Refactor counter style updates

Currently, counter style updates is a phase in UpdateActiveStyles. This
patch moves it out as a standalone phase of UpdateStyleAndLayoutTree,
so that we update counter styles only when needed.

This patch also adds MarkCounterStylesNeedUpdate() to indicate whether
counter style updates are needed.

Note: After refactoring, the framework of counter style updates looks
almost identical to font updates. This is intentional. We will try to
unify them in the future.

Bug: 687225
Change-Id: If07de2878ae5faa6800644a967029a569bef566a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2639535
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Xiaocheng Hu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#846203}

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

@@ -388,6 +388,8 @@ String CounterStyle::GenerateFallbackRepresentation(int value) const {
 }
 
 String CounterStyle::GenerateRepresentation(int value) const {
+  DCHECK(!IsDirty());
+
   if (pad_length_ > kCounterLengthLimit)
     return GenerateFallbackRepresentation(value);
 

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

@@ -82,12 +82,18 @@ CounterStyleMap::CounterStyleMap(Document* document, TreeScope* tree_scope)
 }
 
 void CounterStyleMap::AddCounterStyles(const RuleSet& rule_set) {
+  if (!rule_set.CounterStyleRules().size())
+    return;
+
   for (StyleRuleCounterStyle* rule : rule_set.CounterStyleRules()) {
     CounterStyle* counter_style = CounterStyle::Create(*rule);
     if (!counter_style)
       continue;
     counter_styles_.Set(rule->GetName(), counter_style);
   }
+
+  if (owner_document_)
+    owner_document_->GetStyleEngine().MarkCounterStylesNeedUpdate();
 }
 
 CounterStyleMap* CounterStyleMap::GetAncestorMap() const {
@@ -264,9 +270,15 @@ void CounterStyleMap::ResolveAllReferences(
 }
 
 void CounterStyleMap::Dispose() {
+  if (!counter_styles_.size())
+    return;
+
   for (CounterStyle* counter_style : counter_styles_.Values())
     counter_style->SetIsDirty();
   counter_styles_.clear();
+
+  if (owner_document_)
+    owner_document_->GetStyleEngine().MarkCounterStylesNeedUpdate();
 }
 
 void CounterStyleMap::Trace(Visitor* visitor) const {

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

@@ -541,14 +541,6 @@ void StyleEngine::UpdateActiveStyleSheets() {
       active_tree_scopes_.erase(tree_scope);
   }
 
-  if (RuntimeEnabledFeatures::CSSAtRuleCounterStyleEnabled()) {
-    // TODO(crbug.com/687225): Add a flag to indicate whether counter styles
-    // need updates, so that we don't update them every time.
-    CounterStyleMap::MarkAllDirtyCounterStyles(GetDocument(),
-                                               active_tree_scopes_);
-    CounterStyleMap::ResolveAllReferences(GetDocument(), active_tree_scopes_);
-  }
-
   probe::ActiveStyleSheetsUpdated(document_);
 
   dirty_tree_scopes_.clear();
@@ -864,11 +856,30 @@ void StyleEngine::InvalidateStyleAndLayoutForFontUpdates() {
   }
 }
 
+void StyleEngine::InvalidateStyleAndLayoutForCounterStyleUpdates() {
+  if (!counter_styles_need_update_)
+    return;
+
+  DCHECK(RuntimeEnabledFeatures::CSSAtRuleCounterStyleEnabled());
+
+  counter_styles_need_update_ = false;
+  CounterStyleMap::MarkAllDirtyCounterStyles(GetDocument(),
+                                             active_tree_scopes_);
+  CounterStyleMap::ResolveAllReferences(GetDocument(), active_tree_scopes_);
+
+  // TODO(crbug.com/687225): Actually invalidate style and layout.
+}
+
 void StyleEngine::MarkFontsNeedUpdate() {
   fonts_need_update_ = true;
   GetDocument().ScheduleLayoutTreeUpdateIfNeeded();
 }
 
+void StyleEngine::MarkCounterStylesNeedUpdate() {
+  counter_styles_need_update_ = true;
+  GetDocument().ScheduleLayoutTreeUpdateIfNeeded();
+}
+
 void StyleEngine::FontsNeedUpdate(FontSelector*, FontInvalidationReason) {
   if (!GetDocument().IsActive())
     return;
@@ -1572,7 +1583,7 @@ void StyleEngine::ApplyUserRuleSetChanges(
         EnsureUserCounterStyleMap().AddCounterStyles(*it->second);
     }
 
-    // TODO(crbug.com/687225): Trigger style/Layout invalidations.
+    MarkCounterStylesNeedUpdate();
   }
 
   if (changed_rule_flags & (kPropertyRules | kScrollTimelineRules)) {
@@ -1638,7 +1649,8 @@ void StyleEngine::ApplyRuleSetChanges(
   if (changed_rule_flags & kKeyframesRules)
     ScopedStyleResolver::KeyframesRulesAdded(tree_scope);
 
-  // TODO(crbug.com/687725): Style/layout invalidation for counter style rules.
+  if (changed_rule_flags & kCounterStyleRules)
+    MarkCounterStylesNeedUpdate();
 
   if ((changed_rule_flags & kPropertyRules) || rebuild_at_property_registry) {
     // @property rules are (for now) ignored in shadow trees, per spec.

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

@@ -377,6 +377,9 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
   void MarkFontsNeedUpdate();
   void InvalidateStyleAndLayoutForFontUpdates();
 
+  void MarkCounterStylesNeedUpdate();
+  void InvalidateStyleAndLayoutForCounterStyleUpdates();
+
   StyleRuleKeyframes* KeyframeStylesForAnimation(
       const AtomicString& animation_name);
   StyleRuleScrollTimeline* FindScrollTimelineRule(const AtomicString& name);
@@ -599,6 +602,7 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
   bool in_dom_removal_{false};
   bool viewport_style_dirty_{false};
   bool fonts_need_update_{false};
+  bool counter_styles_need_update_{false};
 
   // Set to true if we allow marking style dirty from style recalc. Ideally, we
   // should get rid of this, but we keep track of where we allow it with

third_party/blink/renderer/core/dom/document.cc

@@ -2405,7 +2405,11 @@ void Document::UpdateStyleAndLayoutTree() {
   UpdateUseShadowTreesIfNeeded();
 
   GetStyleEngine().UpdateActiveStyle();
+
+  // TODO(xiaochengh): These two functions are very similar. Try to unify.
   InvalidateStyleAndLayoutForFontUpdates();
+  GetStyleEngine().InvalidateStyleAndLayoutForCounterStyleUpdates();
+
   UpdateStyleInvalidationIfNeeded();
   UpdateStyle();