From f14f1fa44062f2272437bb73860dce0d6e1e38e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 20 Oct 2022 08:39:18 +0000 Subject: [PATCH] style: fix invalidation of sibling combinators in different slots This extends the code to deal with sibling invalidation to handle the case where the flat tree doesn't match the DOM tree. In the test-case for example, dom is: * details * summary id=a * summary But flat tree is: * details * slot * summary id=a * slot * summary Differential Revision: https://phabricator.services.mozilla.com/D159150 --- components/style/dom.rs | 12 +++++ components/style/dom_apis.rs | 4 ++ .../invalidation/element/document_state.rs | 4 ++ .../style/invalidation/element/invalidator.rs | 6 ++- .../element/state_and_attributes.rs | 45 +++++++++++++++---- components/style/traversal.rs | 6 ++- 6 files changed, 66 insertions(+), 11 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 425b61575d2..c82a438bcc8 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -211,6 +211,18 @@ pub trait TNode: Sized + Copy + Clone + Debug + NodeInfo + PartialEq { self.parent_node().and_then(|n| n.as_element()) } + /// Get this node's parent element, or shadow host if it's a shadow root. + fn parent_element_or_host(&self) -> Option { + let parent = self.parent_node()?; + if let Some(e) = parent.as_element() { + return Some(e); + } + if let Some(root) = parent.as_shadow_root() { + return Some(root.host()); + } + None + } + /// Converts self into an `OpaqueNode`. fn opaque(&self) -> OpaqueNode; diff --git a/components/style/dom_apis.rs b/components/style/dom_apis.rs index e6f1cbf565c..a2bb4c4ecbe 100644 --- a/components/style/dom_apis.rs +++ b/components/style/dom_apis.rs @@ -212,6 +212,10 @@ where Q::append_element(self.results, e); } + fn invalidated_sibling(&mut self, e: E, _of: E) { + Q::append_element(self.results, e); + } + fn recursion_limit_exceeded(&mut self, _e: E) {} fn invalidated_descendants(&mut self, _e: E, _child: E) {} } diff --git a/components/style/invalidation/element/document_state.rs b/components/style/invalidation/element/document_state.rs index 20df408db48..f79b70c6cc3 100644 --- a/components/style/invalidation/element/document_state.rs +++ b/components/style/invalidation/element/document_state.rs @@ -130,4 +130,8 @@ where fn invalidated_self(&mut self, element: E) { state_and_attributes::invalidated_self(element); } + + fn invalidated_sibling(&mut self, sibling: E, of: E) { + state_and_attributes::invalidated_sibling(sibling, of); + } } diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 79131a8008f..ac8f9ed5f81 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -80,6 +80,10 @@ where /// Executes an action when `Self` is invalidated. fn invalidated_self(&mut self, element: E); + /// Executes an action when `sibling` is invalidated as a sibling of + /// `of`. + fn invalidated_sibling(&mut self, sibling: E, of: E); + /// Executes an action when any descendant of `Self` is invalidated. fn invalidated_descendants(&mut self, element: E, child: E); } @@ -397,7 +401,7 @@ where ); if invalidated_sibling { - sibling_invalidator.processor.invalidated_self(sibling); + sibling_invalidator.processor.invalidated_sibling(sibling, self.element); } any_invalidated |= invalidated_sibling; diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index 1726fb21a62..5a462c6942c 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -7,7 +7,7 @@ use crate::context::SharedStyleContext; use crate::data::ElementData; -use crate::dom::TElement; +use crate::dom::{TElement, TNode}; use crate::invalidation::element::element_wrapper::{ElementSnapshot, ElementWrapper}; use crate::invalidation::element::invalidation_map::*; use crate::invalidation::element::invalidator::{DescendantInvalidationLists, InvalidationVector}; @@ -118,14 +118,10 @@ pub fn should_process_descendants(data: &ElementData) -> bool { } /// Propagates the bits after invalidating a descendant child. -pub fn invalidated_descendants(element: E, child: E) +pub fn propagate_dirty_bit_up_to(ancestor: E, child: E) where E: TElement, { - if !child.has_data() { - return; - } - // The child may not be a flattened tree child of the current element, // but may be arbitrarily deep. // @@ -136,10 +132,22 @@ where unsafe { parent.set_dirty_descendants() }; current = parent.traversal_parent(); - if parent == element { - break; + if parent == ancestor { + return; } } + debug_assert!(false, "Should've found {:?} as an ancestor of {:?}", ancestor, child); +} + +/// Propagates the bits after invalidating a descendant child, if needed. +pub fn invalidated_descendants(element: E, child: E) +where + E: TElement, +{ + if !child.has_data() { + return; + } + propagate_dirty_bit_up_to(element, child) } /// Sets the appropriate restyle hint after invalidating the style of a given @@ -153,6 +161,22 @@ where } } +/// Sets the appropriate hint after invalidating the style of a sibling. +pub fn invalidated_sibling(element: E, of: E) +where + E: TElement, +{ + debug_assert_eq!(element.as_node().parent_node(), of.as_node().parent_node(), "Should be siblings"); + invalidated_self(element); + if element.traversal_parent() != of.traversal_parent() { + let parent = element.as_node().parent_element_or_host(); + debug_assert!(parent.is_some(), "How can we have siblings without parent nodes?"); + if let Some(e) = parent { + propagate_dirty_bit_up_to(e, element) + } + } +} + impl<'a, 'b: 'a, E: 'a> InvalidationProcessor<'a, E> for StateAndAttrInvalidationProcessor<'a, 'b, E> where @@ -366,6 +390,11 @@ where debug_assert_ne!(element, self.element); invalidated_self(element); } + + fn invalidated_sibling(&mut self, element: E, of: E) { + debug_assert_ne!(element, self.element); + invalidated_sibling(element, of); + } } impl<'a, 'b, 'selectors, E> Collector<'a, 'b, 'selectors, E> diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 631cc31eb02..3ae066b2fef 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -157,6 +157,8 @@ pub trait DomTraversal: Sync { /// such, we have a pre-traversal step to handle that part and determine whether /// a full traversal is needed. fn pre_traverse(root: E, shared_context: &SharedStyleContext) -> PreTraverseToken { + use crate::invalidation::element::state_and_attributes::propagate_dirty_bit_up_to; + let traversal_flags = shared_context.traversal_flags; let mut data = root.mutate_data(); @@ -174,11 +176,11 @@ pub trait DomTraversal: Sync { ); if invalidation_result.has_invalidated_siblings() { - let actual_root = root.traversal_parent().expect( + let actual_root = root.as_node().parent_element_or_host().expect( "How in the world can you invalidate \ siblings without a parent?", ); - unsafe { actual_root.set_dirty_descendants() } + propagate_dirty_bit_up_to(actual_root, root); return PreTraverseToken(Some(actual_root)); } }