diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs index 319371bb01..2dc6076170 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs @@ -1,5 +1,8 @@ use super::super::{dom_binding_timing_started, record_dom_binding_timing}; -use super::{node_iterators::NodeIteratorRemovalPlan, policy::TreeMutationSourceProfile}; +use super::{ + node_iterators::NodeIteratorRemovalPlan, policy::TreeMutationSourceProfile, + resources::ImageRelevantMutationPlan, +}; use crate::{ custom_elements, document_runtime::{DocumentRuntime, DomHandle}, @@ -18,6 +21,7 @@ pub(super) struct TreeRemovalPlan { pub(super) live_range_previous_sibling: Option, pub(super) node_iterator_plan: Option, pub(super) registry_retargets: Vec, + pub(super) image_relevant_mutation_plan: ImageRelevantMutationPlan, } impl DocumentRuntime { @@ -54,6 +58,8 @@ impl DocumentRuntime { }; let registry_retargets = custom_elements::registry_association_retargets_before_removal(host_ptr, root); + let image_relevant_mutation_plan = + self.image_relevant_mutation_plan_before_remove(parent, root); TreeRemovalPlan { parent, root, @@ -64,6 +70,7 @@ impl DocumentRuntime { live_range_previous_sibling, node_iterator_plan, registry_retargets, + image_relevant_mutation_plan, } } diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs index 55565e1cca..5cfb1dbeba 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs @@ -40,6 +40,12 @@ impl DocumentRuntime { TreeMutationSideEffectSource::ParserTreeSink => {} } self.dispatch_tree_removal_custom_element_reactions(scope, host_ptr, removal_plan, profile); + self.queue_image_relevant_mutation_loads( + scope, + host_ptr, + &removal_plan.image_relevant_mutation_plan, + profile.subresource_request_initiator_type(), + ); } fn dispatch_tree_removal_pre_reaction_followups_after_change( diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs index 68e89ca31c..5bc012e798 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs @@ -274,5 +274,11 @@ impl DocumentRuntime { } } self.preserve_selectedness_for_insertion_plan(scope, host_ptr, insertion_plan); + self.queue_image_relevant_mutation_loads( + scope, + host_ptr, + &replacement_plan.removal.image_relevant_mutation_plan, + crate::types::SubresourceRequestInitiatorType::Script, + ); } } diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs index 806b2d0c1d..898986029d 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs @@ -180,10 +180,12 @@ impl DocumentRuntime { push_unique_handle(&mut plan.pictures, new_parent); } let old_parent = self.dom_host.parent_node(root); - if let Some(old_parent) = old_parent - && self.dom_host.is_html_element_named(old_parent, "picture") - { - push_unique_handle(&mut plan.pictures, old_parent); + if let Some(old_parent) = old_parent { + let removal_plan = + self.image_relevant_mutation_plan_before_remove(old_parent, root); + for image in removal_plan.images { + push_unique_handle(&mut plan.images, image); + } } if root_is_img && (new_parent_is_picture @@ -222,7 +224,38 @@ impl DocumentRuntime { plan } - fn queue_image_relevant_mutation_loads( + pub(super) fn image_relevant_mutation_plan_before_remove( + &self, + parent: DomHandle, + root: DomHandle, + ) -> ImageRelevantMutationPlan { + if !self.dom_host.is_html_element_named(parent, "picture") { + return ImageRelevantMutationPlan::default(); + } + if self.dom_host.is_html_element_named(root, "source") { + return ImageRelevantMutationPlan { + pictures: Vec::new(), + images: self + .dom_host + .child_handles(parent) + .skip_while(|&child| child != root) + .skip(1) + .filter(|&child| self.dom_host.is_html_element_named(child, "img")) + .collect(), + }; + } + ImageRelevantMutationPlan { + pictures: Vec::new(), + images: self + .dom_host + .is_html_element_named(root, "img") + .then_some(root) + .into_iter() + .collect(), + } + } + + pub(super) fn queue_image_relevant_mutation_loads( &self, scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut JsContextHost, @@ -335,28 +368,6 @@ impl DocumentRuntime { "src", ); } - - if !self - .dom_host - .is_html_element_named(removal_plan.root, "source") - || !self - .dom_host - .is_html_element_named(removal_plan.parent, "picture") - { - return; - } - let image = self - .dom_host - .child_handles(removal_plan.parent) - .into_iter() - .find(|child| self.dom_host.is_html_element_named(*child, "img")); - if let Some(image) = image { - crate::native_bridge::element::reset_image_load_dispatch( - unsafe { &mut *host_ptr }, - image, - ); - crate::native_bridge::element::queue_image_load_event_if_needed(scope, host_ptr, image); - } } fn queue_inserted_text_track_loads( diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs index bf0e11beb1..b0e3ddba59 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs @@ -1094,3 +1094,113 @@ async fn changing_picture_source_restarts_intercepted_image_request() { "load:https://example.test/second.png:1" ); } + +#[tokio::test] +async fn picture_source_and_image_tree_mutations_reselect_requests() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); + loader.set_image_fetch_enabled(true); + let mut vm = new_storage_page_task_executor_test_vm_with_loader( + "https://picture-source-removal.test/page.html", + &loader, + ); + vm.set_fetch_subresource_interception(true, Some(crate::types::SubresourceResourceType::Image)); + + vm.eval( + r#" + (() => { + if (!document.documentElement) { + document.append(document.createElement("html")); + } + if (!document.body) { + document.documentElement.append(document.createElement("body")); + } + const picture = document.createElement("picture"); + const source1 = document.createElement("source"); + const source2 = document.createElement("source"); + const source3 = document.createElement("source"); + const source4 = document.createElement("source"); + const source5 = document.createElement("source"); + const middleSource = document.createElement("source"); + const image1 = document.createElement("img"); + const image2 = document.createElement("img"); + source1.srcset = "s1.png"; + source2.srcset = "s2.png"; + source3.srcset = "s3.png"; + source4.srcset = "s4.png"; + source5.srcset = "s5.png"; + middleSource.srcset = "middle.png"; + image1.src = "img1.png"; + image2.src = "img2.png"; + picture.append(source1, source2, source3, image1, middleSource, image2, source4, source5); + const host = document.body; + host.append(picture); + globalThis.__pictureSourceMutation = { + host, + picture, + source1, + source2, + source3, + source4, + source5, + middleSource, + image1, + image2 + }; + })() + "#, + ) + .expect("picture source mutation setup should evaluate"); + + let cases: &[(&str, &[&str])] = &[ + ("", &["s1.png", "s1.png"]), + ("source1.remove()", &["s2.png", "s2.png"]), + ("source2.remove()", &["s3.png", "s3.png"]), + ( + "host.append(__pictureSourceMutation.source3)", + &["img1.png", "middle.png"], + ), + ("middleSource.remove()", &["img2.png"]), + ("source4.remove()", &[]), + ("host.append(__pictureSourceMutation.source5)", &[]), + ( + "picture.prepend(__pictureSourceMutation.source2)", + &["s2.png", "s2.png"], + ), + ( + "picture.prepend(__pictureSourceMutation.source1)", + &["s1.png", "s1.png"], + ), + ( + "source1.replaceWith(document.createElement('div'))", + &["s2.png", "s2.png"], + ), + ("image1.remove()", &["img1.png"]), + ]; + for (mutation, expected_sources) in cases { + if !mutation.is_empty() { + vm.eval(&format!("__pictureSourceMutation.{mutation}")) + .expect("picture tree mutation should evaluate"); + } + let mut requested_sources = vm + .take_pending_subresource_fetch_infos() + .into_iter() + .map(|request| { + assert_eq!( + request.resource_type, + crate::types::SubresourceResourceType::Image + ); + request.url.to_string() + }) + .collect::>(); + requested_sources.sort(); + let mut expected_sources = expected_sources + .iter() + .map(|source| format!("https://picture-source-removal.test/{source}")) + .collect::>(); + expected_sources.sort(); + assert_eq!( + requested_sources, expected_sources, + "mutation: {mutation:?}" + ); + } +}