Don't clear RuleSet for MQ changes until active sheet update.

This is the implementation of phase 2 of [1].

When collecting rules from StyleSheetContents into a RuleSet, push
refptrs to each @media or @import media query along with its current
evaluation into a vector on the RuleSet.

When a media feature value or the media type changes, mark stylesheet
collections which has media queries for active stylesheet update, but no
longer clear their current RuleSet. Instead evaluate the media query
sets in the RuleSets during active stylesheet and clear the RuleSet if
the media query evaluation changed.

This should avoid clearing RuleSets, hence cause less invalidation on
changing media query affecting values.

[1] https://docs.google.com/document/d/1TMqAq4k3aTHNH1m2sYoj-5QAndNmEXLI1QeD-oQ7jWE/edit#heading=h.vxytkk66ckr2

Bug: 1014920, 589083
Change-Id: I6ef9ffdfb2d94987796db15ebb8c39669e7cf96a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2117110Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752788}

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

@@ -18,6 +18,7 @@
 #include "third_party/blink/renderer/core/css/style_sheet_contents.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/frame/settings.h"
 #include "third_party/blink/renderer/core/testing/page_test_base.h"
 
 namespace blink {
@@ -103,15 +104,18 @@ TEST_F(CSSStyleSheetTest,
 }
 
 TEST_F(CSSStyleSheetTest, AdoptedStyleSheetMediaQueryEvalChange) {
-  SetBodyInnerHTML("<div id=green></div>");
+  SetBodyInnerHTML("<div id=green></div><div id=blue></div>");
 
   Element* green = GetDocument().getElementById("green");
+  Element* blue = GetDocument().getElementById("blue");
 
   CSSStyleSheetInit* init = CSSStyleSheetInit::Create();
   CSSStyleSheet* sheet =
       CSSStyleSheet::Create(GetDocument(), init, ASSERT_NO_EXCEPTION);
-  sheet->replaceSync("@media (max-width: 300px) { #green { color: green } }",
-                     ASSERT_NO_EXCEPTION);
+  sheet->replaceSync(
+      "@media (max-width: 300px) {#green{color:green}} @media "
+      "(prefers-reduced-motion: reduce) {#blue{color:blue}}",
+      ASSERT_NO_EXCEPTION);
 
   HeapVector<Member<CSSStyleSheet>> empty_adopted_sheets;
   HeapVector<Member<CSSStyleSheet>> adopted_sheets;
@@ -147,6 +151,26 @@ TEST_F(CSSStyleSheetTest, AdoptedStyleSheetMediaQueryEvalChange) {
   EXPECT_EQ(
       MakeRGB(0, 128, 0),
       green->GetComputedStyle()->VisitedDependentColor(GetCSSPropertyColor()));
+  EXPECT_EQ(Color::kBlack, blue->GetComputedStyle()->VisitedDependentColor(
+                               GetCSSPropertyColor()));
+
+  GetDocument().SetAdoptedStyleSheets(empty_adopted_sheets);
+  GetDocument().GetSettings()->SetPrefersReducedMotion(true);
+  UpdateAllLifecyclePhasesForTest();
+
+  EXPECT_EQ(Color::kBlack, green->GetComputedStyle()->VisitedDependentColor(
+                               GetCSSPropertyColor()));
+  EXPECT_EQ(Color::kBlack, blue->GetComputedStyle()->VisitedDependentColor(
+                               GetCSSPropertyColor()));
+
+  GetDocument().SetAdoptedStyleSheets(adopted_sheets);
+  UpdateAllLifecyclePhasesForTest();
+
+  EXPECT_EQ(
+      MakeRGB(0, 128, 0),
+      green->GetComputedStyle()->VisitedDependentColor(GetCSSPropertyColor()));
+  EXPECT_EQ(MakeRGB(0, 0, 255), blue->GetComputedStyle()->VisitedDependentColor(
+                                    GetCSSPropertyColor()));
 }
 
 }  // namespace blink

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

@@ -44,6 +44,22 @@ class MediaQueryResult {
   bool result_;
 };
 
+class MediaQuerySetResult {
+  DISALLOW_NEW();
+
+ public:
+  MediaQuerySetResult(const MediaQuerySet& media_queries, bool result)
+      : media_queries_(&media_queries), result_(result) {}
+
+  const MediaQuerySet& MediaQueries() const { return *media_queries_; }
+
+  bool Result() const { return result_; }
+
+ private:
+  scoped_refptr<const MediaQuerySet> media_queries_;
+  bool result_;
+};
+
 }  // namespace blink
 
 #endif

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

@@ -320,10 +320,7 @@ void RuleSet::AddChildRules(const HeapVector<Member<StyleRuleBase>>& rules,
     } else if (auto* page_rule = DynamicTo<StyleRulePage>(rule)) {
       AddPageRule(page_rule);
     } else if (auto* media_rule = DynamicTo<StyleRuleMedia>(rule)) {
-      if (!media_rule->MediaQueries() ||
-          medium.Eval(*media_rule->MediaQueries(),
-                      &features_.ViewportDependentMediaQueryResults(),
-                      &features_.DeviceDependentMediaQueryResults()))
+      if (MatchMediaForAddRules(medium, media_rule->MediaQueries()))
         AddChildRules(media_rule->ChildRules(), medium, add_rule_flags);
     } else if (auto* font_face_rule = DynamicTo<StyleRuleFontFace>(rule)) {
       AddFontFaceRule(font_face_rule);
@@ -338,6 +335,18 @@ void RuleSet::AddChildRules(const HeapVector<Member<StyleRuleBase>>& rules,
   }
 }
 
+bool RuleSet::MatchMediaForAddRules(const MediaQueryEvaluator& evaluator,
+                                    const MediaQuerySet* media_queries) {
+  if (!media_queries)
+    return true;
+  bool match_media = evaluator.Eval(
+      *media_queries, &features_.ViewportDependentMediaQueryResults(),
+      &features_.DeviceDependentMediaQueryResults());
+  media_query_set_results_.push_back(
+      MediaQuerySetResult(*media_queries, match_media));
+  return match_media;
+}
+
 void RuleSet::AddRulesFromSheet(StyleSheetContents* sheet,
                                 const MediaQueryEvaluator& medium,
                                 AddRuleFlags add_rule_flags) {
@@ -350,11 +359,9 @@ void RuleSet::AddRulesFromSheet(StyleSheetContents* sheet,
   for (unsigned i = 0; i < import_rules.size(); ++i) {
     StyleRuleImport* import_rule = import_rules[i].Get();
     if (import_rule->GetStyleSheet() &&
-        (!import_rule->MediaQueries() ||
-         medium.Eval(*import_rule->MediaQueries(),
-                     &features_.ViewportDependentMediaQueryResults(),
-                     &features_.DeviceDependentMediaQueryResults())))
+        MatchMediaForAddRules(medium, import_rule->MediaQueries())) {
       AddRulesFromSheet(import_rule->GetStyleSheet(), medium, add_rule_flags);
+    }
   }
 
   AddChildRules(sheet->ChildRules(), medium, add_rule_flags);
@@ -413,6 +420,15 @@ void RuleSet::CompactRules() {
   slotted_pseudo_element_rules_.ShrinkToFit();
 }
 
+bool RuleSet::DidMediaQueryResultsChange(
+    const MediaQueryEvaluator& evaluator) const {
+  for (const auto& result : media_query_set_results_) {
+    if (result.Result() != evaluator.Eval(result.MediaQueries()))
+      return true;
+  }
+  return false;
+}
+
 void MinimalRuleData::Trace(Visitor* visitor) {
   visitor->Trace(rule_);
 }

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

@@ -284,6 +284,8 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
            HasV0BoundaryCrossingRules();
   }
 
+  bool DidMediaQueryResultsChange(const MediaQueryEvaluator& evaluator) const;
+
 #ifndef NDEBUG
   void Show() const;
 #endif
@@ -304,6 +306,8 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
   void AddKeyframesRule(StyleRuleKeyframes*);
   void AddPropertyRule(StyleRuleProperty*);
 
+  bool MatchMediaForAddRules(const MediaQueryEvaluator& evaluator,
+                             const MediaQuerySet* media_queries);
   void AddChildRules(const HeapVector<Member<StyleRuleBase>>&,
                      const MediaQueryEvaluator& medium,
                      AddRuleFlags);
@@ -349,6 +353,7 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
   HeapVector<MinimalRuleData> deep_combinator_or_shadow_pseudo_rules_;
   HeapVector<MinimalRuleData> content_pseudo_element_rules_;
   HeapVector<MinimalRuleData> slotted_pseudo_element_rules_;
+  Vector<MediaQuerySetResult> media_query_set_results_;
 
   unsigned rule_count_;
   Member<PendingRuleMaps> pending_rules_;

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

@@ -383,22 +383,18 @@ void StyleEngine::AddedCustomElementDefaultStyles(
 
 namespace {
 
-bool ClearMediaQueryDependentRuleSets(
-    const ActiveStyleSheetVector& active_style_sheets) {
-  bool needs_active_style_update = false;
+bool HasMediaQueries(const ActiveStyleSheetVector& active_style_sheets) {
   for (const auto& active_sheet : active_style_sheets) {
     if (const MediaQuerySet* media_queries =
             active_sheet.first->MediaQueries()) {
       if (!media_queries->QueryVector().IsEmpty())
-        needs_active_style_update = true;
+        return true;
     }
     StyleSheetContents* contents = active_sheet.first->Contents();
-    if (contents->HasMediaQueries()) {
-      needs_active_style_update = true;
-      contents->ClearRuleSet();
-    }
+    if (contents->HasMediaQueries())
+      return true;
   }
-  return needs_active_style_update;
+  return false;
 }
 
 bool HasSizeDependentMediaQueries(
@@ -424,7 +420,7 @@ bool StyleEngine::MediaQueryAffectingValueChanged(
     return HasSizeDependentMediaQueries(active_sheets);
 
   DCHECK(change == MediaValueChange::kOther);
-  return ClearMediaQueryDependentRuleSets(active_sheets);
+  return HasMediaQueries(active_sheets);
 }
 
 void StyleEngine::MediaQueryAffectingValueChanged(TreeScope& tree_scope,

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

@@ -2548,7 +2548,7 @@ TEST_F(StyleEngineTest, InitialColorChange) {
 }
 
 TEST_F(StyleEngineTest,
-       MediaQueryAffectingValueChanged_InvalidateForChangedQueries) {
+       MediaQueryAffectingValueChanged_InvalidateForChangedSizeQueries) {
   GetDocument().body()->setInnerHTML(R"HTML(
     <style>
       @media (min-width: 1000px) {
@@ -2586,6 +2586,82 @@ TEST_F(StyleEngineTest,
                                     GetCSSPropertyColor()));
 }
 
+TEST_F(StyleEngineTest,
+       MediaQueryAffectingValueChanged_InvalidateForChangedTypeQuery) {
+  GetDocument().body()->setInnerHTML(R"HTML(
+    <style>
+      @media speech {
+        div { color: green }
+      }
+    </style>
+    <style>
+      @media (max-width: 100px) {
+        * { color: red }
+      }
+    </style>
+    <style>
+      @media print {
+        * { color: blue }
+      }
+    </style>
+    <div id="green"></div>
+    <span></span>
+  )HTML");
+  UpdateAllLifecyclePhases();
+
+  Element* div = GetDocument().getElementById("green");
+  EXPECT_EQ(Color::kBlack, div->GetComputedStyle()->VisitedDependentColor(
+                               GetCSSPropertyColor()));
+
+  unsigned initial_count = GetStyleEngine().StyleForElementCount();
+
+  GetDocument().GetSettings()->SetMediaTypeOverride("speech");
+  UpdateAllLifecyclePhases();
+
+  // Only the single div element should have its style recomputed.
+  EXPECT_EQ(1u, GetStyleEngine().StyleForElementCount() - initial_count);
+  EXPECT_EQ(MakeRGB(0, 128, 0), div->GetComputedStyle()->VisitedDependentColor(
+                                    GetCSSPropertyColor()));
+}
+
+TEST_F(StyleEngineTest,
+       MediaQueryAffectingValueChanged_InvalidateForChangedReducedMotionQuery) {
+  GetDocument().body()->setInnerHTML(R"HTML(
+    <style>
+      @media (prefers-reduced-motion: reduce) {
+        div { color: green }
+      }
+    </style>
+    <style>
+      @media (max-width: 100px) {
+        * { color: red }
+      }
+    </style>
+    <style>
+      @media print {
+        * { color: blue }
+      }
+    </style>
+    <div id="green"></div>
+    <span></span>
+  )HTML");
+  UpdateAllLifecyclePhases();
+
+  Element* div = GetDocument().getElementById("green");
+  EXPECT_EQ(Color::kBlack, div->GetComputedStyle()->VisitedDependentColor(
+                               GetCSSPropertyColor()));
+
+  unsigned initial_count = GetStyleEngine().StyleForElementCount();
+
+  GetDocument().GetSettings()->SetPrefersReducedMotion(true);
+  UpdateAllLifecyclePhases();
+
+  // Only the single div element should have its style recomputed.
+  EXPECT_EQ(1u, GetStyleEngine().StyleForElementCount() - initial_count);
+  EXPECT_EQ(MakeRGB(0, 128, 0), div->GetComputedStyle()->VisitedDependentColor(
+                                    GetCSSPropertyColor()));
+}
+
 class ParameterizedStyleEngineTest
     : public testing::WithParamInterface<bool>,
       private ScopedCSSReducedFontLoadingInvalidationsForTest,

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

@@ -601,13 +601,8 @@ void StyleSheetContents::ClearReferencedFromResource() {
 
 RuleSet& StyleSheetContents::EnsureRuleSet(const MediaQueryEvaluator& medium,
                                            AddRuleFlags add_rule_flags) {
-  if (rule_set_ &&
-      (medium.DidResultsChange(
-           rule_set_->Features().ViewportDependentMediaQueryResults()) ||
-       medium.DidResultsChange(
-           rule_set_->Features().DeviceDependentMediaQueryResults()))) {
+  if (rule_set_ && rule_set_->DidMediaQueryResultsChange(medium))
     rule_set_ = nullptr;
-  }
   if (!rule_set_) {
     rule_set_ = MakeGarbageCollected<RuleSet>();
     rule_set_->AddRulesFromSheet(this, medium, add_rule_flags);