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 f4a1904620..1910065fd3 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 @@ -2528,8 +2528,8 @@ fn style_mutation_effects_affect_layout_metric(effects: &[StyleMutationEffect]) StyleMutationEffect::Attribute { name, .. } => { StyleAttributeImpact::for_attribute_name(name).affects_layout_metric() } - StyleMutationEffect::ConnectedSubtree { .. } - | StyleMutationEffect::DisconnectedSubtree { .. } + StyleMutationEffect::ConnectedSubtrees { .. } + | StyleMutationEffect::DisconnectedSubtrees { .. } | StyleMutationEffect::SlotAssignment { .. } | StyleMutationEffect::CharacterData { .. } | StyleMutationEffect::ChildList { .. } => true, diff --git a/moli-renderer-v8/src/native_bridge/document/detached_install/accessors/iframe_content_cache.rs b/moli-renderer-v8/src/native_bridge/document/detached_install/accessors/iframe_content_cache.rs index 7d4f1893df..2ff561f7e5 100644 --- a/moli-renderer-v8/src/native_bridge/document/detached_install/accessors/iframe_content_cache.rs +++ b/moli-renderer-v8/src/native_bridge/document/detached_install/accessors/iframe_content_cache.rs @@ -39,8 +39,8 @@ pub(in crate::native_bridge) fn clear_detached_iframe_cached_context<'s>( detached_iframe_current_content_document_handle(scope, runtime_ptr, iframe_handle) { let effects = [ - crate::style_engine::StyleMutationEffect::DisconnectedSubtree { - root: document_handle, + crate::style_engine::StyleMutationEffect::DisconnectedSubtrees { + roots: vec![document_handle].into(), }, ]; unsafe { &mut *runtime_ptr }.note_style_mutation_effects(&effects); diff --git a/moli-renderer-v8/src/style_engine/cause.rs b/moli-renderer-v8/src/style_engine/cause.rs index bb19234f3f..b6bb7a7775 100644 --- a/moli-renderer-v8/src/style_engine/cause.rs +++ b/moli-renderer-v8/src/style_engine/cause.rs @@ -191,20 +191,28 @@ fn cause_default_fallback_roots( let PendingStyleInvalidationCause::Mutation(effects) = cause else { return IndexSet::new(); }; - stylo_runtime_fallback_roots_for_mutation_inputs( + let mut roots = IndexSet::new(); + for effect in effects { + if let StyleMutationEffect::ConnectedSubtrees { + roots: connected_roots, + } = effect + { + roots.extend(connected_roots.iter().copied()); + } + } + roots.extend(stylo_runtime_fallback_roots_for_mutation_inputs( host, effects .iter() - .map(runtime_fallback_root_input_for_mutation_effect), - ) - .into_iter() - .collect() + .filter_map(runtime_fallback_root_input_for_mutation_effect), + )); + roots } fn runtime_fallback_root_input_for_mutation_effect( effect: &StyleMutationEffect, -) -> StyloRuntimeFallbackRootInput<'_> { - match effect { +) -> Option> { + Some(match effect { StyleMutationEffect::Attribute { element, name, .. } => { StyloRuntimeFallbackRootInput::Attribute { element: *element, @@ -227,12 +235,52 @@ fn runtime_fallback_root_input_for_mutation_effect( .zip(assigned_nodes.as_ref()) .is_some(), }, - StyleMutationEffect::ConnectedSubtree { root } => { - StyloRuntimeFallbackRootInput::ConnectedSubtree { root: *root } + StyleMutationEffect::ConnectedSubtrees { roots } => { + StyloRuntimeFallbackRootInput::ConnectedSubtree { + root: *roots.first()?, + } } StyleMutationEffect::CharacterData { .. } - | StyleMutationEffect::DisconnectedSubtree { .. } => { + | StyleMutationEffect::DisconnectedSubtrees { .. } => { StyloRuntimeFallbackRootInput::OtherMutation } + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::dom::native::NativeDom; + + #[test] + fn batched_connected_subtrees_keep_every_runtime_fallback_root() { + let mut host = DomHost::from_dom(NativeDom::new_html( + url::Url::parse("https://example.test/").expect("valid test url"), + )); + let document = host.document_handle(); + let first = host.create_element("div"); + let second = host.create_element("span"); + assert!(host.append_child(document, first)); + assert!(host.append_child(document, second)); + let cause = PendingStyleInvalidationCause::Mutation(vec![ + StyleMutationEffect::ConnectedSubtrees { + roots: vec![first, second].into(), + }, + StyleMutationEffect::ChildList { + parent: document, + added_nodes: vec![first, second], + removed_nodes: Vec::new(), + removed_element_snapshots: Vec::new(), + previous_sibling: None, + next_sibling: None, + }, + ]); + + assert_eq!( + cause_default_fallback_roots(&host, &cause) + .into_iter() + .collect::>(), + [first, second] + ); } } diff --git a/moli-renderer-v8/src/style_engine/mutation_effect.rs b/moli-renderer-v8/src/style_engine/mutation_effect.rs index 0488bd261e..87a0e056c5 100644 --- a/moli-renderer-v8/src/style_engine/mutation_effect.rs +++ b/moli-renderer-v8/src/style_engine/mutation_effect.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use indexmap::IndexSet; use moli_selector::{ StyloElementDependencySnapshot as StyleElementDependencySnapshot, @@ -19,11 +21,11 @@ pub(crate) enum StyleMutationEffect { old_value: Option, new_value: Option, }, - ConnectedSubtree { - root: DomHandle, + ConnectedSubtrees { + roots: Arc<[DomHandle]>, }, - DisconnectedSubtree { - root: DomHandle, + DisconnectedSubtrees { + roots: Arc<[DomHandle]>, }, SlotAssignment { slot: DomHandle, @@ -70,14 +72,48 @@ impl StyleMutationEffect { effects: &DomMutationEffects, ) -> Vec { let mut style_effects = IndexSet::new(); - for &root in effects.tree().connected_roots() { - style_effects.insert(Self::ConnectedSubtree { root }); + let connected_roots = effects.tree().connected_roots(); + #[cfg(debug_assertions)] + { + let connected_root_set = connected_roots + .iter() + .copied() + .collect::>(); + debug_assert!( + effects + .scripts() + .connected_roots() + .iter() + .all(|root| connected_root_set.contains(root)), + "script connection roots must also be tree connection roots" + ); } - for &root in effects.scripts().connected_roots() { - style_effects.insert(Self::ConnectedSubtree { root }); + if !connected_roots.is_empty() { + style_effects.insert(Self::ConnectedSubtrees { + roots: shared_tree_roots( + connected_roots, + effects + .style() + .child_list_mutations() + .iter() + .map(|mutation| (mutation.added_nodes(), mutation.shared_added_nodes())), + ), + }); } - for &root in effects.tree().disconnected_roots() { - style_effects.insert(Self::DisconnectedSubtree { root }); + let disconnected_roots = effects.tree().disconnected_roots(); + if !disconnected_roots.is_empty() { + style_effects.insert(Self::DisconnectedSubtrees { + roots: shared_tree_roots( + disconnected_roots, + effects + .style() + .child_list_mutations() + .iter() + .map(|mutation| { + (mutation.removed_nodes(), mutation.shared_removed_nodes()) + }), + ), + }); } let detailed_slot_assignment_slots = effects .slots() @@ -152,6 +188,16 @@ impl StyleMutationEffect { } } +fn shared_tree_roots<'a>( + roots: &[DomHandle], + child_list_roots: impl Iterator)>, +) -> Arc<[DomHandle]> { + child_list_roots + .filter_map(|(candidate, shared)| (candidate == roots).then_some(shared)) + .next() + .unwrap_or_else(|| Arc::from(roots)) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum StyleAttributeImpact { None, @@ -218,14 +264,16 @@ pub(super) fn detached_style_subtree_roots_for_mutations( let mut roots = IndexSet::new(); for effect in effects { match effect { - StyleMutationEffect::DisconnectedSubtree { root } => { - roots.insert(*root); + StyleMutationEffect::DisconnectedSubtrees { + roots: disconnected_roots, + } => { + roots.extend(disconnected_roots.iter().copied()); } StyleMutationEffect::ChildList { removed_nodes, .. } => { roots.extend(removed_nodes.iter().copied()); } StyleMutationEffect::Attribute { .. } - | StyleMutationEffect::ConnectedSubtree { .. } + | StyleMutationEffect::ConnectedSubtrees { .. } | StyleMutationEffect::SlotAssignment { .. } | StyleMutationEffect::CharacterData { .. } => {} } @@ -282,7 +330,7 @@ pub(super) fn style_mutation_effects_are_child_list_structural( matches!( effect, StyleMutationEffect::ChildList { .. } - | StyleMutationEffect::ConnectedSubtree { .. } + | StyleMutationEffect::ConnectedSubtrees { .. } | StyleMutationEffect::SlotAssignment { .. } ) }) @@ -383,3 +431,49 @@ fn changed_identifier( .map(str::to_owned), ) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::dom::native::NativeDom; + + fn test_host() -> DomHost { + DomHost::from_dom(NativeDom::new_html( + url::Url::parse("https://example.test/").expect("valid test url"), + )) + } + + #[test] + fn fragment_connection_roots_share_one_batched_child_list_payload() { + let mut host = test_host(); + let document = host.document_handle(); + let parent = host.create_element("main"); + let fragment = host.create_document_fragment(); + let first = host.create_element("div"); + let second = host.create_element("span"); + assert!(host.append_child(document, parent)); + assert!(host.append_child(fragment, first)); + assert!(host.append_child(fragment, second)); + + let effects = host.append_child_effects(parent, fragment); + let child_list = effects + .style() + .child_list_mutations() + .iter() + .find(|mutation| mutation.target() == parent) + .expect("fragment insertion should record one child-list mutation"); + let shared_added_nodes = child_list.shared_added_nodes(); + let style_effects = StyleMutationEffect::from_dom_mutation_effects(&host, &effects); + let connected_batches = style_effects + .iter() + .filter_map(|effect| match effect { + StyleMutationEffect::ConnectedSubtrees { roots } => Some(roots), + _ => None, + }) + .collect::>(); + + assert_eq!(connected_batches.len(), 1); + assert_eq!(connected_batches[0].as_ref(), [first, second]); + assert!(Arc::ptr_eq(connected_batches[0], &shared_added_nodes)); + } +} diff --git a/moli-renderer-v8/src/style_engine/query.rs b/moli-renderer-v8/src/style_engine/query.rs index 9bfa7c825b..39198848ec 100644 --- a/moli-renderer-v8/src/style_engine/query.rs +++ b/moli-renderer-v8/src/style_engine/query.rs @@ -210,7 +210,7 @@ fn retained_stylo_invalidation_queries_for_child_list_mutations( if !matches!(effect, StyleMutationEffect::ChildList { .. }) { if matches!( effect, - StyleMutationEffect::ConnectedSubtree { .. } + StyleMutationEffect::ConnectedSubtrees { .. } | StyleMutationEffect::SlotAssignment { .. } ) { continue; diff --git a/moli-renderer-v8/src/style_engine/runtime_invalidation.rs b/moli-renderer-v8/src/style_engine/runtime_invalidation.rs index 00e3f77abf..2813fe7cd3 100644 --- a/moli-renderer-v8/src/style_engine/runtime_invalidation.rs +++ b/moli-renderer-v8/src/style_engine/runtime_invalidation.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::{collections::HashMap, sync::Arc}; use dom::ElementState as StyloElementState; use moli_selector::StyloStyleSourceScope as StyleSourceScope; @@ -158,7 +158,7 @@ impl MoliStyleEngine { let pending_effects_started = profile_enabled.then(std::time::Instant::now); let pending_effects = effects .iter() - .filter(|effect| !matches!(effect, StyleMutationEffect::DisconnectedSubtree { .. })) + .filter(|effect| !matches!(effect, StyleMutationEffect::DisconnectedSubtrees { .. })) .cloned() .collect(); let pending_effects_us = pending_effects_started @@ -608,34 +608,92 @@ fn style_mutation_effects_by_owner_document( ) -> Vec<(DomHandle, Vec)> { let mut groups = Vec::<(DomHandle, Vec)>::new(); for effect in effects { + match effect { + StyleMutationEffect::ConnectedSubtrees { roots } => { + push_subtree_effects_by_owner_document(host, &mut groups, roots, |roots| { + StyleMutationEffect::ConnectedSubtrees { roots } + }); + continue; + } + StyleMutationEffect::DisconnectedSubtrees { roots } => { + push_subtree_effects_by_owner_document(host, &mut groups, roots, |roots| { + StyleMutationEffect::DisconnectedSubtrees { roots } + }); + continue; + } + _ => {} + } let Some(document) = owner_document_for_mutation_effect(host, effect) else { continue; }; - if let Some((_, group_effects)) = groups - .iter_mut() - .find(|(group_document, _)| *group_document == document) - { - group_effects.push(effect.clone()); - } else { - groups.push((document, vec![effect.clone()])); - } + push_document_effect(&mut groups, document, effect.clone()); } groups } +fn push_subtree_effects_by_owner_document( + host: &DomHost, + groups: &mut Vec<(DomHandle, Vec)>, + roots: &Arc<[DomHandle]>, + make_effect: impl Fn(Arc<[DomHandle]>) -> StyleMutationEffect, +) { + let Some((&first_root, remaining_roots)) = roots.split_first() else { + return; + }; + if let Some(document) = owner_document_for_handle(host, first_root) + && remaining_roots + .iter() + .all(|root| owner_document_for_handle(host, *root) == Some(document)) + { + push_document_effect(groups, document, make_effect(Arc::clone(roots))); + return; + } + + let mut document_roots = Vec::<(DomHandle, Vec)>::new(); + for &root in roots.iter() { + let Some(document) = owner_document_for_handle(host, root) else { + continue; + }; + if let Some((_, roots)) = document_roots + .iter_mut() + .find(|(candidate, _)| *candidate == document) + { + roots.push(root); + } else { + document_roots.push((document, vec![root])); + } + } + for (document, roots) in document_roots { + push_document_effect(groups, document, make_effect(roots.into())); + } +} + +fn push_document_effect( + groups: &mut Vec<(DomHandle, Vec)>, + document: DomHandle, + effect: StyleMutationEffect, +) { + if let Some((_, group_effects)) = groups + .iter_mut() + .find(|(group_document, _)| *group_document == document) + { + group_effects.push(effect); + } else { + groups.push((document, vec![effect])); + } +} + fn owner_document_for_mutation_effect( host: &DomHost, effect: &StyleMutationEffect, ) -> Option { match effect { StyleMutationEffect::Attribute { element, .. } => owner_document_for_handle(host, *element), - StyleMutationEffect::ConnectedSubtree { root } - | StyleMutationEffect::DisconnectedSubtree { root } - | StyleMutationEffect::CharacterData { node: root } => { - owner_document_for_handle(host, *root) - } + StyleMutationEffect::CharacterData { node } => owner_document_for_handle(host, *node), StyleMutationEffect::SlotAssignment { slot, .. } => owner_document_for_handle(host, *slot), StyleMutationEffect::ChildList { parent, .. } => owner_document_for_handle(host, *parent), + StyleMutationEffect::ConnectedSubtrees { .. } + | StyleMutationEffect::DisconnectedSubtrees { .. } => None, } } @@ -767,3 +825,41 @@ fn non_empty_vec(handles: Vec) -> Option> { Some(handles) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::dom::native::NativeDom; + + #[test] + fn batched_subtrees_are_split_by_owner_document() { + let mut host = DomHost::from_dom(NativeDom::new_html( + url::Url::parse("https://example.test/").expect("valid test url"), + )); + let document = host.document_handle(); + let detached_document = host.create_detached_html_document(); + let active_root = host.create_element("main"); + let detached_root = host.create_element("aside"); + assert!(host.append_child(document, active_root)); + assert!(host.append_child(detached_document, detached_root)); + let effects = [StyleMutationEffect::ConnectedSubtrees { + roots: vec![active_root, detached_root].into(), + }]; + + let groups = style_mutation_effects_by_owner_document(&host, &effects); + assert_eq!(groups.len(), 2); + for (expected_document, expected_root) in + [(document, active_root), (detached_document, detached_root)] + { + let (_, effects) = groups + .iter() + .find(|(candidate, _)| *candidate == expected_document) + .expect("each owner document should have a mutation group"); + assert!(matches!( + effects.as_slice(), + [StyleMutationEffect::ConnectedSubtrees { roots }] + if roots.as_ref() == [expected_root] + )); + } + } +} diff --git a/moli-renderer-v8/src/style_engine/scope.rs b/moli-renderer-v8/src/style_engine/scope.rs index 8831fb9f7b..a8014dc31e 100644 --- a/moli-renderer-v8/src/style_engine/scope.rs +++ b/moli-renderer-v8/src/style_engine/scope.rs @@ -17,7 +17,7 @@ pub(super) fn mutation_effects_have_source_scope(effects: &[StyleMutationEffect] !effects.is_empty() && effects .iter() - .any(|effect| !matches!(effect, StyleMutationEffect::DisconnectedSubtree { .. })) + .any(|effect| !matches!(effect, StyleMutationEffect::DisconnectedSubtrees { .. })) } pub(super) fn source_scope_for_element_state_change( @@ -52,10 +52,12 @@ pub(super) fn style_source_scope_for_mutation_effects( StyleMutationEffect::Attribute { element, .. } => { handles.push(*element); } - StyleMutationEffect::ConnectedSubtree { root } - | StyleMutationEffect::DisconnectedSubtree { root } - | StyleMutationEffect::CharacterData { node: root } => { - handles.push(*root); + StyleMutationEffect::ConnectedSubtrees { roots } + | StyleMutationEffect::DisconnectedSubtrees { roots } => { + handles.extend(roots.iter().copied()); + } + StyleMutationEffect::CharacterData { node } => { + handles.push(*node); } StyleMutationEffect::SlotAssignment { slot, .. } => { handles.push(*slot); diff --git a/moli-renderer-v8/src/style_engine/tests/dependency.rs b/moli-renderer-v8/src/style_engine/tests/dependency.rs index 83acbd76fc..e32ccc712b 100644 --- a/moli-renderer-v8/src/style_engine/tests/dependency.rs +++ b/moli-renderer-v8/src/style_engine/tests/dependency.rs @@ -718,8 +718,8 @@ fn pending_cause_default_roots_do_not_get_source_scope_reason() { let target_queries = PendingCauseFallback::from_cause( &host, - &PendingStyleInvalidationCause::Mutation(vec![StyleMutationEffect::ConnectedSubtree { - root: document, + &PendingStyleInvalidationCause::Mutation(vec![StyleMutationEffect::ConnectedSubtrees { + roots: vec![document].into(), }]), ) .target_queries_for_source_scope(&host, &source_scope); diff --git a/moli-renderer-v8/src/style_engine/tests/invalidator.rs b/moli-renderer-v8/src/style_engine/tests/invalidator.rs index 556411b69e..7f8855cfd3 100644 --- a/moli-renderer-v8/src/style_engine/tests/invalidator.rs +++ b/moli-renderer-v8/src/style_engine/tests/invalidator.rs @@ -67,12 +67,16 @@ fn mutation_source_scope_presence_uses_cheap_effect_classification() { "empty effects should not have a source scope", ), ( - vec![StyleMutationEffect::DisconnectedSubtree { root: detached }], + vec![StyleMutationEffect::DisconnectedSubtrees { + roots: vec![detached].into(), + }], false, "only disconnected subtree effects should not have a source scope", ), ( - vec![StyleMutationEffect::ConnectedSubtree { root: connected }], + vec![StyleMutationEffect::ConnectedSubtrees { + roots: vec![connected].into(), + }], true, "connected subtree effects should have a source scope", ), diff --git a/moli-renderer-v8/src/style_engine/tests/slotted_focus.rs b/moli-renderer-v8/src/style_engine/tests/slotted_focus.rs index 4380a27b0e..96275e37a6 100644 --- a/moli-renderer-v8/src/style_engine/tests/slotted_focus.rs +++ b/moli-renderer-v8/src/style_engine/tests/slotted_focus.rs @@ -423,7 +423,7 @@ fn shadow_slot_insertion_uses_retargeted_assignment_snapshots() { assigned_nodes: Some(_), .. } | StyleMutationEffect::ChildList { .. } - | StyleMutationEffect::ConnectedSubtree { .. } + | StyleMutationEffect::ConnectedSubtrees { .. } ) })); let media = crate::protocol_types::EmulatedMediaOverrides::default(); 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 0d02dd1a45..ba58766422 100644 --- a/moli-renderer-v8/src/style_engine/tests/subtree_cache.rs +++ b/moli-renderer-v8/src/style_engine/tests/subtree_cache.rs @@ -7214,8 +7214,8 @@ fn detached_subtree_invalidation_clears_only_affected_shadow_cascade_data() { let media = crate::protocol_types::EmulatedMediaOverrides::default(); engine.invalidate_for_mutations( &host, - &[StyleMutationEffect::DisconnectedSubtree { - root: first_shadow_host, + &[StyleMutationEffect::DisconnectedSubtrees { + roots: vec![first_shadow_host].into(), }], &media, ); diff --git a/moli-renderer-v8/src/style_engine/tests/subtree_context.rs b/moli-renderer-v8/src/style_engine/tests/subtree_context.rs index ad46a32088..fefee10d9e 100644 --- a/moli-renderer-v8/src/style_engine/tests/subtree_context.rs +++ b/moli-renderer-v8/src/style_engine/tests/subtree_context.rs @@ -103,7 +103,9 @@ fn standalone_subtree_context_invalidation_preserves_unrelated_cache_entries() { let generation = engine.computed_cache_generation_for_document_for_test(document); let rebuilds = engine.retained_style_system_rebuild_count_for_document_for_test(document); - let effects = [StyleMutationEffect::ConnectedSubtree { root: target }]; + let effects = [StyleMutationEffect::ConnectedSubtrees { + roots: vec![target].into(), + }]; let media = crate::protocol_types::EmulatedMediaOverrides::default(); engine.invalidate_for_mutations(&host, &effects, &media); @@ -169,7 +171,9 @@ fn pending_mutation_invalidations_drain_before_computed_style_read() { 2 ); - let effects = [StyleMutationEffect::ConnectedSubtree { root: target }]; + let effects = [StyleMutationEffect::ConnectedSubtrees { + roots: vec![target].into(), + }]; let media = crate::protocol_types::EmulatedMediaOverrides::default(); engine.invalidate_for_mutations(&host, &effects, &media); assert_eq!( @@ -245,13 +249,15 @@ fn pending_mutation_invalidations_keep_separate_work_items_until_drain() { let media = crate::protocol_types::EmulatedMediaOverrides::default(); engine.invalidate_for_mutations( &host, - &[StyleMutationEffect::ConnectedSubtree { root: first_target }], + &[StyleMutationEffect::ConnectedSubtrees { + roots: vec![first_target].into(), + }], &media, ); engine.invalidate_for_mutations( &host, - &[StyleMutationEffect::ConnectedSubtree { - root: second_target, + &[StyleMutationEffect::ConnectedSubtrees { + roots: vec![second_target].into(), }], &media, );