V4 debugger: Fix expression evaluation

We need to collect the refs in the debugService's list in order for
them to show up on addRefs() and we need to generate proper error
responses if either the debugger is not stopped or the evaluation
throws an exception.

Task-number: QTBUG-47797
Task-number: QTBUG-47816
Change-Id: I98f17c1f3976859ee50b9bfac41091276ff60982
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

6 files changed, 48 insertions, 25 deletions

diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp b/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp
index a44acdd37..01d2a98a7 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp
+++ b/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp
@@ -317,9 +317,16 @@ ExpressionEvalJob::ExpressionEvalJob(QV4::ExecutionEngine *engine, int frameNr,
 
 void ExpressionEvalJob::handleResult(QV4::ScopedValue &result)
 {
+    if (hasExeption())
+        exception = result->toQStringNoThrow();
     collector->collect(result);
 }
 
+const QString &ExpressionEvalJob::exceptionMessage() const
+{
+    return exception;
+}
+
 GatherSourcesJob::GatherSourcesJob(QV4::ExecutionEngine *engine, int seq)
     : engine(engine)
     , seq(seq)
diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.h b/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.h
index c91b77cb9..ebdde8f96 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.h
+++ b/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.h
@@ -104,11 +104,13 @@ private:
 class ExpressionEvalJob: public QV4::Debugging::Debugger::JavaScriptJob
 {
     QV4DataCollector *collector;
+    QString exception;
 
 public:
     ExpressionEvalJob(QV4::ExecutionEngine *engine, int frameNr, const QString &expression,
                       QV4DataCollector *collector);
     virtual void handleResult(QV4::ScopedValue &result);
+    const QString &exceptionMessage() const;
 };
 
 class GatherSourcesJob: public QV4::Debugging::Debugger::Job
diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.cpp b/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.cpp
index 6b68f9518..89820c9f5 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.cpp
+++ b/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.cpp
@@ -549,31 +549,29 @@ public:
 
     virtual void handleRequest()
     {
-        //decypher the payload:
-        QJsonObject arguments = req.value(QStringLiteral("arguments")).toObject();
-        QString expression = arguments.value(QStringLiteral("expression")).toString();
-        const int frame = arguments.value(QStringLiteral("frame")).toInt(0);
-
         QV4::Debugging::Debugger *debugger = debugService->debuggerAgent.firstDebugger();
-        Q_ASSERT(debugger->state() == QV4::Debugging::Debugger::Paused);
-
-        QV4DataCollector *collector = debugService->collector();
-        QV4DataCollector::Refs refs;
-        RefHolder holder(collector, &refs);
-        Q_ASSERT(debugger->state() == QV4::Debugging::Debugger::Paused);
-
-        ExpressionEvalJob job(debugger->engine(), frame, expression, collector);
-        debugger->runInEngine(&job);
-
-        Q_ASSERT(refs.size() == 1);
-
-        // response:
-        addCommand();
-        addRequestSequence();
-        addSuccess(true);
-        addRunning();
-        addBody(collector->lookupRef(refs.first()));
-        addRefs();
+        if (debugger->state() == QV4::Debugging::Debugger::Paused) {
+            QJsonObject arguments = req.value(QStringLiteral("arguments")).toObject();
+            QString expression = arguments.value(QStringLiteral("expression")).toString();
+            const int frame = arguments.value(QStringLiteral("frame")).toInt(0);
+
+            QV4DataCollector *collector = debugService->collector();
+            RefHolder holder(collector, debugService->refs());
+            ExpressionEvalJob job(debugger->engine(), frame, expression, collector);
+            debugger->runInEngine(&job);
+            if (job.hasExeption()) {
+                createErrorResponse(job.exceptionMessage());
+            } else {
+                addCommand();
+                addRequestSequence();
+                addSuccess(true);
+                addRunning();
+                addBody(collector->lookupRef(debugService->refs()->last()));
+                addRefs();
+            }
+        } else {
+            createErrorResponse(QStringLiteral("Debugger has to be paused for evaluate to work."));
+        }
     }
 };
 } // anonymous namespace
@@ -894,6 +892,11 @@ QV4DataCollector *QV4DebugServiceImpl::collector() const
     return theCollector.data();
 }
 
+QV4DataCollector::Refs *QV4DebugServiceImpl::refs()
+{
+    return &collectedRefs;
+}
+
 void QV4DebugServiceImpl::selectFrame(int frameNr)
 {
     theSelectedFrame = frameNr;
diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.h b/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.h
index c80ad78cc..6c2950de8 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.h
+++ b/src/plugins/qmltooling/qmldbg_debugger/qv4debugservice.h
@@ -90,6 +90,7 @@ public:
 
     QV4DataCollector *collector() const;
     QV4DebuggerAgent debuggerAgent;
+    QV4DataCollector::Refs *refs();
 
 protected:
     void messageReceived(const QByteArray &);
diff --git a/src/qml/jsruntime/qv4debugging.cpp b/src/qml/jsruntime/qv4debugging.cpp
index ceeef80b9..6efc3793c 100644
--- a/src/qml/jsruntime/qv4debugging.cpp
+++ b/src/qml/jsruntime/qv4debugging.cpp
@@ -59,6 +59,7 @@ Debugger::JavaScriptJob::JavaScriptJob(QV4::ExecutionEngine *engine, int frameNr
     : engine(engine)
     , frameNr(frameNr)
     , script(script)
+    , resultIsException(false)
 {}
 
 void Debugger::JavaScriptJob::run()
@@ -85,11 +86,18 @@ void Debugger::JavaScriptJob::run()
     QV4::ScopedValue result(scope);
     if (!scope.engine->hasException)
         result = script.run();
-    if (scope.engine->hasException)
+    if (scope.engine->hasException) {
         result = scope.engine->catchException();
+        resultIsException = true;
+    }
     handleResult(result);
 }
 
+bool Debugger::JavaScriptJob::hasExeption() const
+{
+    return resultIsException;
+}
+
 class EvalJob: public Debugger::JavaScriptJob
 {
     bool result;
diff --git a/src/qml/jsruntime/qv4debugging_p.h b/src/qml/jsruntime/qv4debugging_p.h
index ac0c934d4..86faba45f 100644
--- a/src/qml/jsruntime/qv4debugging_p.h
+++ b/src/qml/jsruntime/qv4debugging_p.h
@@ -95,10 +95,12 @@ public:
         QV4::ExecutionEngine *engine;
         int frameNr;
         const QString &script;
+        bool resultIsException;
 
     public:
         JavaScriptJob(QV4::ExecutionEngine *engine, int frameNr, const QString &script);
         void run();
+        bool hasExeption() const;
 
     protected:
         virtual void handleResult(QV4::ScopedValue &result) = 0;