Title: [217306] trunk
Revision
217306
Author
mattba...@apple.com
Date
2017-05-23 15:58:23 -0700 (Tue, 23 May 2017)

Log Message

Web Inspector: Cannot delete a disabled XHR breakpoint
https://bugs.webkit.org/show_bug.cgi?id=171971
<rdar://problem/32129527>

Reviewed by Devin Rousso.

Source/WebInspectorUI:

* UserInterface/Controllers/DOMDebuggerManager.js:
(WebInspector.DOMDebuggerManager.prototype.removeXHRBreakpoint):
Dispatch XHRBreakpointRemoved event before removing the breakpoint from
the backend. A disabled breakpoint will have already been removed, and
an enabled breakpoint that fails to get removed from the backend should
be removed from the frontend, to prevent it being resolved in the future.
Drive-by fix: remove spurious dispatch of DOMBreakpointRemoved event.

(WebInspector.DOMDebuggerManager.prototype._detachXHRBreakpoint): Deleted.
Merged with removeXHRBreakpoint to simplify implementation.

LayoutTests:

* inspector/dom-debugger/xhr-breakpoints-expected.txt:
* inspector/dom-debugger/xhr-breakpoints.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (217305 => 217306)


--- trunk/LayoutTests/ChangeLog	2017-05-23 22:55:10 UTC (rev 217305)
+++ trunk/LayoutTests/ChangeLog	2017-05-23 22:58:23 UTC (rev 217306)
@@ -1,3 +1,14 @@
+2017-05-23  Matt Baker  <mattba...@apple.com>
+
+        Web Inspector: Cannot delete a disabled XHR breakpoint
+        https://bugs.webkit.org/show_bug.cgi?id=171971
+        <rdar://problem/32129527>
+
+        Reviewed by Devin Rousso.
+
+        * inspector/dom-debugger/xhr-breakpoints-expected.txt:
+        * inspector/dom-debugger/xhr-breakpoints.html:
+
 2017-05-23  Ryan Haddad  <ryanhad...@apple.com>
 
         Skip crashing css-display-3/display-contents tests.

Modified: trunk/LayoutTests/inspector/dom-debugger/xhr-breakpoints-expected.txt (217305 => 217306)


--- trunk/LayoutTests/inspector/dom-debugger/xhr-breakpoints-expected.txt	2017-05-23 22:55:10 UTC (rev 217305)
+++ trunk/LayoutTests/inspector/dom-debugger/xhr-breakpoints-expected.txt	2017-05-23 22:58:23 UTC (rev 217306)
@@ -14,8 +14,8 @@
 -- Running test teardown.
 
 -- Running test case: BreakOnXMLHttpRequestContainingText
+PASS: Added breakpoint for URL: data
 PASS: Breakpoint should not be disabled initially.
-PASS: Added breakpoint for URL: data
 Sending XMLHttpRequest.
 PAUSED:
 PASS: Pause reason should be XHR.
@@ -26,8 +26,8 @@
 -- Running test teardown.
 
 -- Running test case: BreakOnXMLHttpRequestMatchingRegularExpression
+PASS: Added breakpoint for URL: data[A-Z]*.(json|txt|png)
 PASS: Breakpoint should not be disabled initially.
-PASS: Added breakpoint for URL: data[A-Z]*.(json|txt|png)
 Sending XMLHttpRequest.
 PAUSED:
 PASS: Pause reason should be XHR.
@@ -38,9 +38,9 @@
 -- Running test teardown.
 
 -- Running test case: ShouldNotPauseOnDisabledBreakpoint
+PASS: Added breakpoint for URL: data
 PASS: Breakpoint should not be disabled initially.
-Disabling breakpoint.
-PASS: Added breakpoint for URL: data
+Breakpoint disabled.
 Sending XMLHttpRequest.
 Wait for evaluate in page to return.
 PASS: Should not pause for disabled breakpoint.
@@ -47,8 +47,8 @@
 -- Running test teardown.
 
 -- Running test case: ShouldNotPauseOnXMLHttpRequestNotContainingURL
+PASS: Added breakpoint for URL: nonexistant-url
 PASS: Breakpoint should not be disabled initially.
-PASS: Added breakpoint for URL: nonexistant-url
 Sending XMLHttpRequest.
 Wait for evaluate in page to return.
 PASS: Should not pause for breakpoint with different URL.
@@ -55,10 +55,18 @@
 -- Running test teardown.
 
 -- Running test case: RemoveBreakpoint
+PASS: Added breakpoint for URL: data
 PASS: Breakpoint should not be disabled initially.
-PASS: Added breakpoint for URL: data
-Remove breakpoint.
+Breakpoint removed.
 Wait for evaluate in page to return.
 PASS: Should not pause for removed breakpoint.
 -- Running test teardown.
 
+-- Running test case: RemoveDisabledBreakpoint
+PASS: Added breakpoint for URL: data
+PASS: Breakpoint should not be disabled initially.
+Breakpoint disabled.
+Breakpoint removed.
+Wait for evaluate in page to return.
+PASS: Should not pause for removed disabled breakpoint.
+

Modified: trunk/LayoutTests/inspector/dom-debugger/xhr-breakpoints.html (217305 => 217306)


--- trunk/LayoutTests/inspector/dom-debugger/xhr-breakpoints.html	2017-05-23 22:55:10 UTC (rev 217305)
+++ trunk/LayoutTests/inspector/dom-debugger/xhr-breakpoints.html	2017-05-23 22:58:23 UTC (rev 217306)
@@ -41,7 +41,7 @@
         });
     }
 
-    function addBreakpoint(type, url, disabled) {
+    function addBreakpoint(type, url) {
         return new Promise((resolve, reject) => {
             let mainFrame = WebInspector.frameResourceManager.mainFrame;
             InspectorTest.assert(mainFrame, "Missing main frame.");
@@ -51,14 +51,8 @@
             let breakpoint = new WebInspector.XHRBreakpoint(type, url);
             WebInspector.domDebuggerManager.awaitEvent(WebInspector.DOMDebuggerManager.Event.XHRBreakpointAdded)
             .then(() => {
+                InspectorTest.pass("Added breakpoint for URL: " + url);
                 InspectorTest.expectFalse(breakpoint.disabled, "Breakpoint should not be disabled initially.");
-
-                if (disabled) {
-                    InspectorTest.log("Disabling breakpoint.");
-                    breakpoint.disabled = true;
-                }
-
-                InspectorTest.pass("Added breakpoint for URL: " + url);
                 resolve(breakpoint);
             });
 
@@ -153,9 +147,11 @@
         description: "Check that debugger does not pause for disabled breakpoint.",
         teardown,
         test(resolve, reject) {
-            const disabled = true;
-            addBreakpoint(WebInspector.XHRBreakpoint.Type.Text, "data", disabled)
+            addBreakpoint(WebInspector.XHRBreakpoint.Type.Text, "data")
             .then((breakpoint) => {
+                breakpoint.disabled = true;
+                InspectorTest.log("Breakpoint disabled.");
+
                 InspectorTest.log("Sending XMLHttpRequest.");
                 return Promise.race([awaitEvaluateInPage("loadResourceXHR()"), rejectOnPause()]);
             })
@@ -192,13 +188,11 @@
         test(resolve, reject) {
             addBreakpoint(WebInspector.XHRBreakpoint.Type.Text, "data")
             .then((breakpoint) => {
-                let promise = WebInspector.domDebuggerManager.awaitEvent(WebInspector.DOMDebuggerManager.Event.XHRBreakpointRemoved);
+                WebInspector.domDebuggerManager.removeXHRBreakpoint(breakpoint);
+                InspectorTest.log("Breakpoint removed.");
 
-                InspectorTest.log("Remove breakpoint.");
-                WebInspector.domDebuggerManager.removeXHRBreakpoint(breakpoint);
-                return promise;
+                return Promise.race([awaitEvaluateInPage("loadResourceXHR()"), rejectOnPause()]);
             })
-            .then(() => Promise.race([awaitEvaluateInPage("loadResourceXHR()"), rejectOnPause()]))
             .then(() => {
                 InspectorTest.pass("Should not pause for removed breakpoint.");
                 resolve();
@@ -207,6 +201,28 @@
         }
     });
 
+    suite.addTestCase({
+        name: "RemoveDisabledBreakpoint",
+        description: "Check that a disabled breakpoint can be removed.",
+        test(resolve, reject) {
+            addBreakpoint(WebInspector.XHRBreakpoint.Type.Text, "data")
+            .then((breakpoint) => {
+                breakpoint.disabled = true;
+                InspectorTest.log("Breakpoint disabled.");
+
+                WebInspector.domDebuggerManager.removeXHRBreakpoint(breakpoint);
+                InspectorTest.log("Breakpoint removed.");
+
+                return Promise.race([awaitEvaluateInPage("loadResourceXHR()"), rejectOnPause()]);
+            })
+            .then(() => {
+                InspectorTest.pass("Should not pause for removed disabled breakpoint.");
+                resolve();
+            })
+            .catch(reject);
+        }
+    });
+
     suite.runTestCasesAndFinish();
 }
 </script>

Modified: trunk/Source/WebInspectorUI/ChangeLog (217305 => 217306)


--- trunk/Source/WebInspectorUI/ChangeLog	2017-05-23 22:55:10 UTC (rev 217305)
+++ trunk/Source/WebInspectorUI/ChangeLog	2017-05-23 22:58:23 UTC (rev 217306)
@@ -1,3 +1,22 @@
+2017-05-23  Matt Baker  <mattba...@apple.com>
+
+        Web Inspector: Cannot delete a disabled XHR breakpoint
+        https://bugs.webkit.org/show_bug.cgi?id=171971
+        <rdar://problem/32129527>
+
+        Reviewed by Devin Rousso.
+
+        * UserInterface/Controllers/DOMDebuggerManager.js:
+        (WebInspector.DOMDebuggerManager.prototype.removeXHRBreakpoint):
+        Dispatch XHRBreakpointRemoved event before removing the breakpoint from
+        the backend. A disabled breakpoint will have already been removed, and
+        an enabled breakpoint that fails to get removed from the backend should
+        be removed from the frontend, to prevent it being resolved in the future.
+        Drive-by fix: remove spurious dispatch of DOMBreakpointRemoved event.
+
+        (WebInspector.DOMDebuggerManager.prototype._detachXHRBreakpoint): Deleted.
+        Merged with removeXHRBreakpoint to simplify implementation.
+
 2017-05-23  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: use initialLayout for NetworkSidebarPanel

Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js (217305 => 217306)


--- trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js	2017-05-23 22:55:10 UTC (rev 217305)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js	2017-05-23 22:58:23 UTC (rev 217306)
@@ -221,10 +221,16 @@
 
         this._xhrBreakpoints.remove(breakpoint, true);
 
-        this.dispatchEventToListeners(WebInspector.DOMDebuggerManager.Event.DOMBreakpointRemoved, {breakpoint});
+        this._saveXHRBreakpoints();
+        this.dispatchEventToListeners(WebInspector.DOMDebuggerManager.Event.XHRBreakpointRemoved, {breakpoint});
 
-        this._detachXHRBreakpoint(breakpoint);
-        this._saveXHRBreakpoints();
+        if (breakpoint.disabled)
+            return;
+
+        DOMDebuggerAgent.removeXHRBreakpoint(breakpoint.url, (error) => {
+            if (error)
+                console.error(error);
+        });
     }
 
     // Private
@@ -341,19 +347,6 @@
             DOMDebuggerAgent.setDOMBreakpoint(nodeIdentifier, breakpoint.type, breakpointUpdated);
     }
 
-    _detachXHRBreakpoint(breakpoint)
-    {
-        if (breakpoint.disabled)
-            return;
-
-        DOMDebuggerAgent.removeXHRBreakpoint(breakpoint.url, (error) => {
-            if (error)
-                console.error(error);
-
-            this.dispatchEventToListeners(WebInspector.DOMDebuggerManager.Event.XHRBreakpointRemoved, {breakpoint});
-        });
-    }
-
     _updateXHRBreakpoint(breakpoint, callback)
     {
         function breakpointUpdated(error)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to