From 117dce412bb17d4c9da08756a8893b8a2e290f0c Mon Sep 17 00:00:00 2001 From: ldm0 Date: Thu, 1 Oct 2026 04:13:50 +0800 Subject: [PATCH] refactor(cdp): share node lookups with explicit chain execution --- .../src/domains/accessibility/helpers.rs | 12 +- .../src/domains/accessibility/native.rs | 31 +--- moli-protocol/src/domains/css.rs | 13 +- moli-protocol/src/domains/css/native.rs | 36 +--- .../src/domains/dom/node_references.rs | 14 +- .../src/domains/dom/resolve/native.rs | 157 +++++++++--------- .../src/domains/dom/resolve/native/files.rs | 71 ++++---- .../domains/dom/resolve/native/mutation.rs | 80 +++++---- .../dom/resolve/native/remote_object.rs | 12 +- moli-protocol/src/domains/native.rs | 3 + moli-protocol/src/domains/native/node.rs | 69 ++++++++ 11 files changed, 262 insertions(+), 236 deletions(-) create mode 100644 moli-protocol/src/domains/native/node.rs diff --git a/moli-protocol/src/domains/accessibility/helpers.rs b/moli-protocol/src/domains/accessibility/helpers.rs index 8a0348edca..f3040f12fc 100644 --- a/moli-protocol/src/domains/accessibility/helpers.rs +++ b/moli-protocol/src/domains/accessibility/helpers.rs @@ -1,4 +1,5 @@ use crate::conn::CdpConnection; +pub(super) use crate::domains::native::NodeReferenceParams; pub(super) use chromiumoxide_cdp::cdp::browser_protocol::accessibility::{ GetChildAxNodesParams as ChildAxNodesParams, GetFullAxTreeParams, }; @@ -12,17 +13,6 @@ pub(super) struct FrameScopedParams { pub(super) frame_id: Option, } -#[derive(Deserialize, Default)] -#[serde(rename_all = "camelCase")] -pub(super) struct NodeReferenceParams { - #[serde(default)] - pub(super) node_id: Option, - #[serde(default)] - pub(super) backend_node_id: Option, - #[serde(default)] - pub(super) object_id: Option, -} - #[derive(Deserialize)] #[serde(rename_all = "camelCase")] pub(super) struct AncestorsParams { diff --git a/moli-protocol/src/domains/accessibility/native.rs b/moli-protocol/src/domains/accessibility/native.rs index 3c83e2e812..b320307881 100644 --- a/moli-protocol/src/domains/accessibility/native.rs +++ b/moli-protocol/src/domains/accessibility/native.rs @@ -1,9 +1,9 @@ use super::*; -use crate::domains::native::{self, NativeCommandStep}; +use crate::devtools_runtime::DevToolsDomNodeReference; +use crate::domains::native::{self, NativeCommandStep, NodeLookupExecution}; use moli_core::{ - RendererNativeOperation as Operation, RendererNativeOperationStep as Step, - RendererNativeProtocolResponse as Response, RendererPageCommand as Command, - RendererPageReply as Reply, + RendererNativeOperation as Operation, RendererNativeProtocolResponse as Response, + RendererPageCommand as Command, RendererPageReply as Reply, }; pub(crate) fn try_start(conn: &mut CdpConnection, cmd: &Cmd<'_>) -> Option { @@ -231,24 +231,11 @@ fn reference_operation( let frontend_node_id = reference.node_id.ok_or_else(StartError::node_not_found)?; let inspector_session_id = conn.target_renderer_runtime_inspector_session_id_for_session(cmd.session_id); - Ok(Operation::then_on_nested_main( - Command::DocumentFrontendNodeBinding { - inspector_session_id, - frontend_node_id, - }, - move |reply| match reply { - Ok(Reply::DocumentFrontendNodeBinding( - RendererDomFrontendNodeBindingResolution::BackendNodeId(id), - )) => Step::Continue(backend_operation(id, frame_id, top_frame_id, operation)), - Ok(Reply::DocumentFrontendNodeBinding( - RendererDomFrontendNodeBindingResolution::NotFound, - )) => Step::Complete(node_not_found()), - Err(error) => Step::Complete(Response::error( - -32000, - format!("Could not resolve frontend node binding: {error}"), - )), - _ => unreachable!("accessibility frontend-node lookup reply"), - }, + Ok(native::with_backend_node( + inspector_session_id, + DevToolsDomNodeReference::FrontendNodeId(frontend_node_id), + NodeLookupExecution::NestedMain, + move |id| backend_operation(id, frame_id, top_frame_id, operation), )) } diff --git a/moli-protocol/src/domains/css.rs b/moli-protocol/src/domains/css.rs index 0fee9cbde7..524f9925fc 100644 --- a/moli-protocol/src/domains/css.rs +++ b/moli-protocol/src/domains/css.rs @@ -3,6 +3,7 @@ pub(crate) mod native; use crate::conn::{CdpConnection, Cmd, CommandOwnerScope}; use crate::domains::actions::CssAction; use crate::domains::command_output::CommandOutputPlan; +use crate::domains::native::NodeReferenceParams; use chromiumoxide_cdp::cdp::browser_protocol::css::{ GetStyleSheetTextParams as StyleSheetIdParams, SetStyleSheetTextParams, }; @@ -10,23 +11,11 @@ use moli_core::page::{ CompletedPageCommand, Page, PendingPageCommand, RendererDocumentNodeAttributesResolution, }; use moli_css_parse::{DeclarationParseOptions, parse_declaration_list}; -use serde::Deserialize; use serde_json::{Value, json}; mod node_references; mod style_sheets; -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct NodeReferenceParams { - #[serde(default)] - node_id: Option, - #[serde(default)] - backend_node_id: Option, - #[serde(default)] - object_id: Option, -} - pub(crate) struct PendingCssCommandDispatch { command_id: Option, owner_scope: CommandOwnerScope, diff --git a/moli-protocol/src/domains/css/native.rs b/moli-protocol/src/domains/css/native.rs index 053fdaa5cd..dd5c7a1bc5 100644 --- a/moli-protocol/src/domains/css/native.rs +++ b/moli-protocol/src/domains/css/native.rs @@ -1,9 +1,9 @@ use super::*; -use crate::domains::native::{self, NativeCommandStep}; +use crate::devtools_runtime::DevToolsDomNodeReference; +use crate::domains::native::{self, NativeCommandStep, NodeLookupExecution}; use moli_core::{ - RendererNativeOperation as Operation, RendererNativeOperationStep as Step, - RendererNativeProtocolResponse as Response, RendererPageCommand as Command, - RendererPageReply as Reply, + RendererNativeOperation as Operation, RendererNativeProtocolResponse as Response, + RendererPageCommand as Command, RendererPageReply as Reply, }; pub(crate) fn try_start(conn: &mut CdpConnection, cmd: &Cmd<'_>) -> Option { @@ -133,29 +133,11 @@ pub(crate) fn try_start(conn: &mut CdpConnection, cmd: &Cmd<'_>) -> Option { - match node_references::backend_node_id_from_frontend_resolution( - resolution, - ) { - Some(id) => Step::Continue(backend_style_operation(id, query)), - None => Step::Complete(Response::error( - -32000, - "Could not find node with given id", - )), - } - } - Err(error) => Step::Complete(Response::error( - -32000, - format!("Could not resolve frontend node binding: {error}"), - )), - _ => unreachable!("CSS frontend-node lookup reply"), - }, + native::with_backend_node( + inspector_session_id, + DevToolsDomNodeReference::FrontendNodeId(frontend_node_id), + NodeLookupExecution::NestedMain, + move |id| backend_style_operation(id, query), ) } else if let Some(backend_node_id) = params.backend_node_id { backend_style_operation(backend_node_id, query) diff --git a/moli-protocol/src/domains/dom/node_references.rs b/moli-protocol/src/domains/dom/node_references.rs index 63babe9a09..4f0b556ed0 100644 --- a/moli-protocol/src/domains/dom/node_references.rs +++ b/moli-protocol/src/domains/dom/node_references.rs @@ -1,17 +1,5 @@ -use serde::Deserialize; - use crate::devtools_runtime::DevToolsDomNodeReference; - -#[derive(Deserialize, Default)] -#[serde(rename_all = "camelCase")] -pub(super) struct NodeReferenceParams { - #[serde(default)] - pub(super) node_id: Option, - #[serde(default)] - pub(super) backend_node_id: Option, - #[serde(default)] - pub(super) object_id: Option, -} +pub(super) use crate::domains::native::NodeReferenceParams; pub(super) fn devtools_node_reference_from_ids( node_id: Option, diff --git a/moli-protocol/src/domains/dom/resolve/native.rs b/moli-protocol/src/domains/dom/resolve/native.rs index 4ea975b498..f2320d3675 100644 --- a/moli-protocol/src/domains/dom/resolve/native.rs +++ b/moli-protocol/src/domains/dom/resolve/native.rs @@ -1,8 +1,10 @@ use super::*; -use crate::domains::native::{self, NativeCommandStep}; -use moli_core::page::{ - RendererDomFrontendNodeBindingResolution, RendererDomSearchResultsResolution, +use crate::domains::native::{ + self, NativeCommandStep, + NodeLookupExecution::{NestedMain, OwnerTurn}, + with_backend_node as with_backend, }; +use moli_core::page::RendererDomSearchResultsResolution; use moli_core::{ RendererNativeOperation as Operation, RendererNativeOperationStep as Step, RendererNativeProtocolNotification as Notification, RendererNativeProtocolResponse as Response, @@ -84,7 +86,7 @@ fn prepare( return remote_object::prepare(conn, cmd); } if action == DomAction::SetFileInputFiles { - return files::prepare(conn, cmd).map(Operation::require_owner_turn); + return files::prepare(conn, cmd); } if mutation::handles(action) { return mutation::prepare(conn, cmd, action); @@ -146,23 +148,28 @@ fn prepare( } DomAction::GetAttributes => { let params = build_cdp_get_attributes_command(conn, cmd)?; - Ok(with_backend(session, params.reference, |backend_node_id| { - Operation::new( - Command::DocumentNodeAttributesForBackendNodeId { backend_node_id }, - |reply| match reply { - Ok(Reply::DocumentNodeAttributesResolution(resolution)) => { - match attributes_result_from_renderer_resolution(resolution) { - Ok(result) => Response::success( - json!({"attributes": result.attributes.into_iter().flat_map(|attr| [attr.name, attr.value]).collect::>()}), - ), - Err(error) => Response::error(error.code, error.message), + Ok(with_backend( + session, + params.reference, + NestedMain, + |backend_node_id| { + Operation::new( + Command::DocumentNodeAttributesForBackendNodeId { backend_node_id }, + |reply| match reply { + Ok(Reply::DocumentNodeAttributesResolution(resolution)) => { + match attributes_result_from_renderer_resolution(resolution) { + Ok(result) => Response::success( + json!({"attributes": result.attributes.into_iter().flat_map(|attr| [attr.name, attr.value]).collect::>()}), + ), + Err(error) => Response::error(error.code, error.message), + } } - } - Err(error) => Response::error(-32000, error.to_string()), - _ => unreachable!("DOM attributes reply"), - }, - ) - })) + Err(error) => Response::error(-32000, error.to_string()), + _ => unreachable!("DOM attributes reply"), + }, + ) + }, + )) } DomAction::QuerySelector | DomAction::QuerySelectorAll => { let params = @@ -181,24 +188,29 @@ fn prepare( ), Some(reference) => { let frontend = matches!(reference, DevToolsDomNodeReference::FrontendNodeId(_)); - with_backend(session.clone(), reference, move |root_backend_node_id| { - let command = if frontend { - Command::DocumentQuerySelectorWithChildNodeSnapshotEventsForBackendNodeId { + with_backend( + session.clone(), + reference, + NestedMain, + move |root_backend_node_id| { + let command = if frontend { + Command::DocumentQuerySelectorWithChildNodeSnapshotEventsForBackendNodeId { inspector_session_id: session, include_whitespace: whitespace, root_backend_node_id, selector, multiple, } - } else { - Command::DocumentQuerySelectorForBackendNodeId { - inspector_session_id: session, - include_whitespace: whitespace, - root_backend_node_id, - selector, - multiple, - } - }; - Operation::new(command, move |reply| { - project_query(reply, multiple, top_frame) - }) - }) + } else { + Command::DocumentQuerySelectorForBackendNodeId { + inspector_session_id: session, + include_whitespace: whitespace, + root_backend_node_id, + selector, + multiple, + } + }; + Operation::new(command, move |reply| { + project_query(reply, multiple, top_frame) + }) + }, + ) } }) } @@ -212,6 +224,7 @@ fn prepare( Ok(with_backend( session.clone(), params.reference, + NestedMain, move |backend_node_id| { Operation::new( Command::DocumentChildNodeSnapshotEventsForBackendNodeId { @@ -299,6 +312,7 @@ fn prepare( Ok(with_backend( session.clone(), reference, + NestedMain, move |backend_node_id| { Operation::new( Command::DocumentNodeSnapshotForBackendNodeIdInInspectorSession { @@ -359,15 +373,20 @@ fn prepare( params.reference.node_id, params.reference.backend_node_id, ) { - Ok(with_backend(session, reference, move |backend_node_id| { - Operation::new( - Command::OuterHtmlForBackendNodeId { - backend_node_id, - include_shadow_dom: params.include_shadow_dom, - }, - project, - ) - })) + Ok(with_backend( + session, + reference, + NestedMain, + move |backend_node_id| { + Operation::new( + Command::OuterHtmlForBackendNodeId { + backend_node_id, + include_shadow_dom: params.include_shadow_dom, + }, + project, + ) + }, + )) } else { Ok(Operation::new( Command::OuterHtmlForDocument { @@ -413,12 +432,17 @@ fn prepare( let reference = devtools_node_reference_from_ids(params.node_id, params.backend_node_id) .ok_or_else(StartError::node_not_found)?; - Ok(with_backend(session, reference, move |backend_node_id| { - Operation::new( - Command::DocumentGeometryForBackendNodeId { backend_node_id }, - project, - ) - })) + Ok(with_backend( + session, + reference, + NestedMain, + move |backend_node_id| { + Operation::new( + Command::DocumentGeometryForBackendNodeId { backend_node_id }, + project, + ) + }, + )) } } DomAction::PushNodesByBackendIdsToFrontend => { @@ -516,37 +540,6 @@ fn prepare( } } -fn with_backend( - session: Option, - reference: DevToolsDomNodeReference, - next: impl FnOnce(u32) -> Operation + Send + 'static, -) -> Operation { - match reference { - DevToolsDomNodeReference::BackendNodeId(id) => next(id), - DevToolsDomNodeReference::FrontendNodeId(frontend_node_id) => { - Operation::then_on_nested_main( - Command::DocumentFrontendNodeBinding { - inspector_session_id: session, - frontend_node_id, - }, - move |reply| match reply { - Ok(Reply::DocumentFrontendNodeBinding( - RendererDomFrontendNodeBindingResolution::BackendNodeId(id), - )) => Step::Continue(next(id)), - Ok(Reply::DocumentFrontendNodeBinding( - RendererDomFrontendNodeBindingResolution::NotFound, - )) => Step::Complete(node_not_found()), - Err(error) => Step::Complete(Response::error( - -32000, - format!("Could not resolve frontend node binding: {error}"), - )), - _ => unreachable!("DOM node binding reply"), - }, - ) - } - } -} - fn node_not_found() -> Response { Response::error(-32000, "Could not find node with given id") } diff --git a/moli-protocol/src/domains/dom/resolve/native/files.rs b/moli-protocol/src/domains/dom/resolve/native/files.rs index f399dae402..71236e5438 100644 --- a/moli-protocol/src/domains/dom/resolve/native/files.rs +++ b/moli-protocol/src/domains/dom/resolve/native/files.rs @@ -33,38 +33,47 @@ pub(super) fn prepare(conn: &CdpConnection, cmd: &Cmd<'_>) -> Result { - match files { - Ok(files) => Step::Continue(Operation::new( - Command::SetFileInputFilesForBackendNodeId { - backend_node_id, - files, - append: false, - }, - project, - )), - Err(error) => Step::Complete(Response::error(error.code, error.message)), + Ok(with_backend( + session, + reference, + OwnerTurn, + move |backend_node_id| { + Operation::then( + Command::DocumentNodeSnapshotForBackendNodeId { + backend_node_id, + depth: 0, + pierce: false, + }, + move |reply| match reply { + Ok(Reply::OptionalDocumentNodeObjectSnapshot(snapshot)) + if snapshot.is_some() => + { + match files { + Ok(files) => Step::Continue(Operation::new( + Command::SetFileInputFilesForBackendNodeId { + backend_node_id, + files, + append: false, + }, + project, + )), + Err(error) => { + Step::Complete(Response::error(error.code, error.message)) + } + } } - } - Ok(Reply::OptionalDocumentNodeObjectSnapshot(_)) => { - Step::Complete(node_not_found()) - } - Err(error) => Step::Complete(Response::error( - -32000, - format!("Could not preflight file input node: {error}"), - )), - _ => unreachable!("DOM file input preflight reply"), - }, - ) - })) + Ok(Reply::OptionalDocumentNodeObjectSnapshot(_)) => { + Step::Complete(node_not_found()) + } + Err(error) => Step::Complete(Response::error( + -32000, + format!("Could not preflight file input node: {error}"), + )), + _ => unreachable!("DOM file input preflight reply"), + }, + ) + }, + )) } fn project(reply: anyhow::Result) -> Response { diff --git a/moli-protocol/src/domains/dom/resolve/native/mutation.rs b/moli-protocol/src/domains/dom/resolve/native/mutation.rs index d30356bbbc..7624299032 100644 --- a/moli-protocol/src/domains/dom/resolve/native/mutation.rs +++ b/moli-protocol/src/domains/dom/resolve/native/mutation.rs @@ -43,20 +43,26 @@ pub(super) fn prepare( // The first step only resolves a native node binding, but removal // enters V8. Keep the complete chain on its owner turn; agent-only // operations below inherit their backend entry requirement. - Ok(with_backend(session, params.reference, |backend_node_id| { - Operation::new( - Command::RemoveDocumentBackendNodeId { backend_node_id }, - |reply| match reply { - Ok(Reply::Bool(true)) => Response::success(json!({})), - Ok(Reply::Bool(false)) => Response::error(-32000, "Could not remove node"), - Err(error) => { - Response::error(-32000, format!("Could not remove node: {error}")) - } - _ => unreachable!("DOM remove node reply"), - }, - ) - }) - .require_owner_turn()) + Ok(with_backend( + session, + params.reference, + OwnerTurn, + |backend_node_id| { + Operation::new( + Command::RemoveDocumentBackendNodeId { backend_node_id }, + |reply| match reply { + Ok(Reply::Bool(true)) => Response::success(json!({})), + Ok(Reply::Bool(false)) => { + Response::error(-32000, "Could not remove node") + } + Err(error) => { + Response::error(-32000, format!("Could not remove node: {error}")) + } + _ => unreachable!("DOM remove node reply"), + }, + ) + }, + )) } DomAction::Focus => { let params: NodeReferenceParams = cmd @@ -83,13 +89,17 @@ pub(super) fn prepare( } else { "No node found for given backend id" }; - Ok(with_backend(session, reference, move |backend_node_id| { - Operation::new( - Command::FocusDocumentBackendNode { backend_node_id }, - move |reply| focus(reply, missing), - ) - }) - .require_owner_turn()) + Ok(with_backend( + session, + reference, + OwnerTurn, + move |backend_node_id| { + Operation::new( + Command::FocusDocumentBackendNode { backend_node_id }, + move |reply| focus(reply, missing), + ) + }, + )) } } DomAction::SetAttributeValue | DomAction::RemoveAttribute => { @@ -121,6 +131,7 @@ pub(super) fn prepare( Ok(with_backend( session, DevToolsDomNodeReference::FrontendNodeId(id), + OwnerTurn, move |backend_node_id| { Operation::new( Command::MutateDocumentBackendNodeAttribute { @@ -130,8 +141,7 @@ pub(super) fn prepare( attribute, ) }, - ) - .require_owner_turn()) + )) } DomAction::MoveTo | DomAction::SetAttributesAsText @@ -166,16 +176,20 @@ pub(super) fn prepare( params.reference.backend_node_id, ) .ok_or_else(StartError::node_not_found)?; - Ok(with_backend(session, reference, move |backend_node_id| { - Operation::new( - Command::ScrollBackendNodeIntoViewIfNeeded { - backend_node_id, - rect, - }, - scroll, - ) - }) - .require_owner_turn()) + Ok(with_backend( + session, + reference, + OwnerTurn, + move |backend_node_id| { + Operation::new( + Command::ScrollBackendNodeIntoViewIfNeeded { + backend_node_id, + rect, + }, + scroll, + ) + }, + )) } } DomAction::GetNodeForLocation => { diff --git a/moli-protocol/src/domains/dom/resolve/native/remote_object.rs b/moli-protocol/src/domains/dom/resolve/native/remote_object.rs index c818d84458..4461e75cc5 100644 --- a/moli-protocol/src/domains/dom/resolve/native/remote_object.rs +++ b/moli-protocol/src/domains/dom/resolve/native/remote_object.rs @@ -12,8 +12,11 @@ pub(super) fn prepare(conn: &CdpConnection, cmd: &Cmd<'_>) -> Result) -> Result, + #[serde(default)] + pub(crate) backend_node_id: Option, + #[serde(default)] + pub(crate) object_id: Option, +} + +/// The requirement covers the complete lookup and continuation, not merely +/// the native binding query. Each domain retains its own selector precedence. +pub(crate) enum NodeLookupExecution { + OwnerTurn, + NestedMain, +} + +pub(crate) fn with_backend_node( + session: Option, + reference: DevToolsDomNodeReference, + execution: NodeLookupExecution, + next: impl FnOnce(u32) -> Operation + Send + 'static, +) -> Operation { + match reference { + DevToolsDomNodeReference::BackendNodeId(id) => { + let operation = next(id); + match execution { + NodeLookupExecution::OwnerTurn => operation.require_owner_turn(), + NodeLookupExecution::NestedMain => operation, + } + } + DevToolsDomNodeReference::FrontendNodeId(frontend_node_id) => { + let command = Command::DocumentFrontendNodeBinding { + inspector_session_id: session, + frontend_node_id, + }; + let continue_binding = move |reply| match reply { + Ok(Reply::DocumentFrontendNodeBinding( + RendererDomFrontendNodeBindingResolution::BackendNodeId(id), + )) => Step::Continue(next(id)), + Ok(Reply::DocumentFrontendNodeBinding( + RendererDomFrontendNodeBindingResolution::NotFound, + )) => Step::Complete(Response::error(-32000, "Could not find node with given id")), + Err(error) => Step::Complete(Response::error( + -32000, + format!("Could not resolve frontend node binding: {error}"), + )), + _ => unreachable!("native frontend-node lookup reply"), + }; + match execution { + NodeLookupExecution::OwnerTurn => Operation::then(command, continue_binding), + NodeLookupExecution::NestedMain => { + Operation::then_on_nested_main(command, continue_binding) + } + } + } + } +}