Title: [243719] trunk
Revision
243719
Author
[email protected]
Date
2019-04-01 15:41:24 -0700 (Mon, 01 Apr 2019)

Log Message

Web Inspector: DOMDebugger: breakpoints for attribute modifications still fire when breakpoints are disabled
https://bugs.webkit.org/show_bug.cgi?id=196456
<rdar://problem/49489747>

Reviewed by Joseph Pecoraro.

Source/WebCore:

Test: inspector/dom-debugger/dom-breakpoints.html

* inspector/agents/InspectorDOMDebuggerAgent.cpp:
(WebCore::InspectorDOMDebuggerAgent::didInvalidateStyleAttr):
(WebCore::InspectorDOMDebuggerAgent::descriptionForDOMEvent):

LayoutTests:

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

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (243718 => 243719)


--- trunk/LayoutTests/ChangeLog	2019-04-01 22:38:29 UTC (rev 243718)
+++ trunk/LayoutTests/ChangeLog	2019-04-01 22:41:24 UTC (rev 243719)
@@ -1,3 +1,14 @@
+2019-04-01  Devin Rousso  <[email protected]>
+
+        Web Inspector: DOMDebugger: breakpoints for attribute modifications still fire when breakpoints are disabled
+        https://bugs.webkit.org/show_bug.cgi?id=196456
+        <rdar://problem/49489747>
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/dom-debugger/dom-breakpoints.html:
+        * inspector/dom-debugger/dom-breakpoints-expected.txt:
+
 2019-04-01  Per Arne Vollan  <[email protected]>
 
         [Win10] Look into platform specific layout test failures

Modified: trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints-expected.txt (243718 => 243719)


--- trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints-expected.txt	2019-04-01 22:38:29 UTC (rev 243718)
+++ trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints-expected.txt	2019-04-01 22:41:24 UTC (rev 243719)
@@ -9,7 +9,7 @@
 PASS: Breakpoint URL should match document URL.
 -- Running test teardown.
 
--- Running test case: BreakOnSubtreeModified
+-- Running test case: DOMBreakpoints.SubtreeModified.BreakpointEnabled
 PASS: Added 'subtree-modified' breakpoint.
 PASS: Breakpoint should have expected type.
 Call DOM operation.
@@ -20,7 +20,26 @@
 1: [P] Global Code
 -- Running test teardown.
 
--- Running test case: BreakOnAttributeModified
+-- Running test case: DOMBreakpoints.SubtreeModified.BreakpointDisabled
+PASS: Added 'subtree-modified' breakpoint.
+Wait for evaluate in page to return.
+PASS: Should not pause for disabled breakpoint.
+-- Running test teardown.
+
+-- Running test case: DOMBreakpoints.SubtreeModified.DebuggerDisabled
+PASS: Added 'subtree-modified' breakpoint.
+Wait for evaluate in page to return.
+PASS: Should not pause for disabled breakpoint.
+-- Running test teardown.
+
+-- Running test case: DOMBreakpoints.SubtreeModified.RemoveBreakpoint
+PASS: Added 'subtree-modified' breakpoint.
+Remove breakpoint.
+Wait for evaluate in page to return.
+PASS: Should not pause for removed breakpoint.
+-- Running test teardown.
+
+-- Running test case: DOMBreakpoints.AttributeModified.BreakpointEnabled
 PASS: Added 'attribute-modified' breakpoint.
 PASS: Breakpoint should have expected type.
 Call DOM operation.
@@ -31,7 +50,26 @@
 1: [P] Global Code
 -- Running test teardown.
 
--- Running test case: BreakOnNodeRemoved
+-- Running test case: DOMBreakpoints.AttributeModified.BreakpointDisabled
+PASS: Added 'attribute-modified' breakpoint.
+Wait for evaluate in page to return.
+PASS: Should not pause for disabled breakpoint.
+-- Running test teardown.
+
+-- Running test case: DOMBreakpoints.AttributeModified.DebuggerDisabled
+PASS: Added 'attribute-modified' breakpoint.
+Wait for evaluate in page to return.
+PASS: Should not pause for disabled breakpoint.
+-- Running test teardown.
+
+-- Running test case: DOMBreakpoints.AttributeModified.RemoveBreakpoint
+PASS: Added 'attribute-modified' breakpoint.
+Remove breakpoint.
+Wait for evaluate in page to return.
+PASS: Should not pause for removed breakpoint.
+-- Running test teardown.
+
+-- Running test case: DOMBreakpoints.NodeRemoved.BreakpointEnabled
 PASS: Added 'node-removed' breakpoint.
 PASS: Breakpoint should have expected type.
 Call DOM operation.
@@ -42,14 +80,20 @@
 1: [P] Global Code
 -- Running test teardown.
 
--- Running test case: ShouldNotPauseOnDisabledBreakpoint
-PASS: Added 'subtree-modified' breakpoint.
+-- Running test case: DOMBreakpoints.NodeRemoved.BreakpointDisabled
+PASS: Added 'node-removed' breakpoint.
 Wait for evaluate in page to return.
 PASS: Should not pause for disabled breakpoint.
 -- Running test teardown.
 
--- Running test case: RemoveBreakpoint
-PASS: Added 'subtree-modified' breakpoint.
+-- Running test case: DOMBreakpoints.NodeRemoved.DebuggerDisabled
+PASS: Added 'node-removed' breakpoint.
+Wait for evaluate in page to return.
+PASS: Should not pause for disabled breakpoint.
+-- Running test teardown.
+
+-- Running test case: DOMBreakpoints.NodeRemoved.RemoveBreakpoint
+PASS: Added 'node-removed' breakpoint.
 Remove breakpoint.
 Wait for evaluate in page to return.
 PASS: Should not pause for removed breakpoint.

Modified: trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints.html (243718 => 243719)


--- trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints.html	2019-04-01 22:38:29 UTC (rev 243718)
+++ trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints.html	2019-04-01 22:41:24 UTC (rev 243719)
@@ -26,6 +26,8 @@
     const nodeRemovedTestId = "node-removed-test";
     const multipleBreakpointsTestId = "multiple-breakpoints-test";
 
+    let debuggerPausedListener = null;
+
     let suite = InspectorTest.createAsyncSuite("DOMBreakpoints");
 
     function teardown(resolve) {
@@ -32,14 +34,18 @@
         let breakpoints = WI.domDebuggerManager.domBreakpoints;
         for (let breakpoint of breakpoints)
             WI.domDebuggerManager.removeDOMBreakpoint(breakpoint);
+        InspectorTest.assert(!WI.domDebuggerManager.domBreakpoints.length);
 
+        WI.debuggerManager.removeEventListener(WI.DebuggerManager.Event.Paused, debuggerPausedListener);
+        debuggerPausedListener = null;
+
         resolve();
     }
 
     function rejectOnPause() {
         return new Promise((resolve, reject) => {
-            WI.debuggerManager.awaitEvent(WI.DebuggerManager.Event.Paused)
-            .then((event) => {
+            InspectorTest.assert(!debuggerPausedListener);
+            debuggerPausedListener = WI.debuggerManager.addEventListener(WI.DebuggerManager.Event.Paused, (event) => {
                 WI.debuggerManager.resume();
                 InspectorTest.fail("Should not pause.");
                 reject();
@@ -98,14 +104,14 @@
                     resolve(event);
                 });
 
-                WI.domDebuggerManager.addDOMBreakpoint(new WI.DOMBreakpoint(node, type, disabled));
+                WI.domDebuggerManager.addDOMBreakpoint(new WI.DOMBreakpoint(node, type, {disabled}));
             });
         });
     }
 
-    function addSimpleTestCase({name, elementIdentifier, type, _expression_}) {
+    function addTestsForBreakpointType({name, elementIdentifier, type, _expression_}) {
         suite.addTestCase({
-            name: `BreakOn${name}`,
+            name: `DOMBreakpoints.${name}.BreakpointEnabled`,
             description: "Check that debugger pauses for breakpoint type.",
             teardown,
             test(resolve, reject) {
@@ -129,6 +135,62 @@
                 .then(resolve, reject);
             }
         });
+
+        suite.addTestCase({
+            name: `DOMBreakpoints.${name}.BreakpointDisabled`,
+            description: "Check that debugger does not pause for disabled breakpoint.",
+            teardown,
+            test(resolve, reject) {
+                const disabled = true;
+                addBreakpointForElementIdentifier(elementIdentifier, type, disabled)
+                .then(() => Promise.race([awaitEvaluateInPage(_expression_), rejectOnPause()]))
+                .then(() => {
+                    InspectorTest.pass("Should not pause for disabled breakpoint.");
+                    resolve();
+                })
+                .catch(reject);
+            }
+        });
+
+        suite.addTestCase({
+            name: `DOMBreakpoints.${name}.DebuggerDisabled`,
+            description: "Check that debugger does not pause when the debugger is disabled.",
+            teardown,
+            test(resolve, reject) {
+                addBreakpointForElementIdentifier(elementIdentifier, type)
+                .then(() => DebuggerAgent.setBreakpointsActive(false))
+                .then(() => Promise.race([awaitEvaluateInPage(_expression_), rejectOnPause()]))
+                .then(() => DebuggerAgent.setBreakpointsActive(true))
+                .then(() => {
+                    InspectorTest.pass("Should not pause for disabled breakpoint.");
+                    resolve();
+                })
+                .catch(reject);
+            }
+        });
+
+        suite.addTestCase({
+            name: `DOMBreakpoints.${name}.RemoveBreakpoint`,
+            description: "Check that debugger does not pause for removed breakpoint.",
+            teardown,
+            test(resolve, reject) {
+                addBreakpointForElementIdentifier(elementIdentifier, type)
+                .then((event) => {
+                    let promise = WI.domDebuggerManager.awaitEvent(WI.DOMDebuggerManager.Event.DOMBreakpointRemoved);
+                    let breakpoint = event.data.breakpoint;
+
+                    InspectorTest.log("Remove breakpoint.");
+                    WI.domDebuggerManager.removeDOMBreakpoint(breakpoint);
+                    return promise;
+                })
+                .then(() => Promise.race([awaitEvaluateInPage(_expression_), rejectOnPause()]))
+                .then(() => {
+                    InspectorTest.pass("Should not pause for removed breakpoint.");
+                    resolve();
+                })
+                .catch(reject);
+            }
+        });
     }
 
     suite.addTestCase({
@@ -150,7 +212,7 @@
         }
     });
 
-    addSimpleTestCase({
+    addTestsForBreakpointType({
         name: "SubtreeModified",
         elementIdentifier: subtreeModifiedTestId,
         type: WI.DOMBreakpoint.Type.SubtreeModified,
@@ -157,7 +219,7 @@
         _expression_: "subtreeModifiedTest()",
     });
 
-    addSimpleTestCase({
+    addTestsForBreakpointType({
         name: "AttributeModified",
         elementIdentifier: attributeModifiedTestId,
         type: WI.DOMBreakpoint.Type.AttributeModified,
@@ -164,7 +226,7 @@
         _expression_: "attributeModifiedTest()",
     });
 
-    addSimpleTestCase({
+    addTestsForBreakpointType({
         name: "NodeRemoved",
         elementIdentifier: nodeRemovedTestId,
         type: WI.DOMBreakpoint.Type.NodeRemoved,
@@ -172,45 +234,6 @@
     });
 
     suite.addTestCase({
-        name: "ShouldNotPauseOnDisabledBreakpoint",
-        description: "Check that debugger does not pause for disabled breakpoint.",
-        teardown,
-        test(resolve, reject) {
-            const disabled = true;
-            addBreakpointForElementIdentifier(subtreeModifiedTestId, WI.DOMBreakpoint.Type.SubtreeModified, disabled)
-            .then(() => Promise.race([awaitEvaluateInPage("modifySubtreeTest()"), rejectOnPause()]))
-            .then(() => {
-                InspectorTest.pass("Should not pause for disabled breakpoint.");
-                resolve();
-            })
-            .catch(reject);
-        }
-    });
-
-    suite.addTestCase({
-        name: "RemoveBreakpoint",
-        description: "Check that debugger does not pause for removed breakpoint.",
-        teardown,
-        test(resolve, reject) {
-            addBreakpointForElementIdentifier(subtreeModifiedTestId, WI.DOMBreakpoint.Type.SubtreeModified)
-            .then((event) => {
-                let promise = WI.domDebuggerManager.awaitEvent(WI.DOMDebuggerManager.Event.DOMBreakpointRemoved);
-                let breakpoint = event.data.breakpoint;
-
-                InspectorTest.log("Remove breakpoint.");
-                WI.domDebuggerManager.removeDOMBreakpoint(breakpoint);
-                return promise;
-            })
-            .then(() => Promise.race([awaitEvaluateInPage("modifySubtreeTest()"), rejectOnPause()]))
-            .then(() => {
-                InspectorTest.pass("Should not pause for removed breakpoint.");
-                resolve();
-            })
-            .catch(reject);
-        }
-    });
-
-    suite.addTestCase({
         name: "RemoveAllBreakpointsForNode",
         description: "Check that debugger does not pause for removed breakpoints on node.",
         teardown,

Modified: trunk/Source/WebCore/ChangeLog (243718 => 243719)


--- trunk/Source/WebCore/ChangeLog	2019-04-01 22:38:29 UTC (rev 243718)
+++ trunk/Source/WebCore/ChangeLog	2019-04-01 22:41:24 UTC (rev 243719)
@@ -1,3 +1,17 @@
+2019-04-01  Devin Rousso  <[email protected]>
+
+        Web Inspector: DOMDebugger: breakpoints for attribute modifications still fire when breakpoints are disabled
+        https://bugs.webkit.org/show_bug.cgi?id=196456
+        <rdar://problem/49489747>
+
+        Reviewed by Joseph Pecoraro.
+
+        Test: inspector/dom-debugger/dom-breakpoints.html
+
+        * inspector/agents/InspectorDOMDebuggerAgent.cpp:
+        (WebCore::InspectorDOMDebuggerAgent::didInvalidateStyleAttr):
+        (WebCore::InspectorDOMDebuggerAgent::descriptionForDOMEvent):
+
 2019-04-01  Wenson Hsieh  <[email protected]>
 
         Unable to copy and paste a PDF from Notes into Mail compose body

Modified: trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp (243718 => 243719)


--- trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp	2019-04-01 22:38:29 UTC (rev 243718)
+++ trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp	2019-04-01 22:41:24 UTC (rev 243719)
@@ -172,6 +172,9 @@
 
 void InspectorDOMDebuggerAgent::didInvalidateStyleAttr(Node& node)
 {
+    if (!m_debuggerAgent->breakpointsActive())
+        return;
+
     if (hasBreakpoint(&node, AttributeModified)) {
         Ref<JSON::Object> eventData = JSON::Object::create();
         descriptionForDOMEvent(node, AttributeModified, false, eventData.get());
@@ -326,6 +329,7 @@
 
 void InspectorDOMDebuggerAgent::descriptionForDOMEvent(Node& target, int breakpointType, bool insertion, JSON::Object& description)
 {
+    ASSERT(m_debuggerAgent->breakpointsActive());
     ASSERT(hasBreakpoint(&target, breakpointType));
 
     auto* domAgent = m_instrumentingAgents.inspectorDOMAgent();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to