Title: [252730] trunk
- Revision
- 252730
- Author
- [email protected]
- Date
- 2019-11-21 04:14:59 -0800 (Thu, 21 Nov 2019)
Log Message
Web Inspector: removing the blackbox for a specific script doesn't actually remove the blackbox
https://bugs.webkit.org/show_bug.cgi?id=204428
Reviewed by Timothy Hatcher.
Source/_javascript_Core:
Previously, when updating the blackbox state of each existing script, we would only tell the
`Debugger` about when scripts should be blackboxed, not when they shouldn't. This means that
when a given script is un-blackboxed, the `Debugger` would never get told about it and would
therefore still defer pauses as if it was blackboxed.
The solution to this is simple; update the blackboxed state of every script, not just those
that should be blackboxed, and tell the `Debugger` about each.
* inspector/agents/InspectorDebuggerAgent.cpp:
(Inspector::InspectorDebuggerAgent::setShouldBlackboxURL):
LayoutTests:
* inspector/debugger/setShouldBlackboxURL.html:
* inspector/debugger/setShouldBlackboxURL-expected.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (252729 => 252730)
--- trunk/LayoutTests/ChangeLog 2019-11-21 07:25:43 UTC (rev 252729)
+++ trunk/LayoutTests/ChangeLog 2019-11-21 12:14:59 UTC (rev 252730)
@@ -1,3 +1,13 @@
+2019-11-21 Devin Rousso <[email protected]>
+
+ Web Inspector: removing the blackbox for a specific script doesn't actually remove the blackbox
+ https://bugs.webkit.org/show_bug.cgi?id=204428
+
+ Reviewed by Timothy Hatcher.
+
+ * inspector/debugger/setShouldBlackboxURL.html:
+ * inspector/debugger/setShouldBlackboxURL-expected.txt:
+
2019-11-20 Simon Fraser <[email protected]>
getComputedStyle returns "auto" for zIndex property even after it has been set, on non-positioned elements
Modified: trunk/LayoutTests/inspector/debugger/setShouldBlackboxURL-expected.txt (252729 => 252730)
--- trunk/LayoutTests/inspector/debugger/setShouldBlackboxURL-expected.txt 2019-11-21 07:25:43 UTC (rev 252729)
+++ trunk/LayoutTests/inspector/debugger/setShouldBlackboxURL-expected.txt 2019-11-21 12:14:59 UTC (rev 252730)
@@ -302,6 +302,52 @@
PASS: Should not pause in 'CaseSensitiveRegex_PauseInCallee_Outer'.
+-- Running test case: Debugger.setShouldBlackboxURL.Toggle
+Evaluating 'createScripts("Toggle")'...
+Setting breakpoint in 'Toggle_Inner.js'...
+
+Blackboxing 'toggle_middle.js'...
+Evaluating 'Toggle_Outer(10)'...
+
+PAUSED: 'Breakpoint' at 'Toggle_Inner:3:1'.
+{
+ "breakpointId": "Toggle_Inner.js:3:0"
+}
+Stepping over...
+
+PAUSED: 'BlackboxedScript' at 'Toggle_Outer:3:1'.
+{
+ "originalReason": "other",
+ "originalData": {
+ "breakpointId": "Toggle_Inner.js:3:0"
+ }
+}
+Stepping over...
+
+Resuming...
+PASS: Resumed.
+PASS: Should not pause in 'Toggle_Middle'.
+
+Removing blackbox for 'toggle_middle.js'...
+Evaluating 'Toggle_Outer(10)'...
+
+PAUSED: 'Breakpoint' at 'Toggle_Inner:3:1'.
+{
+ "breakpointId": "Toggle_Inner.js:3:0"
+}
+Stepping over...
+
+PAUSED: 'other' at 'Toggle_Middle:3:1'.
+Stepping over...
+
+PAUSED: 'other' at 'Toggle_Outer:3:1'.
+Stepping over...
+
+Resuming...
+PASS: Resumed.
+PASS: Should pause in 'Toggle_Middle'.
+
+
-- Running test case: Debugger.setShouldBlackboxURL.Invalid.emptyURL
PASS: Should produce an exception.
{
Modified: trunk/LayoutTests/inspector/debugger/setShouldBlackboxURL.html (252729 => 252730)
--- trunk/LayoutTests/inspector/debugger/setShouldBlackboxURL.html 2019-11-21 07:25:43 UTC (rev 252729)
+++ trunk/LayoutTests/inspector/debugger/setShouldBlackboxURL.html 2019-11-21 12:14:59 UTC (rev 252730)
@@ -460,6 +460,57 @@
});
suite.addTestCase({
+ name: "Debugger.setShouldBlackboxURL.Toggle",
+ description: "Check that the URL does not remain blackboxed if it's blackboxed state is toggled on and then off.",
+ async test() {
+ let resumePromise = null;
+
+ let [innerSourceURL, middleSourceURL, outerSourceURL] = await Promise.all([
+ listenForSourceParsed(/Toggle_Inner\.js$/),
+ listenForSourceParsed(/Toggle_Middle\.js$/),
+ listenForSourceParsed(/Toggle_Outer\.js$/),
+ evaluate(`createScripts("Toggle")`),
+ ]);
+ await setBreakpoint(innerSourceURL, 3); // last line of function, so it only pauses once
+
+ ProtocolTest.newline();
+
+ resumePromise = new Promise((resolve, reject) => {
+ resumeCallback = function() {
+ ProtocolTest.expectThat(!pausedFunctionNames.includes("Toggle_Middle"), "Should not pause in 'Toggle_Middle'.");
+ resolve();
+ };
+ });
+
+ await setBlackbox(middleSourceURL);
+ await evaluate(`Toggle_Outer(10)`);
+
+ ProtocolTest.newline();
+
+ await resumePromise;
+
+ resumePromise = new Promise((resolve, reject) => {
+ resumeCallback = function() {
+ ProtocolTest.expectThat(pausedFunctionNames.includes("Toggle_Middle"), "Should pause in 'Toggle_Middle'.");
+ resolve();
+ };
+ });
+
+ ProtocolTest.log(`Removing blackbox for '${middleSourceURL.toLowerCase()}'...`);
+ await InspectorProtocol.awaitCommand({
+ method: "Debugger.setShouldBlackboxURL",
+ params: {url: middleSourceURL.toLowerCase(), shouldBlackbox: false},
+ });
+
+ await evaluate(`Toggle_Outer(10)`);
+
+ ProtocolTest.newline();
+
+ await resumePromise;
+ },
+ });
+
+ suite.addTestCase({
name: "Debugger.setShouldBlackboxURL.Invalid.emptyURL",
description: "Check that an error is thrown if the given url is empty.",
async test() {
Modified: trunk/Source/_javascript_Core/ChangeLog (252729 => 252730)
--- trunk/Source/_javascript_Core/ChangeLog 2019-11-21 07:25:43 UTC (rev 252729)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-11-21 12:14:59 UTC (rev 252730)
@@ -1,3 +1,21 @@
+2019-11-21 Devin Rousso <[email protected]>
+
+ Web Inspector: removing the blackbox for a specific script doesn't actually remove the blackbox
+ https://bugs.webkit.org/show_bug.cgi?id=204428
+
+ Reviewed by Timothy Hatcher.
+
+ Previously, when updating the blackbox state of each existing script, we would only tell the
+ `Debugger` about when scripts should be blackboxed, not when they shouldn't. This means that
+ when a given script is un-blackboxed, the `Debugger` would never get told about it and would
+ therefore still defer pauses as if it was blackboxed.
+
+ The solution to this is simple; update the blackboxed state of every script, not just those
+ that should be blackboxed, and tell the `Debugger` about each.
+
+ * inspector/agents/InspectorDebuggerAgent.cpp:
+ (Inspector::InspectorDebuggerAgent::setShouldBlackboxURL):
+
2019-11-20 Yusuke Suzuki <[email protected]>
[JSC] Extend MacroAssemblerARM64::load/store for datasize = 16
Modified: trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp (252729 => 252730)
--- trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp 2019-11-21 07:25:43 UTC (rev 252729)
+++ trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp 2019-11-21 12:14:59 UTC (rev 252730)
@@ -911,12 +911,13 @@
else
m_blackboxedURLs.removeAll(config);
- auto blackboxType = shouldBlackbox ? Optional<JSC::Debugger::BlackboxType>(JSC::Debugger::BlackboxType::Deferred) : WTF::nullopt;
for (auto& [sourceID, script] : m_scripts) {
if (isWebKitInjectedScript(script.sourceURL))
continue;
- if (!shouldBlackboxURL(script.sourceURL) && !shouldBlackboxURL(script.url))
- continue;
+
+ Optional<JSC::Debugger::BlackboxType> blackboxType;
+ if (shouldBlackboxURL(script.sourceURL) || shouldBlackboxURL(script.url))
+ blackboxType = JSC::Debugger::BlackboxType::Deferred;
m_scriptDebugServer.setBlackboxType(sourceID, blackboxType);
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
