Bug 757282 - Pause when an exception is hit; r=rcampbell

browser/devtools/debugger/debugger-controller.js

@@ -363,6 +363,12 @@ StackFrames.prototype = {
    */
   selectedFrame: null,
 
+  /**
+   * A flag that defines whether the debuggee will pause whenever an exception
+   * is thrown.
+   */
+  pauseOnExceptions: false,
+
   /**
    * Gets the current thread the client has connected to.
    */
@@ -379,12 +385,14 @@ StackFrames.prototype = {
   connect: function SF_connect(aCallback) {
     window.addEventListener("Debugger:FetchedVariables", this._onFetchedVars, false);
 
+    this._onFramesCleared();
+
     this.activeThread.addListener("paused", this._onPaused);
     this.activeThread.addListener("resumed", this._onResume);
     this.activeThread.addListener("framesadded", this._onFrames);
     this.activeThread.addListener("framescleared", this._onFramesCleared);
 
-    this._onFramesCleared();
+    this.updatePauseOnExceptions(this.pauseOnExceptions);
 
     aCallback && aCallback();
   },
@@ -406,8 +414,17 @@ StackFrames.prototype = {
 
   /**
    * Handler for the thread client's paused notification.
+   *
+   * @param string aEvent
+   *        The name of the notification ("paused" in this case).
+   * @param object aPacket
+   *        The response packet.
    */
-  _onPaused: function SF__onPaused() {
+  _onPaused: function SF__onPaused(aEvent, aPacket) {
+    // In case the pause was caused by an exception, store the exception value.
+    if (aPacket.why.type == "exception") {
+      this.exception = aPacket.why.exception;
+    }
     this.activeThread.fillFrames(this.pageSize);
   },
 
@@ -445,12 +462,13 @@ StackFrames.prototype = {
    * Handler for the thread client's framescleared notification.
    */
   _onFramesCleared: function SF__onFramesCleared() {
+    this.selectedFrame = null;
+    this.exception = null;
     // After each frame step (in, over, out), framescleared is fired, which
     // forces the UI to be emptied and rebuilt on framesadded. Most of the times
     // this is not necessary, and will result in a brief redraw flicker.
     // To avoid it, invalidate the UI only after a short time if necessary.
     window.setTimeout(this._afterFramesCleared, FRAME_STEP_CACHE_DURATION);
-    this.selectedFrame = null;
   },
 
   /**
@@ -486,6 +504,18 @@ StackFrames.prototype = {
     }
   },
 
+  /**
+   * Inform the debugger client whether the debuggee should be paused whenever
+   * an exception is thrown.
+   *
+   * @param boolean aFlag
+   *        The new value of the flag: true for pausing, false otherwise.
+   */
+  updatePauseOnExceptions: function SF_updatePauseOnExceptions(aFlag) {
+    this.pauseOnExceptions = aFlag;
+    this.activeThread.pauseOnExceptions(this.pauseOnExceptions);
+  },
+
   /**
    * Marks the stack frame in the specified depth as selected and updates the
    * properties view with the stack frame's data.
@@ -557,14 +587,32 @@ StackFrames.prototype = {
 
         let scope = DebuggerView.Properties.addScope(label);
 
-        // Add "this" to the innermost scope.
-        if (frame.this && env == frame.environment) {
-          let thisVar = scope.addVar("this");
-          thisVar.setGrip({
-            type: frame.this.type,
-            class: frame.this.class
-          });
-          this._addExpander(thisVar, frame.this);
+        // Special additions to the innermost scope.
+        if (env == frame.environment) {
+          // Add any thrown exception.
+          if (aDepth == 0 && this.exception) {
+            let excVar = scope.addVar("<exception>");
+            if (typeof this.exception == "object") {
+              excVar.setGrip({
+                type: this.exception.type,
+                class: this.exception.class
+              });
+              this._addExpander(excVar, this.exception);
+            } else {
+              excVar.setGrip(this.exception);
+            }
+          }
+
+          // Add "this".
+          if (frame.this) {
+            let thisVar = scope.addVar("this");
+            thisVar.setGrip({
+              type: frame.this.type,
+              class: frame.this.class
+            });
+            this._addExpander(thisVar, frame.this);
+          }
+
           // Expand the innermost scope by default.
           scope.expand(true);
           scope.addToHierarchy();

browser/devtools/debugger/debugger-view.js

@@ -486,6 +486,7 @@ ScriptsView.prototype = {
  */
 function StackFramesView() {
   this._onFramesScroll = this._onFramesScroll.bind(this);
+  this._onPauseExceptionsClick = this._onPauseExceptionsClick.bind(this);
   this._onCloseButtonClick = this._onCloseButtonClick.bind(this);
   this._onResumeButtonClick = this._onResumeButtonClick.bind(this);
   this._onStepOverClick = this._onStepOverClick.bind(this);
@@ -689,6 +690,14 @@ StackFramesView.prototype = {
     DebuggerController.dispatchEvent("Debugger:Close");
   },
 
+  /**
+   * Listener handling the pause-on-exceptions click event.
+   */
+  _onPauseExceptionsClick: function DVF__onPauseExceptionsClick() {
+    let option = document.getElementById("pause-exceptions");
+    DebuggerController.StackFrames.updatePauseOnExceptions(option.checked);
+  },
+
   /**
    * Listener handling the pause/resume button click event.
    */
@@ -736,6 +745,7 @@ StackFramesView.prototype = {
    */
   initialize: function DVF_initialize() {
     let close = document.getElementById("close");
+    let pauseOnExceptions = document.getElementById("pause-exceptions");
     let resume = document.getElementById("resume");
     let stepOver = document.getElementById("step-over");
     let stepIn = document.getElementById("step-in");
@@ -743,6 +753,10 @@ StackFramesView.prototype = {
     let frames = document.getElementById("stackframes");
 
     close.addEventListener("click", this._onCloseButtonClick, false);
+    pauseOnExceptions.checked = DebuggerController.StackFrames.pauseOnExceptions;
+    pauseOnExceptions.addEventListener("click",
+                                        this._onPauseExceptionsClick,
+                                        false);
     resume.addEventListener("click", this._onResumeButtonClick, false);
     stepOver.addEventListener("click", this._onStepOverClick, false);
     stepIn.addEventListener("click", this._onStepInClick, false);
@@ -759,6 +773,7 @@ StackFramesView.prototype = {
    */
   destroy: function DVF_destroy() {
     let close = document.getElementById("close");
+    let pauseOnExceptions = document.getElementById("pause-exceptions");
     let resume = document.getElementById("resume");
     let stepOver = document.getElementById("step-over");
     let stepIn = document.getElementById("step-in");
@@ -766,6 +781,9 @@ StackFramesView.prototype = {
     let frames = this._frames;
 
     close.removeEventListener("click", this._onCloseButtonClick, false);
+    pauseOnExceptions.removeEventListener("click",
+                                          this._onPauseExceptionsClick,
+                                          false);
     resume.removeEventListener("click", this._onResumeButtonClick, false);
     stepOver.removeEventListener("click", this._onStepOverClick, false);
     stepIn.removeEventListener("click", this._onStepInClick, false);

browser/devtools/debugger/debugger.xul

@@ -68,6 +68,10 @@
       <textbox id="scripts-search" type="search"
                class="devtools-searchinput"
                emptytext="&debuggerUI.emptyFilterText;"/>
+      <checkbox id="pause-exceptions"
+                type="checkbox"
+                tabindex="0"
+                label="&debuggerUI.pauseExceptions;"/>
       <spacer flex="1"/>
 #ifndef XP_MACOSX
       <toolbarbutton id="close"

browser/devtools/debugger/test/Makefile.in

@@ -54,6 +54,7 @@ _BROWSER_TEST_FILES = \
 	browser_dbg_bug731394_editor-contextmenu.js \
 	browser_dbg_displayName.js \
 	browser_dbg_iframes.js \
+	browser_dbg_pause-exceptions.js \
 	head.js \
 	$(NULL)
 
@@ -71,6 +72,7 @@ _BROWSER_TEST_PAGES = \
 	browser_dbg_displayName.html \
 	browser_dbg_iframes.html \
 	browser_dbg_with-frame.html \
+	browser_dbg_pause-exceptions.html \
 	$(NULL)
 
 libs:: $(_BROWSER_TEST_FILES)

browser/devtools/debugger/test/browser_dbg_pause-exceptions.html

@@ -0,0 +1,30 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <meta charset='utf-8'/>
+    <title>Debugger Pause on Exceptions Test</title>
+    <!-- Any copyright is dedicated to the Public Domain.
+         http://creativecommons.org/publicdomain/zero/1.0/ -->
+  </head>
+  <body>
+    <button>Click me!</button>
+    <ul></ul>
+  </body>
+  <script type="text/javascript">
+    window.addEventListener("load", function() {
+      function load() {
+        try {
+          debugger;
+          throw new Error("boom");
+        } catch (e) {
+          var list = document.querySelector("ul");
+          var item = document.createElement("li");
+          item.innerHTML = e.message;
+          list.appendChild(item);
+        }
+      }
+      var button = document.querySelector("button");
+      button.addEventListener("click", load, false);
+    });
+  </script>
+</html>

browser/devtools/debugger/test/browser_dbg_pause-exceptions.js

@@ -0,0 +1,115 @@
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Make sure that the pause-on-exceptions toggle works.
+ */
+
+const TAB_URL = EXAMPLE_URL + "browser_dbg_pause-exceptions.html";
+
+var gPane = null;
+var gTab = null;
+var gDebugger = null;
+var gCount = 0;
+
+function test()
+{
+  debug_tab_pane(TAB_URL, function(aTab, aDebuggee, aPane) {
+    gTab = aTab;
+    gPane = aPane;
+    gDebugger = gPane.contentWindow;
+
+    testWithFrame();
+  });
+}
+
+function testWithFrame()
+{
+  gPane.contentWindow.gClient.addOneTimeListener("paused", function() {
+    gDebugger.addEventListener("Debugger:FetchedVariables", function testA() {
+      // We expect 2 Debugger:FetchedVariables events, one from the global object
+      // scope and the regular one.
+      if (++gCount <2) {
+        is(gCount, 1, "A. First Debugger:FetchedVariables event received.");
+        return;
+      }
+      is(gCount, 2, "A. Second Debugger:FetchedVariables event received.");
+      gDebugger.removeEventListener("Debugger:FetchedVariables", testA, false);
+
+      is(gDebugger.DebuggerController.activeThread.state, "paused",
+        "Should be paused now.");
+
+      EventUtils.sendMouseEvent({ type: "click" },
+        gDebugger.document.getElementById("pause-exceptions"),
+        gDebugger);
+
+      is(gDebugger.DebuggerController.StackFrames.pauseOnExceptions, true,
+        "The option should be enabled now.");
+
+      gCount = 0;
+      gPane.contentWindow.gClient.addOneTimeListener("resumed", function() {
+        gDebugger.addEventListener("Debugger:FetchedVariables", function testB() {
+          // We expect 2 Debugger:FetchedVariables events, one from the global object
+          // scope and the regular one.
+          if (++gCount <2) {
+            is(gCount, 1, "B. First Debugger:FetchedVariables event received.");
+            return;
+          }
+          is(gCount, 2, "B. Second Debugger:FetchedVariables event received.");
+          gDebugger.removeEventListener("Debugger:FetchedVariables", testB, false);
+          Services.tm.currentThread.dispatch({ run: function() {
+
+            var frames = gDebugger.DebuggerView.StackFrames._frames,
+                scopes = gDebugger.DebuggerView.Properties._vars,
+                innerScope = scopes.firstChild,
+                innerNodes = innerScope.querySelector(".details").childNodes;
+
+            is(gDebugger.DebuggerController.activeThread.state, "paused",
+              "Should only be getting stack frames while paused.");
+
+            is(frames.querySelectorAll(".dbg-stackframe").length, 1,
+              "Should have one frame.");
+
+            is(scopes.children.length, 3, "Should have 3 variable scopes.");
+
+            is(innerNodes[0].querySelector(".name").getAttribute("value"), "<exception>",
+              "Should have the right property name for the exception.");
+
+            is(innerNodes[0].querySelector(".value").getAttribute("value"), "[object Error]",
+              "Should have the right property value for the exception.");
+
+            resumeAndFinish();
+          }}, 0);
+        }, false);
+      });
+
+      EventUtils.sendMouseEvent({ type: "click" },
+        gDebugger.document.getElementById("resume"),
+        gDebugger);
+    }, false);
+  });
+
+  EventUtils.sendMouseEvent({ type: "click" },
+    content.document.querySelector("button"),
+    content.window);
+}
+
+function resumeAndFinish() {
+  gPane.contentWindow.gClient.addOneTimeListener("resumed", function() {
+    Services.tm.currentThread.dispatch({ run: function() {
+
+      closeDebuggerAndFinish(false);
+    }}, 0);
+  });
+
+  // Resume to let the exception reach it's catch clause.
+  gDebugger.DebuggerController.activeThread.resume();
+}
+
+registerCleanupFunction(function() {
+  removeTab(gTab);
+  gPane = null;
+  gTab = null;
+  gDebugger = null;
+});

browser/locales/en-US/chrome/browser/devtools/debugger.dtd

@@ -31,6 +31,10 @@
   -  the button that closes the debugger UI. -->
 <!ENTITY debuggerUI.closeButton.tooltip        "Close">
 
+<!-- LOCALIZATION NOTE (debuggerUI.pauseExceptions): This is the label for the
+  -  checkbox that toggles pausing on exceptions. -->
+<!ENTITY debuggerUI.pauseExceptions          "Pause on exceptions">
+
 <!-- LOCALIZATION NOTE (debuggerUI.stepOverButton.tooltip): This is the tooltip for
   -  the button that steps over a function call. -->
 <!ENTITY debuggerUI.stepOverButton.tooltip   "Step Over">

toolkit/devtools/debugger/dbg-client.jsm

@@ -498,6 +498,8 @@ ThreadClient.prototype = {
   get state() { return this._state; },
   get paused() { return this._state === "paused"; },
 
+  _pauseOnExceptions: false,
+
   _actor: null,
   get actor() { return this._actor; },
 
@@ -525,8 +527,12 @@ ThreadClient.prototype = {
     this._state = "resuming";
 
     let self = this;
-    let packet = { to: this._actor, type: DebugProtocolTypes.resume,
-                   resumeLimit: aLimit };
+    let packet = {
+      to: this._actor,
+      type: DebugProtocolTypes.resume,
+      resumeLimit: aLimit,
+      pauseOnExceptions: this._pauseOnExceptions
+    };
     this._client.request(packet, function(aResponse) {
       if (aResponse.error) {
         // There was an error resuming, back to paused state.
@@ -583,6 +589,31 @@ ThreadClient.prototype = {
     });
   },
 
+  /**
+   * Enable or disable pausing when an exception is thrown.
+   *
+   * @param boolean aFlag
+   *        Enables pausing if true, disables otherwise.
+   * @param function aOnResponse
+   *        Called with the response packet.
+   */
+  pauseOnExceptions: function TC_pauseOnExceptions(aFlag, aOnResponse) {
+    this._pauseOnExceptions = aFlag;
+    // If the debuggee is paused, the value of the flag will be communicated in
+    // the next resumption. Otherwise we have to force a pause in order to send
+    // the flag.
+    if (!this.paused) {
+      this.interrupt(function(aResponse) {
+        if (aResponse.error) {
+          // Can't continue if pausing failed.
+          aOnResponse(aResponse);
+          return;
+        }
+        this.resume(aOnResponse);
+      }.bind(this));
+    }
+  },
+
   /**
    * Send a clientEvaluate packet to the debuggee. Response
    * will be a resume packet.

toolkit/devtools/debugger/server/dbg-script-actors.js

@@ -259,6 +259,10 @@ ThreadActor.prototype = {
                    message: "Unknown resumeLimit type" };
       }
     }
+
+    if (aRequest && aRequest.pauseOnExceptions) {
+      this.dbg.onExceptionUnwind = this.onExceptionUnwind.bind(this);
+    }
     let packet = this._resumed();
     DebuggerServer.xpcInspector.exitNestedEventLoop();
     return packet;
@@ -589,6 +593,7 @@ ThreadActor.prototype = {
 
     // Clear stepping hooks.
     this.dbg.onEnterFrame = undefined;
+    this.dbg.onExceptionUnwind = undefined;
     if (aFrame) {
       aFrame.onStep = undefined;
       aFrame.onPop = undefined;
@@ -855,6 +860,33 @@ ThreadActor.prototype = {
     return this._pauseAndRespond(aFrame, { type: "debuggerStatement" });
   },
 
+  /**
+   * A function that the engine calls when an exception has been thrown and has
+   * propagated to the specified frame.
+   *
+   * @param aFrame Debugger.Frame
+   *        The youngest remaining stack frame.
+   * @param aValue object
+   *        The exception that was thrown.
+   */
+  onExceptionUnwind: function TA_onExceptionUnwind(aFrame, aValue) {
+    try {
+      let packet = this._paused(aFrame);
+      if (!packet) {
+        return undefined;
+      }
+
+      packet.why = { type: "exception",
+                     exception: this.createValueGrip(aValue) };
+      this.conn.send(packet);
+      return this._nest();
+    } catch(e) {
+      Cu.reportError("Got an exception during TA_onExceptionUnwind: " + e +
+                     ": " + e.stack);
+      return undefined;
+    }
+  },
+
   /**
    * A function that the engine calls when a new script has been loaded into the
    * scope of the specified debuggee global.

toolkit/devtools/debugger/tests/unit/test_pause_exceptions-01.js

@@ -0,0 +1,51 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Test that setting pauseOnExceptions to true will cause the debuggee to pause
+ * when an exceptions is thrown.
+ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-stack");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function() {
+    attachTestGlobalClientAndResume(gClient, "test-stack", function(aResponse, aThreadClient) {
+      gThreadClient = aThreadClient;
+      test_pause_frame();
+    });
+  });
+  do_test_pending();
+}
+
+function test_pause_frame()
+{
+  gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
+    gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
+      do_check_eq(aPacket.why.type, "exception");
+      do_check_eq(aPacket.why.exception, 42);
+      gThreadClient.resume(function () {
+        finishClient(gClient);
+      });
+    });
+    gThreadClient.pauseOnExceptions(true);
+    gThreadClient.resume();
+  });
+
+  gDebuggee.eval("(" + function() {
+    function stopMe() {
+      debugger;
+      throw 42;
+    };
+    try {
+      stopMe();
+    } catch (e) {}
+    ")"
+  } + ")()");
+}

toolkit/devtools/debugger/tests/unit/test_pause_exceptions-02.js

@@ -0,0 +1,48 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * 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.
+ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-stack");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function() {
+    attachTestGlobalClientAndResume(gClient, "test-stack", function(aResponse, aThreadClient) {
+      gThreadClient = aThreadClient;
+      test_pause_frame();
+    });
+  });
+  do_test_pending();
+}
+
+function test_pause_frame()
+{
+  gThreadClient.pauseOnExceptions(true, function () {
+    gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
+      do_check_eq(aPacket.why.type, "exception");
+      do_check_eq(aPacket.why.exception, 42);
+      gThreadClient.resume(function () {
+        finishClient(gClient);
+      });
+    });
+
+    gDebuggee.eval("(" + function() {
+      function stopMe() {
+        throw 42;
+      };
+      try {
+        stopMe();
+      } catch (e) {}
+      ")"
+    } + ")()");
+  });
+}

toolkit/devtools/debugger/tests/unit/xpcshell.ini

@@ -58,3 +58,5 @@ tail =
 [test_framebindings-03.js]
 [test_framebindings-04.js]
 [test_framebindings-05.js]
+[test_pause_exceptions-01.js]
+[test_pause_exceptions-02.js]