Bug 773590 - Display the return value when stepping out of a function; r=vporof

browser/devtools/debugger/debugger-controller.js

@@ -428,6 +428,7 @@ StackFrames.prototype = {
   currentBreakpointLocation: null,
   currentEvaluation: null,
   currentException: null,
+  currentReturnedValue: null,
 
   /**
    * Connect to the current thread client.
@@ -485,6 +486,17 @@ StackFrames.prototype = {
       case "exception":
         this.currentException = aPacket.why.exception;
         break;
+      // If paused while stepping out of a frame, store the returned value or
+      // thrown exception.
+      case "resumeLimit":
+        if (!aPacket.why.frameFinished) {
+          break;
+        } else if (aPacket.why.frameFinished.throw) {
+          this.currentException = aPacket.why.frameFinished.throw;
+        } else if (aPacket.why.frameFinished.return) {
+          this.currentReturnedValue = aPacket.why.frameFinished.return;
+        }
+        break;
     }
 
     this.activeThread.fillFrames(CALL_STACK_PAGE_SIZE);
@@ -595,6 +607,7 @@ StackFrames.prototype = {
     this.currentBreakpointLocation = null;
     this.currentEvaluation = null;
     this.currentException = null;
+    this.currentReturnedValue = 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.
@@ -832,6 +845,11 @@ StackFrames.prototype = {
       let excRef = aScope.addVar("<exception>", { value: this.currentException });
       this._addVarExpander(excRef, this.currentException);
     }
+    // Add any returned value.
+    if (this.currentReturnedValue) {
+      let retRef = aScope.addVar("<return>", { value: this.currentReturnedValue });
+      this._addVarExpander(retRef, this.currentReturnedValue);
+    }
     // Add "this".
     if (aFrame.this) {
       let thisRef = aScope.addVar("this", { value: aFrame.this });

browser/devtools/debugger/test/Makefile.in

@@ -102,6 +102,7 @@ MOCHITEST_BROWSER_TESTS = \
 	browser_dbg_chrome-debugging.js \
 	browser_dbg_source_maps-01.js \
 	browser_dbg_source_maps-02.js \
+	browser_dbg_step-out.js \
 	head.js \
 	$(NULL)
 
@@ -139,6 +140,7 @@ MOCHITEST_BROWSER_PAGES = \
 	binary_search.map \
 	test-location-changes-bp.js \
 	test-location-changes-bp.html \
+	test-step-out.html \
 	$(NULL)
 
 ifneq (Linux,$(OS_ARCH))

browser/devtools/debugger/test/browser_dbg_step-out.js

@@ -0,0 +1,133 @@
+/* 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 stepping out of a function displays the right return value.
+ */
+
+const TAB_URL = EXAMPLE_URL + "test-step-out.html";
+
+var gPane = null;
+var gTab = null;
+var gDebugger = null;
+
+function test()
+{
+  debug_tab_pane(TAB_URL, function(aTab, aDebuggee, aPane) {
+    gTab = aTab;
+    gPane = aPane;
+    gDebugger = gPane.panelWin;
+
+    expectUncaughtException();
+    testNormalReturn();
+  });
+}
+
+function testNormalReturn()
+{
+  gPane.panelWin.gClient.addOneTimeListener("paused", function() {
+    gDebugger.addEventListener("Debugger:SourceShown", function dbgstmt(aEvent) {
+      gDebugger.removeEventListener(aEvent.type, dbgstmt);
+      is(gDebugger.DebuggerController.activeThread.state, "paused",
+        "Should be paused now.");
+
+      let count = 0;
+      gPane.panelWin.gClient.addOneTimeListener("paused", function() {
+        is(gDebugger.DebuggerController.activeThread.state, "paused",
+          "Should be paused again.");
+
+        gDebugger.addEventListener("Debugger:FetchedVariables", function stepout() {
+          ok(true, "Debugger:FetchedVariables event received.");
+          gDebugger.removeEventListener("Debugger:FetchedVariables", stepout, false);
+
+          Services.tm.currentThread.dispatch({ run: function() {
+
+            var scopes = gDebugger.DebuggerView.Variables._list,
+                innerScope = scopes.firstChild,
+                innerNodes = innerScope.querySelector(".variables-view-element-details").childNodes;
+
+            is(innerNodes[0].querySelector(".name").getAttribute("value"), "<return>",
+              "Should have the right property name for the return value.");
+
+            is(innerNodes[0].querySelector(".value").getAttribute("value"), 10,
+              "Should have the right property value for the return value.");
+
+            testReturnWithException();
+          }}, 0);
+        }, false);
+      });
+
+      EventUtils.sendMouseEvent({ type: "mousedown" },
+        gDebugger.document.getElementById("step-out"),
+        gDebugger);
+    });
+  });
+
+  EventUtils.sendMouseEvent({ type: "click" },
+    content.document.getElementById("return"),
+    content.window);
+}
+
+function testReturnWithException()
+{
+  gDebugger.DebuggerController.activeThread.resume(function() {
+    gPane.panelWin.gClient.addOneTimeListener("paused", function() {
+      gDebugger.addEventListener("Debugger:FetchedVariables", function dbgstmt(aEvent) {
+        gDebugger.removeEventListener(aEvent.type, dbgstmt, false);
+        is(gDebugger.DebuggerController.activeThread.state, "paused",
+          "Should be paused now.");
+
+        let count = 0;
+        gPane.panelWin.gClient.addOneTimeListener("paused", function() {
+          is(gDebugger.DebuggerController.activeThread.state, "paused",
+            "Should be paused again.");
+
+          gDebugger.addEventListener("Debugger:FetchedVariables", function stepout() {
+            ok(true, "Debugger:FetchedVariables event received.");
+            gDebugger.removeEventListener("Debugger:FetchedVariables", stepout, false);
+
+            Services.tm.currentThread.dispatch({ run: function() {
+
+              var scopes = gDebugger.DebuggerView.Variables._list,
+                  innerScope = scopes.firstChild,
+                  innerNodes = innerScope.querySelector(".variables-view-element-details").childNodes;
+
+              is(innerNodes[0].querySelector(".name").getAttribute("value"), "<exception>",
+                "Should have the right property name for the exception value.");
+
+              is(innerNodes[0].querySelector(".value").getAttribute("value"), '"boom"',
+                "Should have the right property value for the exception value.");
+
+              resumeAndFinish();
+
+            }}, 0);
+          }, false);
+        });
+
+        EventUtils.sendMouseEvent({ type: "mousedown" },
+          gDebugger.document.getElementById("step-out"),
+          gDebugger);
+      }, false);
+    });
+
+    EventUtils.sendMouseEvent({ type: "click" },
+      content.document.getElementById("throw"),
+      content.window);
+  });
+}
+
+function resumeAndFinish() {
+  gPane.panelWin.gClient.addOneTimeListener("resumed", function() {
+    Services.tm.currentThread.dispatch({ run: closeDebuggerAndFinish }, 0);
+  });
+
+  gDebugger.DebuggerController.activeThread.resume();
+}
+
+registerCleanupFunction(function() {
+  removeTab(gTab);
+  gPane = null;
+  gTab = null;
+  gDebugger = null;
+});

browser/devtools/debugger/test/test-step-out.html

@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <meta charset='utf-8'/>
+    <title>Debugger Step Out Return Value Test</title>
+    <!-- Any copyright is dedicated to the Public Domain.
+         http://creativecommons.org/publicdomain/zero/1.0/ -->
+  </head>
+  <body>
+    <button id="return">return</button>
+    <button id="throw">throw</button>
+  </body>
+  <script type="text/javascript">
+    window.addEventListener("load", function() {
+      function normal(aArg) {
+        debugger;
+        var r = 10;
+        return r;
+      }
+      function error(aArg) {
+        debugger;
+        var r = 10;
+        throw "boom";
+        return r;
+      }
+      var button = document.getElementById("return");
+      button.addEventListener("click", normal, false);
+      button = document.getElementById("throw");
+      button.addEventListener("click", error, false);
+    });
+  </script>
+</html>

browser/devtools/shared/widgets/VariablesView.jsm

@@ -2256,6 +2256,7 @@ ViewHelpers.create({ constructor: Variable, proto: Scope.prototype }, {
   _onInit: function(aImmediateFlag) {
     if (this._initialDescriptor.enumerable ||
         this._nameString == "this" ||
+        this._nameString == "<return>" ||
         this._nameString == "<exception>") {
       this.ownerView._lazyAppend(aImmediateFlag, true, this._target);
       this.ownerView._enumItems.push(this);
@@ -2433,6 +2434,9 @@ ViewHelpers.create({ constructor: Variable, proto: Scope.prototype }, {
     else if (name == "<exception>") {
       this._target.setAttribute("exception", "");
     }
+    else if (name == "<return>") {
+      this._target.setAttribute("return", "");
+    }
     else if (name == "__proto__") {
       this._target.setAttribute("proto", "");
     }

browser/themes/linux/devtools/widgets.css

@@ -494,7 +494,7 @@
 /* Non enumerable, configurable and writable variables and properties */
 
 .variable-or-property[proto] > .title > .name,
-.variable-or-property[non-enumerable]:not([self]):not([exception]) > .title > .name {
+.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]) > .title > .name {
   opacity: 0.5;
 }
 
@@ -531,6 +531,11 @@
   text-shadow: 0 0 8px #fcc;
 }
 
+.variable-or-property[return]:not(:focus) > .title > .name {
+  color: #0a0;
+  text-shadow: 0 0 8px #cfc;
+}
+
 .variable-or-property[non-extensible]:not([non-writable]) > .title:after {
   content: "N";
   display: inline-block;

browser/themes/osx/devtools/widgets.css

@@ -494,7 +494,7 @@
 /* Non enumerable, configurable and writable variables and properties */
 
 .variable-or-property[proto] > .title > .name,
-.variable-or-property[non-enumerable]:not([self]):not([exception]) > .title > .name {
+.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]) > .title > .name {
   opacity: 0.5;
 }
 
@@ -531,6 +531,11 @@
   text-shadow: 0 0 8px #fcc;
 }
 
+.variable-or-property[return]:not(:focus) > .title > .name {
+  color: #0a0;
+  text-shadow: 0 0 8px #cfc;
+}
+
 .variable-or-property[non-extensible]:not([non-writable]) > .title:after {
   content: "N";
   display: inline-block;

browser/themes/windows/devtools/widgets.css

@@ -497,7 +497,7 @@
 /* Non enumerable, configurable and writable variables and properties */
 
 .variable-or-property[proto] > .title > .name,
-.variable-or-property[non-enumerable]:not([self]):not([exception]) > .title > .name {
+.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]) > .title > .name {
   opacity: 0.5;
 }
 
@@ -534,6 +534,11 @@
   text-shadow: 0 0 8px #fcc;
 }
 
+.variable-or-property[return]:not(:focus) > .title > .name {
+  color: #0a0;
+  text-shadow: 0 0 8px #cfc;
+}
+
 .variable-or-property[non-extensible]:not([non-writable]) > .title:after {
   content: "N";
   display: inline-block;

toolkit/devtools/server/actors/script.js

@@ -327,8 +327,8 @@ ThreadActor.prototype = {
     if (aRequest && aRequest.resumeLimit) {
       // Bind these methods because some of the hooks are called with 'this'
       // set to the current frame.
-      let pauseAndRespond = aFrame => {
+      let pauseAndRespond = (aFrame, onPacket=function (k) k) => {
-        this._pauseAndRespond(aFrame, { type: "resumeLimit" });
+        this._pauseAndRespond(aFrame, { type: "resumeLimit" }, onPacket);
       };
       let createValueGrip = this.createValueGrip.bind(this);
 
@@ -352,7 +352,6 @@ ThreadActor.prototype = {
 
       let onPop = function TA_onPop(aCompletion) {
         // onPop is called with 'this' set to the current frame.
-
         if (thread.sources.isBlackBoxed(this.script.url)) {
           return undefined;
         }
@@ -361,7 +360,19 @@ ThreadActor.prototype = {
         // subsequent step events on its caller.
         this.reportedPop = true;
 
-        return pauseAndRespond(this);
+        return pauseAndRespond(this, (aPacket) => {
+          aPacket.why.frameFinished = {};
+          if (!aCompletion) {
+            aPacket.why.frameFinished.terminated = true;
+          } else if (aCompletion.hasOwnProperty("return")) {
+            aPacket.why.frameFinished.return = createValueGrip(aCompletion.return);
+          } else if (aCompletion.hasOwnProperty("yield")) {
+            aPacket.why.frameFinished.return = createValueGrip(aCompletion.yield);
+          } else {
+            aPacket.why.frameFinished.throw = createValueGrip(aCompletion.throw);
+          }
+          return aPacket;
+        });
       };
 
       let onStep = function TA_onStep() {

toolkit/devtools/server/tests/unit/test_stepping-06.js

@@ -0,0 +1,85 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Check that stepping out of a function returns the right return value.
+ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-stack");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function () {
+    attachTestTabAndResume(gClient, "test-stack", function (aResponse, aTabClient, aThreadClient) {
+      gThreadClient = aThreadClient;
+      test_simple_stepping();
+    });
+  });
+  do_test_pending();
+}
+
+function test_simple_stepping()
+{
+  gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+    gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+      // Check that the return value is 10.
+      do_check_eq(aPacket.type, "paused");
+      do_check_eq(aPacket.frame.where.line, gDebuggee.line0 + 4);
+      do_check_eq(aPacket.why.type, "resumeLimit");
+      do_check_eq(aPacket.why.frameFinished.return, 10);
+
+      gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+        gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+          // Check that the return value is undefined.
+          do_check_eq(aPacket.type, "paused");
+          do_check_eq(aPacket.frame.where.line, gDebuggee.line0 + 7);
+          do_check_eq(aPacket.why.type, "resumeLimit");
+          do_check_eq(aPacket.why.frameFinished.return.type, "undefined");
+
+          gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+            gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+              // Check that the exception was thrown.
+              do_check_eq(aPacket.type, "paused");
+              do_check_eq(aPacket.frame.where.line, gDebuggee.line0 + 12);
+              do_check_eq(aPacket.why.type, "resumeLimit");
+              do_check_eq(aPacket.why.frameFinished.throw, "ah");
+
+              gThreadClient.resume(function () {
+                finishClient(gClient);
+              });
+            });
+            gThreadClient.stepOut();
+          });
+          gThreadClient.resume();
+        });
+        gThreadClient.stepOut();
+      });
+      gThreadClient.resume();
+    });
+    gThreadClient.stepOut();
+
+  });
+
+  gDebuggee.eval("var line0 = Error().lineNumber;\n" +
+                 "function f() {\n" + // line0 + 1
+                 "  debugger;\n" +    // line0 + 2
+                 "  var a = 10;\n" +  // line0 + 3
+                 "  return a;\n" +    // line0 + 4
+                 "}\n" +              // line0 + 5
+                 "function g() {\n" + // line0 + 6
+                 "  debugger;\n" +    // line0 + 7
+                 "}\n" +              // line0 + 8
+                 "function h() {\n" + // line0 + 9
+                 "  debugger;\n" +    // line0 + 10
+                 "  throw 'ah';\n" +  // line0 + 11
+                 "  return 2;\n" +    // line0 + 12
+                 "}\n" +              // line0 + 13
+                 "f();\n" +           // line0 + 14
+                 "g();\n" +           // line0 + 15
+                 "h();\n");           // line0 + 16
+}

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

@@ -109,6 +109,7 @@ reason = bug 820380
 [test_stepping-03.js]
 [test_stepping-04.js]
 [test_stepping-05.js]
+[test_stepping-06.js]
 [test_framebindings-01.js]
 [test_framebindings-02.js]
 [test_framebindings-03.js]