V4 Debugger: Avoid looking up values in debugger thread

To avoid interaction with the engine from the debugger thread we
move the value lookup functionality into the data collector, and
drop the RefHolder. Also, we define some more debugger jobs to
move the work the request handlers do into the GUI thread.

Task-number: QTBUG-50481
Change-Id: Ia73ebd01e715f13110ef010a766e9b0a1e202df9
Reviewed-by: Nils Jeisecke <jeisecke@saltation.de>
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

1 files changed, 73 insertions, 65 deletions

diff --git a/tests/auto/qml/qv4debugger/tst_qv4debugger.cpp b/tests/auto/qml/qv4debugger/tst_qv4debugger.cpp
index 39a1fbc173..2532f73466 100644
--- a/tests/auto/qml/qv4debugger/tst_qv4debugger.cpp
+++ b/tests/auto/qml/qv4debugger/tst_qv4debugger.cpp
@@ -103,19 +103,24 @@ public:
     typedef QV4DataCollector::Refs Refs;
     typedef QV4DataCollector::Ref Ref;
     struct NamedRefs {
-        NamedRefs(QV4DataCollector *collector = 0): collector(collector) {}
-
-        QStringList names;
-        Refs refs;
-        QV4DataCollector *collector;
+        QJsonArray refs;
+        QJsonObject scope;
 
         int size() const {
-            Q_ASSERT(names.size() == refs.size());
-            return names.size();
+            return scope.size();
         }
 
         bool contains(const QString &name) const {
-            return names.contains(name);
+            return scope.contains(name);
+        }
+
+        QJsonObject resolveRef(int ref) const {
+            foreach (const QJsonValue &val, refs) {
+                QJsonObject obj = val.toObject();
+                if (obj.value(QLatin1String("handle")).toInt() == ref)
+                    return obj;
+            }
+            return QJsonObject();
         }
 
 #define DUMP_JSON(x) {\
@@ -125,7 +130,7 @@ public:
 
         QJsonObject rawValue(const QString &name) const {
             Q_ASSERT(contains(name));
-            return collector->lookupRef(refs.at(names.indexOf(name)));
+            return scope[name].toObject();
         }
 
         QJsonValue value(const QString &name) const {
@@ -142,7 +147,7 @@ public:
                 return;
             }
 
-            QJsonObject o = collector->lookupRef(refs.at(names.indexOf(name)));
+            QJsonObject o = scope[name].toObject();
             QJsonDocument d;
             d.setObject(o);
             qDebug() << name << "=" << d.toJson(QJsonDocument::Indented);
@@ -169,12 +174,9 @@ public slots:
 
         if (debugger->state() == V4Debugger::Paused &&
                 debugger->engine()->hasException) {
-            Refs refs;
-            RefHolder holder(&collector, &refs);
             ExceptionCollectJob job(debugger->engine(), &collector);
             debugger->runInEngine(&job);
-            Q_ASSERT(refs.size() > 0);
-            m_thrownValue = refs.first();
+            m_thrownValue = job.exceptionValue();
         }
 
         foreach (const TestBreakPoint &bp, m_breakPointsToAddWhenPaused)
@@ -186,11 +188,11 @@ public slots:
         while (!m_expressionRequests.isEmpty()) {
             Q_ASSERT(debugger->state() == V4Debugger::Paused);
             ExpressionRequest request = m_expressionRequests.takeFirst();
-            m_expressionResults << Refs();
-            RefHolder holder(&collector, &m_expressionResults.last());
             ExpressionEvalJob job(debugger->engine(), request.frameNr, request.expression,
                                   &collector);
             debugger->runInEngine(&job);
+            m_expressionResults << job.returnValue();
+            m_expressionRefs << job.refs();
         }
 
         if (m_captureContextInfo)
@@ -212,17 +214,30 @@ public:
     void captureContextInfo(V4Debugger *debugger)
     {
         for (int i = 0, ei = m_stackTrace.size(); i != ei; ++i) {
-            m_capturedArguments.append(NamedRefs(&collector));
-            RefHolder argHolder(&collector, &m_capturedArguments.last().refs);
-            ArgumentCollectJob argumentsJob(debugger->engine(), &collector,
-                                            &m_capturedArguments.last().names, i, 0);
-            debugger->runInEngine(&argumentsJob);
-
-            m_capturedLocals.append(NamedRefs(&collector));
-            RefHolder localHolder(&collector, &m_capturedLocals.last().refs);
-            LocalCollectJob localsJob(debugger->engine(), &collector,
-                                      &m_capturedLocals.last().names, i, 0);
-            debugger->runInEngine(&localsJob);
+            m_capturedScope.append(NamedRefs());
+            ScopeJob job(&collector, i, 0);
+            debugger->runInEngine(&job);
+            NamedRefs &refs = m_capturedScope.last();
+            refs.refs = job.refs();
+            QJsonObject object = job.returnValue();
+            object = object.value(QLatin1String("object")).toObject();
+            int ref = object.value(QLatin1String("ref")).toInt();
+            object = refs.resolveRef(ref);
+            foreach (const QJsonValue &value, object.value(QLatin1String("properties")).toArray()) {
+                QJsonObject property = value.toObject();
+                QString name = property.value(QLatin1String("name")).toString();
+                property.remove(QLatin1String("name"));
+                if (property.contains(QLatin1String("ref"))) {
+                    int childRef = property.value(QLatin1String("ref")).toInt();
+                    if (childRef >= 0 && refs.refs.size() > childRef) {
+                        property.remove(QLatin1String("ref"));
+                        property.insert(QLatin1String("properties"),
+                                        refs.resolveRef(childRef).value(
+                                            QLatin1String("properties")).toArray());
+                    }
+                }
+                refs.scope.insert(name, property);
+            }
         }
     }
 
@@ -240,8 +255,7 @@ public:
     QList<V4Debugger::ExecutionState> m_statesWhenPaused;
     QList<TestBreakPoint> m_breakPointsToAddWhenPaused;
     QVector<QV4::StackFrame> m_stackTrace;
-    QVector<NamedRefs> m_capturedArguments;
-    QVector<NamedRefs> m_capturedLocals;
+    QVector<NamedRefs> m_capturedScope;
     qint64 m_thrownValue;
     QV4DataCollector collector;
 
@@ -250,7 +264,8 @@ public:
         int frameNr;
     };
     QVector<ExpressionRequest> m_expressionRequests;
-    QVector<Refs> m_expressionResults;
+    QList<QJsonObject> m_expressionResults;
+    QList<QJsonArray> m_expressionRefs;
     V4Debugger *m_debugger;
 
     // Utility methods:
@@ -454,8 +469,8 @@ void tst_qv4debugger::conditionalBreakPoint()
     QCOMPARE(state.fileName, QString("conditionalBreakPoint"));
     QCOMPARE(state.lineNumber, 3);
 
-    QVERIFY(m_debuggerAgent->m_capturedLocals.size() > 1);
-    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedLocals.at(0);
+    QVERIFY(m_debuggerAgent->m_capturedScope.size() > 1);
+    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedScope.at(0);
     QCOMPARE(frame0.size(), 2);
     QVERIFY(frame0.contains("i"));
     QCOMPARE(frame0.value("i").toInt(), 11);
@@ -511,13 +526,13 @@ void tst_qv4debugger::readArguments()
     debugger()->addBreakPoint("readArguments", 2);
     evaluateJavaScript(script, "readArguments");
     QVERIFY(m_debuggerAgent->m_wasPaused);
-    QVERIFY(m_debuggerAgent->m_capturedArguments.size() > 1);
-    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedArguments.at(0);
+    QVERIFY(m_debuggerAgent->m_capturedScope.size() > 1);
+    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedScope.at(0);
     QCOMPARE(frame0.size(), 4);
     QVERIFY(frame0.contains(QStringLiteral("a")));
     QCOMPARE(frame0.type(QStringLiteral("a")), QStringLiteral("number"));
     QCOMPARE(frame0.value(QStringLiteral("a")).toDouble(), 1.0);
-    QVERIFY(frame0.names.contains("b"));
+    QVERIFY(frame0.scope.contains("b"));
     QCOMPARE(frame0.type(QStringLiteral("b")), QStringLiteral("string"));
     QCOMPARE(frame0.value(QStringLiteral("b")).toString(), QStringLiteral("two"));
 }
@@ -535,9 +550,9 @@ void tst_qv4debugger::readLocals()
     debugger()->addBreakPoint("readLocals", 3);
     evaluateJavaScript(script, "readLocals");
     QVERIFY(m_debuggerAgent->m_wasPaused);
-    QVERIFY(m_debuggerAgent->m_capturedLocals.size() > 1);
-    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedLocals.at(0);
-    QCOMPARE(frame0.size(), 2);
+    QVERIFY(m_debuggerAgent->m_capturedScope.size() > 1);
+    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedScope.at(0);
+    QCOMPARE(frame0.size(), 4); // locals and parameters
     QVERIFY(frame0.contains("c"));
     QCOMPARE(frame0.type("c"), QStringLiteral("number"));
     QCOMPARE(frame0.value("c").toDouble(), 3.0);
@@ -557,9 +572,9 @@ void tst_qv4debugger::readObject()
     debugger()->addBreakPoint("readObject", 3);
     evaluateJavaScript(script, "readObject");
     QVERIFY(m_debuggerAgent->m_wasPaused);
-    QVERIFY(m_debuggerAgent->m_capturedLocals.size() > 1);
-    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedLocals.at(0);
-    QCOMPARE(frame0.size(), 1);
+    QVERIFY(m_debuggerAgent->m_capturedScope.size() > 1);
+    const TestAgent::NamedRefs &frame0 = m_debuggerAgent->m_capturedScope.at(0);
+    QCOMPARE(frame0.size(), 2);
     QVERIFY(frame0.contains("b"));
     QCOMPARE(frame0.type("b"), QStringLiteral("object"));
     QJsonObject b = frame0.rawValue("b");
@@ -578,7 +593,7 @@ void tst_qv4debugger::readObject()
     QCOMPARE(b_tail.value("name").toString(), QStringLiteral("tail"));
     QVERIFY(b_tail.contains("ref"));
 
-    QJsonObject b_tail_value = frame0.collector->lookupRef(b_tail.value("ref").toInt());
+    QJsonObject b_tail_value = m_debuggerAgent->collector.lookupRef(b_tail.value("ref").toInt());
     QCOMPARE(b_tail_value.value("type").toString(), QStringLiteral("object"));
     QVERIFY(b_tail_value.contains("properties"));
     QJsonArray b_tail_props = b_tail_value.value("properties").toArray();
@@ -610,28 +625,23 @@ void tst_qv4debugger::readContextInAllFrames()
     evaluateJavaScript(script, "readFormalsInAllFrames");
     QVERIFY(m_debuggerAgent->m_wasPaused);
     QCOMPARE(m_debuggerAgent->m_stackTrace.size(), 13);
-    QCOMPARE(m_debuggerAgent->m_capturedArguments.size(), 13);
-    QCOMPARE(m_debuggerAgent->m_capturedLocals.size(), 13);
+    QCOMPARE(m_debuggerAgent->m_capturedScope.size(), 13);
 
     for (int i = 0; i < 12; ++i) {
-        const TestAgent::NamedRefs &args = m_debuggerAgent->m_capturedArguments.at(i);
-        QCOMPARE(args.size(), 1);
-        QVERIFY(args.contains("n"));
-        QCOMPARE(args.type("n"), QStringLiteral("number"));
-        QCOMPARE(args.value("n").toDouble(), i + 1.0);
-
-        const TestAgent::NamedRefs &locals = m_debuggerAgent->m_capturedLocals.at(i);
-        QCOMPARE(locals.size(), 1);
-        QVERIFY(locals.contains("n_1"));
+        const TestAgent::NamedRefs &scope = m_debuggerAgent->m_capturedScope.at(i);
+        QCOMPARE(scope.size(), 2);
+        QVERIFY(scope.contains("n"));
+        QCOMPARE(scope.type("n"), QStringLiteral("number"));
+        QCOMPARE(scope.value("n").toDouble(), i + 1.0);
+        QVERIFY(scope.contains("n_1"));
         if (i == 0) {
-            QCOMPARE(locals.type("n_1"), QStringLiteral("undefined"));
+            QCOMPARE(scope.type("n_1"), QStringLiteral("undefined"));
         } else {
-            QCOMPARE(locals.type("n_1"), QStringLiteral("number"));
-            QCOMPARE(locals.value("n_1").toInt(), i);
+            QCOMPARE(scope.type("n_1"), QStringLiteral("number"));
+            QCOMPARE(scope.value("n_1").toInt(), i);
         }
     }
-    QCOMPARE(m_debuggerAgent->m_capturedArguments[12].size(), 0);
-    QCOMPARE(m_debuggerAgent->m_capturedLocals[12].size(), 0);
+    QCOMPARE(m_debuggerAgent->m_capturedScope[12].size(), 0);
 }
 
 void tst_qv4debugger::pauseOnThrow()
@@ -711,15 +721,13 @@ void tst_qv4debugger::evaluateExpression()
 
     evaluateJavaScript(script, "evaluateExpression");
 
-    QCOMPARE(m_debuggerAgent->m_expressionResults.count(), 2);
-    QCOMPARE(m_debuggerAgent->m_expressionResults[0].size(), 1);
-    QJsonObject result0 =
-            m_debuggerAgent->collector.lookupRef(m_debuggerAgent->m_expressionResults[0].first());
+    QCOMPARE(m_debuggerAgent->m_expressionRefs.count(), 2);
+    QCOMPARE(m_debuggerAgent->m_expressionRefs[0].size(), 1);
+    QJsonObject result0 = m_debuggerAgent->m_expressionRefs[0].first().toObject();
     QCOMPARE(result0.value("type").toString(), QStringLiteral("number"));
     QCOMPARE(result0.value("value").toInt(), 10);
-    QCOMPARE(m_debuggerAgent->m_expressionResults[1].size(), 1);
-    QJsonObject result1 =
-            m_debuggerAgent->collector.lookupRef(m_debuggerAgent->m_expressionResults[1].first());
+    QCOMPARE(m_debuggerAgent->m_expressionRefs[1].size(), 1);
+    QJsonObject result1 = m_debuggerAgent->m_expressionRefs[1].first().toObject();
     QCOMPARE(result1.value("type").toString(), QStringLiteral("number"));
     QCOMPARE(result1.value("value").toInt(), 20);
 }