[@scope] Save MatchFlags in StyleScopeActivations

ComputedStyles that match pseudo-state-dependent selectors get
a flag set for invalidation purposes. For example:

  p:hover { ... }

When matching this selector against 'p', the MatchResults will
contain MatchFlags with the kAffectedByHover flag set, regardless
of whether or not 'p' is currently hovered or not. (The flag
ultimately ends up on the ComputedStyle of 'p').

Now consider this @scope rule which selects the same element:

  @scope (p:hover) {
    :scope { ... }
  }

Now we may or may not have a scoping root, depending on whether 'p'
is hovered or not. When matching this selector (:scope) against 'p',
we need to make sure that the kAffectedByHover is set in the returned
MatchFlags, even if 'p' is not currently hovered (in which case
we don't technically have a scoping root at 'p').

This means that we have to store the MatchFlags that was produced for
for the scopes that were attempted activated.

This is done in this CL by storing MatchFlags alongside the
activation vector, and propagating these flags during handling
of kScopeActivation. This may over-invalidate a little, since
we're propagating the flags at the start of the compound that
contains :scope (before actually evaluating :scope), but this is
probably OK.

Fixed: 1451091
Change-Id: Ia48664381766ef44e6ade11457e905f068b6103e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4790441
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1185140}

third_party/blink/renderer/core/css/resolver/match_flags.h

@@ -5,6 +5,8 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_RESOLVER_MATCH_FLAGS_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_RESOLVER_MATCH_FLAGS_H_
 
+#include <cstdint>
+
 namespace blink {
 
 // During rule-matching, we collect some information about what the match

third_party/blink/renderer/core/css/selector_checker.cc

@@ -552,11 +552,21 @@ SelectorChecker::MatchStatus SelectorChecker::MatchForRelation(
       if (context.style_scope) {
         const StyleScopeActivations& activations =
             EnsureActivations(context, *context.style_scope);
-        if (activations.empty()) {
+        if (ImpactsSubject(context)) {
+          // For e.g. @scope (:hover) { :scope { ...} },
+          // the StyleScopeActivations may have stored MatchFlags that we
+          // need to propagate. However, this is only needed if :scope
+          // appears in the subject position, since MatchFlags are only
+          // used for subject invalidation. Non-subject flags are set on
+          // Elements directly (e.g. SetChildrenOrSiblingsAffectedByHover)
+          result.flags |= activations.match_flags;
+        }
+        if (activations.vector.empty()) {
           return kSelectorFailsCompletely;
         }
-        for (const StyleScopeActivation& activation : activations) {
+        for (const StyleScopeActivation& activation : activations.vector) {
           next_context.match_visited = context.match_visited;
+          next_context.impact = context.impact;
           next_context.style_scope = nullptr;
           next_context.scope = activation.root;
           if (MatchSelector(next_context, result) == kSelectorMatches) {
@@ -2355,11 +2365,14 @@ namespace {
 
 // CalculateActivations will not produce any activations unless there is
 // an outer activation (i.e. an activation of the outer StyleScope). If there
-// is no outer StyleScope, we use this DefaultActivation as the outer
-// activation. The scope provided to DefaultActivation is typically
+// is no outer StyleScope, we use this DefaultActivations as the outer
+// activation. The scope provided to DefaultActivations is typically
 // a ShadowTree.
-StyleScopeActivation DefaultActivation(const ContainerNode* scope) {
-  return StyleScopeActivation{scope, std::numeric_limits<unsigned>::max()};
+StyleScopeActivations& DefaultActivations(const ContainerNode* scope) {
+  auto* activations = MakeGarbageCollected<StyleScopeActivations>();
+  activations->vector = HeapVector<StyleScopeActivation>(
+      1, StyleScopeActivation{scope, std::numeric_limits<unsigned>::max()});
+  return *activations;
 }
 
 // The activation ceiling is the highest ancestor element that can
@@ -2402,8 +2415,7 @@ const StyleScopeActivations& SelectorChecker::EnsureActivations(
   // of the *parent element*.
   const StyleScopeActivations* outer_activations =
       style_scope.Parent() ? &EnsureActivations(context, *style_scope.Parent())
-                           : MakeGarbageCollected<StyleScopeActivations>(
-                                 1, DefaultActivation(context.scope));
+                           : &DefaultActivations(context.scope);
   // The `match_visited` flag may have been set to false e.g. due to a link
   // having been encountered (see DisallowMatchVisited), but scope activations
   // are calculated lazily when :scope is first seen in a compound selector,
@@ -2459,12 +2471,12 @@ const StyleScopeActivations* SelectorChecker::CalculateActivations(
 
   auto* activations = MakeGarbageCollected<StyleScopeActivations>();
 
-  if (!outer_activations.empty()) {
+  if (!outer_activations.vector.empty()) {
     const StyleScopeActivations* parent_activations = nullptr;
 
     // Remain within the outer scope. I.e. don't look at elements above the
     // highest outer activation.
-    if (&element != ActivationCeiling(outer_activations.front())) {
+    if (&element != ActivationCeiling(outer_activations.vector.front())) {
       if (Element* parent = element.ParentOrShadowHostElement()) {
         // When calculating the activations on the parent element, we pass
         // the parent StyleScopeFrame (if we have it) to be able to use the
@@ -2484,27 +2496,32 @@ const StyleScopeActivations* SelectorChecker::CalculateActivations(
     // The activations of the parent element are still active for this element,
     // unless this element is a scoping limit.
     if (parent_activations) {
-      for (const StyleScopeActivation& activation : *parent_activations) {
+      activations->match_flags = parent_activations->match_flags;
+
+      for (const StyleScopeActivation& activation :
+           parent_activations->vector) {
         if (!ElementIsScopingLimit(style_scope, activation, element,
-                                   match_visited)) {
-          activations->push_back(
+                                   match_visited, activations->match_flags)) {
+          activations->vector.push_back(
               StyleScopeActivation{activation.root, activation.proximity + 1});
         }
       }
     }
 
     // Check if we need to add a new activation for this element.
-    for (const StyleScopeActivation& outer_activation : outer_activations) {
+    for (const StyleScopeActivation& outer_activation :
+         outer_activations.vector) {
       if (style_scope.From()
               ? MatchesWithScope(element, *style_scope.From(),
-                                 outer_activation.root, match_visited)
+                                 outer_activation.root, match_visited,
+                                 activations->match_flags)
               : style_scope.HasImplicitRoot(&element)) {
         StyleScopeActivation activation{&element, 0};
         // It's possible for a newly created activation to be immediately
         // limited (e.g. @scope (.x) to (.x)).
         if (!ElementIsScopingLimit(style_scope, activation, element,
-                                   match_visited)) {
-          activations->push_back(activation);
+                                   match_visited, activations->match_flags)) {
+          activations->vector.push_back(activation);
         }
         break;
       }
@@ -2523,7 +2540,8 @@ const StyleScopeActivations* SelectorChecker::CalculateActivations(
 bool SelectorChecker::MatchesWithScope(Element& element,
                                        const CSSSelector& selector_list,
                                        const ContainerNode* scope,
-                                       bool match_visited) const {
+                                       bool match_visited,
+                                       MatchFlags& match_flags) const {
   SelectorCheckingContext context(&element);
   context.scope = scope;
   context.match_visited = match_visited;
@@ -2536,9 +2554,11 @@ bool SelectorChecker::MatchesWithScope(Element& element,
   context.impact = Impact::kBoth;
   for (context.selector = &selector_list; context.selector;
        context.selector = CSSSelectorList::Next(*context.selector)) {
-    SelectorChecker::MatchResult ignore_result;
-    if (MatchSelector(context, ignore_result) ==
-        SelectorChecker::kSelectorMatches) {
+    MatchResult match_result;
+    bool match = MatchSelector(context, match_result) ==
+                 SelectorChecker::kSelectorMatches;
+    match_flags |= match_result.flags;
+    if (match) {
       return true;
     }
   }
@@ -2549,12 +2569,13 @@ bool SelectorChecker::ElementIsScopingLimit(
     const StyleScope& style_scope,
     const StyleScopeActivation& activation,
     Element& element,
-    bool match_visited) const {
+    bool match_visited,
+    MatchFlags& match_flags) const {
   if (!style_scope.To()) {
     return false;
   }
   return MatchesWithScope(element, *style_scope.To(), activation.root.Get(),
-                          match_visited);
+                          match_visited, match_flags);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/selector_checker.h

@@ -353,12 +353,14 @@ class CORE_EXPORT SelectorChecker {
   bool MatchesWithScope(Element&,
                         const CSSSelector& selector_list,
                         const ContainerNode* scope,
-                        bool match_visited) const;
+                        bool match_visited,
+                        MatchFlags&) const;
   // https://drafts.csswg.org/css-cascade-6/#scoping-limit
   bool ElementIsScopingLimit(const StyleScope&,
                              const StyleScopeActivation&,
                              Element& element,
-                             bool match_visited) const;
+                             bool match_visited,
+                             MatchFlags&) const;
 
   CustomScrollbar* scrollbar_;
   PartNames* part_names_;

third_party/blink/renderer/core/css/selector_checker_test.cc

@@ -10,8 +10,10 @@
 #include "third_party/blink/renderer/core/css/selector_checker-inl.h"
 #include "third_party/blink/renderer/core/css/style_rule.h"
 #include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/core/dom/node_computed_style.h"
 #include "third_party/blink/renderer/core/dom/shadow_root.h"
 #include "third_party/blink/renderer/core/html/html_element.h"
+#include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/core/testing/page_test_base.h"
 #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
 
@@ -660,6 +662,112 @@ TEST_P(MatchFlagsShadowTest, Host) {
   EXPECT_EQ(Bits(param.expected), Bits(result.flags));
 }
 
+class MatchFlagsScopeTest : public PageTestBase {
+ public:
+  void SetUp() override {
+    PageTestBase::SetUp();
+    GetDocument().body()->setInnerHTML(R"HTML(
+      <style id=style>
+      </style>
+      <div id=outer>
+        <div id=inner></div>
+      </div>
+    )HTML");
+    UpdateAllLifecyclePhasesForTest();
+  }
+
+  void SetStyle(String text) {
+    Element* style = GetDocument().getElementById(AtomicString("style"));
+    DCHECK(style);
+    style->setTextContent(text);
+    UpdateAllLifecyclePhasesForTest();
+  }
+
+  Element& Outer() const {
+    return *GetDocument().getElementById(AtomicString("outer"));
+  }
+  Element& Inner() const {
+    return *GetDocument().getElementById(AtomicString("inner"));
+  }
+
+  bool AffectedByHover(Element& element) {
+    return element.ComputedStyleRef().AffectedByHover();
+  }
+};
+
+TEST_F(MatchFlagsScopeTest, NoHover) {
+  SetStyle(R"HTML(
+    @scope (#inner) to (.unknown) {
+      :scope { --x:1; }
+    }
+    @scope (#outer) to (.unknown) {
+      :scope #inner { --x:1; }
+    }
+  )HTML");
+  EXPECT_FALSE(AffectedByHover(Outer()));
+  EXPECT_FALSE(AffectedByHover(Inner()));
+}
+
+TEST_F(MatchFlagsScopeTest, HoverSubject) {
+  SetStyle(R"HTML(
+    @scope (#outer) {
+      :scope #inner:hover { --x:1; }
+    }
+  )HTML");
+  EXPECT_FALSE(AffectedByHover(Outer()));
+  EXPECT_TRUE(AffectedByHover(Inner()));
+}
+
+TEST_F(MatchFlagsScopeTest, HoverNonSubject) {
+  SetStyle(R"HTML(
+    @scope (#outer) {
+      :scope:hover #inner { --x:1; }
+    }
+  )HTML");
+  EXPECT_FALSE(AffectedByHover(Outer()));
+  EXPECT_FALSE(AffectedByHover(Inner()));
+}
+
+TEST_F(MatchFlagsScopeTest, ScopeSubject) {
+  SetStyle(R"HTML(
+    @scope (#inner:hover) {
+      :scope { --x:1; }
+    }
+  )HTML");
+  EXPECT_FALSE(AffectedByHover(Outer()));
+  EXPECT_TRUE(AffectedByHover(Inner()));
+}
+
+TEST_F(MatchFlagsScopeTest, ScopeNonSubject) {
+  SetStyle(R"HTML(
+    @scope (#outer:hover) {
+      :scope #inner { --x:1; }
+    }
+  )HTML");
+  EXPECT_FALSE(AffectedByHover(Outer()));
+  EXPECT_FALSE(AffectedByHover(Inner()));
+}
+
+TEST_F(MatchFlagsScopeTest, ScopeLimit) {
+  SetStyle(R"HTML(
+    @scope (#inner) to (#inner:hover) {
+      :scope { --x:1; }
+    }
+  )HTML");
+  EXPECT_FALSE(AffectedByHover(Outer()));
+  EXPECT_TRUE(AffectedByHover(Inner()));
+}
+
+TEST_F(MatchFlagsScopeTest, ScopeLimitNonSubject) {
+  SetStyle(R"HTML(
+    @scope (#middle) to (#middle:hover) {
+      :scope #inner { --x:1; }
+    }
+  )HTML");
+  EXPECT_FALSE(AffectedByHover(Outer()));
+  EXPECT_FALSE(AffectedByHover(Inner()));
+}
+
 class EasySelectorCheckerTest : public PageTestBase {
  protected:
   bool Matches(const String& selector_text, const char* id);

third_party/blink/renderer/core/css/style_scope_frame.cc

@@ -11,6 +11,10 @@ void StyleScopeActivation::Trace(blink::Visitor* visitor) const {
   visitor->Trace(root);
 }
 
+void StyleScopeActivations::Trace(blink::Visitor* visitor) const {
+  visitor->Trace(vector);
+}
+
 StyleScopeFrame* StyleScopeFrame::GetParentFrameOrNull(
     Element& parent_element) {
   if (parent_ && (&parent_->element_ == &parent_element)) {

third_party/blink/renderer/core/css/style_scope_frame.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_STYLE_SCOPE_FRAME_H_
 
 #include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/core/css/resolver/match_flags.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_map.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_vector.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
@@ -45,7 +46,25 @@ struct CORE_EXPORT StyleScopeActivation {
   unsigned proximity = 0;
 };
 
-using StyleScopeActivations = HeapVector<StyleScopeActivation>;
+struct CORE_EXPORT StyleScopeActivations
+    : public GarbageCollected<StyleScopeActivations> {
+ public:
+  void Trace(blink::Visitor*) const;
+
+  HeapVector<StyleScopeActivation> vector;
+
+  // Even if `vector` is empty, `match_flags` can be set. For example:
+  //
+  //  @scope (p:hover) {
+  //    :scope { ... }
+  //  }
+  //
+  // When matching :scope against 'p', even if 'p' is not currently hovered,
+  // (and therefore won't produce a StyleScopeActivation in the vector),
+  // `match_flags` will contain kAffectedByHover. This allows us to propagate
+  // the flags when matching :scope, also when the selector does not match.
+  MatchFlags match_flags = 0;
+};
 
 // Stores the current @scope activations for a given subject element.
 //