Bug 1092922 - Only pause once when unwinding a particular exception, r=loganfsmyth.

devtools/client/debugger/test/mochitest/browser_dbg-pause-exceptions.js

@@ -17,7 +17,8 @@ function caughtException() {
   4. skip a caught error
 */
 add_task(async function() {
-  const dbg = await initDebugger("doc-exceptions.html");
+  const dbg = await initDebugger("doc-exceptions.html", "exceptions.js");
+  const source = findSource(dbg, "exceptions.js");
 
   log("1. test skipping an uncaught exception");
   await uncaughtException();
@@ -31,6 +32,14 @@ add_task(async function() {
   await resume(dbg);
   await waitForActive(dbg);
 
+  log("2.b Test throwing the same uncaught exception pauses again");
+  await togglePauseOnExceptions(dbg, true, true);
+  uncaughtException();
+  await waitForPaused(dbg);
+  assertPausedLocation(dbg);
+  await resume(dbg);
+  await waitForActive(dbg);
+
   log("3. Test pausing on a caught Error");
   caughtException();
   await waitForPaused(dbg);
@@ -50,4 +59,51 @@ add_task(async function() {
   await waitForPaused(dbg);
   assertPausedLocation(dbg);
   await resume(dbg);
+
+  await togglePauseOnExceptions(dbg, true, true);
+
+  log("5. Only pause once when throwing deep in the stack");
+  invokeInTab("deepError");
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 16);
+  await resume(dbg);
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 22);
+  await resume(dbg);
+
+  log("6. Only pause once on an exception when pausing in a finally block");
+  invokeInTab("deepErrorFinally");
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 34);
+  await resume(dbg);
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 31);
+  await resume(dbg);
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 40);
+  await resume(dbg);
+
+  log("7. Only pause once on an exception when it is rethrown from a catch");
+  invokeInTab("deepErrorCatch");
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 53);
+  await resume(dbg);
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 49);
+  await resume(dbg);
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 59);
+  await resume(dbg);
+
+  log("8. Pause on each exception thrown while unwinding");
+  invokeInTab("deepErrorThrowDifferent");
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 71);
+  await resume(dbg);
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 68);
+  await resume(dbg);
+  await waitForPaused(dbg);
+  assertPausedAtSourceAndLine(dbg, source.id, 77);
+  await resume(dbg);
 });

devtools/client/debugger/test/mochitest/browser_dbg-returnvalues.js

@@ -48,8 +48,6 @@ async function testThrowValue(dbg, val) {
   // displays values, so this test works when `val` is a string.
   is(getValue(dbg, 2), uneval(val), `check exception is ${uneval(val)}`);
 
-  await resume(dbg);
-  await waitForPaused(dbg);
   await resume(dbg);
   assertNotPaused(dbg);
 }

devtools/client/debugger/test/mochitest/examples/exceptions.js

@@ -2,14 +2,6 @@ function uncaughtException() {
   throw "unreachable"
 }
 
-function caughtError() {
-  try {
-    throw new Error("error");
-  } catch (e) {
-    debugger;
-  }
-}
-
 function caughtException() {
   try {
     throw "reachable";
@@ -17,3 +9,70 @@ function caughtException() {
     debugger;
   }
 }
+
+function deepError() {
+  function a() { b(); }
+  function b() { c(); }
+  function c() { throw new Error(); }
+
+  try {
+    a();
+  } catch (e) {}
+
+  debugger;
+}
+
+function deepErrorFinally() {
+  function a() { b(); }
+  function b() {
+    try {
+      c();
+    } finally {
+      debugger;
+    }
+  }
+  function c() { throw new Error(); }
+
+  try {
+    a();
+  } catch (e) {}
+
+  debugger;
+}
+
+function deepErrorCatch() {
+  function a() { b(); }
+  function b() {
+    try {
+      c();
+    } catch (e) {
+      debugger;
+      throw e;
+    }
+  }
+  function c() { throw new Error(); }
+
+  try {
+    a();
+  } catch (e) {}
+
+  debugger;
+}
+
+function deepErrorThrowDifferent() {
+  function a() { b(); }
+  function b() {
+    try {
+      c();
+    } catch (e) {
+      throw new Error();
+    }
+  }
+  function c() { throw new Error(); }
+
+  try {
+    a();
+  } catch (e) {}
+
+  debugger;
+}

devtools/server/actors/thread.js

@@ -76,6 +76,12 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     // the debugger instance.
     this._onLoadBreakpointURLs = new Set();
 
+    // A WeakMap from Debugger.Frame to an exception value which will be ignored
+    // when deciding to pause if the value is thrown by the frame. When we are
+    // pausing on exceptions then we only want to pause when the youngest frame
+    // throws a particular exception, instead of for all older frames as well.
+    this._handledFrameExceptions = new WeakMap();
+
     this.global = global;
 
     this.onNewSourceEvent = this.onNewSourceEvent.bind(this);
@@ -1565,6 +1571,11 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
       return undefined;
     }
 
+    if (this._handledFrameExceptions.has(youngestFrame) &&
+        this._handledFrameExceptions.get(youngestFrame) === value) {
+      return undefined;
+    }
+
     // NS_ERROR_NO_INTERFACE exceptions are a special case in browser code,
     // since they're almost always thrown by QueryInterface functions, and
     // handled cleanly by native code.
@@ -1585,6 +1596,12 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
       return undefined;
     }
 
+    // Now that we've decided to pause, ignore this exception if it's thrown by
+    // any older frames.
+    for (let frame = youngestFrame.older; frame != null; frame = frame.older) {
+      this._handledFrameExceptions.set(frame, value);
+    }
+
     try {
       const packet = this._paused(youngestFrame);
       if (!packet) {

devtools/server/tests/unit/test_ignore_no_interface_exceptions.js

@@ -10,12 +10,9 @@
 
 add_task(threadClientTest(async ({ threadClient, debuggee, client }) => {
   await threadClient.pauseOnExceptions(true, false);
-  await executeOnNextTickAndWaitForPause(() => evaluateTestCode(debuggee), client);
-
-  await resume(threadClient);
-  const paused = await waitForPause(client);
-  Assert.equal(paused.why.type, "exception");
-  equal(paused.frame.where.line, 12, "paused at throw");
+  const paused =
+    await executeOnNextTickAndWaitForPause(() => evaluateTestCode(debuggee), client);
+  equal(paused.frame.where.line, 6, "paused at throw");
 
   await resume(threadClient);
 }, {