⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 278785 in webkit


Ignore:
Timestamp:
Jun 11, 2021, 3:37:40 PM (5 years ago)
Author:
Patrick Angle
Message:

Web Inspector: Add instrumentation to node destruction for InspectorDOMAgent
​https://bugs.webkit.org/show_bug.cgi?id=226624

Reviewed by Devin Rousso.

Source/JavaScriptCore:

Add new DOM.willDestroyDOMNode event to inform the frontend of DOM nodes that no longer exist, even if they
weren't in the DOM tree. This work serves as a prelude to <​https://webkit.org/b/189687> (Web Inspector: preserve
DOM.NodeId if a node is removed and re-added) to eventually only forget about nodes upon destruction, instead of
removal from the DOM tree.

  • inspector/protocol/DOM.json:

Source/WebCore:

Test: inspector/dom/willDestroyDOMNode.html

Add instrumentation for destruction of nodes in order to cease instrumenting nodes and inform the frontend that
the node no longer exists. This work serves as a prelude to <​https://webkit.org/b/189687> (Web Inspector:
preserve DOM.NodeId if a node is removed and re-added) to eventually only forget about nodes upon destruction,
instead of removal from the DOM tree. Additionally, the storage of nodes is simplified down to two inverse maps,
one that maps Node to NodeId, and another that maps NodeId to Node. These are kept in sync throughout,
and both attached and detached nodes are now handled as part of these two maps of Nodes.

  • dom/Node.cpp:

(WebCore::Node::~Node):

  • inspector/InspectorInstrumentation.cpp:

(WebCore::InspectorInstrumentation::willDestroyDOMNodeImpl):

  • inspector/InspectorInstrumentation.h:

(WebCore::InspectorInstrumentation::didRemoveDOMNode):
(WebCore::InspectorInstrumentation::willDestroyDOMNode):

  • inspector/agents/InspectorCSSAgent.cpp:

(WebCore::InspectorCSSAgent::didRemoveDOMNode):

  • inspector/agents/InspectorDOMAgent.cpp:

(WebCore::InspectorDOMAgent::InspectorDOMAgent):
(WebCore::InspectorDOMAgent::reset):
(WebCore::InspectorDOMAgent::bind):
(WebCore::InspectorDOMAgent::unbind):
(WebCore::InspectorDOMAgent::getDocument):
(WebCore::InspectorDOMAgent::pushChildNodesToFrontend):
(WebCore::InspectorDOMAgent::discardBindings):
(WebCore::InspectorDOMAgent::pushNodePathToFrontend):
(WebCore::InspectorDOMAgent::boundNodeId):

  • Add a check that the Node* is a valid key (not nullptr) before getting its id.

(WebCore::InspectorDOMAgent::buildObjectForNode):
(WebCore::InspectorDOMAgent::buildArrayForContainerChildren):
(WebCore::InspectorDOMAgent::buildArrayForPseudoElements):
(WebCore::InspectorDOMAgent::didCommitLoad):
(WebCore::InspectorDOMAgent::didInsertDOMNode):
(WebCore::InspectorDOMAgent::didRemoveDOMNode):
(WebCore::InspectorDOMAgent::willDestroyDOMNode):
(WebCore::InspectorDOMAgent::destroyedNodesTimerFired):

  • Added instrumentation point for DOM nodes being destroyed so they can be removed from the agent, and the

frontend can also be informed of their ceasing to exist.
(WebCore::InspectorDOMAgent::characterDataModified):
(WebCore::InspectorDOMAgent::didInvalidateStyleAttr):
(WebCore::InspectorDOMAgent::didPushShadowRoot):
(WebCore::InspectorDOMAgent::willPopShadowRoot):
(WebCore::InspectorDOMAgent::didChangeCustomElementState):
(WebCore::InspectorDOMAgent::pseudoElementCreated):
(WebCore::InspectorDOMAgent::pseudoElementDestroyed):
(WebCore::InspectorDOMAgent::releaseDanglingNodes): Deleted.

  • Removed usage of NodeToIdMap and nested maps of nodes throughout in favor of two inverse maps for relating

Nodes and NodeIds. Because there is now a single set of canonical node maps, we no longer to to pass a
NodeToIdMap throughout the agent.

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

(WebCore::PageConsoleAgent::PageConsoleAgent):
(WebCore::PageConsoleAgent::clearMessages):

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

(WebCore::PageDOMDebuggerAgent::willDestroyDOMNode):

  • inspector/agents/page/PageDOMDebuggerAgent.h:

Source/WebInspectorUI:

Listen for the new DOM.willDestroyDOMNode event in order to cleanup and remaining references to that Node.
This work serves as a prelude to <​https://webkit.org/b/189687> (Web Inspector: preserve DOM.NodeId if a node is
removed and re-added) to eventually only forget about nodes upon destruction, instead of removal from the DOM
tree.

  • UserInterface/Controllers/DOMManager.js:

(WI.DOMManager.prototype.willDestroyDOMNode):

  • UserInterface/Protocol/DOMObserver.js:

(WI.DOMObserver.prototype.willDestroyDOMNode):

  • UserInterface/Views/DOMTreeUpdater.js:

(WI.DOMTreeUpdater.prototype._nodeRemoved):

LayoutTests:

  • inspector/dom/willDestroyDOMNode-expected.txt: Added.
  • inspector/dom/willDestroyDOMNode.html: Added.
Location:
trunk
Files:
2 added
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r278777 r278785  
     12021-06-11  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Add instrumentation to node destruction for InspectorDOMAgent
     4        https://bugs.webkit.org/show_bug.cgi?id=226624
     5
     6        Reviewed by Devin Rousso.
     7
     8        * inspector/dom/willDestroyDOMNode-expected.txt: Added.
     9        * inspector/dom/willDestroyDOMNode.html: Added.
     10
    1112021-06-11  Wenson Hsieh  <wenson_hsieh@apple.com>
    212

  • trunk/Source/JavaScriptCore/ChangeLog

    r278769 r278785  
     12021-06-11  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Add instrumentation to node destruction for InspectorDOMAgent
     4        https://bugs.webkit.org/show_bug.cgi?id=226624
     5
     6        Reviewed by Devin Rousso.
     7
     8        Add new `DOM.willDestroyDOMNode` event to inform the frontend of DOM nodes that no longer exist, even if they
     9        weren't in the DOM tree. This work serves as a prelude to <https://webkit.org/b/189687> (Web Inspector: preserve
     10        DOM.NodeId if a node is removed and re-added) to eventually only forget about nodes upon destruction, instead of
     11        removal from the DOM tree.
     12
     13        * inspector/protocol/DOM.json:
     14
    1152021-06-11  Yijia Huang  <yijia_huang@apple.com>
    216

  • trunk/Source/JavaScriptCore/inspector/protocol/DOM.json

    r272768 r278785  
    680680        },
    681681        {
     682            "name": "willDestroyDOMNode",
     683            "description": "Fired when a detached DOM node is about to be destroyed. Currently, this event will only be fired when a DOM node that is detached is about to be destructed.",
     684            "parameters": [
     685                { "name": "nodeId", "$ref": "NodeId", "description": "Id of the node that will be destroyed." }
     686            ]
     687        },
     688        {
    682689            "name": "shadowRootPushed",
    683690            "description": "Called when shadow root is pushed into the element.",

  • trunk/Source/WebCore/ChangeLog

    r278782 r278785  
     12021-06-11  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Add instrumentation to node destruction for InspectorDOMAgent
     4        https://bugs.webkit.org/show_bug.cgi?id=226624
     5
     6        Reviewed by Devin Rousso.
     7
     8        Test: inspector/dom/willDestroyDOMNode.html
     9
     10        Add instrumentation for destruction of nodes in order to cease instrumenting nodes and inform the frontend that
     11        the node no longer exists. This work serves as a prelude to <https://webkit.org/b/189687> (Web Inspector:
     12        preserve DOM.NodeId if a node is removed and re-added) to eventually only forget about nodes upon destruction,
     13        instead of removal from the DOM tree. Additionally, the storage of nodes is simplified down to two inverse maps,
     14        one that maps `Node` to `NodeId`, and another that maps `NodeId` to `Node`. These are kept in sync throughout,
     15        and both attached and detached nodes are now handled as part of these two maps of Nodes.
     16
     17        * dom/Node.cpp:
     18        (WebCore::Node::~Node):
     19        * inspector/InspectorInstrumentation.cpp:
     20        (WebCore::InspectorInstrumentation::willDestroyDOMNodeImpl):
     21        * inspector/InspectorInstrumentation.h:
     22        (WebCore::InspectorInstrumentation::didRemoveDOMNode):
     23        (WebCore::InspectorInstrumentation::willDestroyDOMNode):
     24        * inspector/agents/InspectorCSSAgent.cpp:
     25        (WebCore::InspectorCSSAgent::didRemoveDOMNode):
     26        * inspector/agents/InspectorDOMAgent.cpp:
     27        (WebCore::InspectorDOMAgent::InspectorDOMAgent):
     28        (WebCore::InspectorDOMAgent::reset):
     29        (WebCore::InspectorDOMAgent::bind):
     30        (WebCore::InspectorDOMAgent::unbind):
     31        (WebCore::InspectorDOMAgent::getDocument):
     32        (WebCore::InspectorDOMAgent::pushChildNodesToFrontend):
     33        (WebCore::InspectorDOMAgent::discardBindings):
     34        (WebCore::InspectorDOMAgent::pushNodePathToFrontend):
     35        (WebCore::InspectorDOMAgent::boundNodeId):
     36        - Add a check that the `Node*` is a valid key (not `nullptr`) before getting its id.
     37        (WebCore::InspectorDOMAgent::buildObjectForNode):
     38        (WebCore::InspectorDOMAgent::buildArrayForContainerChildren):
     39        (WebCore::InspectorDOMAgent::buildArrayForPseudoElements):
     40        (WebCore::InspectorDOMAgent::didCommitLoad):
     41        (WebCore::InspectorDOMAgent::didInsertDOMNode):
     42        (WebCore::InspectorDOMAgent::didRemoveDOMNode):
     43        (WebCore::InspectorDOMAgent::willDestroyDOMNode):
     44        (WebCore::InspectorDOMAgent::destroyedNodesTimerFired):
     45        - Added instrumentation point for DOM nodes being destroyed so they can be removed from the agent, and the
     46        frontend can also be informed of their ceasing to exist.
     47        (WebCore::InspectorDOMAgent::characterDataModified):
     48        (WebCore::InspectorDOMAgent::didInvalidateStyleAttr):
     49        (WebCore::InspectorDOMAgent::didPushShadowRoot):
     50        (WebCore::InspectorDOMAgent::willPopShadowRoot):
     51        (WebCore::InspectorDOMAgent::didChangeCustomElementState):
     52        (WebCore::InspectorDOMAgent::pseudoElementCreated):
     53        (WebCore::InspectorDOMAgent::pseudoElementDestroyed):
     54        (WebCore::InspectorDOMAgent::releaseDanglingNodes): Deleted.
     55        - Removed usage of NodeToIdMap and nested maps of nodes throughout in favor of two inverse maps for relating
     56        `Node`s and `NodeId`s. Because there is now a single set of canonical node maps, we no longer to to pass a
     57        NodeToIdMap throughout the agent.
     58        * inspector/agents/InspectorDOMAgent.h:
     59        * inspector/agents/page/PageConsoleAgent.cpp:
     60        (WebCore::PageConsoleAgent::PageConsoleAgent):
     61        (WebCore::PageConsoleAgent::clearMessages):
     62        * inspector/agents/page/PageConsoleAgent.h:
     63        * inspector/agents/page/PageDOMDebuggerAgent.cpp:
     64        (WebCore::PageDOMDebuggerAgent::willDestroyDOMNode):
     65        * inspector/agents/page/PageDOMDebuggerAgent.h:
     66
    1672021-06-11  Yusuke Suzuki  <ysuzuki@apple.com>
    268

  • trunk/Source/WebCore/dom/Node.cpp

    r278619 r278785  
    5151#include "InputEvent.h"
    5252#include "InspectorController.h"
     53#include "InspectorInstrumentation.h"
    5354#include "KeyboardEvent.h"
    5455#include "Logging.h"
    … …  
    350351    ASSERT(!m_adoptionIsRequired);
    351352
     353    InspectorInstrumentation::willDestroyDOMNode(*this);
     354
    352355#ifndef NDEBUG
    353356    if (!ignoreSet().remove(*this))

  • trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp

    r278717 r278785  
    177177}
    178178
     179void InspectorInstrumentation::willDestroyDOMNodeImpl(InstrumentingAgents& instrumentingAgents, Node& node)
     180{
     181    if (auto* pageDOMDebuggerAgent = instrumentingAgents.enabledPageDOMDebuggerAgent())
     182        pageDOMDebuggerAgent->willDestroyDOMNode(node);
     183    if (auto* domAgent = instrumentingAgents.persistentDOMAgent())
     184        domAgent->willDestroyDOMNode(node);
     185}
     186
    179187void InspectorInstrumentation::nodeLayoutContextChangedImpl(InstrumentingAgents& instrumentingAgents, Node& node, RenderObject* newRenderer)
    180188{

  • trunk/Source/WebCore/inspector/InspectorInstrumentation.h

    r278717 r278785  
    126126    static void willRemoveDOMNode(Document&, Node&);
    127127    static void didRemoveDOMNode(Document&, Node&);
     128    static void willDestroyDOMNode(Node&);
    128129    static void nodeLayoutContextChanged(Node&, RenderObject*);
    129130    static void willModifyDOMAttr(Document&, Element&, const AtomString& oldValue, const AtomString& newValue);
    … …  
    351352    static void willRemoveDOMNodeImpl(InstrumentingAgents&, Node&);
    352353    static void didRemoveDOMNodeImpl(InstrumentingAgents&, Node&);
     354    static void willDestroyDOMNodeImpl(InstrumentingAgents&, Node&);
    353355    static void nodeLayoutContextChangedImpl(InstrumentingAgents&, Node&, RenderObject*);
    354356    static void willModifyDOMAttrImpl(InstrumentingAgents&, Element&, const AtomString& oldValue, const AtomString& newValue);
    … …  
    602604}
    603605
     606inline void InspectorInstrumentation::willDestroyDOMNode(Node& node)
     607{
     608    FAST_RETURN_IF_NO_FRONTENDS(void());
     609    if (auto* agents = instrumentingAgents(node.document()))
     610        willDestroyDOMNodeImpl(*agents, node);
     611}
     612
    604613inline void InspectorInstrumentation::nodeLayoutContextChanged(Node& node, RenderObject* newRenderer)
    605614{

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

    r278340 r278785  
    11601160void InspectorCSSAgent::didRemoveDOMNode(Node& node, Protocol::DOM::NodeId nodeId)
    11611161{
     1162    // This can be called in response to GC.
    11621163    m_nodeIdToForcedPseudoState.remove(nodeId);
    11631164

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

    r278340 r278785  
    289289    , m_inspectedPage(context.inspectedPage)
    290290    , m_overlay(overlay)
     291    , m_destroyedNodesTimer(*this, &InspectorDOMAgent::destroyedNodesTimerFired)
    291292#if ENABLE(VIDEO)
    292293    , m_mediaMetricsTimer(*this, &InspectorDOMAgent::mediaMetricsTimerFired)
    … …  
    354355        m_revalidateStyleAttrTask->reset();
    355356    m_document = nullptr;
     357
     358    m_destroyedDetachedNodeIdentifiers.clear();
     359    m_destroyedAttachedNodeIdentifiers.clear();
     360    if (m_destroyedNodesTimer.isActive())
     361        m_destroyedNodesTimer.stop();
    356362}
    357363
    … …  
    373379}
    374380
    375 void InspectorDOMAgent::releaseDanglingNodes()
    376 {
    377     m_danglingNodeToIdMaps.clear();
    378 }
    379 
    380 Protocol::DOM::NodeId InspectorDOMAgent::bind(Node* node, NodeToIdMap* nodesMap)
    381 {
    382     auto id = nodesMap->get(node);
    383     if (id)
     381Protocol::DOM::NodeId InspectorDOMAgent::bind(Node& node)
     382{
     383    return m_nodeToId.ensure(&node, [&] {
     384        auto id = m_lastNodeId++;
     385        m_idToNode.set(id, &node);
    384386        return id;
    385     id = m_lastNodeId++;
    386     nodesMap->set(node, id);
    387     m_idToNode.set(id, node);
    388     m_idToNodesMap.set(id, nodesMap);
    389     return id;
    390 }
    391 
    392 void InspectorDOMAgent::unbind(Node* node, NodeToIdMap* nodesMap)
    393 {
    394     auto id = nodesMap->get(node);
     387    }).iterator->value;
     388}
     389
     390void InspectorDOMAgent::unbind(Node& node)
     391{
     392    auto id = m_nodeToId.take(&node);
    395393    if (!id)
    396394        return;
    … …  
    398396    m_idToNode.remove(id);
    399397
    400     if (node->isFrameOwnerElement()) {
    401         const HTMLFrameOwnerElement* frameOwner = static_cast<const HTMLFrameOwnerElement*>(node);
     398    if (node.isFrameOwnerElement()) {
     399        const HTMLFrameOwnerElement* frameOwner = static_cast<const HTMLFrameOwnerElement*>(&node);
    402400        if (Document* contentDocument = frameOwner->contentDocument())
    403             unbind(contentDocument, nodesMap);
    404     }
    405 
    406     if (is<Element>(*node)) {
    407         Element& element = downcast<Element>(*node);
     401            unbind(*contentDocument);
     402    }
     403
     404    if (is<Element>(node)) {
     405        Element& element = downcast<Element>(node);
    408406        if (ShadowRoot* root = element.shadowRoot())
    409             unbind(root, nodesMap);
     407            unbind(*root);
    410408        if (PseudoElement* beforeElement = element.beforePseudoElement())
    411             unbind(beforeElement, nodesMap);
     409            unbind(*beforeElement);
    412410        if (PseudoElement* afterElement = element.afterPseudoElement())
    413             unbind(afterElement, nodesMap);
    414     }
    415 
    416     nodesMap->remove(node);
     411            unbind(*afterElement);
     412    }
    417413
    418414    if (auto* cssAgent = m_instrumentingAgents.enabledCSSAgent())
    419         cssAgent->didRemoveDOMNode(*node, id);
     415        cssAgent->didRemoveDOMNode(node, id);
    420416
    421417    if (m_childrenRequested.remove(id)) {
    422418        // FIXME: Would be better to do this iteratively rather than recursively.
    423         for (Node* child = innerFirstChild(node); child; child = innerNextSibling(child))
    424             unbind(child, nodesMap);
     419        for (Node* child = innerFirstChild(&node); child; child = innerNextSibling(child))
     420            unbind(*child);
    425421    }
    426422}
    … …  
    500496    m_document = document;
    501497
    502     auto root = buildObjectForNode(m_document.get(), 2, &m_documentNodeToIdMap);
     498    auto root = buildObjectForNode(m_document.get(), 2);
    503499
    504500    if (m_nodeToFocus)
    … …  
    513509    if (!node || (node->nodeType() != Node::ELEMENT_NODE && node->nodeType() != Node::DOCUMENT_NODE && node->nodeType() != Node::DOCUMENT_FRAGMENT_NODE))
    514510        return;
    515 
    516     NodeToIdMap* nodeMap = m_idToNodesMap.get(nodeId);
    517511
    518512    if (m_childrenRequested.contains(nodeId)) {
    … …  
    523517
    524518        for (node = innerFirstChild(node); node; node = innerNextSibling(node)) {
    525             auto childNodeId = nodeMap->get(node);
     519            auto childNodeId = boundNodeId(node);
    526520            ASSERT(childNodeId);
    527521            pushChildNodesToFrontend(childNodeId, depth);
    … …  
    531525    }
    532526
    533     auto children = buildArrayForContainerChildren(node, depth, nodeMap);
     527    auto children = buildArrayForContainerChildren(node, depth);
    534528    m_frontendDispatcher->setChildNodes(nodeId, WTFMove(children));
    535529}
    … …  
    537531void InspectorDOMAgent::discardBindings()
    538532{
    539     m_documentNodeToIdMap.clear();
     533    m_nodeToId.clear();
    540534    m_idToNode.clear();
    541535    m_dispatchedEvents.clear();
    542536    m_eventListenerEntries.clear();
    543     releaseDanglingNodes();
    544537    m_childrenRequested.clear();
    545538}
    … …  
    654647
    655648    // FIXME: <https://webkit.org/b/213499> Web Inspector: allow DOM nodes to be instrumented at any point, regardless of whether the main document has also been instrumented
    656     if (!m_documentNodeToIdMap.contains(m_document)) {
     649    if (!m_nodeToId.contains(m_document.get())) {
    657650        errorString = "Document must have been requested"_s;
    658651        return 0;
    … …  
    660653
    661654    // Return id in case the node is known.
    662     if (auto result = m_documentNodeToIdMap.get(nodeToPush))
     655    if (auto result = boundNodeId(nodeToPush))
    663656        return result;
    664657
    665658    Node* node = nodeToPush;
    666659    Vector<Node*> path;
    667     NodeToIdMap* danglingMap = 0;
    668660
    669661    while (true) {
    … …  
    671663        if (!parent) {
    672664            // Node being pushed is detached -> push subtree root.
    673             auto newMap = makeUnique<NodeToIdMap>();
    674             danglingMap = newMap.get();
    675             m_danglingNodeToIdMaps.append(newMap.release());
    676665            auto children = JSON::ArrayOf<Protocol::DOM::Node>::create();
    677             children->addItem(buildObjectForNode(node, 0, danglingMap));
     666            children->addItem(buildObjectForNode(node, 0));
    678667            m_frontendDispatcher->setChildNodes(0, WTFMove(children));
    679668            break;
    680669        } else {
    681670            path.append(parent);
    682             if (m_documentNodeToIdMap.get(parent))
     671            if (boundNodeId(parent))
    683672                break;
    684             else
    685                 node = parent;
     673            node = parent;
    686674        }
    687675    }
    688676
    689     NodeToIdMap* map = danglingMap ? danglingMap : &m_documentNodeToIdMap;
    690677    for (int i = path.size() - 1; i >= 0; --i) {
    691         auto nodeId = map->get(path.at(i));
     678        auto nodeId = boundNodeId(path.at(i));
    692679        ASSERT(nodeId);
    693680        pushChildNodesToFrontend(nodeId);
    694681    }
    695     return map->get(nodeToPush);
     682    return boundNodeId(nodeToPush);
    696683}
    697684
    698685Protocol::DOM::NodeId InspectorDOMAgent::boundNodeId(const Node* node)
    699686{
    700     return m_documentNodeToIdMap.get(const_cast<Node*>(node));
     687    if (!m_nodeToId.isValidKey(node))
     688        return 0;
     689
     690    return m_nodeToId.get(node);
    701691}
    702692
    … …  
    17121702}
    17131703
    1714 Ref<Protocol::DOM::Node> InspectorDOMAgent::buildObjectForNode(Node* node, int depth, NodeToIdMap* nodesMap)
    1715 {
    1716     auto id = bind(node, nodesMap);
     1704Ref<Protocol::DOM::Node> InspectorDOMAgent::buildObjectForNode(Node* node, int depth)
     1705{
     1706    auto id = bind(*node);
    17171707    String nodeName;
    17181708    String localName;
    … …  
    17561746        int nodeCount = innerChildNodeCount(node);
    17571747        value->setChildNodeCount(nodeCount);
    1758         auto children = buildArrayForContainerChildren(node, depth, nodesMap);
     1748        auto children = buildArrayForContainerChildren(node, depth);
    17591749        if (children->length() > 0)
    17601750            value->setChildren(WTFMove(children));
    … …  
    17751765        if (is<HTMLFrameOwnerElement>(element)) {
    17761766            if (auto* document = downcast<HTMLFrameOwnerElement>(element).contentDocument())
    1777                 value->setContentDocument(buildObjectForNode(document, 0, nodesMap));
     1767                value->setContentDocument(buildObjectForNode(document, 0));
    17781768        }
    17791769
    17801770        if (ShadowRoot* root = element.shadowRoot()) {
    17811771            auto shadowRoots = JSON::ArrayOf<Protocol::DOM::Node>::create();
    1782             shadowRoots->addItem(buildObjectForNode(root, 0, nodesMap));
     1772            shadowRoots->addItem(buildObjectForNode(root, 0));
    17831773            value->setShadowRoots(WTFMove(shadowRoots));
    17841774        }
    17851775
    17861776        if (is<HTMLTemplateElement>(element))
    1787             value->setTemplateContent(buildObjectForNode(&downcast<HTMLTemplateElement>(element).content(), 0, nodesMap));
     1777            value->setTemplateContent(buildObjectForNode(&downcast<HTMLTemplateElement>(element).content(), 0));
    17881778
    17891779        if (is<HTMLStyleElement>(element) || (is<HTMLScriptElement>(element) && !element.hasAttributeWithoutSynchronization(HTMLNames::srcAttr)))
    … …  
    17991789                value->setPseudoType(pseudoType);
    18001790        } else {
    1801             if (auto pseudoElements = buildArrayForPseudoElements(element, nodesMap))
     1791            if (auto pseudoElements = buildArrayForPseudoElements(element))
    18021792                value->setPseudoElements(pseudoElements.releaseNonNull());
    18031793        }
    … …  
    18391829}
    18401830
    1841 Ref<JSON::ArrayOf<Protocol::DOM::Node>> InspectorDOMAgent::buildArrayForContainerChildren(Node* container, int depth, NodeToIdMap* nodesMap)
     1831Ref<JSON::ArrayOf<Protocol::DOM::Node>> InspectorDOMAgent::buildArrayForContainerChildren(Node* container, int depth)
    18421832{
    18431833    auto children = JSON::ArrayOf<Protocol::DOM::Node>::create();
    … …  
    18461836        Node* firstChild = container->firstChild();
    18471837        if (firstChild && firstChild->nodeType() == Node::TEXT_NODE && !firstChild->nextSibling()) {
    1848             children->addItem(buildObjectForNode(firstChild, 0, nodesMap));
    1849             m_childrenRequested.add(bind(container, nodesMap));
     1838            children->addItem(buildObjectForNode(firstChild, 0));
     1839            m_childrenRequested.add(bind(*container));
    18501840        }
    18511841        return children;
    … …  
    18541844    Node* child = innerFirstChild(container);
    18551845    depth--;
    1856     m_childrenRequested.add(bind(container, nodesMap));
     1846    m_childrenRequested.add(bind(*container));
    18571847
    18581848    while (child) {
    1859         children->addItem(buildObjectForNode(child, depth, nodesMap));
     1849        children->addItem(buildObjectForNode(child, depth));
    18601850        child = innerNextSibling(child);
    18611851    }
    … …  
    18631853}
    18641854
    1865 RefPtr<JSON::ArrayOf<Protocol::DOM::Node>> InspectorDOMAgent::buildArrayForPseudoElements(const Element& element, NodeToIdMap* nodesMap)
     1855RefPtr<JSON::ArrayOf<Protocol::DOM::Node>> InspectorDOMAgent::buildArrayForPseudoElements(const Element& element)
    18661856{
    18671857    PseudoElement* beforeElement = element.beforePseudoElement();
    … …  
    18721862    auto pseudoElements = JSON::ArrayOf<Protocol::DOM::Node>::create();
    18731863    if (beforeElement)
    1874         pseudoElements->addItem(buildObjectForNode(beforeElement, 0, nodesMap));
     1864        pseudoElements->addItem(buildObjectForNode(beforeElement, 0));
    18751865    if (afterElement)
    1876         pseudoElements->addItem(buildObjectForNode(afterElement, 0, nodesMap));
     1866        pseudoElements->addItem(buildObjectForNode(afterElement, 0));
    18771867    return pseudoElements;
    18781868}
    … …  
    23632353        return;
    23642354
    2365     auto frameOwnerId = m_documentNodeToIdMap.get(frameOwner);
     2355    auto frameOwnerId = boundNodeId(frameOwner.get());
    23662356    if (!frameOwnerId)
    23672357        return;
    23682358
    23692359    // Re-add frame owner element together with its new children.
    2370     auto parentId = m_documentNodeToIdMap.get(innerParentNode(frameOwner.get()));
     2360    auto parentId = boundNodeId(innerParentNode(frameOwner.get()));
    23712361    m_frontendDispatcher->childNodeRemoved(parentId, frameOwnerId);
    2372     unbind(frameOwner.get(), &m_documentNodeToIdMap);
    2373 
    2374     auto value = buildObjectForNode(frameOwner.get(), 0, &m_documentNodeToIdMap);
     2362    unbind(*frameOwner);
     2363
     2364    auto value = buildObjectForNode(frameOwner.get(), 0);
    23752365    Node* previousSibling = innerPreviousSibling(frameOwner.get());
    2376     auto prevId = previousSibling ? m_documentNodeToIdMap.get(previousSibling) : 0;
     2366    auto prevId = boundNodeId(previousSibling);
    23772367    m_frontendDispatcher->childNodeInserted(parentId, prevId, WTFMove(value));
    23782368}
    … …  
    24292419
    24302420    // We could be attaching existing subtree. Forget the bindings.
    2431     unbind(&node, &m_documentNodeToIdMap);
     2421    unbind(node);
    24322422
    24332423    ContainerNode* parent = node.parentNode();
    2434     if (!parent)
    2435         return;
    2436 
    2437     auto parentId = m_documentNodeToIdMap.get(parent);
     2424
     2425    auto parentId = boundNodeId(parent);
    24382426    // Return if parent is not mapped yet.
    24392427    if (!parentId)
    … …  
    24462434        // Children have been requested -> return value of a new child.
    24472435        Node* prevSibling = innerPreviousSibling(&node);
    2448         auto prevId = prevSibling ? m_documentNodeToIdMap.get(prevSibling) : 0;
    2449         auto value = buildObjectForNode(&node, 0, &m_documentNodeToIdMap);
     2436        auto prevId = boundNodeId(prevSibling);
     2437        auto value = buildObjectForNode(&node, 0);
    24502438        m_frontendDispatcher->childNodeInserted(parentId, prevId, WTFMove(value));
    24512439    }
    … …  
    24592447    ContainerNode* parent = node.parentNode();
    24602448
     2449    auto parentId = boundNodeId(parent);
    24612450    // If parent is not mapped yet -> ignore the event.
    2462     if (!m_documentNodeToIdMap.contains(parent))
    2463         return;
    2464 
    2465     auto parentId = m_documentNodeToIdMap.get(parent);
    2466 
     2451    if (!parentId)
     2452        return;
     2453
     2454    // FIXME: <webkit.org/b/189687> Preserve DOM.NodeId if a node is removed and re-added
    24672455    if (!m_childrenRequested.contains(parentId)) {
    24682456        // No children are mapped yet -> only notify on changes of hasChildren.
    … …  
    24702458            m_frontendDispatcher->childNodeCountUpdated(parentId, 0);
    24712459    } else
    2472         m_frontendDispatcher->childNodeRemoved(parentId, m_documentNodeToIdMap.get(&node));
    2473     unbind(&node, &m_documentNodeToIdMap);
     2460        m_frontendDispatcher->childNodeRemoved(parentId, boundNodeId(&node));
     2461    unbind(node);
     2462}
     2463
     2464void InspectorDOMAgent::willDestroyDOMNode(Node& node)
     2465{
     2466    if (containsOnlyHTMLWhitespace(&node))
     2467        return;
     2468
     2469    auto nodeId = m_nodeToId.take(&node);
     2470    if (!nodeId)
     2471        return;
     2472
     2473    m_idToNode.remove(nodeId);
     2474    m_childrenRequested.remove(nodeId);
     2475
     2476    if (auto* cssAgent = m_instrumentingAgents.enabledCSSAgent())
     2477        cssAgent->didRemoveDOMNode(node, nodeId);
     2478
     2479    // This can be called in response to GC. Due to the single-process model used in WebKit1, the
     2480    // event must be dispatched from a timer to prevent the frontend from making JS allocations
     2481    // while the GC is still active.
     2482
     2483    // FIXME: <webkit.org/b/189687> Unify m_destroyedAttachedNodeIdentifiers and m_destroyedDetachedNodeIdentifiers.
     2484    if (auto parentId = boundNodeId(node.parentNode()))
     2485        m_destroyedAttachedNodeIdentifiers.append({ parentId, nodeId });
     2486    else
     2487        m_destroyedDetachedNodeIdentifiers.append(nodeId);
     2488
     2489    if (!m_destroyedNodesTimer.isActive())
     2490        m_destroyedNodesTimer.startOneShot(0_s);
     2491}
     2492
     2493void InspectorDOMAgent::destroyedNodesTimerFired()
     2494{
     2495    for (auto& [parentId, nodeId] : std::exchange(m_destroyedAttachedNodeIdentifiers, { })) {
     2496        if (!m_childrenRequested.contains(parentId)) {
     2497            auto* parent = nodeForId(parentId);
     2498            if (parent && innerChildNodeCount(parent) == 1)
     2499                m_frontendDispatcher->childNodeCountUpdated(parentId, 0);
     2500        } else
     2501            m_frontendDispatcher->childNodeRemoved(parentId, nodeId);
     2502    }
     2503   
     2504    for (auto nodeId : std::exchange(m_destroyedDetachedNodeIdentifiers, { }))
     2505        m_frontendDispatcher->willDestroyDOMNode(nodeId);
    24742506}
    24752507
    … …  
    25262558void InspectorDOMAgent::characterDataModified(CharacterData& characterData)
    25272559{
    2528     auto id = m_documentNodeToIdMap.get(&characterData);
     2560    auto id = boundNodeId(&characterData);
    25292561    if (!id) {
    25302562        // Push text node if it is being created.
    … …  
    25372569void InspectorDOMAgent::didInvalidateStyleAttr(Element& element)
    25382570{
    2539     auto id = m_documentNodeToIdMap.get(&element);
     2571    auto id = boundNodeId(&element);
    25402572    if (!id)
    25412573        return;
    … …  
    25482580void InspectorDOMAgent::didPushShadowRoot(Element& host, ShadowRoot& root)
    25492581{
    2550     auto hostId = m_documentNodeToIdMap.get(&host);
     2582    auto hostId = boundNodeId(&host);
    25512583    if (hostId)
    2552         m_frontendDispatcher->shadowRootPushed(hostId, buildObjectForNode(&root, 0, &m_documentNodeToIdMap));
     2584        m_frontendDispatcher->shadowRootPushed(hostId, buildObjectForNode(&root, 0));
    25532585}
    25542586
    25552587void InspectorDOMAgent::willPopShadowRoot(Element& host, ShadowRoot& root)
    25562588{
    2557     auto hostId = m_documentNodeToIdMap.get(&host);
    2558     auto rootId = m_documentNodeToIdMap.get(&root);
     2589    auto hostId = boundNodeId(&host);
     2590    auto rootId = boundNodeId(&root);
    25592591    if (hostId && rootId)
    25602592        m_frontendDispatcher->shadowRootPopped(hostId, rootId);
    … …  
    25632595void InspectorDOMAgent::didChangeCustomElementState(Element& element)
    25642596{
    2565     auto elementId = m_documentNodeToIdMap.get(&element);
     2597    auto elementId = boundNodeId(&element);
    25662598    if (!elementId)
    25672599        return;
    … …  
    25902622        return;
    25912623
    2592     auto parentId = m_documentNodeToIdMap.get(parent);
     2624    auto parentId = boundNodeId(parent);
    25932625    if (!parentId)
    25942626        return;
    25952627
    25962628    pushChildNodesToFrontend(parentId, 1);
    2597     m_frontendDispatcher->pseudoElementAdded(parentId, buildObjectForNode(&pseudoElement, 0, &m_documentNodeToIdMap));
     2629    m_frontendDispatcher->pseudoElementAdded(parentId, buildObjectForNode(&pseudoElement, 0));
    25982630}
    25992631
    26002632void InspectorDOMAgent::pseudoElementDestroyed(PseudoElement& pseudoElement)
    26012633{
    2602     auto pseudoElementId = m_documentNodeToIdMap.get(&pseudoElement);
     2634    auto pseudoElementId = boundNodeId(&pseudoElement);
    26032635    if (!pseudoElementId)
    26042636        return;
    … …  
    26072639    Element* parent = pseudoElement.hostElement();
    26082640    ASSERT(parent);
    2609     auto parentId = m_documentNodeToIdMap.get(parent);
     2641    auto parentId = boundNodeId(parent);
    26102642    ASSERT(parentId);
    26112643
    2612     unbind(&pseudoElement, &m_documentNodeToIdMap);
     2644    unbind(pseudoElement);
    26132645    m_frontendDispatcher->pseudoElementRemoved(parentId, pseudoElementId);
    26142646}

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

    r278253 r278785  
    160160    void didInsertDOMNode(Node&);
    161161    void didRemoveDOMNode(Node&);
     162    void willDestroyDOMNode(Node&);
    162163    void willModifyDOMAttr(Element&, const AtomString& oldValue, const AtomString& newValue);
    163164    void didModifyDOMAttr(Element&, const AtomString& name, const AtomString& value);
    … …  
    180181    // Callbacks that don't directly correspond to an instrumentation entry point.
    181182    void setDocument(Document*);
    182     void releaseDanglingNodes();
    183183
    184184    void styleAttributeInvalidated(const Vector<Element*>& elements);
    … …  
    219219
    220220    // Node-related methods.
    221     typedef HashMap<RefPtr<Node>, Inspector::Protocol::DOM::NodeId> NodeToIdMap;
    222     Inspector::Protocol::DOM::NodeId bind(Node*, NodeToIdMap*);
    223     void unbind(Node*, NodeToIdMap*);
     221    Inspector::Protocol::DOM::NodeId bind(Node&);
     222    void unbind(Node&);
    224223
    225224    Node* assertEditableNode(Inspector::Protocol::ErrorString&, Inspector::Protocol::DOM::NodeId);
    … …  
    228227    void pushChildNodesToFrontend(Inspector::Protocol::DOM::NodeId, int depth = 1);
    229228
    230     Ref<Inspector::Protocol::DOM::Node> buildObjectForNode(Node*, int depth, NodeToIdMap*);
     229    Ref<Inspector::Protocol::DOM::Node> buildObjectForNode(Node*, int depth);
    231230    Ref<JSON::ArrayOf<String>> buildArrayForElementAttributes(Element*);
    232     Ref<JSON::ArrayOf<Inspector::Protocol::DOM::Node>> buildArrayForContainerChildren(Node* container, int depth, NodeToIdMap* nodesMap);
    233     RefPtr<JSON::ArrayOf<Inspector::Protocol::DOM::Node>> buildArrayForPseudoElements(const Element&, NodeToIdMap* nodesMap);
     231    Ref<JSON::ArrayOf<Inspector::Protocol::DOM::Node>> buildArrayForContainerChildren(Node* container, int depth);
     232    RefPtr<JSON::ArrayOf<Inspector::Protocol::DOM::Node>> buildArrayForPseudoElements(const Element&);
    234233    Ref<Inspector::Protocol::DOM::EventListener> buildObjectForEventListener(const RegisteredEventListener&, Inspector::Protocol::DOM::EventListenerId identifier, EventTarget&, const AtomString& eventType, bool disabled, const RefPtr<JSC::Breakpoint>&);
    235234    Ref<Inspector::Protocol::DOM::AccessibilityProperties> buildObjectForAccessibilityProperties(Node&);
    … …  
    243242    void innerHighlightQuad(std::unique_ptr<FloatQuad>, RefPtr<JSON::Object>&& color, RefPtr<JSON::Object>&& outlineColor, std::optional<bool>&& usePageCoordinates);
    244243
     244    void destroyedNodesTimerFired();
     245
    245246    Inspector::InjectedScriptManager& m_injectedScriptManager;
    246247    std::unique_ptr<Inspector::DOMFrontendDispatcher> m_frontendDispatcher;
    … …  
    248249    Page& m_inspectedPage;
    249250    InspectorOverlay* m_overlay { nullptr };
    250     NodeToIdMap m_documentNodeToIdMap;
    251     // Owns node mappings for dangling nodes.
    252     Vector<std::unique_ptr<NodeToIdMap>> m_danglingNodeToIdMaps;
     251    HashMap<const Node*, Inspector::Protocol::DOM::NodeId> m_nodeToId;
    253252    HashMap<Inspector::Protocol::DOM::NodeId, Node*> m_idToNode;
    254     HashMap<Inspector::Protocol::DOM::NodeId, NodeToIdMap*> m_idToNodesMap;
    255253    HashSet<Inspector::Protocol::DOM::NodeId> m_childrenRequested;
    256254    Inspector::Protocol::DOM::NodeId m_lastNodeId { 1 };
    … …  
    265263    std::unique_ptr<InspectorHistory> m_history;
    266264    std::unique_ptr<DOMEditor> m_domEditor;
     265
     266    Vector<Inspector::Protocol::DOM::NodeId> m_destroyedDetachedNodeIdentifiers;
     267    Vector<std::pair<Inspector::Protocol::DOM::NodeId, Inspector::Protocol::DOM::NodeId>> m_destroyedAttachedNodeIdentifiers;
     268    Timer m_destroyedNodesTimer;
    267269
    268270#if ENABLE(VIDEO)

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

    r271735 r278785  
    4848PageConsoleAgent::PageConsoleAgent(PageAgentContext& context)
    4949    : WebConsoleAgent(context)
    50     , m_instrumentingAgents(context.instrumentingAgents)
    5150    , m_inspectedPage(context.inspectedPage)
    5251{
    … …  
    5453
    5554PageConsoleAgent::~PageConsoleAgent() = default;
    56 
    57 Protocol::ErrorStringOr<void> PageConsoleAgent::clearMessages()
    58 {
    59     if (auto* domAgent = m_instrumentingAgents.persistentDOMAgent())
    60         domAgent->releaseDanglingNodes();
    61 
    62     return WebConsoleAgent::clearMessages();
    63 }
    6455
    6556Protocol::ErrorStringOr<Ref<JSON::ArrayOf<Protocol::Console::Channel>>> PageConsoleAgent::getLoggingChannels()

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

    r266885 r278785  
    4545
    4646    // ConsoleBackendDispatcherHandler
    47     Inspector::Protocol::ErrorStringOr<void> clearMessages();
    4847    Inspector::Protocol::ErrorStringOr<Ref<JSON::ArrayOf<Inspector::Protocol::Console::Channel>>> getLoggingChannels();
    4948    Inspector::Protocol::ErrorStringOr<void> setLoggingChannelLevel(Inspector::Protocol::Console::ChannelSource, Inspector::Protocol::Console::ChannelLevel);
    5049
    5150private:
    52     InstrumentingAgents& m_instrumentingAgents;
    5351    Page& m_inspectedPage;
    5452};

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

    r278340 r278785  
    277277}
    278278
     279void PageDOMDebuggerAgent::willDestroyDOMNode(Node& node)
     280{
     281    // This can be called in response to GC.
     282    // DOM Node destruction should be treated as if the node was removed from the DOM tree.
     283    didRemoveDOMNode(node);
     284}
     285
    279286void PageDOMDebuggerAgent::willModifyDOMAttr(Element& element)
    280287{

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

    r266885 r278785  
    5454    void willRemoveDOMNode(Node&);
    5555    void didRemoveDOMNode(Node&);
     56    void willDestroyDOMNode(Node&);
    5657    void willModifyDOMAttr(Element&);
    5758    void willInvalidateStyleAttr(Element&);

  • trunk/Source/WebInspectorUI/ChangeLog

    r278607 r278785  
     12021-06-11  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Add instrumentation to node destruction for InspectorDOMAgent
     4        https://bugs.webkit.org/show_bug.cgi?id=226624
     5
     6        Reviewed by Devin Rousso.
     7
     8        Listen for the new `DOM.willDestroyDOMNode` event in order to cleanup and remaining references to that Node.
     9        This work serves as a prelude to <https://webkit.org/b/189687> (Web Inspector: preserve DOM.NodeId if a node is
     10        removed and re-added) to eventually only forget about nodes upon destruction, instead of removal from the DOM
     11        tree.
     12
     13        * UserInterface/Controllers/DOMManager.js:
     14        (WI.DOMManager.prototype.willDestroyDOMNode):
     15        * UserInterface/Protocol/DOMObserver.js:
     16        (WI.DOMObserver.prototype.willDestroyDOMNode):
     17        * UserInterface/Views/DOMTreeUpdater.js:
     18        (WI.DOMTreeUpdater.prototype._nodeRemoved):
     19
    1202021-06-08  Razvan Caliman  <rcaliman@apple.com>
    221

  • trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js

    r272566 r278785  
    186186    // DOMObserver
    187187
     188    willDestroyDOMNode(nodeId)
     189    {
     190        let node = this._idToDOMNode[nodeId];
     191        node.markDestroyed();
     192        delete this._idToDOMNode[nodeId];
     193
     194        this.dispatchEventToListeners(WI.DOMManager.Event.NodeRemoved, {node});
     195    }
     196
    188197    didAddEventListener(nodeId)
    189198    {

  • trunk/Source/WebInspectorUI/UserInterface/Protocol/DOMObserver.js

    r251227 r278785  
    7878    }
    7979
     80    willDestroyDOMNode(nodeId)
     81    {
     82        WI.domManager.willDestroyDOMNode(nodeId);
     83    }
     84
    8085    shadowRootPushed(hostId, root)
    8186    {

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

    r269359 r278785  
    104104    _nodeRemoved: function(event)
    105105    {
    106         this._recentlyDeletedNodes.set(event.data.node, {parent: event.data.parent});
     106        let parent = event.data.parent;
     107        if (!parent)
     108            return;
     109
     110        this._recentlyDeletedNodes.set(event.data.node, {parent});
    107111        if (this._treeOutline._visible)
    108112            this._updateModifiedNodesDebouncer.delayForFrame();
Note: See TracChangeset for help on using the changeset viewer.