Web Inspector: Crash when closing the Inspector while debugging an ex…

…ception inside a breakpoint condition.

https://bugs.webkit.org/show_bug.cgi?id=124078

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

The crash would happen because the Debugger is not designed to support nested
breaks. For example, when the debugger handles a breakpoint and the Inspector
executes a console command that would hit the breakpoint again, the Debugger
will just ignore the breakpoint.

There were no checks for conditions and actions. Because of that conditions and actions
could trigger exceptions and breakpoints. This patch disables that functionality as it
cannot be supported without a bigger rewrite of the code.

* debugger/Debugger.cpp:
(JSC::TemporaryPausedState::TemporaryPausedState):
(JSC::TemporaryPausedState::~TemporaryPausedState):
(JSC::Debugger::hasBreakpoint):
(JSC::Debugger::pauseIfNeeded):
* debugger/Debugger.h:

LayoutTests:

Checking that the debugger will not crash nor stall when exceptions are throw while the debugger
is already paused. The cases when that can happen include breakpoint conditions, actions, eval
or runtime object inspection.

The current behavior was to ignore the exceptions or breakpoints while executing "console commands"
when the debugger was already paused. I'm extending this mechanism to breakpoint conditions and
actions as the Debugger is not designed to support nested "debugger breaks".

* http/tests/inspector-protocol/resources/protocol-test.js:
(closeTest): Avoid having internals.closeDummyInspectorFrontend and testRunner.notifyDone
in the same function. The debugger will not have a chance to exit the temporary EventLoop
before loading the next test.
* inspector-protocol/debugger/breakpoint-action-detach-expected.txt: Added.
* inspector-protocol/debugger/breakpoint-action-detach.html: Added.
* inspector-protocol/debugger/breakpoint-action-with-exception-expected.txt: Added.
* inspector-protocol/debugger/breakpoint-action-with-exception.html: Added.
* inspector-protocol/debugger/breakpoint-condition-detach-expected.txt: Added.
* inspector-protocol/debugger/breakpoint-condition-detach.html: Added.
* inspector-protocol/debugger/breakpoint-condition-with-exception-expected.txt: Added.
* inspector-protocol/debugger/breakpoint-condition-with-exception.html: Added.
* inspector-protocol/debugger/breakpoint-eval-with-exception-expected.txt: Added.
* inspector-protocol/debugger/breakpoint-eval-with-exception.html: Added.
* inspector-protocol/debugger/breakpoint-inside-conditons-and-actions-expected.txt: Added.
* inspector-protocol/debugger/breakpoint-inside-conditons-and-actions.html: Added.


Canonical link: https://commits.webkit.org/142396@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@159110 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,35 @@
+2013-11-12  Alexandru Chiculita  <achicu@adobe.com>
+
+        Web Inspector: Crash when closing the Inspector while debugging an exception inside a breakpoint condition.
+        https://bugs.webkit.org/show_bug.cgi?id=124078
+
+        Reviewed by Joseph Pecoraro.
+
+        Checking that the debugger will not crash nor stall when exceptions are throw while the debugger
+        is already paused. The cases when that can happen include breakpoint conditions, actions, eval
+        or runtime object inspection.
+
+        The current behavior was to ignore the exceptions or breakpoints while executing "console commands"
+        when the debugger was already paused. I'm extending this mechanism to breakpoint conditions and
+        actions as the Debugger is not designed to support nested "debugger breaks".
+
+        * http/tests/inspector-protocol/resources/protocol-test.js:
+        (closeTest): Avoid having internals.closeDummyInspectorFrontend and testRunner.notifyDone
+        in the same function. The debugger will not have a chance to exit the temporary EventLoop
+        before loading the next test.
+        * inspector-protocol/debugger/breakpoint-action-detach-expected.txt: Added.
+        * inspector-protocol/debugger/breakpoint-action-detach.html: Added.
+        * inspector-protocol/debugger/breakpoint-action-with-exception-expected.txt: Added.
+        * inspector-protocol/debugger/breakpoint-action-with-exception.html: Added.
+        * inspector-protocol/debugger/breakpoint-condition-detach-expected.txt: Added.
+        * inspector-protocol/debugger/breakpoint-condition-detach.html: Added.
+        * inspector-protocol/debugger/breakpoint-condition-with-exception-expected.txt: Added.
+        * inspector-protocol/debugger/breakpoint-condition-with-exception.html: Added.
+        * inspector-protocol/debugger/breakpoint-eval-with-exception-expected.txt: Added.
+        * inspector-protocol/debugger/breakpoint-eval-with-exception.html: Added.
+        * inspector-protocol/debugger/breakpoint-inside-conditons-and-actions-expected.txt: Added.
+        * inspector-protocol/debugger/breakpoint-inside-conditons-and-actions.html: Added.
+
 2013-11-12  Mario Sanchez Prada  <mario.prada@samsung.com>
 
         Unreviewed GTK gardening. Share expectations for vertical text among WK1 and WK2.

LayoutTests/http/tests/inspector-protocol/resources/protocol-test.js

@@ -58,7 +58,11 @@ function log(text)
 function closeTest()
 {
     window.internals.closeDummyInspectorFrontend();
-    testRunner.notifyDone();
+    // This code might be executed while the debugger is still running through a stack based EventLoop.
+    // Use a setTimeout to defer to a clean stack before letting the testRunner load the next test.
+    setTimeout(function() {
+        testRunner.notifyDone();
+    }, 0);
 }
 
 function runTest()

LayoutTests/inspector-protocol/debugger/breakpoint-action-detach-expected.txt

@@ -0,0 +1,8 @@
+Closing the inspector while executing the code for a breakpoint action should continue executing code.
+
+Breakpoints Enabled
+Found breakpoint.js
+inside breakpointWithCondition a:(1) b:(2) a+b:(3)
+Closing the inspector.
+PASS: Test did not crash after debugger disconnected.
+

LayoutTests/inspector-protocol/debugger/breakpoint-action-detach.html

@@ -0,0 +1,62 @@
+<html>
+<head>
+<script src="../../http/tests/inspector-protocol/resources/protocol-test.js"></script>
+<script src="resources/breakpoint.js"></script>
+<script>
+function runBreakpointWithCondition()
+{
+    // The debugger should hit a breakpoint inside the breakpointWithCondition call.
+    breakpointWithCondition(1, 2);
+    // If we get here it means that the debugger was disconnected and the execution continued as usual.
+    log("PASS: Test did not crash after debugger disconnected.");
+    testRunner.notifyDone();
+}
+
+// This function is called by the breakpoint action.
+function disconnect()
+{
+    log("Closing the inspector.");
+    window.internals.closeDummyInspectorFrontend();
+}
+
+function test()
+{
+    InspectorTest.eventHandler["Debugger.scriptParsed"] = function(messageObject)
+    {
+        if (/resources\/breakpoint\.js$/.test(messageObject.params.url)) {
+            InspectorTest.log("Found breakpoint.js");
+            var breakpoint = {
+                location: {
+                    scriptId: messageObject.params.scriptId,
+                    lineNumber: 8,
+                    columnNumber: 0
+                },
+                options: {
+                    actions: [
+                        {type: "evaluate", data: "disconnect()"},
+                        {type: "evaluate", data: "log('FAIL: This action should not be executed.')"}
+                    ]
+                }
+            };
+            InspectorTest.sendCommand("Debugger.setBreakpoint", breakpoint, InspectorTest.checkForError);
+            InspectorTest.sendCommand("Runtime.evaluate", {expression: "runBreakpointWithCondition()"});
+        }
+    }
+
+    InspectorTest.eventHandler["Debugger.paused"] = function(messageObject)
+    {
+        InspectorTest.log("FAIL: Paused in debugger: reason = \"" + messageObject.params.reason + "\"");
+        InspectorTest.completeTest();
+    }
+
+    InspectorTest.sendCommand("Debugger.enable", {});
+    InspectorTest.sendCommand("Debugger.setBreakpointsActive", {active: true}, function() {
+        InspectorTest.log("Breakpoints Enabled");
+    });
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Closing the inspector while executing the code for a breakpoint action should continue executing code.</p>
+</body>
+</html>

LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception-expected.txt

@@ -0,0 +1,8 @@
+CONSOLE MESSAGE: line 1: TypeError: undefined is not an object (evaluating 'this.will.cause')
+The debugger should not pause on exceptions thrown while executing the breakpoint actions.
+
+Breakpoints Enabled
+Found breakpoint.js
+inside breakpointWithCondition a:(1) b:(2) a+b:(3)
+PASS: Breakpoint action executed, but the exception did not cause the debugger to pause.
+

LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception.html

@@ -0,0 +1,60 @@
+<html>
+<head>
+<script src="../../http/tests/inspector-protocol/resources/protocol-test.js"></script>
+<script src="resources/breakpoint.js"></script>
+<script>
+function test()
+{
+    InspectorTest.eventHandler["Debugger.scriptParsed"] = function(messageObject)
+    {
+        if (/resources\/breakpoint\.js$/.test(messageObject.params.url)) {
+            InspectorTest.log("Found breakpoint.js");
+
+            var breakpoint = {
+                location: {
+                    scriptId: messageObject.params.scriptId,
+                    lineNumber: 8,
+                    columnNumber: 0
+                },
+                options: {
+                    autoContinue: true,
+                    actions: [
+                        {type: "evaluate", data: "this.will.cause.exception"}
+                    ]
+                }
+            };
+            InspectorTest.sendCommand("Debugger.setBreakpoint", breakpoint, InspectorTest.checkForError);
+            InspectorTest.sendCommand("Debugger.setPauseOnExceptions", {state: "all"}, InspectorTest.checkForError);
+            InspectorTest.sendCommand("Runtime.evaluate", {expression: "breakpointWithCondition(1,2)"}, function(messageObject) {
+                InspectorTest.checkForError(messageObject);
+                InspectorTest.log("PASS: Breakpoint action executed, but the exception did not cause the debugger to pause.")
+                completeTest();
+            });
+        }
+    }
+
+    InspectorTest.eventHandler["Debugger.paused"] = function(messageObject)
+    {
+        InspectorTest.log("FAIL: Paused in debugger: reason = \"" + messageObject.params.reason + "\"");
+        completeTest();
+    }
+
+    function completeTest()
+    {
+        // Reset the pauseOnException state before ending the test.
+        InspectorTest.sendCommand("Debugger.setPauseOnExceptions", {state: "none"});
+        InspectorTest.completeTest();
+    }
+
+    InspectorTest.sendCommand("Debugger.enable", {});
+    InspectorTest.sendCommand("Debugger.setBreakpointsActive", {active: true}, function(messageObject) {
+        InspectorTest.checkForError(messageObject);
+        InspectorTest.log("Breakpoints Enabled");
+    });
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>The debugger should not pause on exceptions thrown while executing the breakpoint actions.</p>
+</body>
+</html>

LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach-expected.txt

@@ -0,0 +1,8 @@
+Debugger.setBreakpoint with an action that throws an exception should not pause the debugger.
+
+Breakpoints Enabled
+Found breakpoint.js
+inside breakpointWithCondition a:(1) b:(2) a+b:(3)
+Closing the inspector.
+PASS: Test did not crash after debugger disconnected.
+

LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach.html

@@ -0,0 +1,63 @@
+<html>
+<head>
+<script src="../../http/tests/inspector-protocol/resources/protocol-test.js"></script>
+<script src="resources/breakpoint.js"></script>
+<script>
+function runBreakpointWithCondition()
+{
+    // The debugger should hit a breakpoint inside the breakpointWithCondition call.
+    breakpointWithCondition(1, 2);
+    // If we get here it means that the debugger was disconnected and the execution continued as usual.
+    log("PASS: Test did not crash after debugger disconnected.");
+    testRunner.notifyDone();
+}
+
+// This function is called by the breakpoint condition.
+function disconnect()
+{
+    log("Closing the inspector.");
+    window.internals.closeDummyInspectorFrontend();
+    // Throwing an exception will make the debugger execute some code using a dead context.
+    // Test passes if the debugger is not crashing.
+    throw new Error();
+}
+
+function test()
+{
+    InspectorTest.eventHandler["Debugger.scriptParsed"] = function(messageObject)
+    {
+        if (/resources\/breakpoint\.js$/.test(messageObject.params.url)) {
+            InspectorTest.log("Found breakpoint.js");
+            var breakpoint = {
+                location: {
+                    scriptId: messageObject.params.scriptId,
+                    lineNumber: 8,
+                    columnNumber: 0
+                },
+                options: {
+                    condition: "disconnect()"
+                }
+            };
+            InspectorTest.sendCommand("Debugger.setBreakpoint", breakpoint, InspectorTest.checkForError);
+            InspectorTest.sendCommand("Runtime.evaluate", {expression: "runBreakpointWithCondition()"});
+        }
+    }
+
+    InspectorTest.eventHandler["Debugger.paused"] = function(messageObject)
+    {
+        InspectorTest.log("FAIL: Paused in debugger: reason = \"" + messageObject.params.reason + "\"");
+        InspectorTest.completeTest();
+    }
+
+    InspectorTest.sendCommand("Debugger.enable", {});
+    InspectorTest.sendCommand("Debugger.setBreakpointsActive", {active: true}, function(messageObject) {
+        InspectorTest.checkForError(messageObject);
+        InspectorTest.log("Breakpoints Enabled");
+    });
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Debugger.setBreakpoint with an action that throws an exception should not pause the debugger.</p>
+</body>
+</html>

LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception-expected.txt

@@ -0,0 +1,8 @@
+CONSOLE MESSAGE: line 1: TypeError: undefined is not an object (evaluating 'this.will.cause')
+The debugger should not pause on exceptions thrown while executing the breakpoint conditions.
+
+Breakpoints Enabled
+Found breakpoint.js
+inside breakpointWithCondition a:(1) b:(2) a+b:(3)
+PASS: The debugger should not pause on the exception inside the condition.
+

LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception.html

@@ -0,0 +1,59 @@
+<html>
+<head>
+<script src="../../http/tests/inspector-protocol/resources/protocol-test.js"></script>
+<script src="resources/breakpoint.js"></script>
+<script>
+function test()
+{
+    InspectorTest.eventHandler["Debugger.scriptParsed"] = function(messageObject)
+    {
+        if (/resources\/breakpoint\.js$/.test(messageObject.params.url)) {
+            InspectorTest.log("Found breakpoint.js");
+            var breakpoint = {
+                location: {
+                    scriptId: messageObject.params.scriptId,
+                    lineNumber: 8,
+                    columnNumber: 0
+                },
+                options: {
+                    autoContinue: true,
+                    condition: "this.will.cause.exception",
+                    action: [
+                        {type: "log", data: "FAIL: The breakpoint action should not be executed if the condition had an exception."}
+                    ]
+                }
+            };
+            InspectorTest.sendCommand("Debugger.setBreakpoint", breakpoint, InspectorTest.checkForError);
+            InspectorTest.sendCommand("Debugger.setPauseOnExceptions", {state: "all"}, InspectorTest.checkForError);
+            InspectorTest.sendCommand("Runtime.evaluate", {expression: "breakpointWithCondition(1,2)"}, function() {
+                InspectorTest.log("PASS: The debugger should not pause on the exception inside the condition.");
+                completeTest();
+            });
+        }
+    }
+
+    InspectorTest.eventHandler["Debugger.paused"] = function(messageObject)
+    {
+        InspectorTest.log("FAIL: Paused in debugger: reason = \"" + messageObject.params.reason + "\"");
+        completeTest();
+    }
+
+    function completeTest()
+    {
+        // Reset the pauseOnException state before ending the test.
+        InspectorTest.sendCommand("Debugger.setPauseOnExceptions", {state: "none"});
+        InspectorTest.completeTest();
+    }
+
+    InspectorTest.sendCommand("Debugger.enable", {});
+    InspectorTest.sendCommand("Debugger.setBreakpointsActive", {active: true}, function(messageObject) {
+        InspectorTest.checkForError(messageObject);
+        InspectorTest.log("Breakpoints Enabled");
+    });
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>The debugger should not pause on exceptions thrown while executing the breakpoint conditions.</p>
+</body>
+</html>

LayoutTests/inspector-protocol/debugger/breakpoint-eval-with-exception-expected.txt

@@ -0,0 +1,8 @@
+While being paused on a breakpoint the debugger should ignore the exceptions from console commands.
+
+Breakpoints Enabled
+Found breakpoint.js
+inside breakpointBasic
+PASS: Paused in debugger: reason = "other"
+PASS: The debugger should not stop on the exception thrown by a console command while paused on a breakpoint.
+