fix(service-worker): preserve Window containers when navigator properties are shadowed

moli-renderer-v8/src/context_bootstrap/navigator_runtime.rs

@@ -33,7 +33,8 @@ pub(in crate::context_bootstrap) use self::navigator::{
     STORAGE_MANAGER_POPUP_ID_SLOT, build_storage_manager_worker_template,
     current_protocol_user_gesture_activation, finalize_navigator_credentials_realm_bindings,
     install_storage_manager_constructor_template_bindings, navigator_identity_profile,
-    service_worker_owner_token_value, set_navigator_identity_profile,
+    service_worker_container_for_window, service_worker_owner_token_value,
+    set_navigator_identity_profile,
 };
 #[cfg(test)]
 pub(crate) use self::navigator::{

moli-renderer-v8/src/context_bootstrap/navigator_runtime/navigator.rs

@@ -1759,3 +1759,25 @@ fn navigator_runtime_data_object<'s>(
     get_private_value(scope, navigator, NAVIGATOR_RUNTIME_DATA_SLOT)
         .and_then(|value| v8::Local::<v8::Object>::try_from(value).ok())
 }
+
+pub(in crate::context_bootstrap) fn service_worker_container_for_window<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    window: v8::Local<'s, v8::Object>,
+) -> Option<v8::Local<'s, v8::Object>> {
+    // Native notifications address the Window's associated container. Neither
+    // Window.navigator nor Navigator.serviceWorker invokes author code here.
+    let navigator = get_private_value(scope, window, WINDOW_NAVIGATOR_SLOT)
+        .and_then(|value| v8::Local::<v8::Object>::try_from(value).ok())
+        .or_else(|| {
+            super::super::window_lazy_surface::ensure_window_lazy_surface_object(
+                scope,
+                window,
+                super::super::window_lazy_surface::WindowLazySurface::Navigator,
+            )
+            .ok()
+        })?;
+    let backing = navigator_runtime_data_object(scope, navigator)?;
+    ensure_navigator_subobject(scope, backing, NavigatorSubobject::ServiceWorker)
+        .ok()
+        .and_then(|value| v8::Local::<v8::Object>::try_from(value).ok())
+}

moli-renderer-v8/src/context_bootstrap/window_runtime/service_worker.rs

@@ -1,6 +1,7 @@
 use super::*;
 use crate::context_bootstrap::navigator_runtime::{
-    SERVICE_WORKER_OWNER_TOKEN_SLOT, service_worker_owner_token_value,
+    SERVICE_WORKER_OWNER_TOKEN_SLOT, service_worker_container_for_window,
+    service_worker_owner_token_value,
 };
 use crate::context_bootstrap::service_worker_interfaces::{
     CONTAINER_READY, NavigationPreloadManagerObjectDeclaration, REGISTRATION_UPDATE_VIA_CACHE,
@@ -882,12 +883,7 @@ fn service_worker_container_from_window<'s>(
     scope: &mut v8::PinScope<'s, '_>,
     window: v8::Local<'s, v8::Object>,
 ) -> Option<v8::Local<'s, v8::Object>> {
-    let navigator = window
-        .get(scope, v8str(scope, "navigator").into())
-        .and_then(|value| v8::Local::<v8::Object>::try_from(value).ok())?;
-    navigator
-        .get(scope, v8str(scope, "serviceWorker").into())
-        .and_then(|value| v8::Local::<v8::Object>::try_from(value).ok())
+    service_worker_container_for_window(scope, window)
 }
 
 fn service_worker_container_registration_cache<'s>(

moli-renderer-v8/src/runtime/page_vm/tests/service_worker_client_message.rs

@@ -279,7 +279,8 @@ navigator.serviceWorker.startMessages();
 }
 
 #[tokio::test(flavor = "current_thread")]
-async fn current_service_worker_client_without_a_dispatchable_container_is_checkpoint_only() {
+async fn service_worker_client_message_to_a_shadowed_container_without_listener_is_checkpoint_only()
+{
     run_page_vm_async_test(async move {
         let loader =
             crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader");
@@ -312,11 +313,17 @@ Object.defineProperty(navigator, "serviceWorker", {
 
         let task = page_vm
             .take_service_worker_client_message_body_task_for_test()
-            .expect("container-free current target should consume one bounded turn");
+            .expect("current target with a shadowed container should consume one bounded turn");
         let outcome = page_vm.apply_selected_page_service_worker_client_message_turn(task)?;
+        // An author property cannot remove the Window's associated container.
+        // Dispatching without a listener still only checkpoints this task.
         assert_eq!(
             outcome.action.target_effect,
-            PageServiceWorkerClientMessageTargetEffect::CurrentTargetProducedNoDispatchableEvent
+            PageServiceWorkerClientMessageTargetEffect::EventDispatchedToCurrentTarget {
+                event_kind: ServiceWorkerClientMessageEventKind::Message,
+                callback_effect:
+                    ServiceWorkerClientMessageCallbackEffect::CurrentTargetHadNoCallback,
+            }
         );
         let completion = outcome.action.into_page_task_completion();
         assert!(matches!(completion, PageTaskCompletion::CheckpointOnly));
@@ -336,7 +343,7 @@ Object.defineProperty(navigator, "serviceWorker", {
         Ok::<_, anyhow::Error>(())
     })
     .await
-    .expect("ServiceWorker current-target no-event completion test should run");
+    .expect("ServiceWorker shadowed-container no-listener completion test should run");
 }
 
 #[tokio::test(flavor = "current_thread")]

moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/service_worker_events.rs

@@ -1,6 +1,7 @@
 use super::*;
 
 mod navigation_preload;
+mod window_container;
 
 #[tokio::test]
 async fn service_worker_navigation_fallback_preserves_reload_validation_on_wire() {

moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/service_worker_events/window_container.rs

@@ -0,0 +1,120 @@
+use super::*;
+
+#[tokio::test]
+async fn window_service_worker_notifications_ignore_shadowed_navigator_properties() {
+    for mode in [
+        "normal",
+        "navigator-value",
+        "navigator-getter",
+        "container-value",
+        "container-getter",
+        "prototype-getter",
+    ] {
+        let (base_url, server) = spawn_service_worker_response_server(vec![(
+            "/app/worker.js",
+            "text/javascript",
+            r#"
+oninstall = event => event.waitUntil(skipWaiting());
+onactivate = event => event.waitUntil(clients.claim());
+onmessage = event => {
+  const source = event.source;
+  if (event.data === 'reply') {
+    source.postMessage({kind: 'source-ack'});
+    return;
+  }
+  const channel = new MessageChannel();
+  channel.port1.onmessage = message => {
+    source.postMessage({kind: 'port-ack', value: message.data});
+  };
+  source.postMessage({kind: 'message', index: 0, port: channel.port2}, [channel.port2]);
+  source.postMessage({kind: 'message', index: 1});
+};
+            "#,
+        )])
+        .await;
+        let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).unwrap();
+        let (mut vm, browser_context_runtime) =
+            new_service_worker_page_test_vm_with_loader_and_browser_context_runtime(
+                &format!("{base_url}/app/page.html"),
+                &loader,
+            );
+        vm.eval(&format!(
+            r#"
+globalThis.intrinsicContainerResult = 'pending';
+(async () => {{
+  const mode = {mode:?}, savedNavigator = navigator, container = savedNavigator.serviceWorker;
+  let getterCalls = 0, globalMessages = 0, controllerChanges = 0, portAck = false, sourceAck = false;
+  let controller, resolveMessages, resolveChanged;
+  const messages = new Promise(resolve => resolveMessages = resolve);
+  const changed = new Promise(resolve => resolveChanged = resolve);
+  const rows = [], trace = [];
+  function checkDone() {{
+    if (rows.length === 2 && portAck && sourceAck) resolveMessages();
+  }}
+  addEventListener('message', () => globalMessages++);
+  container.oncontrollerchange = event => {{
+    if (event.target !== container || event.currentTarget !== container) throw Error('wrong controllerchange target');
+    controllerChanges++;
+    resolveChanged();
+  }};
+  container.onmessage = event => {{
+    if (event.data.kind === 'source-ack') sourceAck = true;
+    else if (event.data.kind === 'port-ack') portAck = event.data.value === 'port-reply';
+    else {{
+      const index = event.data.index;
+      rows.push({{
+        index,
+        target: event.target === container,
+        currentTarget: event.currentTarget === container,
+        source: event.source === controller && event.source instanceof ServiceWorker,
+        sourceURL: event.source.scriptURL === new URL('worker.js', location.href).href,
+        origin: event.origin === location.origin,
+        lastEventId: event.lastEventId,
+        ports: event.ports.length,
+        port: index !== 0 || (event.ports[0] === event.data.port && event.ports[0] instanceof MessagePort)
+      }});
+      trace.push('message:' + index);
+      Promise.resolve().then(() => trace.push('microtask:' + index));
+      if (index === 0) {{
+        event.ports[0].postMessage('port-reply');
+        event.source.postMessage('reply');
+      }}
+    }}
+    checkDone();
+  }};
+  const throwingGetter = () => {{ getterCalls++; throw Error('native notification read an author property'); }};
+  if (mode === 'navigator-value') Object.defineProperty(globalThis, 'navigator', {{value: null, configurable: true}});
+  if (mode === 'navigator-getter') Object.defineProperty(globalThis, 'navigator', {{get: throwingGetter, configurable: true}});
+  if (mode === 'container-value') Object.defineProperty(savedNavigator, 'serviceWorker', {{value: null, configurable: true}});
+  if (mode === 'container-getter') Object.defineProperty(savedNavigator, 'serviceWorker', {{get: throwingGetter, configurable: true}});
+  if (mode === 'prototype-getter') Object.defineProperty(Object.getPrototypeOf(savedNavigator), 'serviceWorker', {{get: throwingGetter, configurable: true}});
+  const registration = await container.register('worker.js', {{scope: './'}});
+  await __moliTestActivatedServiceWorker(container);
+  await changed;
+  controller = container.controller;
+  if (controller !== registration.active) throw Error('controller does not share the registration binding');
+  controller.postMessage('burst');
+  await messages;
+  const valid = rows.every((row, index) => row.index === index && row.target && row.currentTarget &&
+    row.source && row.sourceURL && row.origin && row.lastEventId === '' && row.ports === (index === 0 ? 1 : 0) && row.port);
+  intrinsicContainerResult = JSON.stringify({{valid, getterCalls, globalMessages, controllerChanges, trace, portAck, sourceAck}});
+}})().catch(error => intrinsicContainerResult = String(error));
+            "#,
+        ))
+        .unwrap();
+        drain_service_worker_test_until_eval_equals(
+            &mut vm,
+            &browser_context_runtime,
+            &loader,
+            "String(intrinsicContainerResult !== 'pending')",
+            "true",
+        )
+        .await;
+        assert_eq!(
+            vm.eval("intrinsicContainerResult").unwrap(),
+            r#"{"valid":true,"getterCalls":0,"globalMessages":0,"controllerChanges":1,"trace":["message:0","microtask:0","message:1","microtask:1"],"portAck":true,"sourceAck":true}"#,
+            "{mode}"
+        );
+        server.await.unwrap();
+    }
+}