diff --git a/moli-renderer-v8/src/style_engine/mutation_effect.rs b/moli-renderer-v8/src/style_engine/mutation_effect.rs index 2e58900c9f..bacfec7fc1 100644 --- a/moli-renderer-v8/src/style_engine/mutation_effect.rs +++ b/moli-renderer-v8/src/style_engine/mutation_effect.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{borrow::Cow, sync::Arc}; use indexmap::IndexSet; use moli_selector::{ @@ -271,6 +271,73 @@ pub(crate) fn normalized_style_attribute_name( .unwrap_or_else(|| name.to_owned()) } +/// Keep only live structural history for an emptied ordinary fragment. Removed +/// roots must already have been invalidated from the original effects, and the +/// caller must retain the original source scope. A fragment with children, an +/// element parent, and a ShadowRoot can still have selector-dependent styles. +/// Their histories must pass the ordinary retained-topology checks unchanged. +pub(super) fn normalized_deferred_structural_mutation_effects<'a>( + host: &DomHost, + effects: &'a [StyleMutationEffect], +) -> Cow<'a, [StyleMutationEffect]> { + if !effects + .iter() + .any(|effect| is_empty_ordinary_fragment_child_list(host, effect)) + { + return Cow::Borrowed(effects); + } + + let mut normalized = Vec::with_capacity(effects.len()); + let mut connected_roots = IndexSet::new(); + let mut disconnected_roots = IndexSet::new(); + let mut connected_slot = None; + let mut disconnected_slot = None; + for effect in effects { + if is_empty_ordinary_fragment_child_list(host, effect) { + continue; + } + match effect { + StyleMutationEffect::ConnectedSubtrees { roots } => { + if connected_slot.is_none() { + connected_slot = Some(normalized.len()); + normalized.push(effect.clone()); + } + connected_roots.extend(roots.iter().copied()); + } + StyleMutationEffect::DisconnectedSubtrees { roots } => { + if disconnected_slot.is_none() { + disconnected_slot = Some(normalized.len()); + normalized.push(effect.clone()); + } + disconnected_roots.extend(roots.iter().copied()); + } + // Connection facts are sets, but child-list and slot histories are + // ordered inputs. Do not deduplicate or collapse their splices. + _ => normalized.push(effect.clone()), + } + } + if let Some(slot) = connected_slot { + normalized[slot] = StyleMutationEffect::ConnectedSubtrees { + roots: connected_roots.into_iter().collect::>().into(), + }; + } + if let Some(slot) = disconnected_slot { + normalized[slot] = StyleMutationEffect::DisconnectedSubtrees { + roots: disconnected_roots.into_iter().collect::>().into(), + }; + } + Cow::Owned(normalized) +} + +fn is_empty_ordinary_fragment_child_list(host: &DomHost, effect: &StyleMutationEffect) -> bool { + let StyleMutationEffect::ChildList { parent, .. } = effect else { + return false; + }; + host.node(*parent) + .is_some_and(|node| node.is_document_fragment() && node.first_child().is_none()) + && !host.is_shadow_root(*parent) +} + pub(super) fn detached_style_subtree_roots_for_mutations( effects: &[StyleMutationEffect], ) -> IndexSet { @@ -490,4 +557,178 @@ mod tests { assert_eq!(connected_batches[0].as_ref(), [first, second]); assert!(Arc::ptr_eq(connected_batches[0], &shared_added_nodes)); } + + fn collect_native_effects( + output: &mut Vec, + host: &DomHost, + effects: &DomMutationEffects, + ) { + assert!(effects.did_change()); + output.extend(StyleMutationEffect::from_dom_mutation_effects( + host, effects, + )); + } + + #[test] + fn consumed_fragment_normalization_preserves_real_splices_and_the_effect_limit() { + use crate::style_engine::{ + eligibility::child_list_effects_have_current_topology, + scope::style_source_scope_for_mutation_effects, + }; + + let mut host = test_host(); + let parent = host.create_element("div"); + assert!(host.append_child(host.document_handle(), parent)); + let mut effects = Vec::new(); + let mut inserted = Vec::new(); + let mut fragments = Vec::new(); + // Match textContent + ParentNode.append's native commits, including + // the staging insertion and fragment consumption as separate facts. + for _ in 0..64 { + let child = host.create_element("div"); + let text = host.set_text_content_effects(child, "ABCD"); + collect_native_effects(&mut effects, &host, &text); + let fragment = host.create_document_fragment(); + let staged = host.append_child_effects(fragment, child); + collect_native_effects(&mut effects, &host, &staged); + let consumed = host.append_child_effects(parent, fragment); + collect_native_effects(&mut effects, &host, &consumed); + inserted.push(child); + fragments.push(fragment); + } + assert!(effects.len() > 256); + assert!(!child_list_effects_have_current_topology(&host, &effects)); + assert_eq!( + detached_style_subtree_roots_for_mutations(&effects) + .into_iter() + .collect::>(), + inserted, + "cleanup must see the original removed roots before normalization" + ); + let normalized = normalized_deferred_structural_mutation_effects(&host, &effects); + assert!(matches!(&normalized, Cow::Owned(_))); + assert!(normalized.len() <= 256); + assert!(child_list_effects_have_current_topology(&host, &normalized)); + assert_eq!( + style_source_scope_for_mutation_effects(&host, &effects), + style_source_scope_for_mutation_effects(&host, &normalized) + ); + let real_splices = effects + .iter() + .filter(|effect| match effect { + StyleMutationEffect::ChildList { parent, .. } => !fragments.contains(parent), + _ => false, + }) + .cloned() + .collect::>(); + let kept_splices = normalized + .iter() + .filter(|effect| matches!(effect, StyleMutationEffect::ChildList { .. })) + .cloned() + .collect::>(); + assert_eq!(real_splices.len(), 128); + assert_eq!(kept_splices, real_splices); + drop(normalized); + + // Normalization must not turn an actually large structural history + // into a small batch. These are 300 distinct, selector-visible splices. + for _ in 0..300 { + let child = host.create_element("span"); + let added = host.append_child_effects(parent, child); + collect_native_effects(&mut effects, &host, &added); + } + let normalized = normalized_deferred_structural_mutation_effects(&host, &effects); + assert_eq!( + normalized + .iter() + .filter(|effect| matches!(effect, StyleMutationEffect::ChildList { .. })) + .count(), + 428 + ); + assert!(normalized.len() > 256); + } + + #[test] + fn ordinary_element_shadow_and_nonempty_fragment_histories_stay_borrowed() { + use crate::style_engine::eligibility::child_list_effects_have_current_topology; + + for parent_kind in ["detached-element", "shadow-root", "nonempty-fragment"] { + let mut host = test_host(); + let document = host.document_handle(); + let destination = host.create_element("main"); + assert!(host.append_child(document, destination)); + let old_parent = match parent_kind { + "detached-element" => host.create_element("section"), + "shadow-root" => { + let shadow_host = host.create_element("div"); + assert!(host.append_child(document, shadow_host)); + host.attach_shadow_root(shadow_host, "open").unwrap() + } + "nonempty-fragment" => { + let fragment = host.create_document_fragment(); + let remaining = host.create_element("span"); + assert!(host.append_child(fragment, remaining)); + fragment + } + _ => unreachable!(), + }; + let moved = host.create_element("span"); + let mut effects = Vec::new(); + let staged = host.append_child_effects(old_parent, moved); + collect_native_effects(&mut effects, &host, &staged); + let relocated = host.append_child_effects(destination, moved); + collect_native_effects(&mut effects, &host, &relocated); + assert!(effects.iter().any(|effect| matches!( + effect, + StyleMutationEffect::ChildList { parent, removed_nodes, .. } + if *parent == old_parent && removed_nodes == &[moved] + ))); + let normalized = normalized_deferred_structural_mutation_effects(&host, &effects); + assert!(matches!(&normalized, Cow::Borrowed(_)), "{parent_kind}"); + assert_eq!(normalized.as_ref(), effects.as_slice(), "{parent_kind}"); + assert!( + !child_list_effects_have_current_topology(&host, &normalized), + "real reparenting must retain the conservative fallback: {parent_kind}" + ); + } + } + + #[test] + fn consumed_fragment_does_not_erase_an_existing_elements_removal() { + use crate::style_engine::eligibility::child_list_effects_have_current_topology; + + let mut host = test_host(); + let document = host.document_handle(); + let old_parent = host.create_element("section"); + let destination = host.create_element("main"); + let moved = host.create_element("span"); + assert!(host.append_child(document, old_parent)); + assert!(host.append_child(document, destination)); + assert!(host.append_child(old_parent, moved)); + let fragment = host.create_document_fragment(); + let mut effects = Vec::new(); + let staged = host.append_child_effects(fragment, moved); + collect_native_effects(&mut effects, &host, &staged); + let consumed = host.append_child_effects(destination, fragment); + collect_native_effects(&mut effects, &host, &consumed); + let normalized = normalized_deferred_structural_mutation_effects(&host, &effects); + assert!(matches!(&normalized, Cow::Owned(_))); + assert!(normalized.iter().any(|effect| matches!( + effect, + StyleMutationEffect::ChildList { parent, removed_nodes, .. } + if *parent == old_parent && removed_nodes == &[moved] + ))); + assert!(normalized.iter().any(|effect| matches!( + effect, + StyleMutationEffect::ConnectedSubtrees { roots } if roots.contains(&moved) + ))); + assert!(normalized.iter().any(|effect| matches!( + effect, + StyleMutationEffect::DisconnectedSubtrees { roots } if roots.contains(&moved) + ))); + assert!( + !child_list_effects_have_current_topology(&host, &normalized), + "connected and disconnected facts must not cancel a real element-parent removal" + ); + } } diff --git a/moli-renderer-v8/src/style_engine/runtime_invalidation.rs b/moli-renderer-v8/src/style_engine/runtime_invalidation.rs index 4d200eec80..59c126852b 100644 --- a/moli-renderer-v8/src/style_engine/runtime_invalidation.rs +++ b/moli-renderer-v8/src/style_engine/runtime_invalidation.rs @@ -19,7 +19,9 @@ use super::{ cause::PendingStyleInvalidationCause, document_world::DocumentStyleWorld, eligibility::child_list_effects_have_current_topology, - mutation_effect::detached_style_subtree_roots_for_mutations, + mutation_effect::{ + detached_style_subtree_roots_for_mutations, normalized_deferred_structural_mutation_effects, + }, schedule::queue_style_invalidation_for_scope, scope::{ mutation_effects_have_source_scope, source_scope_for_custom_state_change, @@ -105,10 +107,10 @@ impl MoliStyleEngine { .unwrap_or_default(); let queue_started = profile_enabled.then(std::time::Instant::now); self.queue_style_invalidation_for_mutations( - document, &world, host, document_effects, + document_effects, emulated_media, viewport, ); @@ -151,9 +153,9 @@ impl MoliStyleEngine { fn queue_style_invalidation_for_mutations( &self, - document: DomHandle, world: &DocumentStyleWorld, host: &DomHost, + scope_effects: &[StyleMutationEffect], effects: &[StyleMutationEffect], emulated_media: &EmulatedMediaOverrides, viewport: StyleViewport, @@ -161,7 +163,9 @@ impl MoliStyleEngine { let profile_enabled = moli_trace::cpu_profile_enabled(); let total_started = profile_enabled.then(std::time::Instant::now); let source_scope_started = profile_enabled.then(std::time::Instant::now); - let source_scope = source_scope_for_mutations(host, effects); + // The retained cause may omit exhausted fragment history, but source + // ownership still comes from every original mutation handle. + let source_scope = source_scope_for_mutations(host, scope_effects); let source_scope_us = source_scope_started .map(|started| started.elapsed().as_micros()) .unwrap_or_default(); @@ -176,7 +180,7 @@ impl MoliStyleEngine { .unwrap_or_default(); let queue_scope_started = profile_enabled.then(std::time::Instant::now); self.queue_style_invalidation_scope( - document, + world.document, world, host, emulated_media, @@ -229,15 +233,21 @@ impl MoliStyleEngine { emulated_media: &EmulatedMediaOverrides, viewport: StyleViewport, ) { + // Even an exhausted ordinary fragment can have removed previously + // styled descendants. Retire those inputs before filtering history. self.invalidate_detached_style_subtrees_for_mutations(host, effects); - if effects.len() <= IMMEDIATE_STRUCTURAL_MUTATION_EFFECT_LIMIT - && child_list_effects_have_current_topology(host, effects) + let normalized = normalized_deferred_structural_mutation_effects(host, effects); + if !mutation_effects_have_source_scope(&normalized) { + return; + } + if normalized.len() <= IMMEDIATE_STRUCTURAL_MUTATION_EFFECT_LIMIT + && child_list_effects_have_current_topology(host, &normalized) { self.queue_style_invalidation_for_mutations( - document, world, host, effects, + &normalized, emulated_media, viewport, ); diff --git a/moli-renderer-v8/src/style_engine/tests/subtree_cache.rs b/moli-renderer-v8/src/style_engine/tests/subtree_cache.rs index 0e4e664c10..50fb8484f0 100644 --- a/moli-renderer-v8/src/style_engine/tests/subtree_cache.rs +++ b/moli-renderer-v8/src/style_engine/tests/subtree_cache.rs @@ -80,6 +80,8 @@ fn element_style_is_dirty_for_test( }) } +#[path = "subtree_cache/consumed_fragments.rs"] +mod consumed_fragments; #[path = "subtree_cache/detached_sources.rs"] mod detached_sources; #[path = "subtree_cache/document_ownership.rs"] diff --git a/moli-renderer-v8/src/style_engine/tests/subtree_cache/consumed_fragments.rs b/moli-renderer-v8/src/style_engine/tests/subtree_cache/consumed_fragments.rs new file mode 100644 index 0000000000..738280c49a --- /dev/null +++ b/moli-renderer-v8/src/style_engine/tests/subtree_cache/consumed_fragments.rs @@ -0,0 +1,205 @@ +use super::*; + +fn queue_native_fragment_effects( + engine: &mut MoliStyleEngine, + host: &DomHost, + effects: &crate::dom::native::DomMutationEffects, +) { + assert!(effects.did_change()); + let effects = StyleMutationEffect::from_dom_mutation_effects(host, effects); + assert!(!effects.is_empty()); + engine.invalidate_for_mutations( + host, + &effects, + &crate::protocol_types::EmulatedMediaOverrides::default(), + ); +} + +// ParentNode.append converts its arguments through an ordinary fragment and +// commits the staging and insertion separately. Do not drain between them: +// the removed node already has its final parent when deferred work is read. +fn insert_through_consumed_fragment( + host: &mut DomHost, + engine: &mut MoliStyleEngine, + parent: DomHandle, + child: DomHandle, + before: Option, +) { + let fragment = host.create_document_fragment(); + let staged = host.append_child_effects(fragment, child); + queue_native_fragment_effects(engine, host, &staged); + let inserted = host.insert_before_effects(parent, fragment, before); + assert!(host.node(fragment).unwrap().first_child().is_none()); + assert!(!host.is_shadow_root(fragment)); + assert_eq!(host.node(child).unwrap().parent_node(), Some(parent)); + assert!( + inserted + .style() + .child_list_mutations() + .iter() + .any(|effect| { effect.target() == fragment && effect.removed_nodes() == [child] }) + ); + queue_native_fragment_effects(engine, host, &inserted); +} + +fn fragment_test_styles( + document: DomHandle, + css: &str, +) -> (MoliStyleEngine, url::Url, FullStyleWorldSnapshot) { + let mut engine = MoliStyleEngine::new(); + let url = url::Url::parse("https://example.test/").unwrap(); + let source = StyloStylesheetSource::new(css.into(), url.clone()); + engine.set_document_adopted_style_sheet_sources(document, vec![source.clone()]); + let mut inputs = FullStyleWorldSnapshot::default(); + inputs.document_stylesheet_sources.push(source); + (engine, url, inputs) +} + +#[test] +fn consumed_fragment_tail_append_preserves_existing_canonical_styles() { + let mut host = test_host(); + let document = host.document_handle(); + let html = host.create_element("html"); + let body = host.create_element("body"); + let list = host.create_element("div"); + let unrelated = host.create_element("aside"); + assert!(host.append_child(document, html)); + assert!(host.append_child(html, body)); + assert!(host.append_child(body, list)); + assert!(host.append_child(body, unrelated)); + let old_items: [DomHandle; 2] = std::array::from_fn(|_| { + let item = host.create_element("div"); + let text = host.create_text_node("ABCD"); + assert!(host.set_attribute(item, "class", "item")); + assert!(host.append_child(item, text)); + assert!(host.append_child(list, item)); + item + }); + let (mut engine, url, inputs) = fragment_test_styles( + document, + ".item { font: 16px/20px monospace; color: rgb(1, 2, 3); }", + ); + let retained = [html, body, list, old_items[0], old_items[1], unrelated].map(|handle| { + let snapshot = computed_style_snapshot_for_test(&engine, &host, &url, handle, &inputs); + (handle, snapshot.computed_values()) + }); + + for _ in 0..2 { + let item = host.create_element("div"); + let text = host.create_text_node("ABCD"); + assert!(host.set_attribute(item, "class", "item")); + assert!(host.append_child(item, text)); + insert_through_consumed_fragment(&mut host, &mut engine, list, item, None); + } + engine.drain_pending_style_invalidations_for_document_for_test(&host, document); + let resolutions = engine.element_style_resolution_count_for_document_for_test(document); + for (handle, before) in retained { + let after = computed_style_snapshot_for_test(&engine, &host, &url, handle, &inputs); + assert!( + ServoArc::ptr_eq(&before, &after.computed_values()), + "tail append through a consumed ordinary fragment must keep existing style {handle:?}" + ); + let canonical = retained_primary_style_for_test(&engine, &host, handle).unwrap(); + assert!(ServoArc::ptr_eq(&before, &canonical)); + } + assert_eq!( + engine.element_style_resolution_count_for_document_for_test(document), + resolutions, + "reading unaffected styles must not recascade them" + ); +} + +#[test] +fn consumed_fragment_insertion_still_invalidates_structural_selectors() { + const BASE: &str = "rgb(1, 2, 3)"; + const MATCHED: &str = "rgb(9, 8, 7)"; + // The values are sampled by stable handles, not by changing sibling index. + // The sibling case inserts before the second item; the other cases append. + for (selector, before_values, after_values, insert_before_second, changed) in [ + ( + ".item:last-child", + [BASE, MATCHED], + [BASE, BASE, MATCHED], + false, + 1, + ), + ( + ".item:nth-last-child(2)", + [MATCHED, BASE], + [BASE, MATCHED, BASE], + false, + 0, + ), + ( + "#list:has(> .tail) > .item", + [BASE, BASE], + [MATCHED, MATCHED, MATCHED], + false, + 0, + ), + ( + ".tail + .item", + [BASE, BASE], + [BASE, MATCHED, BASE], + true, + 1, + ), + ] { + let mut host = test_host(); + let document = host.document_handle(); + let html = host.create_element("html"); + let list = host.create_element("div"); + assert!(host.set_attribute(list, "id", "list")); + assert!(host.append_child(document, html)); + assert!(host.append_child(html, list)); + let old_items: [DomHandle; 2] = std::array::from_fn(|_| { + let item = host.create_element("div"); + assert!(host.set_attribute(item, "class", "item")); + assert!(host.append_child(list, item)); + item + }); + let css = format!( + ".item {{ background-color: {BASE}; }} {selector} {{ background-color: {MATCHED}; }}" + ); + let (mut engine, url, inputs) = fragment_test_styles(document, &css); + let read_color = |engine: &MoliStyleEngine, host: &DomHost, handle| { + engine + .computed_style_property_value( + host, + &url, + handle, + "background-color", + None, + &inputs, + None, + ) + .unwrap() + }; + assert_eq!( + old_items.map(|item| read_color(&engine, &host, item)), + before_values, + "initial selector precondition: {selector}" + ); + let before = retained_primary_style_for_test(&engine, &host, old_items[changed]).unwrap(); + let added = host.create_element("div"); + assert!(host.set_attribute(added, "class", "item tail")); + insert_through_consumed_fragment( + &mut host, + &mut engine, + list, + added, + insert_before_second.then_some(old_items[1]), + ); + engine.drain_pending_style_invalidations_for_document_for_test(&host, document); + assert_eq!( + [old_items[0], old_items[1], added].map(|item| read_color(&engine, &host, item)), + after_values, + "the final insertion must preserve real selector invalidation: {selector}" + ); + let after = retained_primary_style_for_test(&engine, &host, old_items[changed]).unwrap(); + assert!( + !ServoArc::ptr_eq(&before, &after), + "an existing element whose computed value changed must be republished: {selector}" + ); + } +}