diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs index 1191edbff6..93f16fe133 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs @@ -1,5 +1,7 @@ use super::*; +mod child_list; + #[test] fn element_current_css_zoom_observes_fresh_effective_style_for_rendered_boxes() { let mut vm = new_parsed_test_vm( diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/child_list.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/child_list.rs new file mode 100644 index 0000000000..b5ba444851 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/child_list.rs @@ -0,0 +1,203 @@ +use super::*; + +#[test] +fn removed_source_with_moved_old_neighbor_invalidates_remaining_target() { + let mut vm = new_parsed_test_vm( + "https://moved-old-neighbor.test/", + r#"
ABC
"#, + ); + let result = vm + .eval( + r#"(() => { + const oldParent = document.getElementById('old'); + const newParent = document.getElementById('new'); + const held = getComputedStyle(document.getElementById('target')); + const values = [held.color]; + document.getElementById('source').remove(); + newParent.appendChild(document.getElementById('neighbor')); + values.push(oldParent.innerText, newParent.innerText, held.color); + return values.join('|'); + })()"#, + ) + .expect("moving an old neighbor must not hide the remaining affected siblings"); + assert_eq!(result, "rgb(255, 0, 0)|C|B|rgb(0, 0, 255)"); +} + +#[test] +fn sibling_move_between_shadow_roots_invalidates_both_style_scopes() { + let mut vm = new_parsed_test_vm( + "https://cross-shadow-sibling-move.test/", + "
", + ); + let result = vm + .eval( + r#"(() => { + const oldRoot = document.getElementById('old').attachShadow({mode:'open'}); + const newRoot = document.getElementById('new').attachShadow({mode:'open'}); + const css = ''; + oldRoot.innerHTML = css + 'AB'; + newRoot.innerHTML = css + 'C'; + const oldTarget = oldRoot.querySelector('.target'); + const newTarget = newRoot.querySelector('.target'); + const oldStyle = getComputedStyle(oldTarget); + const newStyle = getComputedStyle(newTarget); + const values = [oldStyle.color, newStyle.color]; + newRoot.insertBefore(oldRoot.querySelector('.source'), newTarget); + values.push(oldTarget.innerText, newTarget.innerText, oldStyle.color, newStyle.color); + return values.join('|'); + })()"#, + ) + .expect("source-scope fallback must include the old and new shadow trees"); + assert_eq!( + result, + "rgb(255, 0, 0)|rgb(0, 0, 255)|B|C|rgb(0, 0, 255)|rgb(255, 0, 0)" + ); +} + +#[test] +fn detached_then_reinserted_sibling_does_not_reuse_queued_old_links() { + let mut vm = new_parsed_test_vm( + "https://two-step-sibling-move.test/", + r#"
AB
"#, + ); + let result = vm + .eval( + r#"(() => { + const container = document.getElementById('container'); + const source = document.getElementById('source'); + const held = getComputedStyle(document.getElementById('target')); + const values = [held.color]; + source.remove(); + container.appendChild(source); + values.push(container.innerText, held.color); + return values.join('|'); + })()"#, + ) + .expect("separate remove and insert calls must validate topology together at drain time"); + assert_eq!(result, "rgb(255, 0, 0)|BA|rgb(0, 0, 255)"); +} + +#[test] +fn inner_text_and_held_style_update_after_sibling_reorder() { + let mut vm = new_parsed_test_vm( + "https://sibling-reorder.test/", + r#"
AB
"#, + ); + let result = vm + .eval( + r#"(() => { + const container = document.getElementById('container'); + const source = document.getElementById('source'); + const held = getComputedStyle(document.getElementById('target')); + const values = [held.color]; + container.appendChild(source); + values.push(container.innerText, held.color); + container.prepend(source); + values.push(container.innerText, held.color); + return values.join('|'); + })()"#, + ) + .expect("innerText must finish after a sibling move and preserve held style liveness"); + assert_eq!(result, "rgb(255, 0, 0)|BA|rgb(0, 0, 255)|AB|rgb(255, 0, 0)"); +} + +#[test] +fn relative_selector_updates_after_backward_sibling_reorder() { + let mut vm = new_parsed_test_vm( + "https://backward-sibling-reorder.test/", + r#"
AB
"#, + ); + let result = vm + .eval( + r#"(() => { + const container = document.getElementById('container'); + const target = document.getElementById('target'); + const held = getComputedStyle(document.getElementById('source')); + const values = [held.color]; + container.prepend(target); + values.push(container.innerText, held.color); + container.appendChild(target); + values.push(container.innerText, held.color); + return values.join('|'); + })()"#, + ) + .expect("relative invalidation must finish after reversing a sibling relation"); + assert_eq!(result, "rgb(255, 0, 0)|BA|rgb(0, 0, 255)|AB|rgb(255, 0, 0)"); +} + +#[test] +fn sibling_move_between_parents_invalidates_old_and_new_targets() { + let mut vm = new_parsed_test_vm( + "https://cross-parent-sibling-move.test/", + r#" +
AB
+
C
"#, + ); + let result = vm + .eval( + r#"(() => { + const oldParent = document.getElementById('old'); + const newParent = document.getElementById('new'); + const source = document.getElementById('source'); + const oldStyle = getComputedStyle(document.getElementById('old-target')); + const newStyle = getComputedStyle(document.getElementById('new-target')); + const values = [oldStyle.color, newStyle.color]; + newParent.prepend(source); + values.push(oldParent.innerText, newParent.innerText, oldStyle.color, newStyle.color); + return values.join('|'); + })()"#, + ) + .expect("moving a source must invalidate both sibling regions"); + assert_eq!( + result, + "rgb(255, 0, 0)|rgb(0, 0, 255)|B|AC|rgb(0, 0, 255)|rgb(255, 0, 0)" + ); +} + +#[test] +fn batched_sibling_reorders_update_adjacent_and_general_sibling_styles() { + let mut vm = new_parsed_test_vm( + "https://batched-sibling-reorder.test/", + r#"
ABC
"#, + ); + let result = vm + .eval( + r#"(() => { + const container = document.getElementById('container'); + const source = document.getElementById('source'); + const other = document.getElementById('other'); + const held = getComputedStyle(document.getElementById('target')); + const values = [held.color, held.backgroundColor]; + container.appendChild(source); + container.prepend(other); + values.push(container.innerText, held.color, held.backgroundColor); + container.prepend(source); + values.push(container.innerText, held.color, held.backgroundColor); + return values.join('|'); + })()"#, + ) + .expect("batched moves must not retain obsolete sibling edges or computed styles"); + assert_eq!( + result, + "rgb(255, 0, 0)|rgb(0, 0, 0)|CBA|rgb(0, 0, 255)|rgb(255, 255, 255)|ACB|rgb(255, 0, 0)|rgb(255, 255, 255)" + ); +} diff --git a/moli-renderer-v8/src/style_engine/eligibility.rs b/moli-renderer-v8/src/style_engine/eligibility.rs index 1d11769bfc..398552102f 100644 --- a/moli-renderer-v8/src/style_engine/eligibility.rs +++ b/moli-renderer-v8/src/style_engine/eligibility.rs @@ -1,6 +1,97 @@ +use std::collections::HashSet; + use moli_selector::stylo_attribute_change_can_use_retained_invalidator; -use super::StyleAttributeImpact; +use crate::{ + document_runtime::DomHandle, + dom::native::{DomHost, Node}, +}; + +use super::{StyleAttributeImpact, StyleMutationEffect}; + +/// Retained sibling traversal can restore one removed element's links, not a +/// historical tree. Only use it while each recorded splice still agrees with +/// the live tree, excluding subsequent insertions validated in reverse order. +/// Check at drain time: later mutations can move either the changed nodes or +/// their old neighbors, including within the same parent. +pub(super) fn child_list_effects_have_current_topology( + host: &DomHost, + effects: &[StyleMutationEffect], +) -> bool { + let mut later_insertions = HashSet::new(); + effects.iter().rev().all(|effect| { + let StyleMutationEffect::ChildList { + parent, + added_nodes, + removed_nodes, + removed_element_snapshots, + previous_sibling, + next_sibling, + } = effect + else { + return true; + }; + if removed_nodes.iter().any(|&root| { + host.node(root) + .is_none_or(|node| node.parent_node().is_some()) + }) { + return false; + } + // A removed root can stay detached while one of its snapshotted + // descendants is moved back into the live tree. + if removed_element_snapshots.iter().any(|snapshot| { + let mut current = Some(snapshot.handle()); + while let Some(handle) = current { + if removed_nodes.contains(&handle) { + return false; + } + current = host.node(handle).and_then(Node::parent_node); + } + true + }) { + return false; + } + if previous_sibling + .iter() + .chain(next_sibling) + .any(|&sibling| host.node(sibling).and_then(Node::parent_node) != Some(*parent)) + { + return false; + } + let Some(parent_node) = host.node(*parent) else { + return false; + }; + let mut current = match previous_sibling { + Some(previous) => host.next_sibling(*previous), + None => parent_node.first_child(), + }; + current = skip_later_insertions(host, current, &later_insertions); + for &added in added_nodes { + if current != Some(added) { + return false; + } + current = skip_later_insertions(host, host.next_sibling(added), &later_insertions); + } + if current != *next_sibling { + return false; + } + // Pure insertion batches are safe: every new subtree is also queried. + // Do not mistake an appended suffix for a rearranged old sibling chain. + later_insertions.extend(added_nodes.iter().copied()); + true + }) +} + +fn skip_later_insertions( + host: &DomHost, + mut current: Option, + later_insertions: &HashSet, +) -> Option { + while let Some(handle) = current.filter(|handle| later_insertions.contains(handle)) { + current = host.next_sibling(handle); + } + current +} pub(super) fn attribute_effect_can_use_retained_stylo_invalidator(name: &str) -> bool { stylo_attribute_change_can_use_retained_invalidator( diff --git a/moli-renderer-v8/src/style_engine/mutation_effect.rs b/moli-renderer-v8/src/style_engine/mutation_effect.rs index 87a0e056c5..ea25c3be90 100644 --- a/moli-renderer-v8/src/style_engine/mutation_effect.rs +++ b/moli-renderer-v8/src/style_engine/mutation_effect.rs @@ -331,6 +331,7 @@ pub(super) fn style_mutation_effects_are_child_list_structural( effect, StyleMutationEffect::ChildList { .. } | StyleMutationEffect::ConnectedSubtrees { .. } + | StyleMutationEffect::DisconnectedSubtrees { .. } | StyleMutationEffect::SlotAssignment { .. } ) }) diff --git a/moli-renderer-v8/src/style_engine/query.rs b/moli-renderer-v8/src/style_engine/query.rs index 39198848ec..546747f2c2 100644 --- a/moli-renderer-v8/src/style_engine/query.rs +++ b/moli-renderer-v8/src/style_engine/query.rs @@ -211,6 +211,7 @@ fn retained_stylo_invalidation_queries_for_child_list_mutations( if matches!( effect, StyleMutationEffect::ConnectedSubtrees { .. } + | StyleMutationEffect::DisconnectedSubtrees { .. } | 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 2813fe7cd3..43ff0e2a79 100644 --- a/moli-renderer-v8/src/style_engine/runtime_invalidation.rs +++ b/moli-renderer-v8/src/style_engine/runtime_invalidation.rs @@ -17,6 +17,7 @@ use super::{ MoliStyleEngine, StyleMutationEffect, StyleViewport, cause::PendingStyleInvalidationCause, document_world::DocumentStyleWorld, + eligibility::child_list_effects_have_current_topology, mutation_effect::detached_style_subtree_roots_for_mutations, schedule::queue_style_invalidation_for_scope, scope::{ @@ -219,8 +220,10 @@ impl MoliStyleEngine { emulated_media: &EmulatedMediaOverrides, viewport: StyleViewport, ) { - if effects.len() <= IMMEDIATE_STRUCTURAL_MUTATION_EFFECT_LIMIT { - self.invalidate_detached_style_subtrees_for_mutations(host, effects); + 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) + { self.queue_style_invalidation_for_mutations( document, world, @@ -250,6 +253,8 @@ impl MoliStyleEngine { emulated_media: &EmulatedMediaOverrides, viewport: StyleViewport, ) { + // Use all mutation handles to cover both the old and new tree scopes. + // Cause-local fallback roots are not enough when old neighbors moved. let source_scope = source_scope_for_mutations(host, effects); let cause = PendingStyleInvalidationCause::Mutation(Vec::new()); self.queue_style_invalidation_scope( diff --git a/moli-renderer-v8/src/style_engine/tests/eligibility.rs b/moli-renderer-v8/src/style_engine/tests/eligibility.rs new file mode 100644 index 0000000000..6ba656432d --- /dev/null +++ b/moli-renderer-v8/src/style_engine/tests/eligibility.rs @@ -0,0 +1,101 @@ +use super::super::eligibility::child_list_effects_have_current_topology; +use super::*; +use crate::dom::native::Node; + +fn sibling_fixture() -> (DomHost, DomHandle, [DomHandle; 3]) { + let mut host = test_host(); + let parent = host.create_element("div"); + assert!(host.append_child(host.document_handle(), parent)); + let children = std::array::from_fn(|_| { + let child = host.create_element("span"); + assert!(host.append_child(parent, child)); + child + }); + (host, parent, children) +} + +#[test] +fn stable_single_insertions_and_removals_keep_retained_topology() { + for index in 0..3 { + let (mut host, parent, children) = sibling_fixture(); + let removed = host.remove_child_effects(parent, children[index]); + let removed = StyleMutationEffect::from_dom_mutation_effects(&host, &removed); + assert!(child_list_effects_have_current_topology(&host, &removed)); + + let inserted = + host.insert_before_effects(parent, children[index], children.get(index + 1).copied()); + let inserted = StyleMutationEffect::from_dom_mutation_effects(&host, &inserted); + assert!(child_list_effects_have_current_topology(&host, &inserted)); + assert!( + !child_list_effects_have_current_topology(&host, &removed), + "reinserting the removed node invalidates the earlier snapshot" + ); + } +} + +#[test] +fn consecutive_insertions_keep_retained_topology() { + let (mut host, parent, [_, b, _]) = sibling_fixture(); + let mut effects = Vec::new(); + for reference in [None, None, Some(b), Some(b)] { + let added = host.create_element("span"); + let inserted = host.insert_before_effects(parent, added, reference); + effects.extend(StyleMutationEffect::from_dom_mutation_effects( + &host, &inserted, + )); + } + assert!(child_list_effects_have_current_topology(&host, &effects)); +} + +#[test] +fn removal_snapshot_rejects_old_neighbor_moved_to_another_parent() { + let (mut host, parent, [a, b, _]) = sibling_fixture(); + let removed = host.remove_child_effects(parent, a); + let removed = StyleMutationEffect::from_dom_mutation_effects(&host, &removed); + let other = host.create_element("div"); + assert!(host.append_child(host.document_handle(), other)); + assert!(host.append_child(other, b)); + + assert_eq!(host.node(a).and_then(Node::parent_node), None); + assert!(!child_list_effects_have_current_topology(&host, &removed)); +} + +#[test] +fn removal_snapshot_rejects_old_neighbor_reordered_within_parent() { + let (mut host, parent, [a, b, _]) = sibling_fixture(); + let removed = host.remove_child_effects(parent, b); + let removed = StyleMutationEffect::from_dom_mutation_effects(&host, &removed); + assert!(host.append_child(parent, a)); + + assert_eq!(host.node(b).and_then(Node::parent_node), None); + assert_eq!(host.node(a).and_then(Node::parent_node), Some(parent)); + assert!(!child_list_effects_have_current_topology(&host, &removed)); +} + +#[test] +fn removal_snapshot_rejects_reattached_descendant_of_detached_root() { + let (mut host, parent, [_, b, c]) = sibling_fixture(); + let descendant = host.create_element("span"); + assert!(host.append_child(b, descendant)); + let removed = host.remove_child_effects(parent, b); + let removed = StyleMutationEffect::from_dom_mutation_effects(&host, &removed); + assert!(child_list_effects_have_current_topology(&host, &removed)); + assert!(host.append_child(c, descendant)); + + assert_eq!(host.node(b).and_then(Node::parent_node), None); + assert!(!child_list_effects_have_current_topology(&host, &removed)); +} + +#[test] +fn insertion_snapshot_rejects_changed_range_at_consumption_time() { + let (mut host, parent, [a, _, _]) = sibling_fixture(); + let added = host.create_element("span"); + let inserted = host.insert_before_effects(parent, added, Some(a)); + let inserted = StyleMutationEffect::from_dom_mutation_effects(&host, &inserted); + assert!(child_list_effects_have_current_topology(&host, &inserted)); + assert!(host.append_child(parent, added)); + + // Check the old record alone, without relying on the later mutation being + // in the same viewport/media group. + assert!(!child_list_effects_have_current_topology(&host, &inserted)); +} diff --git a/moli-renderer-v8/src/style_engine/tests/invalidator.rs b/moli-renderer-v8/src/style_engine/tests/invalidator.rs index 7f8855cfd3..3a42b2ac8b 100644 --- a/moli-renderer-v8/src/style_engine/tests/invalidator.rs +++ b/moli-renderer-v8/src/style_engine/tests/invalidator.rs @@ -4219,7 +4219,7 @@ fn retained_stylo_invalidator_narrows_child_list_inserted_sibling_invalidation() } #[test] -fn retained_stylo_invalidator_keeps_ua_structural_boundary_when_author_source_has_no_target() { +fn reordered_summary_invalidates_ua_styles_when_author_source_has_no_target() { let mut host = test_host(); let document = host.document_handle(); let details = host.create_element("details"); @@ -4306,8 +4306,19 @@ fn retained_stylo_invalidator_keeps_ua_structural_boundary_when_author_source_ha ); engine.drain_pending_style_invalidations_for_document_for_test(&host, document); - assert!(!engine.computed_style_cache_contains_handle_for_document_for_test(document, first)); - assert!(!engine.computed_style_cache_contains_handle_for_document_for_test(document, second)); + // Reordering uses lazy source-scope invalidation, not eager eviction of + // exact sibling targets. Both cached styles must be stale before a read. + let world = engine.world_for_document(document); + let invalidation = &world.document_state.lazy_invalidation_roots; + for handle in [first, second] { + assert!( + invalidation + .validation_path(&host, document, handle) + .iter() + .any(|entry| entry.element == handle + && !invalidation.element_is_current(handle, entry.required_generation)) + ); + } assert!(engine.computed_style_cache_contains_handle_for_document_for_test(document, unrelated)); assert_eq!( engine.computed_style_property_value( diff --git a/moli-renderer-v8/src/style_engine/tests/mod.rs b/moli-renderer-v8/src/style_engine/tests/mod.rs index 55c550530a..f76d706fa7 100644 --- a/moli-renderer-v8/src/style_engine/tests/mod.rs +++ b/moli-renderer-v8/src/style_engine/tests/mod.rs @@ -524,6 +524,7 @@ fn collect_source_invalidation_roots_for_test( mod char_child; mod dependency; +mod eligibility; mod invalidator; mod lifecycle; mod outcome;