Reduce css @keyframes memory

Currently, for each tree scope, we gather all @keyframes rules into a
hash map. This may cause memory issues when we have web components that
carry @keyframes rules in its style sheet.

This patch makes each tree scope iterate the already stored
ActiveStyleSheetVector instead, so that we can save some memory.

Bug: 1141814
Change-Id: I09a1fda77f10a22376ed83a0ac7bd1f6f9f3cca9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2493431Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823619}

third_party/blink/renderer/core/css/resolver/scoped_style_resolver.cc

@@ -58,6 +58,9 @@ ScopedStyleResolver* ScopedStyleResolver::Parent() const {
 }
 
 void ScopedStyleResolver::AddKeyframeRules(const RuleSet& rule_set) {
+  if (RuntimeEnabledFeatures::CSSKeyframesMemoryReductionEnabled())
+    return;
+
   const HeapVector<Member<StyleRuleKeyframes>> keyframes_rules =
       rule_set.KeyframesRules();
   for (auto rule : keyframes_rules)
@@ -142,8 +145,40 @@ void ScopedStyleResolver::ResetAuthorStyle() {
   needs_append_all_sheets_ = false;
 }
 
+const ActiveStyleSheetVector& ScopedStyleResolver::ActiveAuthorStyleSheets() {
+  StyleSheetCollection* collection =
+      GetTreeScope().GetDocument().GetStyleEngine().StyleSheetCollectionFor(
+          *scope_);
+  DCHECK(collection);
+  return collection->ActiveAuthorStyleSheets();
+}
+
+// static
+StyleRuleKeyframes*
+ScopedStyleResolver::KeyframeStylesForAnimationFromActiveSheets(
+    const AtomicString& name,
+    const ActiveStyleSheetVector& sheets) {
+  // We prefer non-vendor-prefixed over vendor-prefixed rules.
+  StyleRuleKeyframes* vendor_prefixed_result = nullptr;
+  for (auto sheet = sheets.rbegin(); sheet != sheets.rend(); ++sheet) {
+    RuleSet* rule_set = sheet->second;
+    if (StyleRuleKeyframes* rule = rule_set->KeyframeStylesForAnimation(name)) {
+      if (!rule->IsVendorPrefixed())
+        return rule;
+      if (!vendor_prefixed_result)
+        vendor_prefixed_result = rule;
+    }
+  }
+  return vendor_prefixed_result;
+}
+
 StyleRuleKeyframes* ScopedStyleResolver::KeyframeStylesForAnimation(
     const AtomicString& animation_name) {
+  if (RuntimeEnabledFeatures::CSSKeyframesMemoryReductionEnabled()) {
+    return KeyframeStylesForAnimationFromActiveSheets(
+        animation_name, ActiveAuthorStyleSheets());
+  }
+
   if (keyframes_rule_map_.IsEmpty())
     return nullptr;
 
@@ -155,6 +190,7 @@ StyleRuleKeyframes* ScopedStyleResolver::KeyframeStylesForAnimation(
 }
 
 void ScopedStyleResolver::AddKeyframeStyle(StyleRuleKeyframes* rule) {
+  DCHECK(!RuntimeEnabledFeatures::CSSKeyframesMemoryReductionEnabled());
   AtomicString name = rule->GetName();
 
   if (rule->IsVendorPrefixed()) {

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

@@ -56,6 +56,9 @@ class CORE_EXPORT ScopedStyleResolver final
   const TreeScope& GetTreeScope() const { return *scope_; }
   ScopedStyleResolver* Parent() const;
 
+  static StyleRuleKeyframes* KeyframeStylesForAnimationFromActiveSheets(
+      const AtomicString& name,
+      const ActiveStyleSheetVector& sheets);
   StyleRuleKeyframes* KeyframeStylesForAnimation(
       const AtomicString& animation_name);
 
@@ -100,6 +103,8 @@ class CORE_EXPORT ScopedStyleResolver final
   void AddFontFaceRules(const RuleSet&);
   void AddKeyframeStyle(StyleRuleKeyframes*);
 
+  const ActiveStyleSheetVector& ActiveAuthorStyleSheets();
+
   Member<TreeScope> scope_;
 
   HeapVector<Member<CSSStyleSheet>> author_style_sheets_;

third_party/blink/renderer/core/css/resolver/style_cascade_test.cc

@@ -323,6 +323,10 @@ class StyleCascadeTest
     active_sheets.push_back(
         std::make_pair(sheet, &sheet->Contents()->GetRuleSet()));
     scoped_resolver.AppendActiveStyleSheets(0, active_sheets);
+    GetDocument()
+        .GetStyleEngine()
+        .GetDocumentStyleSheetCollection()
+        .AppendActiveStyleSheet(active_sheets[0]);
   }
 
   Element* DocumentElement() const { return GetDocument().documentElement(); }

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

@@ -319,6 +319,50 @@ void RuleSet::AddFontFaceRule(StyleRuleFontFace* rule) {
 void RuleSet::AddKeyframesRule(StyleRuleKeyframes* rule) {
   EnsurePendingRules();  // So that keyframes_rules_.ShrinkToFit() gets called.
   keyframes_rules_.push_back(rule);
+  keyframes_rules_sorted_ = false;
+}
+
+void RuleSet::SortKeyframesRulesIfNeeded() {
+  if (keyframes_rules_sorted_)
+    return;
+  // Sort keyframes rules by name, breaking ties with vendor prefixing.
+  // Since equal AtomicStrings always have the same impl, there's no need to
+  // actually compare the contents of two AtomicStrings. Comparing their impl
+  // addresses is enough.
+  std::stable_sort(
+      keyframes_rules_.begin(), keyframes_rules_.end(),
+      [](const StyleRuleKeyframes* lhs, const StyleRuleKeyframes* rhs) {
+        if (lhs->GetName() != rhs->GetName())
+          return lhs->GetName().Impl() < rhs->GetName().Impl();
+        if (lhs->IsVendorPrefixed() != rhs->IsVendorPrefixed())
+          return lhs->IsVendorPrefixed();
+        return false;
+      });
+  // Deduplicate rules, erase all but the last one for each animation name,
+  // since all the preceding ones are overridden.
+  auto boundary = std::unique(
+      keyframes_rules_.rbegin(), keyframes_rules_.rend(),
+      [](const StyleRuleKeyframes* lhs, const StyleRuleKeyframes* rhs) {
+        return lhs->GetName() == rhs->GetName();
+      });
+  keyframes_rules_.erase(keyframes_rules_.begin(), boundary.base());
+  keyframes_rules_.ShrinkToFit();
+  keyframes_rules_sorted_ = true;
+}
+
+StyleRuleKeyframes* RuleSet::KeyframeStylesForAnimation(
+    const AtomicString& name) {
+  SortKeyframesRulesIfNeeded();
+  Member<StyleRuleKeyframes>* rule_iterator = std::lower_bound(
+      keyframes_rules_.begin(), keyframes_rules_.end(), name,
+      [](const StyleRuleKeyframes* rule, const AtomicString& name) {
+        return rule->GetName().Impl() < name.Impl();
+      });
+  if (rule_iterator != keyframes_rules_.end() &&
+      (*rule_iterator)->GetName() == name) {
+    return *rule_iterator;
+  }
+  return nullptr;
 }
 
 void RuleSet::AddPropertyRule(StyleRuleProperty* rule) {

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

@@ -261,6 +261,7 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
   const HeapVector<Member<StyleRuleKeyframes>>& KeyframesRules() const {
     return keyframes_rules_;
   }
+  StyleRuleKeyframes* KeyframeStylesForAnimation(const AtomicString& name);
   const HeapVector<Member<StyleRuleProperty>>& PropertyRules() const {
     return property_rules_;
   }
@@ -330,6 +331,8 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
                      AddRuleFlags);
   bool FindBestRuleSetAndAdd(const CSSSelector&, RuleData*);
 
+  void SortKeyframesRulesIfNeeded();
+
   void CompactRules();
   static void CompactPendingRules(PendingRuleMap&, CompactRuleMap&);
 
@@ -374,6 +377,8 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
   HeapVector<MinimalRuleData> slotted_pseudo_element_rules_;
   Vector<MediaQuerySetResult> media_query_set_results_;
 
+  bool keyframes_rules_sorted_ = true;
+
   unsigned rule_count_;
   Member<PendingRuleMaps> pending_rules_;
 

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

@@ -30,7 +30,10 @@
 #include "third_party/blink/renderer/core/css/rule_set.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/css/css_keyframes_rule.h"
+#include "third_party/blink/renderer/core/css/css_rule_list.h"
 #include "third_party/blink/renderer/core/css/css_test_helpers.h"
+#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 
 namespace blink {
@@ -331,4 +334,61 @@ TEST(RuleSetTest, RuleCountNotIncreasedByInvalidRuleData) {
   EXPECT_EQ(1u, rule_set->RuleCount());
 }
 
+TEST(RuleSetTest, KeyframesRulesBasic) {
+  ScopedCSSKeyframesMemoryReductionForTest enabled_scope(true);
+
+  css_test_helpers::TestStyleSheet sheet;
+  sheet.AddCSSRules("@keyframes foo { from {top: 0;} to {top: 100px;} }");
+  sheet.AddCSSRules("@keyframes bar { from {top: 100px;} to {top: 0;} }");
+
+  RuleSet& rule_set = sheet.GetRuleSet();
+
+  StyleRuleKeyframes* foo = rule_set.KeyframeStylesForAnimation("foo");
+  EXPECT_TRUE(foo);
+  EXPECT_EQ("foo", foo->GetName());
+
+  StyleRuleKeyframes* bar = rule_set.KeyframeStylesForAnimation("bar");
+  EXPECT_TRUE(bar);
+  EXPECT_EQ("bar", bar->GetName());
+
+  StyleRuleKeyframes* nonexist =
+      rule_set.KeyframeStylesForAnimation("nonexist");
+  EXPECT_FALSE(nonexist);
+}
+
+TEST(RuleSetTest, KeyframesRulesOverriding) {
+  ScopedCSSKeyframesMemoryReductionForTest enabled_scope(true);
+
+  // Among multiple @keyframes rules with the same name, the last one wins.
+  css_test_helpers::TestStyleSheet sheet;
+  sheet.AddCSSRules("@keyframes foo { from1 {top: 0;} to1 {top: 100px;} }");
+  sheet.AddCSSRules("@keyframes foo { from2 {top: 100px;} to2 {top: 0;} }");
+
+  RuleSet& rule_set = sheet.GetRuleSet();
+
+  StyleRuleKeyframes* rule = rule_set.KeyframeStylesForAnimation("foo");
+  EXPECT_TRUE(rule);
+  EXPECT_EQ("foo", rule->GetName());
+
+  CSSKeyframesRule* css_rule = To<CSSKeyframesRule>(sheet.CssRules()->item(1));
+  EXPECT_EQ(rule, css_rule->Keyframes());
+}
+
+TEST(RuleSetTest, KeyframesRulesVendorPrefixed) {
+  ScopedCSSKeyframesMemoryReductionForTest enabled_scope(true);
+
+  // Non-vendor-prefixed keyframes rules win against vendor-prefixed ones.
+  css_test_helpers::TestStyleSheet sheet;
+  sheet.AddCSSRules("@keyframes foo { from1 {top: 0;} to1 {top: 100px;} }");
+  sheet.AddCSSRules(
+      "@-webkit-keyframes foo { from2 {top: 100px;} to2 {top: 0;} }");
+
+  RuleSet& rule_set = sheet.GetRuleSet();
+
+  StyleRuleKeyframes* rule = rule_set.KeyframeStylesForAnimation("foo");
+  EXPECT_TRUE(rule);
+  EXPECT_EQ("foo", rule->GetName());
+  EXPECT_FALSE(rule->IsVendorPrefixed());
+}
+
 }  // namespace blink

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

@@ -1930,6 +1930,9 @@ bool StyleEngine::AddUserFontFaceRules(const RuleSet& rule_set) {
 }
 
 void StyleEngine::AddUserKeyframeRules(const RuleSet& rule_set) {
+  if (RuntimeEnabledFeatures::CSSKeyframesMemoryReductionEnabled())
+    return;
+
   const HeapVector<Member<StyleRuleKeyframes>> keyframes_rules =
       rule_set.KeyframesRules();
   for (unsigned i = 0; i < keyframes_rules.size(); ++i)
@@ -1937,6 +1940,8 @@ void StyleEngine::AddUserKeyframeRules(const RuleSet& rule_set) {
 }
 
 void StyleEngine::AddUserKeyframeStyle(StyleRuleKeyframes* rule) {
+  DCHECK(!RuntimeEnabledFeatures::CSSKeyframesMemoryReductionEnabled());
+
   AtomicString animation_name(rule->GetName());
 
   if (rule->IsVendorPrefixed()) {
@@ -1973,6 +1978,11 @@ void StyleEngine::AddScrollTimelineRules(const RuleSet& rule_set) {
 
 StyleRuleKeyframes* StyleEngine::KeyframeStylesForAnimation(
     const AtomicString& animation_name) {
+  if (RuntimeEnabledFeatures::CSSKeyframesMemoryReductionEnabled()) {
+    return ScopedStyleResolver::KeyframeStylesForAnimationFromActiveSheets(
+        animation_name, active_user_style_sheets_);
+  }
+
   if (keyframes_rule_map_.IsEmpty())
     return nullptr;
 

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

@@ -417,6 +417,8 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
   Color ForcedBackgroundColor() const { return forced_background_color_; }
   Color ColorAdjustBackgroundColor() const;
 
+  TreeScopeStyleSheetCollection* StyleSheetCollectionFor(TreeScope&);
+
   void Trace(Visitor*) const override;
   const char* NameInHeapSnapshot() const override { return "StyleEngine"; }
 
@@ -433,7 +435,6 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
   }
 
   TreeScopeStyleSheetCollection& EnsureStyleSheetCollectionFor(TreeScope&);
-  TreeScopeStyleSheetCollection* StyleSheetCollectionFor(TreeScope&);
   bool ShouldUpdateDocumentStyleSheetCollection() const;
   bool ShouldUpdateShadowTreeStyleSheetCollection() const;
 
@@ -671,6 +672,7 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
   friend class NodeTest;
   friend class StyleEngineTest;
   friend class WhitespaceAttacherTest;
+  friend class StyleCascadeTest;
 
   HeapHashSet<Member<TextTrack>> text_tracks_;
   Member<Element> vtt_originating_element_;

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -502,6 +502,14 @@
       name: "CSSIndependentTransformProperties",
       status: "experimental",
     },
+    {
+      // Change ScopedStyleResolve and StyleEngine to use the already stored
+      // StyleSheetCollection to find @keyframes rules instead of creating their
+      // own hashmaps, so that we can save memory when there are web components
+      // with @keyframes rules in their stylesheets.
+      name: "CSSKeyframesMemoryReduction",
+      status: "test",
+    },
     {
       name: "CSSLayoutAPI",
       status: "experimental",