Web Inspector: DOMDebugger: breakpoints for attribute modifications still fire when...
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Apr 2019 22:41:24 +0000 (22:41 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Apr 2019 22:41:24 +0000 (22:41 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243719 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/dom-debugger/dom-breakpoints-expected.txt
LayoutTests/inspector/dom-debugger/dom-breakpoints.html
Source/WebCore/ChangeLog
Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp

index 52c9718..1e5dbae 100644 (file)
@@ -1,3 +1,14 @@
+2019-04-01  Devin Rousso  <drousso@apple.com>
+
+        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  <pvollan@apple.com>
 
         [Win10] Look into platform specific layout test failures
index d5a9c2e..3f789f5 100644 (file)
@@ -9,7 +9,7 @@ PASS: Breakpoint should have node identifier.
 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 @@ CALL STACK:
 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 @@ CALL STACK:
 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 @@ CALL STACK:
 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.
index 26fc4ca..047d69c 100644 (file)
@@ -26,20 +26,26 @@ function test()
     const nodeRemovedTestId = "node-removed-test";
     const multipleBreakpointsTestId = "multiple-breakpoints-test";
 
+    let debuggerPausedListener = null;
+
     let suite = InspectorTest.createAsyncSuite("DOMBreakpoints");
 
     function teardown(resolve) {
         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 @@ function test()
                     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 @@ function test()
                 .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,21 +212,21 @@ function test()
         }
     });
 
-    addSimpleTestCase({
+    addTestsForBreakpointType({
         name: "SubtreeModified",
         elementIdentifier: subtreeModifiedTestId,
         type: WI.DOMBreakpoint.Type.SubtreeModified,
         expression: "subtreeModifiedTest()",
     });
 
-    addSimpleTestCase({
+    addTestsForBreakpointType({
         name: "AttributeModified",
         elementIdentifier: attributeModifiedTestId,
         type: WI.DOMBreakpoint.Type.AttributeModified,
         expression: "attributeModifiedTest()",
     });
 
-    addSimpleTestCase({
+    addTestsForBreakpointType({
         name: "NodeRemoved",
         elementIdentifier: nodeRemovedTestId,
         type: WI.DOMBreakpoint.Type.NodeRemoved,
@@ -172,45 +234,6 @@ function test()
     });
 
     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,
index d9e9022..64debca 100644 (file)
@@ -1,3 +1,17 @@
+2019-04-01  Devin Rousso  <drousso@apple.com>
+
+        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  <wenson_hsieh@apple.com>
 
         Unable to copy and paste a PDF from Notes into Mail compose body
index e467679..3f7d9c8 100644 (file)
@@ -172,6 +172,9 @@ void InspectorDOMDebuggerAgent::removeEventBreakpoint(ErrorString& error, const
 
 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::willModifyDOMAttr(Element& element)
 
 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();