[webkit-reviews] review granted: [Bug 226624] Web Inspector: Add instrumentation to node destruction for InspectorDOMAgent : [Attachment 431219] Patch v2.3 - Add valid key check to InspectorDOMAgent::boundNodeId
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jun 11 12:36:51 PDT 2021
Devin Rousso <drousso at apple.com> has granted Patrick Angle <pangle at apple.com>'s
request for review:
Bug 226624: Web Inspector: Add instrumentation to node destruction for
InspectorDOMAgent
https://bugs.webkit.org/show_bug.cgi?id=226624
Attachment 431219: Patch v2.3 - Add valid key check to
InspectorDOMAgent::boundNodeId
https://bugs.webkit.org/attachment.cgi?id=431219&action=review
--- Comment #14 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 431219
--> https://bugs.webkit.org/attachment.cgi?id=431219
Patch v2.3 - Add valid key check to InspectorDOMAgent::boundNodeId
View in context: https://bugs.webkit.org/attachment.cgi?id=431219&action=review
r=mews, awesome awesome awesome
> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:382
> +Protocol::DOM::NodeId InspectorDOMAgent::bind(Node* node)
NIT: I wonder if we should make this into `Node&` while here so that we know
for sure that `m_nodeToId.ensure` won't complain about an invalid key (e.g. not
`nullptr`). Alternatively you could check it just like `boundNodeId`.
> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:391
> +void InspectorDOMAgent::unbind(Node* node)
ditto (:382)
> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:2367
> + auto prevId = previousSibling ? boundNodeId(previousSibling) : 0;
you can drop the `previousSibling ? ` check now that `boundNodeId` does it for
you
> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:2425
> if (!parent)
ditto (:2367)
> Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:2439
> + auto prevId = prevSibling ? boundNodeId(prevSibling) : 0;
ditto (:2367)
> Source/WebCore/inspector/agents/page/PageConsoleAgent.cpp:56
> Protocol::ErrorStringOr<void> PageConsoleAgent::clearMessages()
you can just remove this function entirely now
More information about the webkit-reviews
mailing list