Bug 1518961 - Show evaluated frames when pausing on exceptions, r=lsmyth.
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 10 Jan 2019 15:09:43 -1000
changeset 453381 fbe6548db11ded24b5221180719ce66e785dc3c6
parent 453380 472bcde2c2d1142a518cbe0ba6f06ba28896b7a9
child 453382 4e60d1fb951f89e3363db4a0918f2bf8f925bb4b
push id35354
push userrmaries@mozilla.com
push dateFri, 11 Jan 2019 09:31:48 +0000
treeherdermozilla-central@340d5146c405 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsmyth
bugs1518961
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518961 - Show evaluated frames when pausing on exceptions, r=lsmyth.
devtools/client/debugger/new/test/mochitest/browser.ini
devtools/client/debugger/new/test/mochitest/browser_dbg-eval-throw.js
devtools/client/debugger/new/test/mochitest/examples/doc-eval-throw.html
devtools/server/actors/thread.js
devtools/server/actors/webconsole.js
devtools/server/tests/unit/test_pause_exceptions-01.js
devtools/server/tests/unit/test_pause_exceptions-02.js
--- a/devtools/client/debugger/new/test/mochitest/browser.ini
+++ b/devtools/client/debugger/new/test/mochitest/browser.ini
@@ -649,16 +649,17 @@ support-files =
   examples/frames.js
   examples/pause-points.js
   examples/script-mutate.js
   examples/script-switching-02.js
   examples/script-switching-01.js
   examples/times2.js
   examples/doc-windowless-workers.html
   examples/simple-worker.js
+  examples/doc-eval-throw.html
   examples/doc_rr_basic.html
   examples/doc_rr_continuous.html
   examples/doc_rr_logs.html
   examples/doc_rr_recovery.html
   examples/doc_rr_error.html
 
 [browser_dbg-asm.js]
 [browser_dbg-async-stepping.js]
@@ -766,16 +767,17 @@ skip-if = os == "win"
 [browser_dbg-tabs-pretty-print.js]
 [browser_dbg-tabs-without-urls.js]
 [browser_dbg-toggling-tools.js]
 [browser_dbg-react-app.js]
 skip-if = os == "win"
 [browser_dbg-wasm-sourcemaps.js]
 skip-if = true
 [browser_dbg-windowless-workers.js]
+[browser_dbg-eval-throw.js]
 [browser_dbg_rr_breakpoints-01.js]
 skip-if = os != "mac" || debug || !nightly_build
 [browser_dbg_rr_breakpoints-02.js]
 skip-if = os != "mac" || debug || !nightly_build
 [browser_dbg_rr_breakpoints-03.js]
 skip-if = os != "mac" || debug || !nightly_build
 [browser_dbg_rr_breakpoints-04.js]
 skip-if = os != "mac" || debug || !nightly_build
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-eval-throw.js
@@ -0,0 +1,17 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that exceptions thrown while evaluating code will pause at the point the
+// exception was generated when pausing on uncaught exceptions.
+add_task(async function() {
+  const dbg = await initDebugger("doc-eval-throw.html");
+  await togglePauseOnExceptions(dbg, true, true);
+
+  invokeInTab("evaluateException");
+
+  await waitForPaused(dbg);
+
+  const state = dbg.store.getState();
+  const source = dbg.selectors.getSelectedSource(state);
+  ok(!source.url, "Selected source should not have a URL");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/new/test/mochitest/examples/doc-eval-throw.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<meta charset=UTF-8>
+<script>
+function evaluateException() {
+  try {
+    eval("throw new Error()");
+  } catch (e) {}
+}
+</script>
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -1820,19 +1820,23 @@ const ThreadActor = ActorClassWithSpec(t
     // handled cleanly by native code.
     if (value == Cr.NS_ERROR_NO_INTERFACE) {
       return undefined;
     }
 
     const { generatedSourceActor } = this.sources.getFrameLocation(youngestFrame);
     const url = generatedSourceActor ? generatedSourceActor.url : null;
 
-    // We ignore sources without a url because we do not
-    // want to pause at console evaluations or watch expressions.
-    if (!url || this.skipBreakpoints || this.sources.isBlackBoxed(url)) {
+    // Don't pause on exceptions thrown while inside an evaluation being done on
+    // behalf of the client.
+    if (this.insideClientEvaluation) {
+      return undefined;
+    }
+
+    if (this.skipBreakpoints || this.sources.isBlackBoxed(url)) {
       return undefined;
     }
 
     try {
       const packet = this._paused(youngestFrame);
       if (!packet) {
         return undefined;
       }
--- a/devtools/server/actors/webconsole.js
+++ b/devtools/server/actors/webconsole.js
@@ -1025,17 +1025,24 @@ WebConsoleActor.prototype =
       bindObjectActor: request.bindObjectActor,
       frameActor: request.frameActor,
       url: request.url,
       selectedNodeActor: request.selectedNodeActor,
       selectedObjectActor: request.selectedObjectActor,
     };
     const {mapped} = request;
 
+    // Set a flag on the thread actor which indicates an evaluation is being
+    // done for the client. This can affect how debugger handlers behave.
+    this.parentActor.threadActor.insideClientEvaluation = true;
+
     const evalInfo = evalWithDebugger(input, evalOptions, this);
+
+    this.parentActor.threadActor.insideClientEvaluation = false;
+
     const evalResult = evalInfo.result;
     const helperResult = evalInfo.helperResult;
 
     let result, errorDocURL, errorMessage, errorNotes = null, errorGrip = null,
       frame = null, awaitResult;
     if (evalResult) {
       if ("return" in evalResult) {
         result = evalResult.return;
--- a/devtools/server/tests/unit/test_pause_exceptions-01.js
+++ b/devtools/server/tests/unit/test_pause_exceptions-01.js
@@ -31,31 +31,29 @@ function run_test() {
                            });
   });
   do_test_pending();
 }
 
 function test_pause_frame() {
   gThreadClient.addOneTimeListener("paused", function(event, packet) {
     gThreadClient.addOneTimeListener("paused", function(event, packet) {
-      Assert.equal(packet.why.type, "debuggerStatement");
-      Assert.equal(packet.frame.where.line, 9);
+      Assert.equal(packet.why.type, "exception");
+      Assert.equal(packet.why.exception, 42);
       gThreadClient.resume(() => finishClient(gClient));
     });
 
     gThreadClient.pauseOnExceptions(true);
     gThreadClient.resume();
   });
 
   /* eslint-disable */
   gDebuggee.eval("(" + function () {
     function stopMe() {
       debugger;
       throw 42;
     }
     try {
       stopMe();
-    } catch (e) {
-      debugger
-    }
+    } catch (e) {}
   } + ")()");
   /* eslint-enable */
 }
--- a/devtools/server/tests/unit/test_pause_exceptions-02.js
+++ b/devtools/server/tests/unit/test_pause_exceptions-02.js
@@ -30,27 +30,25 @@ function run_test() {
                            });
   });
   do_test_pending();
 }
 
 function test_pause_frame() {
   gThreadClient.pauseOnExceptions(true, false, function() {
     gThreadClient.addOneTimeListener("paused", function(event, packet) {
-      Assert.equal(packet.why.type, "debuggerStatement");
-      Assert.equal(packet.frame.where.line, 8);
+      Assert.equal(packet.why.type, "exception");
+      Assert.equal(packet.why.exception, 42);
       gThreadClient.resume(() => finishClient(gClient));
     });
 
     /* eslint-disable */
     gDebuggee.eval("(" + function () {   // 1
       function stopMe() {                // 2
         throw 42;                        // 3
       }                                  // 4
       try {                              // 5
         stopMe();                        // 6
-      } catch (e) {                      // 7
-        debugger;                        // 8
-      }                                  // 9
+      } catch (e) {}                     // 7
     } + ")()");
     /* eslint-enable */
   });
 }