diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index 84562cc277..b2f6c6bf89 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -2825,7 +2825,6 @@ html/interaction/focus/the-autofocus-attribute/update-the-rendering.html html/semantics/document-metadata/the-link-element/link-error-fired-before-scripting-unblocked.html html/semantics/document-metadata/the-link-element/link-load-error-events.html html/semantics/document-metadata/the-link-element/link-load-fired-before-scripting-unblocked.html -html/semantics/document-metadata/the-link-element/stylesheet-not-removed-until-next-stylesheet-loads.html html/semantics/document-metadata/the-style-element/tentative/style-element-basic-import.html html/semantics/document-metadata/the-style-element/tentative/style-element-csp-allowed.html html/semantics/document-metadata/the-style-element/tentative/style-element-csp-nonce-allowed.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index cca9e64e2d..ae46714b8f 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -6231,6 +6231,7 @@ html/semantics/document-metadata/the-link-element/link-type-attribute-crash.html html/semantics/document-metadata/the-link-element/stylesheet-bad-mime-type.html html/semantics/document-metadata/the-link-element/stylesheet-media-change-no-fetch.html html/semantics/document-metadata/the-link-element/stylesheet-non-OK-status.html +html/semantics/document-metadata/the-link-element/stylesheet-not-removed-until-next-stylesheet-loads.html html/semantics/document-metadata/the-meta-element/color-scheme/meta-color-scheme-attribute-changes.html html/semantics/document-metadata/the-meta-element/color-scheme/meta-color-scheme-empty-content-value.html html/semantics/document-metadata/the-meta-element/color-scheme/meta-color-scheme-first-valid-applies.html diff --git a/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs b/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs index f3b3800913..ea5a2dec41 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs @@ -1202,6 +1202,11 @@ impl JsContextHost { owner: DomHandle, request_url: &url::Url, ) -> bool { + // A loaded owner keeps its current sheet until the successor fetch + // settles. Parsed URL cache hydration must not replace it in advance. + if self.linked_stylesheet_source_for_owner(owner).is_some() { + return false; + } // A URL-only parsed source has no proof for this link's integrity. // Admission through the stylesheet fetch cache can reuse only a terminal // validated with the same request options, including integrity metadata. diff --git a/moli-renderer-v8/src/native_bridge/document/css_state/projection.rs b/moli-renderer-v8/src/native_bridge/document/css_state/projection.rs index 0fabe2ebd0..794a1f04e7 100644 --- a/moli-renderer-v8/src/native_bridge/document/css_state/projection.rs +++ b/moli-renderer-v8/src/native_bridge/document/css_state/projection.rs @@ -237,6 +237,17 @@ fn owner_change_detaches_cached_link_sheet( element.attribute("title"), ); } + if matches!( + change.kind(), + DomStylesheetOwnerChangeKind::Attribute { .. } + ) { + // The source lifecycle retains an installed sheet while its successor + // loads. Preserve the corresponding JS object until an actual install + // changes its identity, or the owner stops being a stylesheet. + return host + .linked_stylesheet_source_for_owner(change.owner()) + .is_none(); + } true } diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs b/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs index a03a55c6dd..0146c25bb0 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs @@ -621,6 +621,194 @@ document.head.append(late); .expect("shared cache-hit stylesheet import test should run"); } +#[tokio::test(flavor = "current_thread")] +async fn stylesheet_reload_retains_cssom_until_the_successor_installs() { + use base64::{Engine as _, engine::general_purpose::STANDARD}; + + run_page_vm_async_test(async move { + let wrong = format!( + "sha384-{}", + STANDARD.encode(moli_crypto::DigestAlgorithm::Sha384.digest_bytes(b"wrong")) + ); + for (status, integrity, expected) in [ + ( + "HTTP/1.1 200 OK", + None, + "false|false|false|1|1|2|rgb(4, 5, 6)|load", + ), + ( + "HTTP/1.1 404 Not Found", + None, + "false|false|false|1|0|2|rgb(0, 0, 0)|error", + ), + ( + "HTTP/1.1 200 OK", + Some(wrong.as_str()), + "true|true|true|1|2|2|rgb(7, 8, 9)|error", + ), + ] { + let (base_url, server) = spawn_path_response_http_server(vec![ + ( + "/old.css", + "HTTP/1.1 200 OK", + "#reload-target { color: rgb(1, 2, 3); }".to_owned(), + Duration::ZERO, + ), + ( + "/next.css", + status, + "#reload-target { color: rgb(4, 5, 6); }".to_owned(), + Duration::ZERO, + ), + ]) + .await; + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let (mut page_vm, _resource_source, mut wake_rx) = + page_vm_with_bound_task_sources_and_owner_wake( + &loader, + Url::parse(&format!("{base_url}/page.html"))?, + ); + let old_href = serde_json::to_string(&format!("{base_url}/old.css"))?; + let next_href = serde_json::to_string(&format!("{base_url}/next.css"))?; + page_vm.vm_mut().eval(&format!( + r#" +globalThis.__reloadEvents = []; +const target = document.body.appendChild(document.createElement("div")); +target.id = "reload-target"; +const link = document.createElement("link"); +link.rel = "stylesheet"; +link.href = {old_href}; +link.onload = () => __reloadEvents.push("load"); +link.onerror = () => __reloadEvents.push("error"); +document.head.append(link); +"ready" +"#, + ))?; + + for stage in 0..2 { + page_vm + .vm_mut() + .prime_document_lifecycle_processing_and_record_stylesheet_network_results(); + wait_for_stylesheet_source(&mut wake_rx, RendererOwnerWakeSource::NetworkingTask) + .await; + assert!( + page_vm + .run_exact_selected_page_task_for_test( + PageSelectedTaskTestSelector::StylesheetCompletion, + &loader, + ) + .await? + ); + let event = take_next_link_element_event_task_for_test(&mut page_vm) + .expect("the selected stylesheet response must publish its result"); + page_vm + .run_claimed_dom_manipulation_task_through_selected_dispatcher_for_test( + crate::page_task_queue::RendererPageDomManipulationTask::ConnectedStyleEvent( + event, + ), + &loader, + ) + .await?; + if stage == 0 { + assert_eq!(page_vm.vm_mut().eval("__reloadEvents.join('|')")?, "load"); + let integrity = serde_json::to_string(integrity.unwrap_or_default())?; + let pending = page_vm.vm_mut().eval(&format!( + r##" +const old = link.sheet; +old.insertRule("#reload-target {{ color: rgb(7, 8, 9); }}", old.cssRules.length); +globalThis.__reloadSnapshot = () => [ + link.sheet === old, old.ownerNode === link, document.styleSheets[0] === old, + document.styleSheets.length, link.sheet.cssRules.length, old.cssRules.length, + getComputedStyle(target).color, __reloadEvents.join(",") +].join("|"); +__reloadEvents.length = 0; +link.integrity = {integrity}; +link.href = {next_href}; +__reloadSnapshot() +"##, + ))?; + assert_eq!( + pending, + "true|true|true|1|2|2|rgb(7, 8, 9)|", + "changing href must retain the old JS sheet and its CSSOM edits until replacement" + ); + } else { + assert_eq!(page_vm.vm_mut().eval("__reloadSnapshot()")?, expected); + assert_eq!( + page_vm.vm_mut().eval(&format!("old.href === {old_href}"))?, + "true", + "the old sheet's URL must not follow the owner's new href" + ); + assert!(!page_vm.vm().document_runtime.has_pending_style_loads()); + } + } + server.await.expect("both stylesheet responses should be consumed"); + } + Ok::<_, anyhow::Error>(()) + }) + .await + .expect("stylesheet reload CSSOM lifetimes should run"); +} + +#[tokio::test(flavor = "current_thread")] +async fn stylesheet_reload_removal_retires_the_sheet_and_pending_successor() { + run_page_vm_async_test(async move { + let (base_url, server) = spawn_path_response_http_server(vec![ + ("/old.css", "HTTP/1.1 200 OK", "body { color: red; }".to_owned(), Duration::ZERO), + ("/next.css", "HTTP/1.1 200 OK", "body { color: green; }".to_owned(), Duration::ZERO), + ]) + .await; + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let (mut page_vm, _resource_source, mut wake_rx) = + page_vm_with_bound_task_sources_and_owner_wake( + &loader, + Url::parse(&format!("{base_url}/page.html"))?, + ); + let old_href = serde_json::to_string(&format!("{base_url}/old.css"))?; + let next_href = serde_json::to_string(&format!("{base_url}/next.css"))?; + page_vm.vm_mut().eval(&format!( + r#" +globalThis.__reloadEvents = []; +const link = document.createElement("link"); +link.rel = "stylesheet"; +link.href = {old_href}; +link.onload = () => __reloadEvents.push("load"); +link.onerror = () => __reloadEvents.push("error"); +document.head.append(link); +"ready" +"#, + ))?; + page_vm.vm_mut().prime_document_lifecycle_processing_and_record_stylesheet_network_results(); + wait_for_stylesheet_source(&mut wake_rx, RendererOwnerWakeSource::NetworkingTask).await; + assert!(page_vm.run_exact_selected_page_task_for_test(PageSelectedTaskTestSelector::StylesheetCompletion, &loader).await?); + let event = take_next_link_element_event_task_for_test(&mut page_vm).expect("initial load event"); + page_vm.run_claimed_dom_manipulation_task_through_selected_dispatcher_for_test( + crate::page_task_queue::RendererPageDomManipulationTask::ConnectedStyleEvent(event), &loader, + ).await?; + assert_eq!(page_vm.vm_mut().eval(&format!( + "const old = link.sheet; __reloadEvents.length = 0; link.href = {next_href}; link.sheet === old && old.ownerNode === link" + ))?, "true"); + + page_vm.vm_mut().prime_document_lifecycle_processing_and_record_stylesheet_network_results(); + wait_for_stylesheet_source(&mut wake_rx, RendererOwnerWakeSource::NetworkingTask).await; + assert_eq!(page_vm.vm_mut().eval( + "link.remove(); [link.sheet === null, old.ownerNode === null, document.styleSheets.length].join('|')" + )?, "true|true|0"); + let _ = page_vm.run_exact_selected_page_task_for_test(PageSelectedTaskTestSelector::StylesheetCompletion, &loader).await?; + assert!(take_next_link_element_event_task_for_test(&mut page_vm).is_none()); + assert_eq!(page_vm.vm_mut().eval( + "[link.sheet === null, old.ownerNode === null, document.styleSheets.length, __reloadEvents.length].join('|')" + )?, "true|true|0|0"); + assert!(!page_vm.vm().document_runtime.has_pending_style_loads()); + server.await.expect("both requests reached the fixture before removal"); + Ok::<_, anyhow::Error>(()) + }) + .await + .expect("removal must invalidate a reloading stylesheet owner"); +} + #[tokio::test(flavor = "current_thread")] async fn stylesheet_integrity_does_not_reuse_an_unverified_css_source() { use base64::{Engine as _, engine::general_purpose::STANDARD}; diff --git a/moli-renderer-v8/src/style_engine/source/mod.rs b/moli-renderer-v8/src/style_engine/source/mod.rs index 77dfad2127..0112ed1139 100644 --- a/moli-renderer-v8/src/style_engine/source/mod.rs +++ b/moli-renderer-v8/src/style_engine/source/mod.rs @@ -580,6 +580,26 @@ impl MoliStyleEngine { continue; } let previous_documents = self.linked_stylesheet_owner_documents(owner); + if matches!( + change.kind(), + DomStylesheetOwnerChangeKind::Attribute { .. } + ) && crate::stylesheet_blocking::stylesheet_link_disposition( + host, + moli_dom::NodeId::new(owner.index()), + ) + .is_some() + { + // Reprocessing a valid link starts its successor load. Keep the + // installed sheet, including CSSOM edits, until that response + // installs a replacement; integrity rejection installs nothing. + self.invalidate_linked_stylesheet_owner_lifecycle_change( + host, + owner, + previous_documents, + owner_document_for_source_owner(host, owner), + ); + continue; + } self.remove_linked_stylesheet_owner_from_documents(host, owner, previous_documents); } } diff --git a/moli-renderer-v8/src/style_engine/tests/lifecycle.rs b/moli-renderer-v8/src/style_engine/tests/lifecycle.rs index ee648ef7e5..d38596faa7 100644 --- a/moli-renderer-v8/src/style_engine/tests/lifecycle.rs +++ b/moli-renderer-v8/src/style_engine/tests/lifecycle.rs @@ -2080,11 +2080,10 @@ fn linked_owner_binding_changes_only_on_explicit_install() { let media = crate::protocol_types::EmulatedMediaOverrides::default(); engine.invalidate_for_mutations(&host, &style_effects, &media); - assert!( - engine - .retained_stylesheet_source_ids_for_document_for_test(&host, document) - .is_empty(), - "the old loaded stylesheet must not remain bound after href changes" + assert_eq!( + engine.retained_stylesheet_source_ids_for_document_for_test(&host, document), + vec![linked_source_id.clone()], + "the loaded stylesheet remains bound while the new href is pending" ); engine.record_stylesheet_source_for_url_for_document_for_test( @@ -2092,17 +2091,32 @@ fn linked_owner_binding_changes_only_on_explicit_install() { &new_url, StyloStylesheetSource::new(".new { color: blue; }".into(), new_url.clone()), ); - assert!( + assert_eq!( + engine.retained_stylesheet_source_ids_for_document_for_test(&host, document), + vec![linked_source_id.clone()], + "recording a resource by URL must not replace the pending owner's sheet" + ); + assert_eq!( engine - .retained_stylesheet_source_ids_for_document_for_test(&host, document) - .is_empty(), - "recording a resource by URL must not infer an owner binding from live href" + .linked_stylesheet_source_for_owner_with_host(&host, link) + .unwrap() + .serialized_css_text() + .as_ref(), + ".old { color: red; }" ); assert!(engine.install_recorded_linked_stylesheet_source_for_test(&host, link, &new_url,)); assert_eq!( engine.retained_stylesheet_source_ids_for_document_for_test(&host, document), vec![linked_source_id] ); + assert_eq!( + engine + .linked_stylesheet_source_for_owner_with_host(&host, link) + .unwrap() + .serialized_css_text() + .as_ref(), + ".new { color: blue; }" + ); } #[test]