Diff
Modified: trunk/LayoutTests/ChangeLog (207266 => 207267)
--- trunk/LayoutTests/ChangeLog 2016-10-13 01:42:53 UTC (rev 207266)
+++ trunk/LayoutTests/ChangeLog 2016-10-13 02:05:44 UTC (rev 207267)
@@ -1,3 +1,16 @@
+2016-10-12 Joseph Pecoraro <pecor...@apple.com>
+
+ Web Inspector: step-into `console.log(o)` should not step through inspector _javascript_
+ https://bugs.webkit.org/show_bug.cgi?id=161656
+ <rdar://problem/28181123>
+
+ Reviewed by Timothy Hatcher.
+
+ * inspector/debugger/stepping/stepping-internal-scripts-expected.txt: Added.
+ * inspector/debugger/stepping/stepping-internal-scripts.html: Added.
+ Ensure step-into a console.log statement steps past it, and doesn't pause
+ inside the non-visible internal script.
+
2016-10-12 Yusuke Suzuki <utatane....@gmail.com>
Unreviewed, add expected file for new test after r207239
Modified: trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html (207266 => 207267)
--- trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html 2016-10-13 01:42:53 UTC (rev 207266)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html 2016-10-13 02:05:44 UTC (rev 207267)
@@ -21,7 +21,7 @@
name: "TopLevelSyntaxErrorDontCrash",
description: "Make sure exceptions from top-level syntax errors don't cause us to crash.",
test: (resolve, reject) => {
- InspectorProtocol.eventHandler["Debugger.paused"] = function(messageObject) {
+ InspectorProtocol.eventHandler["Debugger.paused"] = function(messageObject) {
InspectorProtocol.sendCommand("Debugger.resume");
ProtocolTest.pass("Paused on the breakpoint after syntax error at top level without crashing.");
@@ -37,6 +37,6 @@
</script>
</head>
<body _onload_="runTest()">
-<p> Making sure we don't crash when having a top-level syntax error. </p>
+<p>Making sure we don't crash when having a top-level syntax error.</p>
</body>
</html>
Added: trunk/LayoutTests/inspector/debugger/stepping/stepping-internal-scripts-expected.txt (0 => 207267)
--- trunk/LayoutTests/inspector/debugger/stepping/stepping-internal-scripts-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector/debugger/stepping/stepping-internal-scripts-expected.txt 2016-10-13 02:05:44 UTC (rev 207267)
@@ -0,0 +1,124 @@
+CONSOLE MESSAGE: line 9: [object Window]
+CONSOLE MESSAGE: line 10: [object Console]
+CONSOLE MESSAGE: line 11: string
+CONSOLE MESSAGE: line 9: [object Window]
+CONSOLE MESSAGE: line 10: [object Console]
+CONSOLE MESSAGE: line 11: string
+Checking pause locations when stepping in and over statements that make use of Internal scripts.
+
+
+== Running test suite: Debugger.stepping.internal
+-- Running test case: Debugger.stepping.internal.ConsoleLog.StepOver
+_expression_: setTimeout(entryConsoleLog)
+STEPS: over, over, over, over, resume
+PAUSED (debugger-statement)
+PAUSE AT entryConsoleLog:8:5
+ 4 <script src=""
+ 5 <script>
+ 6 function entryConsoleLog() {
+ -> 7 |debugger;
+ 8 console.log(window);
+ 9 console.log(console);
+ 10 console.log("string");
+
+ACTION: step-over
+PAUSE AT entryConsoleLog:9:5
+ 5 <script>
+ 6 function entryConsoleLog() {
+ 7 debugger;
+ -> 8 |console.log(window);
+ 9 console.log(console);
+ 10 console.log("string");
+ 11 }
+
+ACTION: step-over
+PAUSE AT entryConsoleLog:10:5
+ 6 function entryConsoleLog() {
+ 7 debugger;
+ 8 console.log(window);
+ -> 9 |console.log(console);
+ 10 console.log("string");
+ 11 }
+ 12
+
+ACTION: step-over
+PAUSE AT entryConsoleLog:11:5
+ 7 debugger;
+ 8 console.log(window);
+ 9 console.log(console);
+ -> 10 |console.log("string");
+ 11 }
+ 12
+ 13 // ---------
+
+ACTION: step-over
+PAUSE AT entryConsoleLog:12:2
+ 8 console.log(window);
+ 9 console.log(console);
+ 10 console.log("string");
+ -> 11 }|
+ 12
+ 13 // ---------
+ 14
+
+ACTION: resume
+RESUMED
+PASS: Should have used all steps.
+
+-- Running test case: Debugger.stepping.internal.ConsoleLog.StepIn
+_expression_: setTimeout(entryConsoleLog)
+STEPS: over, in, in, in, resume
+PAUSED (debugger-statement)
+PAUSE AT entryConsoleLog:8:5
+ 4 <script src=""
+ 5 <script>
+ 6 function entryConsoleLog() {
+ -> 7 |debugger;
+ 8 console.log(window);
+ 9 console.log(console);
+ 10 console.log("string");
+
+ACTION: step-over
+PAUSE AT entryConsoleLog:9:5
+ 5 <script>
+ 6 function entryConsoleLog() {
+ 7 debugger;
+ -> 8 |console.log(window);
+ 9 console.log(console);
+ 10 console.log("string");
+ 11 }
+
+ACTION: step-in
+PAUSE AT entryConsoleLog:10:5
+ 6 function entryConsoleLog() {
+ 7 debugger;
+ 8 console.log(window);
+ -> 9 |console.log(console);
+ 10 console.log("string");
+ 11 }
+ 12
+
+ACTION: step-in
+PAUSE AT entryConsoleLog:11:5
+ 7 debugger;
+ 8 console.log(window);
+ 9 console.log(console);
+ -> 10 |console.log("string");
+ 11 }
+ 12
+ 13 // ---------
+
+ACTION: step-in
+PAUSE AT entryConsoleLog:12:2
+ 8 console.log(window);
+ 9 console.log(console);
+ 10 console.log("string");
+ -> 11 }|
+ 12
+ 13 // ---------
+ 14
+
+ACTION: resume
+RESUMED
+PASS: Should have used all steps.
+
Added: trunk/LayoutTests/inspector/debugger/stepping/stepping-internal-scripts.html (0 => 207267)
--- trunk/LayoutTests/inspector/debugger/stepping/stepping-internal-scripts.html (rev 0)
+++ trunk/LayoutTests/inspector/debugger/stepping/stepping-internal-scripts.html 2016-10-13 02:05:44 UTC (rev 207267)
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+<script>
+function entryConsoleLog() {
+ debugger;
+ console.log(window);
+ console.log(console);
+ console.log("string");
+}
+
+// ---------
+
+function test()
+{
+ let suite = InspectorTest.createAsyncSuite("Debugger.stepping.internal");
+
+ window.initializeSteppingTestSuite(suite);
+
+ addSteppingTestCase({
+ name: "Debugger.stepping.internal.ConsoleLog.StepOver",
+ description: "Should be able to step over console.log statements which use the Internal InjectedScript.",
+ _expression_: "setTimeout(entryConsoleLog)",
+ steps: [
+ "over",
+ "over", // console.log(window)
+ "over", // console.log(console)
+ "over", // console.log("string")
+ "resume",
+ ]
+ });
+
+ addSteppingTestCase({
+ name: "Debugger.stepping.internal.ConsoleLog.StepIn",
+ description: "Should not be able to step into console.log statements which use the Internal InjectedScript.",
+ _expression_: "setTimeout(entryConsoleLog)",
+ steps: [
+ "over",
+ "in", // console.log(window)
+ "in", // console.log(console)
+ "in", // console.log("string")
+ "resume",
+ ]
+ });
+
+ loadMainPageContent().then(() => {
+ suite.runTestCasesAndFinish();
+ });
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<p>Checking pause locations when stepping in and over statements that make use of Internal scripts.</p>
+</body>
+</html>
Modified: trunk/Source/_javascript_Core/ChangeLog (207266 => 207267)
--- trunk/Source/_javascript_Core/ChangeLog 2016-10-13 01:42:53 UTC (rev 207266)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-10-13 02:05:44 UTC (rev 207267)
@@ -1,3 +1,30 @@
+2016-10-12 Joseph Pecoraro <pecor...@apple.com>
+
+ Web Inspector: step-into `console.log(o)` should not step through inspector _javascript_
+ https://bugs.webkit.org/show_bug.cgi?id=161656
+ <rdar://problem/28181123>
+
+ Reviewed by Timothy Hatcher.
+
+ * debugger/Debugger.h:
+ * debugger/Debugger.cpp:
+ (JSC::Debugger::pauseIfNeeded):
+ If the Script is blacklisted skip checking if we need to pause.
+
+ (JSC::Debugger::isBlacklisted):
+ (JSC::Debugger::addToBlacklist):
+ (JSC::Debugger::clearBlacklist):
+ Add the ability to add a Script to a blacklist. Currently the blacklist
+ only prevents pausing in the Script.
+
+ * inspector/agents/InspectorDebuggerAgent.cpp:
+ (Inspector::isWebKitInjectedScript):
+ (Inspector::InspectorDebuggerAgent::didParseSource):
+ Always add Internal InjectedScripts to the Debugger's blacklist.
+
+ (Inspector::InspectorDebuggerAgent::clearDebuggerBreakpointState):
+ Clear blacklists when clearing debugger state.
+
2016-10-12 Keith Miller <keith_mil...@apple.com>
B3 needs a special WasmBoundsCheck Opcode
Modified: trunk/Source/_javascript_Core/debugger/Debugger.cpp (207266 => 207267)
--- trunk/Source/_javascript_Core/debugger/Debugger.cpp 2016-10-13 01:42:53 UTC (rev 207266)
+++ trunk/Source/_javascript_Core/debugger/Debugger.cpp 2016-10-13 02:05:44 UTC (rev 207267)
@@ -691,6 +691,10 @@
if (m_suppressAllPauses)
return;
+ intptr_t sourceID = DebuggerCallFrame::sourceIDForCallFrame(m_currentCallFrame);
+ if (isBlacklisted(sourceID))
+ return;
+
DebuggerPausedScope debuggerPausedScope(*this);
bool pauseNow = m_pauseAtNextOpportunity;
@@ -700,7 +704,6 @@
bool didHitBreakpoint = false;
Breakpoint breakpoint;
- intptr_t sourceID = DebuggerCallFrame::sourceIDForCallFrame(m_currentCallFrame);
TextPosition position = DebuggerCallFrame::positionForCallFrame(m_currentCallFrame);
pauseNow |= didHitBreakpoint = hasBreakpoint(sourceID, position, &breakpoint);
m_lastExecutedLine = position.m_line.zeroBasedInt();
@@ -912,4 +915,19 @@
return m_currentDebuggerCallFrame.get();
}
+bool Debugger::isBlacklisted(SourceID sourceID) const
+{
+ return m_blacklistedScripts.contains(sourceID);
+}
+
+void Debugger::addToBlacklist(SourceID sourceID)
+{
+ m_blacklistedScripts.add(sourceID);
+}
+
+void Debugger::clearBlacklist()
+{
+ m_blacklistedScripts.clear();
+}
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/debugger/Debugger.h (207266 => 207267)
--- trunk/Source/_javascript_Core/debugger/Debugger.h 2016-10-13 01:42:53 UTC (rev 207266)
+++ trunk/Source/_javascript_Core/debugger/Debugger.h 2016-10-13 02:05:44 UTC (rev 207267)
@@ -110,6 +110,10 @@
void stepOverStatement();
void stepOutOfFunction();
+ bool isBlacklisted(SourceID) const;
+ void addToBlacklist(SourceID);
+ void clearBlacklist();
+
bool isPaused() const { return m_isPaused; }
bool isStepping() const { return m_steppingMode == SteppingModeEnabled; }
@@ -216,7 +220,8 @@
VM& m_vm;
HashSet<JSGlobalObject*> m_globalObjects;
- HashMap<SourceID, DebuggerParseData> m_parseDataMap;
+ HashMap<SourceID, DebuggerParseData, WTF::IntHash<SourceID>, WTF::UnsignedWithZeroKeyHashTraits<SourceID>> m_parseDataMap;
+ HashSet<SourceID, WTF::IntHash<SourceID>, WTF::UnsignedWithZeroKeyHashTraits<SourceID>> m_blacklistedScripts;
PauseOnExceptionsState m_pauseOnExceptionsState;
bool m_pauseAtNextOpportunity : 1;
Modified: trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp (207266 => 207267)
--- trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp 2016-10-13 01:42:53 UTC (rev 207266)
+++ trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp 2016-10-13 02:05:44 UTC (rev 207267)
@@ -681,6 +681,11 @@
return script.sourceMappingURL;
}
+static bool isWebKitInjectedScript(const String& sourceURL)
+{
+ return sourceURL.startsWith("__InjectedScript_") && sourceURL.endsWith(".js");
+}
+
void InspectorDebuggerAgent::didParseSource(JSC::SourceID sourceID, const Script& script)
{
String scriptIDStr = String::number(sourceID);
@@ -696,6 +701,9 @@
m_scripts.set(sourceID, script);
+ if (hasSourceURL && isWebKitInjectedScript(sourceURL))
+ m_scriptDebugServer.addToBlacklist(sourceID);
+
String scriptURLForBreakpoints = hasSourceURL ? script.sourceURL : script.url;
if (scriptURLForBreakpoints.isEmpty())
return;
@@ -868,6 +876,7 @@
{
m_scriptDebugServer.clearBreakpointActions();
m_scriptDebugServer.clearBreakpoints();
+ m_scriptDebugServer.clearBlacklist();
m_pausedScriptState = nullptr;
m_currentCallStack = { };
