From a7b577126aaf3d6a8feb9076b7e71ad634afdc44 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 11 Mar 2025 07:55:05 -0400 Subject: [PATCH] LibDevTools: Send an unknown actor error for failed DOM node lookups DevTools will ask us to find nodes by their actor names. If the actor is missing, we should inform DevTools of the error instead of just dropping the request. The diff here is a bit noisy, just due to a leftward shift of code that used to be in an if-statement. --- Libraries/LibDevTools/Actors/NodeActor.cpp | 74 ++-- .../LibDevTools/Actors/PageStyleActor.cpp | 4 +- Libraries/LibDevTools/Actors/WalkerActor.cpp | 401 ++++++++++-------- 3 files changed, 270 insertions(+), 209 deletions(-) diff --git a/Libraries/LibDevTools/Actors/NodeActor.cpp b/Libraries/LibDevTools/Actors/NodeActor.cpp index 68b58f653cb..a6a58aa13e7 100644 --- a/Libraries/LibDevTools/Actors/NodeActor.cpp +++ b/Libraries/LibDevTools/Actors/NodeActor.cpp @@ -99,9 +99,13 @@ void NodeActor::handle_message(StringView type, JsonObject const& message) response.set("from"sv, name()); if (type == "getUniqueSelector"sv) { - if (auto dom_node = WalkerActor::dom_node_for(m_walker, name()); dom_node.has_value()) - response.set("value"sv, dom_node->node.get_string("name"sv)->to_ascii_lowercase()); + auto dom_node = WalkerActor::dom_node_for(m_walker, name()); + if (!dom_node.has_value()) { + send_unknown_actor_error(name()); + return; + } + response.set("value"sv, dom_node->node.get_string("name"sv)->to_ascii_lowercase()); send_message(move(response)); return; } @@ -117,24 +121,28 @@ void NodeActor::handle_message(StringView type, JsonObject const& message) if (!attribute_to_replace.has_value() && replacement_attributes.is_empty()) return; - if (auto dom_node = WalkerActor::dom_node_for(m_walker, name()); dom_node.has_value()) { - auto block_token = block_responses(); + auto dom_node = WalkerActor::dom_node_for(m_walker, name()); + if (!dom_node.has_value()) { + send_unknown_actor_error(name()); + return; + } - auto on_complete = [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } + auto block_token = block_responses(); - if (auto self = weak_self.strong_ref()) - self->finished_editing_dom_node(move(block_token)); - }; - - if (attribute_to_replace.has_value()) { - devtools().delegate().replace_dom_node_attribute(dom_node->tab->description(), dom_node->identifier.id, *attribute_to_replace, move(replacement_attributes), move(on_complete)); - } else { - devtools().delegate().add_dom_node_attributes(dom_node->tab->description(), dom_node->identifier.id, move(replacement_attributes), move(on_complete)); + auto on_complete = [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; } + + if (auto self = weak_self.strong_ref()) + self->finished_editing_dom_node(move(block_token)); + }; + + if (attribute_to_replace.has_value()) { + devtools().delegate().replace_dom_node_attribute(dom_node->tab->description(), dom_node->identifier.id, *attribute_to_replace, move(replacement_attributes), move(on_complete)); + } else { + devtools().delegate().add_dom_node_attributes(dom_node->tab->description(), dom_node->identifier.id, move(replacement_attributes), move(on_complete)); } return; @@ -147,22 +155,26 @@ void NodeActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(m_walker, name()); dom_node.has_value()) { - auto block_token = block_responses(); - - devtools().delegate().set_dom_node_text( - dom_node->tab->description(), dom_node->identifier.id, *value, - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } - - if (auto self = weak_self.strong_ref()) - self->finished_editing_dom_node(move(block_token)); - }); + auto dom_node = WalkerActor::dom_node_for(m_walker, name()); + if (!dom_node.has_value()) { + send_unknown_actor_error(name()); + return; } + auto block_token = block_responses(); + + devtools().delegate().set_dom_node_text( + dom_node->tab->description(), dom_node->identifier.id, *value, + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; + } + + if (auto self = weak_self.strong_ref()) + self->finished_editing_dom_node(move(block_token)); + }); + return; } diff --git a/Libraries/LibDevTools/Actors/PageStyleActor.cpp b/Libraries/LibDevTools/Actors/PageStyleActor.cpp index cf3ba1814eb..2d48a6c0c38 100644 --- a/Libraries/LibDevTools/Actors/PageStyleActor.cpp +++ b/Libraries/LibDevTools/Actors/PageStyleActor.cpp @@ -96,8 +96,10 @@ template void PageStyleActor::inspect_dom_node(StringView node_actor, Callback&& callback) { auto dom_node = WalkerActor::dom_node_for(InspectorActor::walker_for(m_inspector), node_actor); - if (!dom_node.has_value()) + if (!dom_node.has_value()) { + send_unknown_actor_error(node_actor); return; + } auto block_token = block_responses(); diff --git a/Libraries/LibDevTools/Actors/WalkerActor.cpp b/Libraries/LibDevTools/Actors/WalkerActor.cpp index 3e13624b90d..cd99239d8a1 100644 --- a/Libraries/LibDevTools/Actors/WalkerActor.cpp +++ b/Libraries/LibDevTools/Actors/WalkerActor.cpp @@ -55,15 +55,18 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } + auto ancestor_node = WalkerActor::dom_node_for(*this, *node); + if (!ancestor_node.has_value()) { + send_unknown_actor_error(*node); + return; + } + JsonArray nodes; - if (auto ancestor_node = m_actor_to_dom_node_map.get(*node); ancestor_node.has_value()) { - if (auto children = ancestor_node.value()->get_array("children"sv); children.has_value()) { - - children->for_each([&](JsonValue const& child) { - nodes.must_append(serialize_node(child.as_object())); - }); - } + if (auto children = ancestor_node->node.get_array("children"sv); children.has_value()) { + children->for_each([&](JsonValue const& child) { + nodes.must_append(serialize_node(child.as_object())); + }); } response.set("hasFirst"sv, !nodes.is_empty()); @@ -80,25 +83,29 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - auto block_token = block_responses(); - - devtools().delegate().clone_dom_node( - dom_node->tab->description(), dom_node->identifier.id, - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } - - if (auto self = weak_self.strong_ref()) { - JsonObject message; - message.set("from"sv, self->name()); - self->send_message(move(message), move(block_token)); - } - }); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + auto block_token = block_responses(); + + devtools().delegate().clone_dom_node( + dom_node->tab->description(), dom_node->identifier.id, + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; + } + + if (auto self = weak_self.strong_ref()) { + JsonObject message; + message.set("from"sv, self->name()); + self->send_message(move(message), move(block_token)); + } + }); + return; } @@ -115,25 +122,29 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - auto block_token = block_responses(); - - devtools().delegate().set_dom_node_tag( - dom_node->tab->description(), dom_node->identifier.id, *tag_name, - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } - - if (auto self = weak_self.strong_ref()) { - JsonObject message; - message.set("from"sv, self->name()); - self->send_message(move(message), move(block_token)); - } - }); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + auto block_token = block_responses(); + + devtools().delegate().set_dom_node_tag( + dom_node->tab->description(), dom_node->identifier.id, *tag_name, + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; + } + + if (auto self = weak_self.strong_ref()) { + JsonObject message; + message.set("from"sv, self->name()); + self->send_message(move(message), move(block_token)); + } + }); + return; } @@ -170,26 +181,30 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - auto block_token = block_responses(); - - devtools().delegate().get_dom_node_inner_html( - dom_node->tab->description(), dom_node->identifier.id, - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr html) mutable { - if (html.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", html.error()); - return; - } - - if (auto self = weak_self.strong_ref()) { - JsonObject message; - message.set("from"sv, self->name()); - message.set("value"sv, html.release_value()); - self->send_message(move(message), move(block_token)); - } - }); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + auto block_token = block_responses(); + + devtools().delegate().get_dom_node_inner_html( + dom_node->tab->description(), dom_node->identifier.id, + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr html) mutable { + if (html.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", html.error()); + return; + } + + if (auto self = weak_self.strong_ref()) { + JsonObject message; + message.set("from"sv, self->name()); + message.set("value"sv, html.release_value()); + self->send_message(move(message), move(block_token)); + } + }); + return; } @@ -203,34 +218,38 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - auto block_token = block_responses(); - - devtools().delegate().create_child_element( - dom_node->tab->description(), dom_node->identifier.id, - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } - - if (auto self = weak_self.strong_ref()) { - JsonArray nodes; - - if (auto actor = self->m_dom_node_id_to_actor_map.get(node_id.value()); actor.has_value()) { - if (auto dom_node = WalkerActor::dom_node_for(self, *actor); dom_node.has_value()) - nodes.must_append(self->serialize_node(dom_node->node)); - } - - JsonObject message; - message.set("from"sv, self->name()); - message.set("newParents"sv, JsonArray {}); - message.set("nodes"sv, move(nodes)); - self->send_message(move(message), move(block_token)); - } - }); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + auto block_token = block_responses(); + + devtools().delegate().create_child_element( + dom_node->tab->description(), dom_node->identifier.id, + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; + } + + if (auto self = weak_self.strong_ref()) { + JsonArray nodes; + + if (auto actor = self->m_dom_node_id_to_actor_map.get(node_id.value()); actor.has_value()) { + if (auto dom_node = WalkerActor::dom_node_for(self, *actor); dom_node.has_value()) + nodes.must_append(self->serialize_node(dom_node->node)); + } + + JsonObject message; + message.set("from"sv, self->name()); + message.set("newParents"sv, JsonArray {}); + message.set("nodes"sv, move(nodes)); + self->send_message(move(message), move(block_token)); + } + }); + return; } @@ -248,35 +267,44 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) } auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; + } + auto parent_dom_node = WalkerActor::dom_node_for(*this, *parent); + if (!parent_dom_node.has_value()) { + send_unknown_actor_error(*parent); + return; + } Optional sibling_node_id; if (auto sibling = message.get_string("sibling"sv); sibling.has_value()) { auto sibling_dom_node = WalkerActor::dom_node_for(*this, *sibling); - if (!sibling_dom_node.has_value()) + if (!sibling_dom_node.has_value()) { + send_unknown_actor_error(*sibling); return; + } sibling_node_id = sibling_dom_node->identifier.id; } - if (dom_node.has_value() && parent_dom_node.has_value()) { - auto block_token = block_responses(); + auto block_token = block_responses(); - devtools().delegate().insert_dom_node_before( - dom_node->tab->description(), dom_node->identifier.id, parent_dom_node->identifier.id, sibling_node_id, - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } + devtools().delegate().insert_dom_node_before( + dom_node->tab->description(), dom_node->identifier.id, parent_dom_node->identifier.id, sibling_node_id, + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; + } - if (auto self = weak_self.strong_ref()) { - JsonObject message; - message.set("from"sv, self->name()); - self->send_message(move(message), move(block_token)); - } - }); - } + if (auto self = weak_self.strong_ref()) { + JsonObject message; + message.set("from"sv, self->name()); + self->send_message(move(message), move(block_token)); + } + }); return; } @@ -300,26 +328,30 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - auto block_token = block_responses(); - - devtools().delegate().get_dom_node_outer_html( - dom_node->tab->description(), dom_node->identifier.id, - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr html) mutable { - if (html.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", html.error()); - return; - } - - if (auto self = weak_self.strong_ref()) { - JsonObject message; - message.set("from"sv, self->name()); - message.set("value"sv, html.release_value()); - self->send_message(move(message), move(block_token)); - } - }); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + auto block_token = block_responses(); + + devtools().delegate().get_dom_node_outer_html( + dom_node->tab->description(), dom_node->identifier.id, + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr html) mutable { + if (html.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", html.error()); + return; + } + + if (auto self = weak_self.strong_ref()) { + JsonObject message; + message.set("from"sv, self->name()); + message.set("value"sv, html.release_value()); + self->send_message(move(message), move(block_token)); + } + }); + return; } @@ -330,13 +362,16 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - JsonValue previous_sibling; - - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - if (auto previous_sibling_node = previous_sibling_for_node(dom_node->node); previous_sibling_node.has_value()) - previous_sibling = serialize_node(*previous_sibling_node); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + JsonValue previous_sibling; + if (auto previous_sibling_node = previous_sibling_for_node(dom_node->node); previous_sibling_node.has_value()) + previous_sibling = serialize_node(*previous_sibling_node); + response.set("node"sv, move(previous_sibling)); send_message(move(response)); return; @@ -355,17 +390,21 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto ancestor_node = m_actor_to_dom_node_map.get(*node); ancestor_node.has_value()) { - if (auto selected_node = find_node_by_selector(*ancestor_node.value(), *selector); selected_node.has_value()) { - response.set("node"sv, serialize_node(*selected_node)); + auto ancestor_node = WalkerActor::dom_node_for(*this, *node); + if (!ancestor_node.has_value()) { + send_unknown_actor_error(*node); + return; + } - if (auto parent = m_dom_node_to_parent_map.get(&selected_node.value()); parent.value() && parent.value() != ancestor_node.value()) { - // FIXME: Should this be a stack of nodes leading to `ancestor_node`? - JsonArray new_parents; - new_parents.must_append(serialize_node(*parent.value())); + if (auto selected_node = find_node_by_selector(ancestor_node->node, *selector); selected_node.has_value()) { + response.set("node"sv, serialize_node(*selected_node)); - response.set("newParents"sv, move(new_parents)); - } + if (auto parent = m_dom_node_to_parent_map.get(&selected_node.value()); parent.value() && parent.value() != &ancestor_node->node) { + // FIXME: Should this be a stack of nodes leading to `ancestor_node`? + JsonArray new_parents; + new_parents.must_append(serialize_node(*parent.value())); + + response.set("newParents"sv, move(new_parents)); } } @@ -380,34 +419,38 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - JsonValue next_sibling; - if (auto next_sibling_node = next_sibling_for_node(dom_node->node); next_sibling_node.has_value()) - next_sibling = serialize_node(*next_sibling_node); - - auto parent_node = remove_node(dom_node->node); - if (!parent_node.has_value()) - return; - - auto block_token = block_responses(); - - devtools().delegate().remove_dom_node( - dom_node->tab->description(), dom_node->identifier.id, - [weak_self = make_weak_ptr(), next_sibling = move(next_sibling), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } - - if (auto self = weak_self.strong_ref()) { - JsonObject message; - message.set("from"sv, self->name()); - message.set("nextSibling"sv, move(next_sibling)); - self->send_message(move(message), move(block_token)); - } - }); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + JsonValue next_sibling; + if (auto next_sibling_node = next_sibling_for_node(dom_node->node); next_sibling_node.has_value()) + next_sibling = serialize_node(*next_sibling_node); + + auto parent_node = remove_node(dom_node->node); + if (!parent_node.has_value()) + return; + + auto block_token = block_responses(); + + devtools().delegate().remove_dom_node( + dom_node->tab->description(), dom_node->identifier.id, + [weak_self = make_weak_ptr(), next_sibling = move(next_sibling), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; + } + + if (auto self = weak_self.strong_ref()) { + JsonObject message; + message.set("from"sv, self->name()); + message.set("nextSibling"sv, move(next_sibling)); + self->send_message(move(message), move(block_token)); + } + }); + return; } @@ -429,25 +472,29 @@ void WalkerActor::handle_message(StringView type, JsonObject const& message) return; } - if (auto dom_node = WalkerActor::dom_node_for(*this, *node); dom_node.has_value()) { - auto block_token = block_responses(); - - devtools().delegate().set_dom_node_outer_html( - dom_node->tab->description(), dom_node->identifier.id, value.release_value(), - [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { - if (node_id.is_error()) { - dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); - return; - } - - if (auto self = weak_self.strong_ref()) { - JsonObject message; - message.set("from"sv, self->name()); - self->send_message(move(message), move(block_token)); - } - }); + auto dom_node = WalkerActor::dom_node_for(*this, *node); + if (!dom_node.has_value()) { + send_unknown_actor_error(*node); + return; } + auto block_token = block_responses(); + + devtools().delegate().set_dom_node_outer_html( + dom_node->tab->description(), dom_node->identifier.id, value.release_value(), + [weak_self = make_weak_ptr(), block_token = move(block_token)](ErrorOr node_id) mutable { + if (node_id.is_error()) { + dbgln_if(DEVTOOLS_DEBUG, "Unable to edit DOM node: {}", node_id.error()); + return; + } + + if (auto self = weak_self.strong_ref()) { + JsonObject message; + message.set("from"sv, self->name()); + self->send_message(move(message), move(block_token)); + } + }); + return; }