Bug 1931432: Fix up how :empty, :first-child and :last-child invalidation are skipped. r=firefox-style-system-reviewers,emilio

Previous go at optimizing in Bug 1896380 was not quite correct.
For example, given `.anchor:has(:first-child)`, when `.anchor`
has a new child inserted, we need to be able to invalidate (Which
we did not).
Thankfully, we can reason that the previously-optimized pseudo-
classes, i.e. :empty, :first-child, :last-child, can be trivially
rejected from activating the invalidation machinery by by examining
its descendants/siblings - e.g. `:has(:first-child)` is irrelevant
to elements we know aren't in the first sibling position.

Differential Revision: https://phabricator.services.mozilla.com/D229670

layout/base/RestyleManager.cpp

@@ -160,7 +160,7 @@ void RestyleManager::ContentAppended(nsIContent* aFirstNewContent) {
         PostRestyleEvent(element, RestyleHint::RestyleSubtree(),
                          nsChangeHint(0));
         StyleSet()->MaybeInvalidateRelativeSelectorForNthEdgeDependency(
-            *element);
+            *element, StyleRelativeSelectorNthEdgeInvalidateFor::Last);
         break;
       }
     }
@@ -218,7 +218,7 @@ void RestyleManager::MaybeRestyleForEdgeChildChange(nsINode* aContainer,
         PostRestyleEvent(element, RestyleHint::RestyleSubtree(),
                          nsChangeHint(0));
         StyleSet()->MaybeInvalidateRelativeSelectorForNthEdgeDependency(
-            *element);
+            *element, StyleRelativeSelectorNthEdgeInvalidateFor::First);
       }
       break;
     }
@@ -237,7 +237,7 @@ void RestyleManager::MaybeRestyleForEdgeChildChange(nsINode* aContainer,
         PostRestyleEvent(element, RestyleHint::RestyleSubtree(),
                          nsChangeHint(0));
         StyleSet()->MaybeInvalidateRelativeSelectorForNthEdgeDependency(
-            *element);
+            *element, StyleRelativeSelectorNthEdgeInvalidateFor::Last);
       }
       break;
     }
@@ -541,7 +541,7 @@ void RestyleManager::ContentWillBeRemoved(nsIContent* aOldChild) {
           PostRestyleEvent(element, RestyleHint::RestyleSubtree(),
                            nsChangeHint(0));
           StyleSet()->MaybeInvalidateRelativeSelectorForNthEdgeDependency(
-              *element);
+              *element, StyleRelativeSelectorNthEdgeInvalidateFor::First);
         }
         break;
       }
@@ -560,7 +560,7 @@ void RestyleManager::ContentWillBeRemoved(nsIContent* aOldChild) {
           PostRestyleEvent(element, RestyleHint::RestyleSubtree(),
                            nsChangeHint(0));
           StyleSet()->MaybeInvalidateRelativeSelectorForNthEdgeDependency(
-              *element);
+              *element, StyleRelativeSelectorNthEdgeInvalidateFor::Last);
         }
         break;
       }

layout/style/ServoStyleSet.cpp

@@ -1442,9 +1442,10 @@ void ServoStyleSet::MaybeInvalidateRelativeSelectorForEmptyDependency(
 }
 
 void ServoStyleSet::MaybeInvalidateRelativeSelectorForNthEdgeDependency(
-    const Element& aElement) {
+    const Element& aElement,
+    StyleRelativeSelectorNthEdgeInvalidateFor aInvalidateFor) {
   Servo_StyleSet_MaybeInvalidateRelativeSelectorNthEdgeDependency(
-      mRawData.get(), &aElement);
+      mRawData.get(), &aElement, aInvalidateFor);
 }
 
 void ServoStyleSet::MaybeInvalidateRelativeSelectorForNthDependencyFromSibling(

layout/style/ServoStyleSet.h

@@ -31,6 +31,7 @@ namespace mozilla {
 enum class MediaFeatureChangeReason : uint8_t;
 enum class StylePageSizeOrientation : uint8_t;
 enum class StyleRuleChangeKind : uint32_t;
+enum class StyleRelativeSelectorNthEdgeInvalidateFor : uint8_t;
 
 class ErrorResult;
 
@@ -514,7 +515,8 @@ class ServoStyleSet {
    * by a selector that can only selector first/last child, that
    * might require us to restyle the relative selector it refers to.
    */
-  void MaybeInvalidateRelativeSelectorForNthEdgeDependency(const dom::Element&);
+  void MaybeInvalidateRelativeSelectorForNthEdgeDependency(
+      const dom::Element&, StyleRelativeSelectorNthEdgeInvalidateFor);
 
   /**
    * Maybe invalidate if a state change on an element that might be selected by

servo/components/style/invalidation/element/invalidation_map.rs

@@ -291,25 +291,33 @@ pub struct InvalidationMap {
 
 /// Tree-structural pseudoclasses that we care about for (Relative selector) invalidation.
 /// Specifically, we need to store information on ones that don't generate the inner selector.
+/// Given the nature of these selectors:
+/// * These are only relevant during DOM mutation invalidations
+/// * Some invalidations may be optimized away.
 #[derive(Clone, Copy, Debug, MallocSizeOf)]
 pub struct TSStateForInvalidation(u8);
 
 bitflags! {
     impl TSStateForInvalidation : u8 {
-        /// :empty
+        /// :empty. This only needs to be considered for DOM mutation, and for
+        /// elements that do not have any children.
         const EMPTY = 1 << 0;
         /// :nth and related selectors, without of.
         const NTH = 1 << 1;
-        /// "Simple" edge child selectors, like :first-child, :last-child, etc.
+        /// :first-child. This only needs to be considered for DOM mutation, and
-        /// Excludes :*-of-type as well as :only-child.
+        /// for elements that have no previous sibling.
-        const NTH_EDGE = 1 << 2;
+        const NTH_EDGE_FIRST = 1 << 2;
+        /// :last-child. This only needs to be considered for DOM mutation,
+        /// and for elements have no next sibling.
+        const NTH_EDGE_LAST = 1 << 3;
     }
 }
 
 impl TSStateForInvalidation {
-    /// Return true if this state invalidation should not result in a blanket
+    /// Return true if this state invalidation could be skipped (As per comment
-    pub fn avoid_blanket_invalidation_on_dom_mutation(&self) -> bool {
+    /// in the definition of this bitflags)
-        (Self::EMPTY | Self::NTH_EDGE).contains(*self)
+    pub fn may_be_optimized(&self) -> bool {
+        (Self::EMPTY | Self::NTH_EDGE_FIRST | Self::NTH_EDGE_LAST).contains(*self)
     }
 }
 
@@ -1166,7 +1174,11 @@ fn on_simple_selector<C: Collector>(
         Component::Empty => Ok(ComponentVisitResult::Handled(TSStateForInvalidation::EMPTY)),
         Component::Nth(data) => {
             let kind = if data.is_simple_edge() {
-                TSStateForInvalidation::NTH_EDGE
+                if data.ty.is_from_end() {
+                    TSStateForInvalidation::NTH_EDGE_LAST
+                } else {
+                    TSStateForInvalidation::NTH_EDGE_FIRST
+                }
             } else {
                 TSStateForInvalidation::NTH
             };

servo/components/style/invalidation/element/relative_selector.rs

@@ -11,7 +11,7 @@ use crate::gecko_bindings::structs::ServoElementSnapshotTable;
 use crate::invalidation::element::element_wrapper::ElementWrapper;
 use crate::invalidation::element::invalidation_map::{
     Dependency, DependencyInvalidationKind, NormalDependencyInvalidationKind,
-    RelativeDependencyInvalidationKind, RelativeSelectorInvalidationMap,
+    RelativeDependencyInvalidationKind, RelativeSelectorInvalidationMap, TSStateForInvalidation,
 };
 use crate::invalidation::element::invalidator::{
     DescendantInvalidationLists, Invalidation, InvalidationProcessor, InvalidationResult,
@@ -515,14 +515,46 @@ where
                 if !operation.accept(&dependency.dep, element) {
                     return true;
                 }
+                // This section contain potential optimization for not running full invalidation -
+                // consult documentation in `TSStateForInvalidation`.
+                if dependency.state.may_be_optimized() {
+                    debug_assert!(
+                        self.optimization_context.is_some(),
+                        "Optimization context not available for DOM mutation?"
+                    );
+                    if operation.is_side_effect() {
+                        // Side effect operations act on element not being mutated, so they can't
+                        // change the match outcome of these optimizable pseudoclasses.
+                        return true;
+                    }
+
+                    if dependency.state.contains(TSStateForInvalidation::EMPTY) &&
+                        element.first_element_child().is_some()
+                    {
+                        return true;
+                    }
+
+                    let sibling_traversal_map = self
+                        .optimization_context
+                        .as_ref()
+                        .unwrap()
+                        .sibling_traversal_map;
                     if dependency
                         .state
-                    .avoid_blanket_invalidation_on_dom_mutation()
+                        .contains(TSStateForInvalidation::NTH_EDGE_FIRST) &&
+                        sibling_traversal_map.prev_sibling_for(&element).is_some()
                     {
-                    // We assume here that these dependencies are handled elsewhere,
-                    // in a more constrained manner.
                         return true;
                     }
+
+                    if dependency
+                        .state
+                        .contains(TSStateForInvalidation::NTH_EDGE_LAST) &&
+                        sibling_traversal_map.next_sibling_for(&element).is_some()
+                    {
+                        return true;
+                    }
+                }
                 self.add_dependency(&dependency.dep, element, scope);
                 true
             },

servo/ports/geckolib/glue.rs

@@ -7497,15 +7497,26 @@ pub extern "C" fn Servo_StyleSet_MaybeInvalidateRelativeSelectorEmptyDependency(
     );
 }
 
+/// Which edge side should the invalidation run for?
+#[repr(u8)]
+pub enum RelativeSelectorNthEdgeInvalidateFor {
+    First,
+    Last
+}
+
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_MaybeInvalidateRelativeSelectorNthEdgeDependency(
     raw_data: &PerDocumentStyleData,
     element: &RawGeckoElement,
+    invalidate_for: RelativeSelectorNthEdgeInvalidateFor,
 ) {
     invalidate_relative_selector_ts_dependency(
         &raw_data.borrow().stylist,
         GeckoElement(element),
-        TSStateForInvalidation::NTH_EDGE,
+        match invalidate_for {
+            RelativeSelectorNthEdgeInvalidateFor::First => TSStateForInvalidation::NTH_EDGE_FIRST,
+            RelativeSelectorNthEdgeInvalidateFor::Last => TSStateForInvalidation::NTH_EDGE_LAST,
+        },
     );
 }
 

testing/web-platform/tests/css/selectors/invalidation/has-pseudoclass-only.html

@@ -0,0 +1,84 @@
+<!DOCTYPE html>
+<title>CSS Selectors Invalidation: :has() containing :empty, :first-child, :last-child, pseudoclasses only</title>
+<link rel="author" title="David Shin" href="mailto:dshin@mozilla.com">
+<link rel="help" href="https://drafts.csswg.org/selectors/#relational">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1931432">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+#subject {
+  color: grey;
+}
+
+#subject.empty:has(:empty) {
+  color: red;
+}
+
+#subject.first:has(:first-child) {
+  color: orange;
+}
+
+#subject.last:has(:last-child) {
+  color: yellow;
+}
+
+/* :empty :empty never matches, so use something else. */
+#subject.empty-subtree:has(:first-child :empty) {
+  color: green;
+}
+
+#subject.first-subtree:has(:first-child :first-child) {
+  color: blue;
+}
+
+#subject.last-subtree:has(:last-child :last-child) {
+  color: purple;
+}
+</style>
+<div id="subject"></div>
+<script>
+const grey = "rgb(128, 128, 128)";
+const red = "rgb(255, 0, 0)";
+const orange = "rgb(255, 165, 0)";
+const yellow = "rgb(255, 255, 0)";
+const green = "rgb(0, 128, 0)";
+const blue = 'rgb(0, 0, 255)';
+const purple = 'rgb(128, 0, 128)';
+
+function testColor(test_name, color) {
+  test(function() {
+    assert_equals(getComputedStyle(subject).color, color);
+  }, test_name);
+}
+
+function runTest(test_class, child, insert, color_after_add, desc) {
+  subject.classList.add(test_class);
+  testColor(desc + ' color initial', grey);
+  insert(child);
+  testColor(desc + ' color after adding', color_after_add);
+  subject.replaceChildren();
+  testColor(desc + ' color after removing', grey);
+  subject.classList.remove(test_class);
+}
+
+function createSimpleTree() {
+  const root = document.createElement('div');
+  root.replaceChildren(document.createElement('div'), document.createElement('div'));
+  return root;
+}
+
+const tests = {
+  ':empty': {cls: 'empty', child: document.createElement('div'), color_after_add: red},
+  ':first-child': {cls: 'first', child: document.createElement('div'), color_after_add: orange},
+  ':last-child': {cls: 'last', child: document.createElement('div'), color_after_add: yellow},
+  ':empty subtree': {cls: 'empty-subtree', child: createSimpleTree(), color_after_add: green},
+  ':first-child subtree': {cls: 'first-subtree', child: createSimpleTree(), color_after_add: blue},
+  ':last-child subtree': {cls: 'last-subtree', child: createSimpleTree(), color_after_add: purple},
+};
+
+const element = document.createElement('div');
+for (const [t, options] of Object.entries(tests)) {
+  runTest(options.cls, options.child, (e) => subject.appendChild(e), options.color_after_add, t + ' append');
+  runTest(options.cls, options.child, (e) => subject.insertBefore(e, null), options.color_after_add, t + ' insert');
+}
+</script>