Fix column number provided to QScriptEngineAgent

Introduced a helper function in our custom source provider,
columnNumberFromOffset(), that maps an absolute offset in the source
input to a relative column number.

Reviewed-by: Jedrzej Nowacki

5 files changed, 88 insertions, 68 deletions

diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp
index 9604fff..059b102 100644
--- a/src/script/api/qscriptengine.cpp
+++ b/src/script/api/qscriptengine.cpp
@@ -382,65 +382,6 @@ private:
     bool m_shouldAbortEvaluation;
 };
 
-/*Helper class. Main purpose is to give debugger feedback about unloading and loading scripts.
-  It keeps pointer to JSGlobalObject assuming that it is always the same - there is no way to update
-  this data. Class is internal and used as an implementation detail in and only in QScriptEngine::evaluate.*/
-class UStringSourceProviderWithFeedback: public JSC::UStringSourceProvider
-{
-public:
-
-    static PassRefPtr<UStringSourceProviderWithFeedback> create(const JSC::UString& source, const JSC::UString& url, int lineNumber, QScriptEnginePrivate* engine)
-    {
-        return adoptRef(new UStringSourceProviderWithFeedback(source, url, lineNumber, engine));
-    }
-
-    /* Destruction means that there is no more copies of script so create scriptUnload event
-       and unregister script in QScriptEnginePrivate::loadedScripts */
-    virtual ~UStringSourceProviderWithFeedback()
-    {
-        if (m_ptr) {
-            if (JSC::Debugger* debugger = this->debugger())
-                debugger->scriptUnload(asID());
-            m_ptr->loadedScripts.remove(this);
-        }
-    }
-
-    /* set internal QScriptEnginePrivate pointer to null and create unloadScript event, should be called
-       only if QScriptEnginePrivate is about to be  destroyed.*/
-    void disconnectFromEngine()
-    {        
-        if (JSC::Debugger* debugger = this->debugger())
-            debugger->scriptUnload(asID());
-        m_ptr = 0;
-    }
-
-protected:
-    UStringSourceProviderWithFeedback(const JSC::UString& source, const JSC::UString& url, int lineNumber, QScriptEnginePrivate* engine)
-            : UStringSourceProvider(source, url),
-            m_ptr(engine)
-    {
-        if (JSC::Debugger* debugger = this->debugger())
-            debugger->scriptLoad(asID(), source, url, lineNumber);
-        if (m_ptr)
-            m_ptr->loadedScripts.insert(this);
-    }
-
-    JSC::Debugger* debugger()
-    {
-        //if m_ptr is null it mean that QScriptEnginePrivate was destroyed and scriptUnload was called
-        //else m_ptr is stable and we can use it as normal pointer without hesitation
-        if(!m_ptr)
-            return 0; //we are in ~QScriptEnginePrivate
-        else
-            return m_ptr->originalGlobalObject()->debugger(); //QScriptEnginePrivate is still alive
-    }
-
-    //trace global object and debugger instance
-    QScriptEnginePrivate* m_ptr;
-};
-
-
-
 static int toDigit(char c)
 {
     if ((c >= '0') && (c <= '9'))
@@ -900,11 +841,9 @@ QScriptEnginePrivate::QScriptEnginePrivate()
 QScriptEnginePrivate::~QScriptEnginePrivate()
 {
     //disconnect all loadedScripts and generate all jsc::debugger::scriptUnload events
-    QSet<QScript::UStringSourceProviderWithFeedback*>::const_iterator i = loadedScripts.constBegin();
-    while(i!=loadedScripts.constEnd()) {
-        (*i)->disconnectFromEngine();
-        i++;
-    }
+    QHash<intptr_t,QScript::UStringSourceProviderWithFeedback*>::const_iterator it;
+    for (it = loadedScripts.constBegin(); it != loadedScripts.constEnd(); ++it)
+        it.value()->disconnectFromEngine();
 
     while (!ownedAgents.isEmpty())
         delete ownedAgents.takeFirst();
diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h
index b8b805e..5f31054 100644
--- a/src/script/api/qscriptengine_p.h
+++ b/src/script/api/qscriptengine_p.h
@@ -60,7 +60,10 @@
 #include "qscriptvalue_p.h"
 #include "qscriptstring_p.h"
 
+#include "Debugger.h"
+#include "Lexer.h"
 #include "RefPtr.h"
+#include "SourceProvider.h"
 #include "Structure.h"
 #include "JSGlobalObject.h"
 #include "JSValue.h"
@@ -259,7 +262,7 @@ public:
     QSet<QString> importedExtensions;
     QSet<QString> extensionsBeingImported;
     
-    QSet<QScript::UStringSourceProviderWithFeedback*> loadedScripts;
+    QHash<intptr_t, QScript::UStringSourceProviderWithFeedback*> loadedScripts;
 
 #ifndef QT_NO_QOBJECT
     QHash<QObject*, QScript::QObjectData*> m_qobjectData;
@@ -273,6 +276,74 @@ public:
 namespace QScript
 {
 
+/*Helper class. Main purpose is to give debugger feedback about unloading and loading scripts.
+  It keeps pointer to JSGlobalObject assuming that it is always the same - there is no way to update
+  this data. Class is internal and used as an implementation detail in and only in QScriptEngine::evaluate.*/
+class UStringSourceProviderWithFeedback: public JSC::UStringSourceProvider
+{
+public:
+    static PassRefPtr<UStringSourceProviderWithFeedback> create(
+        const JSC::UString& source, const JSC::UString& url,
+        int lineNumber, QScriptEnginePrivate* engine)
+    {
+        return adoptRef(new UStringSourceProviderWithFeedback(source, url, lineNumber, engine));
+    }
+
+    /* Destruction means that there is no more copies of script so create scriptUnload event
+       and unregister script in QScriptEnginePrivate::loadedScripts */
+    virtual ~UStringSourceProviderWithFeedback()
+    {
+        if (m_ptr) {
+            if (JSC::Debugger* debugger = this->debugger())
+                debugger->scriptUnload(asID());
+            m_ptr->loadedScripts.remove(asID());
+        }
+    }
+
+    /* set internal QScriptEnginePrivate pointer to null and create unloadScript event, should be called
+       only if QScriptEnginePrivate is about to be  destroyed.*/
+    void disconnectFromEngine()
+    {
+        if (JSC::Debugger* debugger = this->debugger())
+            debugger->scriptUnload(asID());
+        m_ptr = 0;
+    }
+
+    int columnNumberFromOffset(int offset) const
+    {
+        for (const UChar *c = m_source.data() + offset; c >= m_source.data(); --c) {
+            if (JSC::Lexer::isLineTerminator(*c))
+                return offset - static_cast<int>(c - data());
+        }
+        return offset + 1;
+    }
+
+protected:
+    UStringSourceProviderWithFeedback(const JSC::UString& source, const JSC::UString& url,
+                                      int lineNumber, QScriptEnginePrivate* engine)
+        : UStringSourceProvider(source, url),
+          m_ptr(engine)
+    {
+        if (JSC::Debugger* debugger = this->debugger())
+            debugger->scriptLoad(asID(), source, url, lineNumber);
+        if (m_ptr)
+            m_ptr->loadedScripts.insert(asID(), this);
+    }
+
+    JSC::Debugger* debugger()
+    {
+        //if m_ptr is null it mean that QScriptEnginePrivate was destroyed and scriptUnload was called
+        //else m_ptr is stable and we can use it as normal pointer without hesitation
+        if(!m_ptr)
+            return 0; //we are in ~QScriptEnginePrivate
+        else
+            return m_ptr->originalGlobalObject()->debugger(); //QScriptEnginePrivate is still alive
+    }
+
+    //trace global object and debugger instance
+    QScriptEnginePrivate* m_ptr;
+};
+
 class SaveFrameHelper
 {
 public:
diff --git a/src/script/api/qscriptengineagent.cpp b/src/script/api/qscriptengineagent.cpp
index 84ae380..bc2eea2 100644
--- a/src/script/api/qscriptengineagent.cpp
+++ b/src/script/api/qscriptengineagent.cpp
@@ -169,6 +169,9 @@ void QScriptEngineAgentPrivate::exceptionCatch(const JSC::DebuggerCallFrame& fra
 
 void QScriptEngineAgentPrivate::atStatement(const JSC::DebuggerCallFrame& frame, intptr_t sourceID, int lineno, int column)
 {
+    QScript::UStringSourceProviderWithFeedback *source = engine->loadedScripts.value(sourceID);
+    Q_ASSERT(source != 0);
+    column = source->columnNumberFromOffset(column);
     JSC::CallFrame *oldFrame = engine->currentFrame;
     int oldAgentLineNumber = engine->agentLineNumber;
     engine->currentFrame = frame.callFrame();
@@ -195,6 +198,9 @@ void QScriptEngineAgentPrivate::didReachBreakpoint(const JSC::DebuggerCallFrame&
                                                    intptr_t sourceID, int lineno, int column)
 {
     if (q_ptr->supportsExtension(QScriptEngineAgent::DebuggerInvocationRequest)) {
+        QScript::UStringSourceProviderWithFeedback *source = engine->loadedScripts.value(sourceID);
+        Q_ASSERT(source != 0);
+        column = source->columnNumberFromOffset(column);
         JSC::CallFrame *oldFrame = engine->currentFrame;
         int oldAgentLineNumber = engine->agentLineNumber;
         engine->currentFrame = frame.callFrame();
diff --git a/tests/auto/qscriptengine/tst_qscriptengine.cpp b/tests/auto/qscriptengine/tst_qscriptengine.cpp
index 183aa3f..f2c7157 100644
--- a/tests/auto/qscriptengine/tst_qscriptengine.cpp
+++ b/tests/auto/qscriptengine/tst_qscriptengine.cpp
@@ -1632,7 +1632,6 @@ void tst_QScriptEngine::errorMessage_QT679()
     engine.globalObject().setProperty("foo", 15);
     QScriptValue error = engine.evaluate("'hello world';\nfoo.bar.blah");
     QVERIFY(error.isError());
-    QEXPECT_FAIL("", "Task QT-679: the error message always contains the first line of the script, even if the error was on a different line", Continue);
     QCOMPARE(error.toString(), QString::fromLatin1("TypeError: Result of expression 'foo.bar' [undefined] is not an object."));
 }
 
diff --git a/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp b/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp
index 82bca8f..283e489 100644
--- a/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp
+++ b/tests/auto/qscriptengineagent/tst_qscriptengineagent.cpp
@@ -1193,10 +1193,15 @@ void tst_QScriptEngineAgent::positionChange_1()
         QCOMPARE(spy->at(0).columnNumber, 1);
     }
 
-    {
+    QStringList lineTerminators;
+    lineTerminators << "\n" << "\r" << "\n\r" << "\r\n";
+    for (int i = 0; i < lineTerminators.size(); ++i) {
         spy->clear();
         int lineNumber = 456;
-        eng.evaluate("1 + 2; 3 + 4;\n5 + 6", "foo.qs", lineNumber);
+        QString code = "1 + 2; 3 + 4;";
+        code.append(lineTerminators.at(i));
+        code.append("5 + 6");
+        eng.evaluate(code, "foo.qs", lineNumber);
         QCOMPARE(spy->count(), 3);
 
         // 1 + 2