From ddea67034f81ca1b68edc8720601e28d232a445a Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 6 Mar 2025 19:49:27 -0500 Subject: [PATCH] LibDevTools: Associate node actors with a DOM node identifier This is a prepatory commit to be able to handle DOM mutations. Once a node actor is created, the DOM node it is created for must continue to be associated with the same actor even after DOM mutations. This change stores an identifier on the node actor, and only creates new actors when an actor for a node does not exist. --- .../LibDevTools/Actors/HighlighterActor.cpp | 2 +- Libraries/LibDevTools/Actors/NodeActor.cpp | 24 +++++++-- Libraries/LibDevTools/Actors/NodeActor.h | 19 ++++++- .../LibDevTools/Actors/PageStyleActor.cpp | 2 +- Libraries/LibDevTools/Actors/WalkerActor.cpp | 50 ++++++++++++------- Libraries/LibDevTools/Actors/WalkerActor.h | 11 ++-- 6 files changed, 80 insertions(+), 28 deletions(-) diff --git a/Libraries/LibDevTools/Actors/HighlighterActor.cpp b/Libraries/LibDevTools/Actors/HighlighterActor.cpp index a69e4caaecf..463ad4dbc7b 100644 --- a/Libraries/LibDevTools/Actors/HighlighterActor.cpp +++ b/Libraries/LibDevTools/Actors/HighlighterActor.cpp @@ -42,7 +42,7 @@ void HighlighterActor::handle_message(StringView type, JsonObject const& message response.set("value"sv, false); if (auto dom_node = WalkerActor::dom_node_for(InspectorActor::walker_for(m_inspector), *node); dom_node.has_value()) { - devtools().delegate().highlight_dom_node(dom_node->tab->description(), dom_node->id, dom_node->pseudo_element); + devtools().delegate().highlight_dom_node(dom_node->tab->description(), dom_node->identifier.id, dom_node->identifier.pseudo_element); response.set("value"sv, true); } diff --git a/Libraries/LibDevTools/Actors/NodeActor.cpp b/Libraries/LibDevTools/Actors/NodeActor.cpp index d826bdecd76..1c9869e6e3f 100644 --- a/Libraries/LibDevTools/Actors/NodeActor.cpp +++ b/Libraries/LibDevTools/Actors/NodeActor.cpp @@ -11,13 +11,31 @@ namespace DevTools { -NonnullRefPtr NodeActor::create(DevToolsServer& devtools, String name, WeakPtr walker) +NodeIdentifier NodeIdentifier::for_node(JsonObject const& node) { - return adopt_ref(*new NodeActor(devtools, move(name), move(walker))); + NodeIdentifier identifier; + + identifier.pseudo_element = node.get_integer>("pseudo-element"sv).map([](auto value) { + VERIFY(value < to_underlying(Web::CSS::Selector::PseudoElement::Type::KnownPseudoElementCount)); + return static_cast(value); + }); + + if (identifier.pseudo_element.has_value()) + identifier.id = node.get_integer("parent-id"sv).value(); + else + identifier.id = node.get_integer("id"sv).value(); + + return identifier; } -NodeActor::NodeActor(DevToolsServer& devtools, String name, WeakPtr walker) +NonnullRefPtr NodeActor::create(DevToolsServer& devtools, String name, NodeIdentifier node_identifier, WeakPtr walker) +{ + return adopt_ref(*new NodeActor(devtools, move(name), move(node_identifier), move(walker))); +} + +NodeActor::NodeActor(DevToolsServer& devtools, String name, NodeIdentifier node_identifier, WeakPtr walker) : Actor(devtools, move(name)) + , m_node_identifier(move(node_identifier)) , m_walker(move(walker)) { } diff --git a/Libraries/LibDevTools/Actors/NodeActor.h b/Libraries/LibDevTools/Actors/NodeActor.h index db42597b92e..71cc1a52378 100644 --- a/Libraries/LibDevTools/Actors/NodeActor.h +++ b/Libraries/LibDevTools/Actors/NodeActor.h @@ -8,20 +8,35 @@ #include #include +#include +#include namespace DevTools { +struct NodeIdentifier { + static NodeIdentifier for_node(JsonObject const& node); + + bool operator==(NodeIdentifier const&) const = default; + + Web::UniqueNodeID id { 0 }; + Optional pseudo_element; +}; + class NodeActor final : public Actor { public: static constexpr auto base_name = "node"sv; - static NonnullRefPtr create(DevToolsServer&, String name, WeakPtr); + static NonnullRefPtr create(DevToolsServer&, String name, NodeIdentifier, WeakPtr); virtual ~NodeActor() override; virtual void handle_message(StringView type, JsonObject const&) override; + NodeIdentifier const& node_identifier() const { return m_node_identifier; } + private: - NodeActor(DevToolsServer&, String name, WeakPtr); + NodeActor(DevToolsServer&, String name, NodeIdentifier, WeakPtr); + + NodeIdentifier m_node_identifier; WeakPtr m_walker; }; diff --git a/Libraries/LibDevTools/Actors/PageStyleActor.cpp b/Libraries/LibDevTools/Actors/PageStyleActor.cpp index b5f690c998b..cf3ba1814eb 100644 --- a/Libraries/LibDevTools/Actors/PageStyleActor.cpp +++ b/Libraries/LibDevTools/Actors/PageStyleActor.cpp @@ -102,7 +102,7 @@ void PageStyleActor::inspect_dom_node(StringView node_actor, Callback&& callback auto block_token = block_responses(); devtools().delegate().inspect_dom_node( - dom_node->tab->description(), dom_node->id, dom_node->pseudo_element, + dom_node->tab->description(), dom_node->identifier.id, dom_node->identifier.pseudo_element, [weak_self = make_weak_ptr(), block_token = move(block_token), callback = forward(callback)](ErrorOr properties) mutable { if (properties.is_error()) { dbgln_if(DEVTOOLS_DEBUG, "Unable to inspect DOM node: {}", properties.error()); diff --git a/Libraries/LibDevTools/Actors/WalkerActor.cpp b/Libraries/LibDevTools/Actors/WalkerActor.cpp index 2b9a4444fc3..6e7352f24a2 100644 --- a/Libraries/LibDevTools/Actors/WalkerActor.cpp +++ b/Libraries/LibDevTools/Actors/WalkerActor.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -25,7 +24,7 @@ WalkerActor::WalkerActor(DevToolsServer& devtools, String name, WeakPtr WalkerActor::dom_node(StringView actor) return {}; auto const& dom_node = *maybe_dom_node.value(); + auto identifier = NodeIdentifier::for_node(dom_node); - auto pseudo_element = dom_node.get_integer>("pseudo-element"sv).map([](auto value) { - VERIFY(value < to_underlying(Web::CSS::Selector::PseudoElement::Type::KnownPseudoElementCount)); - return static_cast(value); - }); - - Web::UniqueNodeID node_id { 0 }; - if (pseudo_element.has_value()) - node_id = dom_node.get_integer("parent-id"sv).value(); - else - node_id = dom_node.get_integer("id"sv).value(); - - return DOMNode { .node = dom_node, .id = node_id, .pseudo_element = pseudo_element, .tab = tab.release_nonnull() }; + return DOMNode { .node = dom_node, .identifier = move(identifier), .tab = tab.release_nonnull() }; } Optional WalkerActor::find_node_by_selector(JsonObject const& node, StringView selector) @@ -306,14 +295,25 @@ Optional WalkerActor::find_node_by_selector(JsonObject const& return {}; } +void WalkerActor::populate_dom_tree_cache() +{ + m_dom_node_to_parent_map.clear(); + m_actor_to_dom_node_map.clear(); + m_dom_node_id_to_actor_map.clear(); + + populate_dom_tree_cache(m_dom_tree, nullptr); +} + void WalkerActor::populate_dom_tree_cache(JsonObject& node, JsonObject const* parent) { - auto& node_actor = devtools().register_actor(*this); + auto const& node_actor = actor_for_node(node); + node.set("actor"sv, node_actor.name()); m_dom_node_to_parent_map.set(&node, parent); - m_actor_to_dom_node_map.set(node_actor.name(), &node); - node.set("actor"sv, node_actor.name()); + + if (!node_actor.node_identifier().pseudo_element.has_value()) + m_dom_node_id_to_actor_map.set(node_actor.node_identifier().id, node_actor.name()); auto children = node.get_array("children"sv); if (!children.has_value()) @@ -328,4 +328,20 @@ void WalkerActor::populate_dom_tree_cache(JsonObject& node, JsonObject const* pa }); } +NodeActor const& WalkerActor::actor_for_node(JsonObject const& node) +{ + auto identifier = NodeIdentifier::for_node(node); + + for (auto const& actor : devtools().actor_registry()) { + auto const* node_actor = as_if(*actor.value); + if (!node_actor) + continue; + + if (node_actor->node_identifier() == identifier) + return *node_actor; + } + + return devtools().register_actor(move(identifier), *this); +} + } diff --git a/Libraries/LibDevTools/Actors/WalkerActor.h b/Libraries/LibDevTools/Actors/WalkerActor.h index ed492797f5c..6664516f2f7 100644 --- a/Libraries/LibDevTools/Actors/WalkerActor.h +++ b/Libraries/LibDevTools/Actors/WalkerActor.h @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include namespace DevTools { @@ -30,8 +30,7 @@ public: struct DOMNode { JsonObject const& node; - Web::UniqueNodeID id { 0 }; - Optional pseudo_element; + NodeIdentifier identifier; NonnullRefPtr tab; }; static Optional dom_node_for(WeakPtr const&, StringView actor); @@ -43,7 +42,10 @@ private: JsonValue serialize_node(JsonObject const&) const; Optional find_node_by_selector(JsonObject const& node, StringView selector); - void populate_dom_tree_cache(JsonObject& node, JsonObject const* parent = nullptr); + void populate_dom_tree_cache(); + void populate_dom_tree_cache(JsonObject& node, JsonObject const* parent); + + NodeActor const& actor_for_node(JsonObject const& node); WeakPtr m_tab; WeakPtr m_layout_inspector; @@ -52,6 +54,7 @@ private: HashMap m_dom_node_to_parent_map; HashMap m_actor_to_dom_node_map; + HashMap m_dom_node_id_to_actor_map; }; }