From c56bf8ac932143a5b4bd4ec902e1390b40ebd77f Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 21 Feb 2025 15:19:35 -0500 Subject: [PATCH] LibDevTools: Implement a real actor for DOM nodes The DevTools client will now send requests to the node actor, rather than just sending messages to other actors on the node's behalf. This exposed a slight issue in the way we assign actor IDs. Node actors are created in the walker actor constructor, which executes before the actor ID is incremented. So we must be sure to increment the actor ID before invoking any actor constructors. Otherwise, the walker actor and the first node actor have the same numeric ID. --- Libraries/LibDevTools/Actors/NodeActor.cpp | 44 ++++++++++++++++++++ Libraries/LibDevTools/Actors/NodeActor.h | 29 +++++++++++++ Libraries/LibDevTools/Actors/WalkerActor.cpp | 9 ++-- Libraries/LibDevTools/Actors/WalkerActor.h | 1 - Libraries/LibDevTools/CMakeLists.txt | 1 + Libraries/LibDevTools/DevToolsServer.h | 4 +- Libraries/LibDevTools/Forward.h | 1 + 7 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 Libraries/LibDevTools/Actors/NodeActor.cpp create mode 100644 Libraries/LibDevTools/Actors/NodeActor.h diff --git a/Libraries/LibDevTools/Actors/NodeActor.cpp b/Libraries/LibDevTools/Actors/NodeActor.cpp new file mode 100644 index 00000000000..9cabe5441c5 --- /dev/null +++ b/Libraries/LibDevTools/Actors/NodeActor.cpp @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2025, Tim Flynn + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include + +namespace DevTools { + +NonnullRefPtr NodeActor::create(DevToolsServer& devtools, String name, WeakPtr walker) +{ + return adopt_ref(*new NodeActor(devtools, move(name), move(walker))); +} + +NodeActor::NodeActor(DevToolsServer& devtools, String name, WeakPtr walker) + : Actor(devtools, move(name)) + , m_walker(move(walker)) +{ +} + +NodeActor::~NodeActor() = default; + +void NodeActor::handle_message(StringView type, JsonObject const&) +{ + JsonObject response; + response.set("from"sv, name()); + + if (type == "getUniqueSelector"sv) { + if (auto walker = m_walker.strong_ref()) { + if (auto const& dom_node = walker->dom_node(name()); dom_node.has_value()) + response.set("value"sv, dom_node->node.get_string("name"sv)->to_ascii_lowercase()); + } + + send_message(move(response)); + return; + } + + send_unrecognized_packet_type_error(type); +} + +} diff --git a/Libraries/LibDevTools/Actors/NodeActor.h b/Libraries/LibDevTools/Actors/NodeActor.h new file mode 100644 index 00000000000..db42597b92e --- /dev/null +++ b/Libraries/LibDevTools/Actors/NodeActor.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2025, Tim Flynn + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include + +namespace DevTools { + +class NodeActor final : public Actor { +public: + static constexpr auto base_name = "node"sv; + + static NonnullRefPtr create(DevToolsServer&, String name, WeakPtr); + virtual ~NodeActor() override; + + virtual void handle_message(StringView type, JsonObject const&) override; + +private: + NodeActor(DevToolsServer&, String name, WeakPtr); + + WeakPtr m_walker; +}; + +} diff --git a/Libraries/LibDevTools/Actors/WalkerActor.cpp b/Libraries/LibDevTools/Actors/WalkerActor.cpp index 3a89f0f2030..d793cf8d123 100644 --- a/Libraries/LibDevTools/Actors/WalkerActor.cpp +++ b/Libraries/LibDevTools/Actors/WalkerActor.cpp @@ -6,8 +6,10 @@ #include #include +#include #include #include +#include #include namespace DevTools { @@ -276,11 +278,12 @@ Optional WalkerActor::find_node_by_selector(JsonObject const& void WalkerActor::populate_dom_tree_cache(JsonObject& node, JsonObject const* parent) { + auto& node_actor = devtools().register_actor(*this); + m_dom_node_to_parent_map.set(&node, parent); - auto actor = MUST(String::formatted("{}-node{}", name(), m_dom_node_count++)); - m_actor_to_dom_node_map.set(actor, &node); - node.set("actor"sv, actor); + m_actor_to_dom_node_map.set(node_actor.name(), &node); + node.set("actor"sv, node_actor.name()); auto children = node.get_array("children"sv); if (!children.has_value()) diff --git a/Libraries/LibDevTools/Actors/WalkerActor.h b/Libraries/LibDevTools/Actors/WalkerActor.h index 969a26f4d93..90cc69919be 100644 --- a/Libraries/LibDevTools/Actors/WalkerActor.h +++ b/Libraries/LibDevTools/Actors/WalkerActor.h @@ -48,7 +48,6 @@ private: HashMap m_dom_node_to_parent_map; HashMap m_actor_to_dom_node_map; - size_t m_dom_node_count { 0 }; }; } diff --git a/Libraries/LibDevTools/CMakeLists.txt b/Libraries/LibDevTools/CMakeLists.txt index 4d6d2bb73e8..2babaaee865 100644 --- a/Libraries/LibDevTools/CMakeLists.txt +++ b/Libraries/LibDevTools/CMakeLists.txt @@ -5,6 +5,7 @@ set(SOURCES Actors/FrameActor.cpp Actors/HighlighterActor.cpp Actors/InspectorActor.cpp + Actors/NodeActor.cpp Actors/PageStyleActor.cpp Actors/PreferenceActor.cpp Actors/ProcessActor.cpp diff --git a/Libraries/LibDevTools/DevToolsServer.h b/Libraries/LibDevTools/DevToolsServer.h index 9f8e6ac4c83..0d48ac99958 100644 --- a/Libraries/LibDevTools/DevToolsServer.h +++ b/Libraries/LibDevTools/DevToolsServer.h @@ -32,16 +32,16 @@ public: ActorType& register_actor(Args&&... args) { String name; + auto id = m_actor_count++; if constexpr (IsSame) { name = String::from_utf8_without_validation(ActorType::base_name.bytes()); } else { - name = MUST(String::formatted("server{}-{}{}", m_server_id, ActorType::base_name, m_actor_count)); + name = MUST(String::formatted("server{}-{}{}", m_server_id, ActorType::base_name, id)); } auto actor = ActorType::create(*this, name, forward(args)...); m_actor_registry.set(name, actor); - ++m_actor_count; return actor; } diff --git a/Libraries/LibDevTools/Forward.h b/Libraries/LibDevTools/Forward.h index 71442c1de2e..637eab76eb1 100644 --- a/Libraries/LibDevTools/Forward.h +++ b/Libraries/LibDevTools/Forward.h @@ -17,6 +17,7 @@ class DevToolsServer; class FrameActor; class HighlighterActor; class InspectorActor; +class NodeActor; class PageStyleActor; class PreferenceActor; class ProcessActor;