diff --git a/moli-protocol/src/conn/runtime_eval/remote_objects.rs b/moli-protocol/src/conn/runtime_eval/remote_objects.rs index 813411fca1..80866096ff 100644 --- a/moli-protocol/src/conn/runtime_eval/remote_objects.rs +++ b/moli-protocol/src/conn/runtime_eval/remote_objects.rs @@ -2,7 +2,7 @@ use super::*; use moli_inspector_json::{response_remote_object_ids_in_value, response_result_remote_object_ids}; impl CdpConnection { - pub(super) fn register_runtime_remote_object_ids_from_response_for_owner( + pub(crate) fn register_runtime_remote_object_ids_from_response_for_owner( &mut self, owner: &CommandOwnerScope, message: &InspectorJson, @@ -20,7 +20,7 @@ impl CdpConnection { self.register_runtime_remote_object_ids_for_owner(owner, object_ids, object_group); } - pub(super) fn register_runtime_remote_object_ids_from_response_result_for_owner( + pub(crate) fn register_runtime_remote_object_ids_from_response_result_for_owner( &mut self, owner: &CommandOwnerScope, result: &Value, @@ -149,6 +149,8 @@ impl CdpConnection { .is_some_and(|state| state.has_runtime_remote_object_id(object_id)) } + // For notifications and native RemoteObject fragments. Inspector command + // responses must use the response entry points above to keep values opaque. pub(crate) fn register_runtime_remote_object_ids_from_value_for_owner( &mut self, owner: &CommandOwnerScope, diff --git a/moli-protocol/src/domains/runtime/dispatcher.rs b/moli-protocol/src/domains/runtime/dispatcher.rs index c70525c43c..ad184a40c9 100644 --- a/moli-protocol/src/domains/runtime/dispatcher.rs +++ b/moli-protocol/src/domains/runtime/dispatcher.rs @@ -304,17 +304,25 @@ impl RuntimeInspectorRoutedOutput { if let Some((_, _, BackgroundCommandResponsePayloadRef::Success { result })) = event.command_response_payload_ref() { - conn.register_runtime_remote_object_ids_from_value_for_owner_with_group( + conn.register_runtime_remote_object_ids_from_response_result_for_owner( owner, result, - object_group, - ); - } else if let Some(message) = event.protocol_message() { - conn.register_runtime_remote_object_ids_from_value_for_owner_with_group( - owner, - message, - object_group, + Some(object_group), ); + } else if let Some(message) = event.protocol_json() { + if message.id_u64().is_some() { + conn.register_runtime_remote_object_ids_from_response_for_owner( + owner, + message, + Some(object_group), + ); + } else { + conn.register_runtime_remote_object_ids_from_value_for_owner_with_group( + owner, + message.value(), + object_group, + ); + } } } } @@ -2841,6 +2849,9 @@ pub(crate) use worker_binding_replay::{ mod worker_completion; pub(crate) use worker_completion::{PreparedWorkerRuntimeCommand, WorkerRuntimeContinuation}; +#[cfg(test)] +mod response_handle_tests; + #[cfg(test)] fn worker_workflow_fixture() -> CdpConnection { let mut conn = CdpConnection::new(); diff --git a/moli-protocol/src/domains/runtime/dispatcher/response_handle_tests.rs b/moli-protocol/src/domains/runtime/dispatcher/response_handle_tests.rs new file mode 100644 index 0000000000..d498313d45 --- /dev/null +++ b/moli-protocol/src/domains/runtime/dispatcher/response_handle_tests.rs @@ -0,0 +1,133 @@ +use super::*; +use crate::conn::BrowserContext; +use moli_inspector_json::InspectorJson; + +#[derive(Clone, Copy, Debug)] +enum Owner { + Page, + PageSession, + SharedWorker, +} + +impl Owner { + fn fixture(self) -> (CdpConnection, CommandOwnerScope) { + if matches!(self, Self::SharedWorker) { + return ( + worker_workflow_fixture(), + CommandOwnerScope::for_session("SID-owned-worker"), + ); + } + let mut conn = CdpConnection::default(); + let mut context = BrowserContext::new("BID-response-dispatch".to_owned()); + context.set_active_target_id("TID-response-dispatch".to_owned()); + context.attach_active_session("SID-response-dispatch".to_owned()); + conn.install_browser_context_fixture_for_test(context); + let session = matches!(self, Self::PageSession).then_some("SID-response-dispatch"); + let owner = CommandOwnerScope::capture(&conn, session); + (conn, owner) + } +} + +#[derive(Clone, Copy, Debug)] +enum Container { + Raw, + Parsed, + Typed, +} + +#[test] +fn routed_output_response_registration_preserves_groups_without_materializing() { + for owner_kind in [Owner::Page, Owner::PageSession, Owner::SharedWorker] { + for container in [Container::Raw, Container::Parsed, Container::Typed] { + for real_handles in [false, true] { + let (mut conn, owner) = owner_kind.fixture(); + conn.register_runtime_remote_object_ids_from_value_for_owner_with_group( + &owner, + &json!({"objectId":"existing-handle"}), + "group-A", + ); + let mut body = json!({"result":{"type":"object","value":{ + "objectId":"existing-handle","nested":{"errorObjectId":"user-only"} + }}}); + if real_handles { + body["result"]["objectId"] = json!("actual"); + body["exceptionDetails"] = json!({"exception":{ + "type":"object","objectId":"exception","value":{"objectId":"user-only"} + }}); + } + let envelope = json!({"id":45,"result":body}); + let raw = InspectorJson::from_slice(envelope.to_string().as_bytes()).unwrap(); + let wire = raw.to_string(); + let event = match container { + Container::Raw => BackgroundProtocolEvent::immediate(raw.clone()), + Container::Parsed => BackgroundProtocolEvent::immediate(envelope), + Container::Typed => { + BackgroundProtocolEvent::command_success(Some(45), owner.session_id(), body) + } + }; + let routed = RuntimeInspectorRoutedOutput { + events: vec![event], + ..Default::default() + }; + routed.register_object_group_for_success(&mut conn, &owner, None); + assert!(!raw.is_materialized()); + routed.register_object_group_for_success(&mut conn, &owner, Some("group-B")); + assert_eq!( + conn.runtime_remote_object_group_for_owner(&owner, "existing-handle") + .as_deref(), + Some("group-A"), + "{owner_kind:?}, {container:?}, real_handles={real_handles}" + ); + assert!(!conn.runtime_remote_object_id_known_for_owner(&owner, "user-only")); + assert!(!raw.is_materialized()); + assert_eq!(raw.to_string(), wire); + for id in ["actual", "exception"] { + assert_eq!( + conn.runtime_remote_object_id_known_for_owner(&owner, id), + real_handles + ); + assert_eq!( + conn.runtime_remote_object_group_for_owner(&owner, id) + .as_deref(), + real_handles.then_some("group-B") + ); + } + conn.unregister_runtime_remote_object_group_for_owner(&owner, "group-B"); + assert!(conn.runtime_remote_object_id_known_for_owner(&owner, "existing-handle")); + assert_eq!( + conn.runtime_remote_object_group_for_owner(&owner, "existing-handle") + .as_deref(), + Some("group-A") + ); + for id in ["actual", "exception"] { + assert!(!conn.runtime_remote_object_id_known_for_owner(&owner, id)); + } + } + } + } +} + +#[test] +fn routed_output_registration_retains_notification_handle_semantics() { + let notification = json!({"method":"Runtime.futureNotification","params":{ + "value":{"objectId":"notification"},"nested":[{"promiseObjectId":"promise"}] + }}); + for message in [ + InspectorJson::from_slice(notification.to_string().as_bytes()).unwrap(), + InspectorJson::from(notification), + ] { + let (mut conn, owner) = Owner::PageSession.fixture(); + let routed = RuntimeInspectorRoutedOutput { + events: vec![BackgroundProtocolEvent::immediate(message)], + ..Default::default() + }; + routed.register_object_group_for_success(&mut conn, &owner, Some("notifications")); + for id in ["notification", "promise"] { + assert_eq!( + conn.runtime_remote_object_group_for_owner(&owner, id) + .as_deref(), + Some("notifications") + ); + } + } +}