From 3b9100236a1046d569747a0393d70721c1298695 Mon Sep 17 00:00:00 2001
From: Panos Astithas <past@mozilla.com>
Date: Wed, 12 Sep 2012 14:10:05 +0300
Subject: [PATCH] Bug 785689 - Debugger breaking when it shouldn't.
 r=msucan,vporof

---
 .../debugger/server/dbg-script-actors.js      | 34 +++----
 .../debugger/tests/unit/test_stepping-05.js   | 94 +++++++++++++++++++
 .../devtools/debugger/tests/unit/xpcshell.ini |  1 +
 3 files changed, 112 insertions(+), 17 deletions(-)
 create mode 100644 toolkit/devtools/debugger/tests/unit/test_stepping-05.js

diff --git a/toolkit/devtools/debugger/server/dbg-script-actors.js b/toolkit/devtools/debugger/server/dbg-script-actors.js
index 9c85f9345bb1..1f8018cb31e1 100644
--- a/toolkit/devtools/debugger/server/dbg-script-actors.js
+++ b/toolkit/devtools/debugger/server/dbg-script-actors.js
@@ -261,26 +261,26 @@ ThreadActor.prototype = {
         return undefined;
       }
 
-      switch (aRequest.resumeLimit.type) {
-        case "step":
-          this.dbg.onEnterFrame = onEnterFrame;
-          // Fall through.
-        case "next":
-          let stepFrame = this._getNextStepFrame(startFrame);
-          if (stepFrame) {
+      let steppingType = aRequest.resumeLimit.type;
+      if (["step", "next", "finish"].indexOf(steppingType) == -1) {
+            return { error: "badParameterType",
+                     message: "Unknown resumeLimit type" };
+      }
+      // Make sure there is still a frame on the stack if we are to continue
+      // stepping.
+      let stepFrame = this._getNextStepFrame(startFrame);
+      if (stepFrame) {
+        switch (steppingType) {
+          case "step":
+            this.dbg.onEnterFrame = onEnterFrame;
+            // Fall through.
+          case "next":
             stepFrame.onStep = onStep;
             stepFrame.onPop = onPop;
-          }
-          break;
-        case "finish":
-          stepFrame = this._getNextStepFrame(startFrame);
-          if (stepFrame) {
+            break;
+          case "finish":
             stepFrame.onPop = onPop;
-          }
-          break;
-        default:
-          return { error: "badParameterType",
-                   message: "Unknown resumeLimit type" };
+        }
       }
     }
 
diff --git a/toolkit/devtools/debugger/tests/unit/test_stepping-05.js b/toolkit/devtools/debugger/tests/unit/test_stepping-05.js
new file mode 100644
index 000000000000..75c15809009c
--- /dev/null
+++ b/toolkit/devtools/debugger/tests/unit/test_stepping-05.js
@@ -0,0 +1,94 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Make sure that stepping in the last statement of the last frame doesn't
+ * cause an unexpected pause, when another JS frame is pushed on the stack
+ * (bug 785689).
+ */
+
+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_stepping_last();
+    });
+  });
+  do_test_pending();
+}
+
+function test_stepping_last()
+{
+  gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+    gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+      // Check the return value.
+      do_check_eq(aPacket.type, "paused");
+      do_check_eq(aPacket.frame.where.line, gDebuggee.line0 + 2);
+      do_check_eq(aPacket.why.type, "resumeLimit");
+      // Check that stepping worked.
+      do_check_eq(gDebuggee.a, undefined);
+      do_check_eq(gDebuggee.b, undefined);
+
+      gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+        // Check the return value.
+        do_check_eq(aPacket.type, "paused");
+        do_check_eq(aPacket.frame.where.line, gDebuggee.line0 + 3);
+        do_check_eq(aPacket.why.type, "resumeLimit");
+        // Check that stepping worked.
+        do_check_eq(gDebuggee.a, 1);
+        do_check_eq(gDebuggee.b, undefined);
+
+        gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+          // Check the return value.
+          do_check_eq(aPacket.type, "paused");
+          // When leaving a stack frame the line number doesn't change.
+          do_check_eq(aPacket.frame.where.line, gDebuggee.line0 + 3);
+          do_check_eq(aPacket.why.type, "resumeLimit");
+          // Check that stepping worked.
+          do_check_eq(gDebuggee.a, 1);
+          do_check_eq(gDebuggee.b, 2);
+
+          gThreadClient.stepIn(function () {
+            test_next_pause();
+          });
+        });
+        gThreadClient.stepIn();
+      });
+      gThreadClient.stepIn();
+
+    });
+    gThreadClient.stepIn();
+
+  });
+
+  gDebuggee.eval("var line0 = Error().lineNumber;\n" +
+                 "debugger;\n" +   // line0 + 1
+                 "var a = 1;\n" +  // line0 + 2
+                 "var b = 2;\n");  // line0 + 3
+}
+
+function test_next_pause()
+{
+  gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+    // Check the return value.
+    do_check_eq(aPacket.type, "paused");
+    // Before fixing bug 785689, the type was resumeLimit.
+    do_check_eq(aPacket.why.type, "debuggerStatement");
+
+    gThreadClient.resume(function () {
+      finishClient(gClient);
+    });
+  });
+
+  gDebuggee.eval("debugger;");
+}
diff --git a/toolkit/devtools/debugger/tests/unit/xpcshell.ini b/toolkit/devtools/debugger/tests/unit/xpcshell.ini
index bd01b0013029..e1644c033792 100644
--- a/toolkit/devtools/debugger/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/debugger/tests/unit/xpcshell.ini
@@ -53,6 +53,7 @@ tail =
 [test_stepping-02.js]
 [test_stepping-03.js]
 [test_stepping-04.js]
+[test_stepping-05.js]
 [test_framebindings-01.js]
 [test_framebindings-02.js]
 [test_framebindings-03.js]