Diff
Modified: trunk/LayoutTests/ChangeLog (159109 => 159110)
--- trunk/LayoutTests/ChangeLog 2013-11-12 15:23:12 UTC (rev 159109)
+++ trunk/LayoutTests/ChangeLog 2013-11-12 15:41:55 UTC (rev 159110)
@@ -1,3 +1,35 @@
+2013-11-12 Alexandru Chiculita <[email protected]>
+
+ 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 <[email protected]>
Unreviewed GTK gardening. Share expectations for vertical text among WK1 and WK2.
Modified: trunk/LayoutTests/http/tests/inspector-protocol/resources/protocol-test.js (159109 => 159110)
--- trunk/LayoutTests/http/tests/inspector-protocol/resources/protocol-test.js 2013-11-12 15:23:12 UTC (rev 159109)
+++ trunk/LayoutTests/http/tests/inspector-protocol/resources/protocol-test.js 2013-11-12 15:41:55 UTC (rev 159110)
@@ -58,7 +58,11 @@
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()
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-detach-expected.txt (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-detach-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-detach-expected.txt 2013-11-12 15:41:55 UTC (rev 159110)
@@ -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.
+
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-detach.html (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-detach.html (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-detach.html 2013-11-12 15:41:55 UTC (rev 159110)
@@ -0,0 +1,62 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<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>
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception-expected.txt (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception-expected.txt 2013-11-12 15:41:55 UTC (rev 159110)
@@ -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.
+
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception.html (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception.html (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-action-with-exception.html 2013-11-12 15:41:55 UTC (rev 159110)
@@ -0,0 +1,60 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<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>
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach-expected.txt (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach-expected.txt 2013-11-12 15:41:55 UTC (rev 159110)
@@ -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.
+
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach.html (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach.html (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-detach.html 2013-11-12 15:41:55 UTC (rev 159110)
@@ -0,0 +1,63 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<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>
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception-expected.txt (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception-expected.txt 2013-11-12 15:41:55 UTC (rev 159110)
@@ -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.
+
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception.html (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception.html (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-condition-with-exception.html 2013-11-12 15:41:55 UTC (rev 159110)
@@ -0,0 +1,59 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<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>
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-eval-with-exception-expected.txt (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-eval-with-exception-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-eval-with-exception-expected.txt 2013-11-12 15:41:55 UTC (rev 159110)
@@ -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.
+
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-eval-with-exception.html (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-eval-with-exception.html (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-eval-with-exception.html 2013-11-12 15:41:55 UTC (rev 159110)
@@ -0,0 +1,69 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<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: 3,
+ columnNumber: 0
+ }
+ };
+ InspectorTest.sendCommand("Debugger.setBreakpoint", breakpoint, InspectorTest.checkForError);
+ InspectorTest.sendCommand("Debugger.setPauseOnExceptions", {state: "all"}, InspectorTest.checkForError);
+ InspectorTest.sendCommand("Runtime.evaluate", {_expression_: "breakpointBasic()"}, function() {
+ completeTest();
+ });
+ }
+ }
+
+ InspectorTest.eventHandler["Debugger.paused"] = onBreakpointHit;
+
+ function onBreakpointHit(messageObject)
+ {
+ InspectorTest.log("PASS: Paused in debugger: reason = \"" + messageObject.params.reason + "\"");
+
+ // Make sure we do not pause again in the debugger.
+ InspectorTest.eventHandler["Debugger.paused"] = onPausedOnException;
+
+ InspectorTest.sendCommand("Runtime.evaluate", {_expression_: "this.should.trigger.an.exception"}, function(messageObject) {
+ InspectorTest.checkForError(messageObject);
+ InspectorTest.log("PASS: The debugger should not stop on the exception thrown by a console command while paused on a breakpoint.");
+ InspectorTest.sendCommand("Debugger.resume");
+ });
+ }
+
+ function onPausedOnException(messageObject)
+ {
+ InspectorTest.log("FAIL: Should not break on exceptions while paused on a breakpoint.");
+ completeTest();
+ }
+
+ function completeTest()
+ {
+ // InspectorTest.sendCommand("Debugger.resume", {});
+ // Reset the pauseOnException state before ending the test.
+ InspectorTest.sendCommand("Debugger.setPauseOnExceptions", {state: "none"});
+ InspectorTest.sendCommand("Debugger.disable");
+ 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>While being paused on a breakpoint the debugger should ignore the exceptions from console commands.</p>
+</body>
+</html>
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-inside-conditons-and-actions-expected.txt (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-inside-conditons-and-actions-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-inside-conditons-and-actions-expected.txt 2013-11-12 15:41:55 UTC (rev 159110)
@@ -0,0 +1,13 @@
+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)
+Executing condition
+inside breakpointBasic
+Condition executed
+Executing action
+inside breakpointBasic
+Action executed
+PASS: Breakpoint condition and action executed, but the exception did not cause the debugger to pause.
+
Added: trunk/LayoutTests/inspector-protocol/debugger/breakpoint-inside-conditons-and-actions.html (0 => 159110)
--- trunk/LayoutTests/inspector-protocol/debugger/breakpoint-inside-conditons-and-actions.html (rev 0)
+++ trunk/LayoutTests/inspector-protocol/debugger/breakpoint-inside-conditons-and-actions.html 2013-11-12 15:41:55 UTC (rev 159110)
@@ -0,0 +1,73 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<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: "log('Executing condition'); breakpointBasic(); log('Condition executed'); true;",
+ actions: [
+ {type: "evaluate", data: "log('Executing action'); breakpointBasic(); log('Action executed');"}
+ ]
+ }
+ };
+
+ // This breakpoint should not be hit at all. The only callers are the condition and action
+ // and breakpoints should be disabled while executing their code.
+ var breakpointInsideConditonAndAction = {
+ location: {
+ scriptId: messageObject.params.scriptId,
+ lineNumber: 3,
+ columnNumber: 0
+ }
+ };
+
+ InspectorTest.sendCommand("Debugger.setBreakpoint", breakpoint, InspectorTest.checkForError);
+ InspectorTest.sendCommand("Debugger.setBreakpoint", breakpointInsideConditonAndAction, 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 condition and 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>
Modified: trunk/Source/_javascript_Core/ChangeLog (159109 => 159110)
--- trunk/Source/_javascript_Core/ChangeLog 2013-11-12 15:23:12 UTC (rev 159109)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-11-12 15:41:55 UTC (rev 159110)
@@ -1,3 +1,26 @@
+2013-11-12 Alexandru Chiculita <[email protected]>
+
+ 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.
+
+ 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:
+
2013-11-12 Julien Brianceau <[email protected]>
InvalidIndex shouldn't be private in GPRInfo and FPRInfo for sh4, mips and arm64 architectures.
Modified: trunk/Source/_javascript_Core/debugger/Debugger.cpp (159109 => 159110)
--- trunk/Source/_javascript_Core/debugger/Debugger.cpp 2013-11-12 15:23:12 UTC (rev 159109)
+++ trunk/Source/_javascript_Core/debugger/Debugger.cpp 2013-11-12 15:41:55 UTC (rev 159110)
@@ -115,7 +115,26 @@
Debugger& m_debugger;
};
+// This is very similar to TemporaryChange<bool>, but that cannot be used
+// as the m_isPaused field uses only one bit.
+class TemporaryPausedState {
+public:
+ TemporaryPausedState(Debugger& debugger)
+ : m_debugger(debugger)
+ {
+ ASSERT(!m_debugger.m_isPaused);
+ m_debugger.m_isPaused = true;
+ }
+ ~TemporaryPausedState()
+ {
+ m_debugger.m_isPaused = false;
+ }
+
+private:
+ Debugger& m_debugger;
+};
+
Debugger::Debugger(bool isInWorkerThread)
: m_pauseOnExceptionsState(DontPauseOnExceptions)
, m_pauseOnNextStatement(false)
@@ -264,7 +283,7 @@
updateNeedForOpDebugCallbacks();
}
-bool Debugger::hasBreakpoint(SourceID sourceID, const TextPosition& position, Breakpoint *hitBreakpoint) const
+bool Debugger::hasBreakpoint(SourceID sourceID, const TextPosition& position, Breakpoint *hitBreakpoint)
{
if (!m_breakpointsActivated)
return false;
@@ -303,13 +322,23 @@
if (breakpoints[i].condition.isEmpty())
return true;
+ // We cannot stop in the debugger while executing condition code,
+ // so make it looks like the debugger is already paused.
+ TemporaryPausedState pausedState(*this);
+
JSValue exception;
JSValue result = DebuggerCallFrame::evaluateWithCallFrame(m_currentCallFrame, breakpoints[i].condition, exception);
+
+ // We can lose the debugger while executing _javascript_.
+ if (!m_currentCallFrame)
+ return false;
+
if (exception) {
// An erroneous condition counts as "false".
handleExceptionInBreakpointCondition(m_currentCallFrame, exception);
return false;
}
+
return result.toBoolean(m_currentCallFrame);
}
@@ -427,16 +456,20 @@
DebuggerCallFrameScope debuggerCallFrameScope(*this);
+ // Make sure we are not going to pause again on breakpoint actions by
+ // reseting the pause state before executing any breakpoint actions.
+ TemporaryPausedState pausedState(*this);
+ m_pauseOnCallFrame = 0;
+ m_pauseOnNextStatement = false;
+
if (didHitBreakpoint) {
handleBreakpointHit(breakpoint);
- if (breakpoint.autoContinue)
+ // Note that the actions can potentially stop the debugger, so we need to check that
+ // we still have a current call frame when we get back.
+ if (breakpoint.autoContinue || !m_currentCallFrame)
return;
}
- m_pauseOnCallFrame = 0;
- m_pauseOnNextStatement = false;
- m_isPaused = true;
-
handlePause(m_reasonForPause, dynamicGlobalObject);
if (!m_pauseOnNextStatement && !m_pauseOnCallFrame) {
@@ -444,8 +477,6 @@
if (!needsOpDebugCallbacks())
m_currentCallFrame = 0;
}
-
- m_isPaused = false;
}
void Debugger::exception(CallFrame* callFrame, JSValue exception, bool hasHandler)
Modified: trunk/Source/_javascript_Core/debugger/Debugger.h (159109 => 159110)
--- trunk/Source/_javascript_Core/debugger/Debugger.h 2013-11-12 15:23:12 UTC (rev 159109)
+++ trunk/Source/_javascript_Core/debugger/Debugger.h 2013-11-12 15:41:55 UTC (rev 159110)
@@ -144,7 +144,7 @@
Debugger& m_debugger;
};
- bool hasBreakpoint(SourceID, const TextPosition&, Breakpoint* hitBreakpoint) const;
+ bool hasBreakpoint(SourceID, const TextPosition&, Breakpoint* hitBreakpoint);
bool shouldPause() const { return m_shouldPause; }
void setShouldPause(bool);
@@ -185,6 +185,7 @@
RefPtr<JSC::DebuggerCallFrame> m_currentDebuggerCallFrame;
friend class DebuggerCallFrameScope;
+ friend class TemporaryPausedState;
friend class LLIntOffsetsExtractor;
};
