Bug 1448431 - Don't pause on console expression exceptions. r=jimb draft
authorJason Laster <jason.laster.11@gmail.com>
Thu, 22 Mar 2018 20:03:21 -0400
changeset 777296 e4d35c97ee1692c68a846accea040a11dddbfa53
parent 776308 e1dd5af91068dc36b3715f3bf5f0741fa69fa29a
push id105154
push userbmo:jlaster@mozilla.com
push dateWed, 04 Apr 2018 15:35:24 +0000
reviewersjimb
bugs1448431
milestone61.0a1
Bug 1448431 - Don't pause on console expression exceptions. r=jimb MozReview-Commit-ID: JrQjvtBy3LP
devtools/client/debugger/new/test/mochitest/browser_dbg-expressions-error.js
devtools/server/actors/thread.js
devtools/server/tests/unit/test_ignore_caught_exceptions.js
devtools/server/tests/unit/test_ignore_no_interface_exceptions.js
devtools/server/tests/unit/test_pause_exceptions-01.js
devtools/server/tests/unit/test_pause_exceptions-02.js
devtools/server/tests/unit/test_pause_exceptions-03.js
devtools/server/tests/unit/xpcshell.ini
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions-error.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions-error.js
@@ -17,65 +17,45 @@ function getLabel(dbg, index) {
   return findElement(dbg, "expressionNode", index).innerText;
 }
 
 function getValue(dbg, index) {
   return findElement(dbg, "expressionValue", index).innerText;
 }
 
 async function addExpression(dbg, input) {
-  info("Adding an expression");
+  const evaluation = waitForDispatch(dbg, "EVALUATE_EXPRESSION");
   findElementWithSelector(dbg, expressionSelectors.input).focus();
   type(dbg, input);
   pressKey(dbg, "Enter");
-  await waitForDispatch(dbg, "EVALUATE_EXPRESSION");
+  await evaluation;
 }
 
 async function editExpression(dbg, input) {
   info("updating the expression");
   dblClickElement(dbg, "expressionNode", 1);
   // Position cursor reliably at the end of the text.
   const evaluation = waitForDispatch(dbg, "EVALUATE_EXPRESSION");
   pressKey(dbg, "End");
   type(dbg, input);
   pressKey(dbg, "Enter");
   await evaluation;
 }
 
-/*
- * When we add a bad expression, we'll pause,
- * resume, and wait for the expression to finish being evaluated.
- */
-async function addBadExpression(dbg, input) {
-  const evaluation = waitForDispatch(dbg, "EVALUATE_EXPRESSION");
-
-  findElementWithSelector(dbg, expressionSelectors.input).focus();
-  type(dbg, input);
-  pressKey(dbg, "Enter");
-
-  await waitForPaused(dbg);
-
-  ok(dbg.selectors.isEvaluatingExpression(dbg.getState()));
-  await resume(dbg);
-  await evaluation;
-}
-
 add_task(async function() {
   const dbg = await initDebugger("doc-script-switching.html");
 
-  const onPausedOnException = togglePauseOnExceptions(dbg, true, false);
+  await togglePauseOnExceptions(dbg, true, false);
 
   // add a good expression, 2 bad expressions, and another good one
   await addExpression(dbg, "location");
-  await addBadExpression(dbg, "foo.bar");
-  await addBadExpression(dbg, "foo.batt");
+  await addExpression(dbg, "foo.bar");
+  await addExpression(dbg, "foo.batt");
   await addExpression(dbg, "2");
 
-  await onPausedOnException;
-
   // check the value of
   is(getValue(dbg, 2), "(unavailable)");
   is(getValue(dbg, 3), "(unavailable)");
   is(getValue(dbg, 4), 2);
 
   await toggleExpressionNode(dbg, 1);
   is(findAllElements(dbg, "expressionNodes").length, 20);
 });
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -1531,17 +1531,19 @@ const ThreadActor = ActorClassWithSpec(t
       return undefined;
     }
 
     const generatedLocation = this.sources.getFrameLocation(youngestFrame);
     const { originalSourceActor } = this.unsafeSynchronize(
       this.sources.getOriginalLocation(generatedLocation));
     const url = originalSourceActor ? originalSourceActor.url : null;
 
-    if (this.sources.isBlackBoxed(url)) {
+    // We ignore sources without a url because we do not
+    // want to pause at console evaluations or watch expressions.
+    if (!url || this.sources.isBlackBoxed(url)) {
       return undefined;
     }
 
     try {
       let packet = this._paused(youngestFrame);
       if (!packet) {
         return undefined;
       }
--- a/devtools/server/tests/unit/test_ignore_caught_exceptions.js
+++ b/devtools/server/tests/unit/test_ignore_caught_exceptions.js
@@ -6,51 +6,57 @@
 
 /**
  * Test that setting ignoreCaughtExceptions will cause the debugger to ignore
  * caught exceptions, but not uncaught ones.
  */
 
 var gDebuggee;
 var gClient;
-var gThreadClient;
 
 function run_test() {
-  initTestDebuggerServer();
-  gDebuggee = addTestGlobal("test-stack");
-  gClient = new DebuggerClient(DebuggerServer.connectPipe());
-  gClient.connect().then(function() {
-    attachTestTabAndResume(gClient, "test-stack",
-                           function(response, tabClient, threadClient) {
-                             gThreadClient = threadClient;
-                             test_pause_frame();
-                           });
+  do_test_pending();
+  run_test_with_server(DebuggerServer, function() {
+    run_test_with_server(WorkerDebuggerServer, do_test_finished);
   });
-  do_test_pending();
+}
+
+function run_test_with_server(server, callback) {
+  initTestDebuggerServer(server);
+  gDebuggee = addTestGlobal("test-pausing", server);
+  gClient = new DebuggerClient(server.connectPipe());
+  gClient.connect(test_pause_frame);
 }
 
-function test_pause_frame() {
-  gThreadClient.addOneTimeListener("paused", function(event, packet) {
-    gThreadClient.addOneTimeListener("paused", function(event, packet) {
-      Assert.equal(packet.why.type, "exception");
-      Assert.equal(packet.why.exception, "bar");
-      gThreadClient.resume(function() {
-        finishClient(gClient);
-      });
-    });
-    gThreadClient.pauseOnExceptions(true, true);
-    gThreadClient.resume();
-  });
+async function test_pause_frame() {
+  const [,, threadClient] = await attachTestTabAndResume(gClient, "test-pausing");
+  await executeOnNextTickAndWaitForPause(evaluateTestCode, gClient);
+
+  evaluateTestCode();
 
+  threadClient.pauseOnExceptions(true, true);
+  await resume(threadClient);
+  const paused = await waitForPause(gClient);
+  Assert.equal(paused.why.type, "exception");
+  equal(paused.frame.where.line, 6, "paused at throw");
+
+  await resume(threadClient);
+  finishClient(gClient);
+}
+
+function evaluateTestCode() {
+  /* eslint-disable */
   try {
-    /* eslint-disable */
-    gDebuggee.eval("(" + function () {
-      debugger;
-      try {
-        throw "foo";
-      } catch (e) {}
-      throw "bar";
-    } + ")()");
-    /* eslint-enable */
-  } catch (e) {
-    /* Empty */
-  }
+  Cu.evalInSandbox(`                    // 1
+   debugger;                            // 2
+   try {                                // 3           
+     throw "foo";                       // 4
+   } catch (e) {}                       // 5
+   throw "bar";                         // 6  
+  `,                                    // 7
+    gDebuggee,
+    "1.8",
+    "test_pause_exceptions-03.js",
+    1
+  );
+  } catch (e) {}
+  /* eslint-disable */
 }
--- a/devtools/server/tests/unit/test_ignore_no_interface_exceptions.js
+++ b/devtools/server/tests/unit/test_ignore_no_interface_exceptions.js
@@ -5,52 +5,60 @@
 
 /**
  * Test that the debugger automatically ignores NS_ERROR_NO_INTERFACE
  * exceptions, but not normal ones.
  */
 
 var gDebuggee;
 var gClient;
-var gThreadClient;
 
 function run_test() {
-  initTestDebuggerServer();
-  gDebuggee = addTestGlobal("test-no-interface");
-  gClient = new DebuggerClient(DebuggerServer.connectPipe());
-  gClient.connect().then(function() {
-    attachTestTabAndResume(gClient, "test-no-interface",
-                           function(response, tabClient, threadClient) {
-                             gThreadClient = threadClient;
-                             test_pause_frame();
-                           });
+  do_test_pending();
+  run_test_with_server(DebuggerServer, function() {
+    run_test_with_server(WorkerDebuggerServer, do_test_finished);
   });
-  do_test_pending();
+}
+
+function run_test_with_server(server, callback) {
+  initTestDebuggerServer(server);
+  gDebuggee = addTestGlobal("test-pausing", server);
+  gClient = new DebuggerClient(server.connectPipe());
+  gClient.connect(test_pause_frame);
 }
 
-function test_pause_frame() {
-  gThreadClient.pauseOnExceptions(true, false, function() {
-    gThreadClient.addOneTimeListener("paused", function(event, packet) {
-      Assert.equal(packet.why.type, "exception");
-      Assert.equal(packet.why.exception, 42);
-      gThreadClient.resume(function() {
-        finishClient(gClient);
-      });
-    });
+async function test_pause_frame() {
+  const [,, threadClient] = await attachTestTabAndResume(gClient, "test-pausing");
+
+  await threadClient.pauseOnExceptions(true, false);
+  await executeOnNextTickAndWaitForPause(evaluateTestCode, gClient);
+
+  await resume(threadClient);
+  const paused = await waitForPause(gClient);
+  Assert.equal(paused.why.type, "exception");
+  equal(paused.frame.where.line, 12, "paused at throw");
+
+  await resume(threadClient);
+  finishClient(gClient);
+}
 
-    /* eslint-disable */
-    gDebuggee.eval("(" + function () {
-      function QueryInterface() {
-        throw Cr.NS_ERROR_NO_INTERFACE;
-      }
-      function stopMe() {
-        throw 42;
-      }
-      try {
-        QueryInterface();
-      } catch (e) {}
-      try {
-        stopMe();
-      } catch (e) {}
-    } + ")()");
-    /* eslint-enable */
-  });
+function evaluateTestCode() {
+  /* eslint-disable */
+  Cu.evalInSandbox(`                    // 1
+    function QueryInterface() {         // 2
+      throw Cr.NS_ERROR_NO_INTERFACE;   // 3
+    }                                   // 4
+    function stopMe() {                 // 5
+      throw 42;                         // 6
+    }                                   // 7
+    try {                               // 8
+      QueryInterface();                 // 9
+    } catch (e) {}                      // 10
+    try {                               // 11
+      stopMe();                         // 12
+    } catch (e) {}`,                    // 13
+    gDebuggee,
+    "1.8",
+    "test_ignore_no_interface_exceptions.js",
+    1
+  );
+  /* eslint-disable */
 }
--- a/devtools/server/tests/unit/test_pause_exceptions-01.js
+++ b/devtools/server/tests/unit/test_pause_exceptions-01.js
@@ -25,30 +25,31 @@ 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, "exception");
-      Assert.equal(packet.why.exception, 42);
-      gThreadClient.resume(function() {
-        finishClient(gClient);
-      });
+      Assert.equal(packet.why.type, "debuggerStatement");
+      Assert.equal(packet.frame.where.line, 9);
+      gThreadClient.resume(() => finishClient(gClient));
     });
+
     gThreadClient.pauseOnExceptions(true);
     gThreadClient.resume();
   });
 
   /* eslint-disable */
   gDebuggee.eval("(" + function () {
     function stopMe() {
       debugger;
       throw 42;
     }
     try {
       stopMe();
-    } catch (e) {}
+    } catch (e) {
+      debugger
+    }
   } + ")()");
   /* eslint-enable */
 }
--- a/devtools/server/tests/unit/test_pause_exceptions-02.js
+++ b/devtools/server/tests/unit/test_pause_exceptions-02.js
@@ -1,16 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 /**
  * Test that setting pauseOnExceptions to true when the debugger isn't in a
- * paused state will cause the debuggee to pause when an exceptions is thrown.
+ * paused state will not cause the debuggee to pause when an exceptions is thrown.
  */
 
 var gDebuggee;
 var gClient;
 var gThreadClient;
 
 function run_test() {
   initTestDebuggerServer();
@@ -24,27 +24,27 @@ 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, "exception");
-      Assert.equal(packet.why.exception, 42);
-      gThreadClient.resume(function() {
-        finishClient(gClient);
-      });
+      Assert.equal(packet.why.type, "debuggerStatement");
+      Assert.equal(packet.frame.where.line, 8);
+      gThreadClient.resume(() => finishClient(gClient));
     });
 
     /* eslint-disable */
-    gDebuggee.eval("(" + function () {
-      function stopMe() {
-        throw 42;
-      }
-      try {
-        stopMe();
-      } catch (e) {}
+    gDebuggee.eval("(" + function () {   // 1
+      function stopMe() {                // 2
+        throw 42;                        // 3
+      }                                  // 4
+      try {                              // 5
+        stopMe();                        // 6
+      } catch (e) {                      // 7
+        debugger;                        // 8
+      }                                  // 9
     } + ")()");
     /* eslint-enable */
   });
 }
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_pause_exceptions-03.js
@@ -0,0 +1,60 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+/* eslint-disable no-shadow */
+
+"use strict";
+
+/**
+ * Test that setting pauseOnExceptions to true will cause the debuggee to pause
+ * when an exception is thrown.
+ */
+
+var gDebuggee;
+var gClient;
+
+function run_test() {
+  do_test_pending();
+  run_test_with_server(DebuggerServer, function() {
+    run_test_with_server(WorkerDebuggerServer, do_test_finished);
+  });
+}
+
+function run_test_with_server(server, callback) {
+  initTestDebuggerServer(server);
+  gDebuggee = addTestGlobal("test-pausing", server);
+  gClient = new DebuggerClient(server.connectPipe());
+  gClient.connect(test_pause_frame);
+}
+
+async function test_pause_frame() {
+  const [,, threadClient] = await attachTestTabAndResume(gClient, "test-pausing");
+  await executeOnNextTickAndWaitForPause(evaluateTestCode, gClient);
+
+  threadClient.pauseOnExceptions(true);
+  await resume(threadClient);
+  const paused = await waitForPause(gClient);
+  Assert.equal(paused.why.type, "exception");
+  equal(paused.frame.where.line, 4, "paused at throw");
+
+  await resume(threadClient);
+  finishClient(gClient);
+}
+
+function evaluateTestCode() {
+  /* eslint-disable */
+  Cu.evalInSandbox(
+    `                                   // 1
+    function stopMe() {                 // 2
+      debugger;                         // 3
+      throw 42;                         // 4
+    }                                   // 5
+    try {                               // 6
+      stopMe();                         // 7
+    } catch (e) {}`,                    // 8
+    gDebuggee,
+    "1.8",
+    "test_pause_exceptions-03.js",
+    1
+  );
+  /* eslint-disable */
+}
\ No newline at end of file
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -192,16 +192,17 @@ reason = bug 1104838
 [test_framebindings-02.js]
 [test_framebindings-03.js]
 [test_framebindings-04.js]
 [test_framebindings-05.js]
 [test_framebindings-06.js]
 [test_framebindings-07.js]
 [test_pause_exceptions-01.js]
 [test_pause_exceptions-02.js]
+[test_pause_exceptions-03.js]
 [test_longstringactor.js]
 [test_longstringgrips-01.js]
 [test_longstringgrips-02.js]
 [test_source-01.js]
 [test_wasm_source-01.js]
 [test_breakpoint-actor-map.js]
 [test_unsafeDereference.js]
 [test_add_actors.js]