Changeset 251871 in webkit


Ignore:
Timestamp:
Oct 31, 2019 1:20:43 PM (4 years ago)
Author:
Devin Rousso
Message:

Web Inspector: DOMDebugger: Node Removed breakpoints should fire whenever the node is removed from the main DOM tree, not just when it's removed from it's parent
​https://bugs.webkit.org/show_bug.cgi?id=203349

Reviewed by Matt Baker.

Source/WebCore:

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

Replace targetNode (which was a Runtime.RemoteObject) with a targetNodeId (which is a
DOM.NodeId) when dispatching DOMDebugger pauses. Additionally, include the ancestor's
DOM.NodeId as the targetNodeId whenever an ancestor is removed that has a descendant
with a node removed DOM breakpoint.

  • inspector/agents/page/PageDOMDebuggerAgent.h:
  • inspector/agents/page/PageDOMDebuggerAgent.cpp:

(WebCore::PageDOMDebuggerAgent::willRemoveDOMNode):
(WebCore::PageDOMDebuggerAgent::descriptionForDOMEvent):

  • inspector/agents/InspectorDOMAgent.h:
  • inspector/agents/InspectorDOMAgent.cpp:

(WebCore::InspectorDOMAgent::pushNodeToFrontend): Added.

Source/WebInspectorUI:

Replace targetNode (which was a Runtime.RemoteObject) with a targetNodeId (which is a
DOM.NodeId) when dispatching DOMDebugger pauses. Additionally, include the ancestor's
DOM.NodeId as the targetNodeId whenever an ancestor is removed that has a descendant
with a node removed DOM breakpoint.

  • UserInterface/Views/SourcesNavigationSidebarPanel.js:

(WI.SourcesNavigationSidebarPanel.prototype._updatePauseReasonSection):

  • Localizations/en.lproj/localizedStrings.js:

LayoutTests:

  • inspector/dom-debugger/dom-breakpoints.html:
  • inspector/dom-debugger/dom-breakpoints-expected.txt:
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r251867 r251871  
     12019-10-31  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: DOMDebugger: Node Removed breakpoints should fire whenever the node is removed from the main DOM tree, not just when it's removed from it's parent
     4        https://bugs.webkit.org/show_bug.cgi?id=203349
     5
     6        Reviewed by Matt Baker.
     7
     8        * inspector/dom-debugger/dom-breakpoints.html:
     9        * inspector/dom-debugger/dom-breakpoints-expected.txt:
     10
    1112019-10-31  Ryosuke Niwa  <rniwa@webkit.org>
    212

  • trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints-expected.txt

    r249330 r251871  
    1616PAUSED:
    1717PASS: Pause reason should be DOM.
     18PASS: Pause type should be 'subtree-modified'.
     19PASS: Pause nodeId should be expected value.
     20PASS: Pause insertion should be 'true'.
     21PASS: Pause targetNodeId should match nodeId.
    1822CALL STACK:
    19230: [F] subtreeModifiedTest
    … …  
    4650PAUSED:
    4751PASS: Pause reason should be DOM.
     52PASS: Pause type should be 'attribute-modified'.
     53PASS: Pause nodeId should be expected value.
    4854CALL STACK:
    49550: [F] attributeModifiedTest
    … …  
    7076-- Running test teardown.
    7177
    72 -- Running test case: DOMBreakpoints.NodeRemoved.BreakpointEnabled
     78-- Running test case: DOMBreakpoints.NodeRemovedSelf.BreakpointEnabled
    7379PASS: Added 'node-removed' breakpoint.
    7480PASS: Breakpoint should have expected type.
    … …  
    7682PAUSED:
    7783PASS: Pause reason should be DOM.
     84PASS: Pause type should be 'node-removed'.
     85PASS: Pause nodeId should be expected value.
    7886CALL STACK:
    79 0: [F] nodeRemovedTest
     870: [F] nodeRemovedDirectTest
    80881: [P] Global Code
    8189-- Running test teardown.
    8290
    83 -- Running test case: DOMBreakpoints.NodeRemoved.BreakpointDisabled
     91-- Running test case: DOMBreakpoints.NodeRemovedSelf.BreakpointDisabled
    8492PASS: Added 'node-removed' breakpoint.
    8593Wait for evaluate in page to return.
    … …  
    8795-- Running test teardown.
    8896
    89 -- Running test case: DOMBreakpoints.NodeRemoved.DebuggerDisabled
     97-- Running test case: DOMBreakpoints.NodeRemovedSelf.DebuggerDisabled
    9098PASS: Added 'node-removed' breakpoint.
    9199Wait for evaluate in page to return.
    … …  
    93101-- Running test teardown.
    94102
    95 -- Running test case: DOMBreakpoints.NodeRemoved.RemoveBreakpoint
     103-- Running test case: DOMBreakpoints.NodeRemovedSelf.RemoveBreakpoint
     104PASS: Added 'node-removed' breakpoint.
     105Remove breakpoint.
     106Wait for evaluate in page to return.
     107PASS: Should not pause for removed breakpoint.
     108-- Running test teardown.
     109
     110-- Running test case: DOMBreakpoints.NodeRemovedAncestor.BreakpointEnabled
     111PASS: Added 'node-removed' breakpoint.
     112PASS: Breakpoint should have expected type.
     113Call DOM operation.
     114PAUSED:
     115PASS: Pause reason should be DOM.
     116PASS: Pause type should be 'node-removed'.
     117PASS: Pause nodeId should be expected value.
     118PASS: Pause targetNodeId should not match nodeId.
     119CALL STACK:
     1200: [F] nodeRemovedAncestorTest
     1211: [P] Global Code
     122-- Running test teardown.
     123
     124-- Running test case: DOMBreakpoints.NodeRemovedAncestor.BreakpointDisabled
     125PASS: Added 'node-removed' breakpoint.
     126Wait for evaluate in page to return.
     127PASS: Should not pause for disabled breakpoint.
     128-- Running test teardown.
     129
     130-- Running test case: DOMBreakpoints.NodeRemovedAncestor.DebuggerDisabled
     131PASS: Added 'node-removed' breakpoint.
     132Wait for evaluate in page to return.
     133PASS: Should not pause for disabled breakpoint.
     134-- Running test teardown.
     135
     136-- Running test case: DOMBreakpoints.NodeRemovedAncestor.RemoveBreakpoint
    96137PASS: Added 'node-removed' breakpoint.
    97138Remove breakpoint.

  • trunk/LayoutTests/inspector/dom-debugger/dom-breakpoints.html

    r243719 r251871  
    1313}
    1414
    15 function nodeRemovedTest() {
    16     let node = document.getElementById("node-removed-test");
     15function nodeRemovedDirectTest() {
     16    let node = document.getElementById("node-removed-direct-test");
    1717    let parent = node.parentNode;
    1818    node.remove();
    1919    parent.append(node);
     20}
     21
     22function nodeRemovedAncestorTest() {
     23    let node = document.getElementById("node-removed-ancestor-test");
     24    let parent = node.parentNode;
     25    let grandparent = parent.parentNode;
     26    parent.remove(node);
     27    grandparent.append(parent);
    2028}
    2129
    … …  
    2432    const subtreeModifiedTestId = "subtree-modified-test";
    2533    const attributeModifiedTestId = "attribute-modified-test";
    26     const nodeRemovedTestId = "node-removed-test";
     34    const nodeRemovedDirectTestId = "node-removed-direct-test";
     35    const nodeRemovedAncestorTestId = "node-removed-ancestor-test";
    2736    const multipleBreakpointsTestId = "multiple-breakpoints-test";
    2837
    … …  
    110119    }
    111120
    112     function addTestsForBreakpointType({name, elementIdentifier, type, expression}) {
     121    function addTestsForBreakpointType({name, elementIdentifier, type, expression, insertion, shouldMatch}) {
    113122        suite.addTestCase({
    114123            name: `DOMBreakpoints.${name}.BreakpointEnabled`,
    … …  
    129138                    InspectorTest.log("PAUSED:");
    130139                    InspectorTest.expectEqual(targetData.pauseReason, WI.DebuggerManager.PauseReason.DOM, "Pause reason should be DOM.");
     140
     141                    let pauseData = targetData.pauseData;
     142                    InspectorTest.expectEqual(pauseData.type, type, `Pause type should be '${type}'.`);
     143                    InspectorTest.expectEqual(pauseData.nodeId, domNodeIdentifier, `Pause nodeId should be expected value.`);
     144
     145                    if (insertion !== undefined)
     146                        InspectorTest.expectEqual(insertion, target.pauseData.insertion, `Pause insertion should be '${insertion}'.`);
     147
     148                    if (pauseData.targetNodeId) {
     149                        if (shouldMatch)
     150                            InspectorTest.expectEqual(pauseData.targetNodeId, pauseData.nodeId, "Pause targetNodeId should match nodeId.");
     151                        else
     152                            InspectorTest.expectNotEqual(pauseData.targetNodeId, pauseData.nodeId, "Pause targetNodeId should not match nodeId.");
     153                    }
     154
    131155                    logActiveStackTrace();
    132 
    133156                    return WI.debuggerManager.resume();
    134157                })
    … …  
    218241        type: WI.DOMBreakpoint.Type.SubtreeModified,
    219242        expression: "subtreeModifiedTest()",
     243        insertion: true,
     244        shouldMatch: true,
    220245    });
    221246
    … …  
    228253
    229254    addTestsForBreakpointType({
    230         name: "NodeRemoved",
    231         elementIdentifier: nodeRemovedTestId,
     255        name: "NodeRemovedSelf",
     256        elementIdentifier: nodeRemovedDirectTestId,
    232257        type: WI.DOMBreakpoint.Type.NodeRemoved,
    233         expression: "nodeRemovedTest()",
     258        expression: "nodeRemovedDirectTest()",
     259    });
     260
     261    addTestsForBreakpointType({
     262        name: "NodeRemovedAncestor",
     263        elementIdentifier: nodeRemovedAncestorTestId,
     264        type: WI.DOMBreakpoint.Type.NodeRemoved,
     265        expression: "nodeRemovedAncestorTest()",
     266        shouldMatch: true,
    234267    });
    235268
    … …  
    354387    <div id="subtree-modified-test"></div>
    355388    <div id="attribute-modified-test"></div>
    356     <div id="node-removed-test"></div>
     389    <div id="node-removed-direct-test"></div>
     390    <div><div id="node-removed-ancestor-test"></div></div>
    357391    <div id="multiple-breakpoints-test"></div>
    358392</div>

  • trunk/Source/WebCore/ChangeLog

    r251870 r251871  
     12019-10-31  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: DOMDebugger: Node Removed breakpoints should fire whenever the node is removed from the main DOM tree, not just when it's removed from it's parent
     4        https://bugs.webkit.org/show_bug.cgi?id=203349
     5
     6        Reviewed by Matt Baker.
     7
     8        Test: inspector/dom-debugger/dom-breakpoints.html
     9
     10        Replace `targetNode` (which was a `Runtime.RemoteObject`) with a `targetNodeId` (which is a
     11        `DOM.NodeId`) when dispatching `DOMDebugger` pauses. Additionally, include the ancestor's
     12        `DOM.NodeId` as the `targetNodeId` whenever an ancestor is removed that has a descendant
     13        with a node removed DOM breakpoint.
     14
     15        * inspector/agents/page/PageDOMDebuggerAgent.h:
     16        * inspector/agents/page/PageDOMDebuggerAgent.cpp:
     17        (WebCore::PageDOMDebuggerAgent::willRemoveDOMNode):
     18        (WebCore::PageDOMDebuggerAgent::descriptionForDOMEvent):
     19
     20        * inspector/agents/InspectorDOMAgent.h:
     21        * inspector/agents/InspectorDOMAgent.cpp:
     22        (WebCore::InspectorDOMAgent::pushNodeToFrontend): Added.
     23
    1242019-10-31  Andres Gonzalez  <andresg_22@apple.com>
    225

  • trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp

    r251798 r251871  
    542542}
    543543
     544int InspectorDOMAgent::pushNodeToFrontend(Node* nodeToPush)
     545{
     546    if (!nodeToPush)
     547        return 0;
     548
     549    ErrorString ignored;
     550    return pushNodeToFrontend(ignored, boundNodeId(&nodeToPush->document()), nodeToPush);
     551}
     552
    544553int InspectorDOMAgent::pushNodeToFrontend(ErrorString& errorString, int documentNodeId, Node* nodeToPush)
    545554{

  • trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.h

    r251798 r251871  
    179179    void styleAttributeInvalidated(const Vector<Element*>& elements);
    180180
     181    int pushNodeToFrontend(Node*);
    181182    int pushNodeToFrontend(ErrorString&, int documentNodeId, Node*);
    182183    Node* nodeForId(int nodeId);

  • trunk/Source/WebCore/inspector/agents/page/PageDOMDebuggerAgent.cpp

    r249714 r251871  
    188188        return;
    189189
    190     Node* parentNode = InspectorDOMAgent::innerParentNode(&node);
    191190    if (hasBreakpoint(&node, NodeRemoved)) {
    192         Ref<JSON::Object> eventData = JSON::Object::create();
     191        auto eventData = JSON::Object::create();
    193192        descriptionForDOMEvent(node, NodeRemoved, false, eventData.get());
    194193        m_debuggerAgent->breakProgram(Inspector::DebuggerFrontendDispatcher::Reason::DOM, WTFMove(eventData));
    195     } else if (parentNode && hasBreakpoint(parentNode, SubtreeModified)) {
    196         Ref<JSON::Object> eventData = JSON::Object::create();
     194        return;
     195    }
     196
     197    uint32_t rootBit = 1 << NodeRemoved;
     198    uint32_t derivedBit = rootBit << domBreakpointDerivedTypeShift;
     199    uint32_t matchBit = rootBit | derivedBit;
     200    for (auto& [nodeWithBreakpoint, breakpointTypes] : m_domBreakpoints) {
     201        if (node.contains(nodeWithBreakpoint) && (breakpointTypes & matchBit)) {
     202            auto eventData = JSON::Object::create();
     203            descriptionForDOMEvent(*nodeWithBreakpoint, NodeRemoved, false, eventData.get());
     204            if (auto* domAgent = m_instrumentingAgents.inspectorDOMAgent())
     205                eventData->setInteger("targetNodeId"_s, domAgent->pushNodeToFrontend(&node));
     206            m_debuggerAgent->breakProgram(Inspector::DebuggerFrontendDispatcher::Reason::DOM, WTFMove(eventData));
     207            return;
     208        }
     209    }
     210
     211    auto* parentNode = InspectorDOMAgent::innerParentNode(&node);
     212    if (parentNode && hasBreakpoint(parentNode, SubtreeModified)) {
     213        auto eventData = JSON::Object::create();
    197214        descriptionForDOMEvent(node, SubtreeModified, false, eventData.get());
    198215        m_debuggerAgent->breakProgram(Inspector::DebuggerFrontendDispatcher::Reason::DOM, WTFMove(eventData));
     216        return;
    199217    }
    200218}
    … …  
    274292            // For inheritable breakpoint types, target node isn't always the same as the node that owns a breakpoint.
    275293            // Target node may be unknown to frontend, so we need to push it first.
    276             RefPtr<Inspector::Protocol::Runtime::RemoteObject> targetNodeObject = domAgent->resolveNode(&target, InspectorDebuggerAgent::backtraceObjectGroup);
    277             description.setValue("targetNode", targetNodeObject);
     294            description.setInteger("targetNodeId"_s, domAgent->pushNodeToFrontend(&target));
    278295        }
    279296

  • trunk/Source/WebCore/inspector/agents/page/PageDOMDebuggerAgent.h

    r250996 r251871  
    6565    void descriptionForDOMEvent(Node&, int breakpointType, bool insertion, JSON::Object& description);
    6666    void updateSubtreeBreakpoints(Node*, uint32_t rootMask, bool set);
     67    bool checkSubtreeForNodeRemoved(Node*);
    6768    bool hasBreakpoint(Node*, int type);
    6869

  • trunk/Source/WebInspectorUI/ChangeLog

    r251853 r251871  
     12019-10-31  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: DOMDebugger: Node Removed breakpoints should fire whenever the node is removed from the main DOM tree, not just when it's removed from it's parent
     4        https://bugs.webkit.org/show_bug.cgi?id=203349
     5
     6        Reviewed by Matt Baker.
     7
     8        Replace `targetNode` (which was a `Runtime.RemoteObject`) with a `targetNodeId` (which is a
     9        `DOM.NodeId`) when dispatching `DOMDebugger` pauses. Additionally, include the ancestor's
     10        `DOM.NodeId` as the `targetNodeId` whenever an ancestor is removed that has a descendant
     11        with a node removed DOM breakpoint.
     12
     13        * UserInterface/Views/SourcesNavigationSidebarPanel.js:
     14        (WI.SourcesNavigationSidebarPanel.prototype._updatePauseReasonSection):
     15
     16        * Localizations/en.lproj/localizedStrings.js:
     17
    1182019-10-31  Yury Semikhatsky  <yurys@chromium.org>
    219

  • trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js

    r251624 r251871  
    892892localizedStrings["Remove probe"] = "Remove probe";
    893893localizedStrings["Remove this breakpoint action"] = "Remove this breakpoint action";
     894localizedStrings["Removed ancestor "] = "Removed ancestor ";
    894895localizedStrings["Removed descendant "] = "Removed descendant ";
    895896localizedStrings["Render Pipeline %d"] = "Render Pipeline %d";

  • trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js

    r251531 r251871  
    14661466            this._pauseReasonGroup.rows = [domBreakpointRow, ownerElementRow];
    14671467
    1468             if (domBreakpoint.type !== WI.DOMBreakpoint.Type.SubtreeModified)
    1469                 return true;
    1470 
    1471             console.assert(pauseData.targetNode);
    1472 
    1473             let remoteObject = WI.RemoteObject.fromPayload(pauseData.targetNode, target);
    1474             remoteObject.pushNodeToFrontend((nodeId) => {
     1468            let updateTargetDescription = (nodeId) => {
    14751469                if (!nodeId)
    14761470                    return;
    … …  
    14821476
    14831477                let fragment = document.createDocumentFragment();
    1484                 let description = pauseData.insertion ? WI.UIString("Child added to ") : WI.UIString("Removed descendant ");
     1478
     1479                let description = null;
     1480                switch (domBreakpoint.type) {
     1481                case WI.DOMBreakpoint.Type.SubtreeModified:
     1482                    description = pauseData.insertion ? WI.UIString("Child added to ") : WI.UIString("Removed descendant ");
     1483                    break;
     1484                case WI.DOMBreakpoint.Type.NodeRemoved:
     1485                    description = WI.UIString("Removed ancestor ");
     1486                    break;
     1487                }
     1488                console.assert(description);
    14851489                fragment.append(description, WI.linkifyNodeReference(node));
    14861490
    … …  
    14891493
    14901494                this._pauseReasonGroup.rows = this._pauseReasonGroup.rows.concat(targetDescriptionRow);
    1491             });
     1495            };
     1496
     1497            if (pauseData.targetNodeId) {
     1498                console.assert(domBreakpoint.type === WI.DOMBreakpoint.Type.SubtreeModified || domBreakpoint.type === WI.DOMBreakpoint.Type.NodeRemoved);
     1499                updateTargetDescription(pauseData.targetNodeId);
     1500            } else if (pauseData.targetNode) { // COMPATIBILITY (iOS 13): `targetNode` was renamed to `targetNodeId` and was changed from a `Runtime.RemoteObject` to a `DOM.NodeId`.
     1501                console.assert(domBreakpoint.type === WI.DOMBreakpoint.Type.SubtreeModified);
     1502                let remoteObject = WI.RemoteObject.fromPayload(pauseData.targetNode, target);
     1503                remoteObject.pushNodeToFrontend(updateTargetDescription);
     1504            }
    14921505
    14931506            return true;
Note: See TracChangeset for help on using the changeset viewer.