Always rebuild CascadeLayerMap if active style sheets changed

The existing implementation:
- Resets a tree scope's style (include cascade layers) if the change
  type is ActiveSheetsChanged
- Rebuilds a tree scope's cascade layers only if the changed sheets
  involve cascade layers

This results in a bug that we if modify a sheet that does not have
cascade layers, then the previous CascadeLayerMap is cleared but not
rebuilt, and as a result, all previously previously layered rules
break into the default layer.

This patch fixes it by rebuilding the CascadeLayerMap in that case.
Layer-related invalidations remain the same as before.

Fixed: 1313357
Change-Id: I702a6421416aea2634537b6d9650399b68e25f0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3572544
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989813}

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

@@ -67,6 +67,7 @@ class CORE_EXPORT ScopedStyleResolver final
   static void CounterStyleRulesChanged(TreeScope& scope);
 
   void RebuildCascadeLayerMap(const ActiveStyleSheetVector&);
+  bool HasCascadeLayerMap() const { return cascade_layer_map_.Get(); }
   const CascadeLayerMap* GetCascadeLayerMap() const {
     return cascade_layer_map_;
   }

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

@@ -16,6 +16,7 @@
 #include "third_party/blink/renderer/core/css/css_value_list.h"
 #include "third_party/blink/renderer/core/css/properties/computed_style_utils.h"
 #include "third_party/blink/renderer/core/css/properties/css_property_ref.h"
+#include "third_party/blink/renderer/core/css/resolver/scoped_style_resolver.h"
 #include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
 #include "third_party/blink/renderer/core/css/style_change_reason.h"
 #include "third_party/blink/renderer/core/css/style_engine.h"
@@ -1553,6 +1554,52 @@ TEST_F(StyleResolverTest, CascadeLayersInDifferentTreeScopes) {
             &GetDocument());
 }
 
+// https://crbug.com/1313357
+TEST_F(StyleResolverTest, CascadeLayersAfterModifyingAnotherSheet) {
+  GetDocument().documentElement()->setInnerHTML(R"HTML(
+    <style>
+      @layer {
+        target { color: red; }
+      }
+    </style>
+    <style id="addrule"></style>
+    <target></target>
+  )HTML");
+
+  UpdateAllLifecyclePhasesForTest();
+
+  GetDocument().getElementById("addrule")->appendChild(
+      GetDocument().createTextNode("target { font-size: 10px; }"));
+
+  UpdateAllLifecyclePhasesForTest();
+
+  ASSERT_TRUE(GetDocument().GetScopedStyleResolver()->GetCascadeLayerMap());
+
+  StyleResolverState state(GetDocument(),
+                           *GetDocument().QuerySelector("target"));
+  SelectorFilter filter;
+  MatchResult match_result;
+  ElementRuleCollector collector(state.ElementContext(), StyleRecalcContext(),
+                                 filter, match_result, state.Style(),
+                                 EInsideLink::kNotInsideLink);
+  MatchAllRules(state, collector);
+  const auto& properties = match_result.GetMatchedProperties();
+  ASSERT_EQ(properties.size(), 2u);
+
+  const uint16_t kImplicitOuterLayerOrder =
+      CascadeLayerMap::kImplicitOuterLayerOrder;
+
+  // @layer { target { color: red } }"
+  EXPECT_TRUE(properties[0].properties->HasProperty(CSSPropertyID::kColor));
+  EXPECT_EQ(0u, properties[0].types_.layer_order);
+  EXPECT_EQ(properties[0].types_.origin, CascadeOrigin::kAuthor);
+
+  // target { font-size: 10px }
+  EXPECT_TRUE(properties[1].properties->HasProperty(CSSPropertyID::kFontSize));
+  EXPECT_EQ(kImplicitOuterLayerOrder, properties[1].types_.layer_order);
+  EXPECT_EQ(properties[1].types_.origin, CascadeOrigin::kAuthor);
+}
+
 // TODO(crbug.com/1095765): We should have a WPT for this test case, but
 // currently Blink web test runner can't test @page rules in WPT.
 TEST_F(StyleResolverTest, CascadeLayersAndPageRules) {

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

@@ -2049,29 +2049,30 @@ void StyleEngine::ApplyRuleSetChanges(
     MarkCounterStylesNeedUpdate();
 
   unsigned append_start_index = 0;
+  bool rebuild_cascade_layer_map = changed_rule_flags & kLayerRules;
   if (scoped_resolver) {
     // - If all sheets were removed, we remove the ScopedStyleResolver
     // - If new sheets were appended to existing ones, start appending after the
-    //   common prefix.
+    //   common prefix, and rebuild CascadeLayerMap only if layers are changed.
     // - For other diffs, reset author style and re-add all sheets for the
-    //   TreeScope.
-    if (new_style_sheets.IsEmpty())
+    //   TreeScope. If there is an existing CascadeLayerMap, rebuild it.
+    if (new_style_sheets.IsEmpty()) {
+      rebuild_cascade_layer_map = false;
       ResetAuthorStyle(tree_scope);
-    else if (change == kActiveSheetsAppended)
+    } else if (change == kActiveSheetsAppended) {
       append_start_index = old_style_sheets.size();
-    else
+    } else {
+      rebuild_cascade_layer_map = scoped_resolver->HasCascadeLayerMap();
       scoped_resolver->ResetStyle();
+    }
+  }
+
+  if (rebuild_cascade_layer_map) {
+    tree_scope.EnsureScopedStyleResolver().RebuildCascadeLayerMap(
+        new_style_sheets);
   }
 
-  // Cascade layer map must be built before adding other at-rules, because other
-  // at-rules rely on layer order to resolve name conflicts.
   if (changed_rule_flags & kLayerRules) {
-    if (!new_style_sheets.IsEmpty()) {
-      // Rebuild cascade layer map in all cases, because a newly inserted
-      // sub-layer can precede an original layer in the final ordering.
-      tree_scope.EnsureScopedStyleResolver().RebuildCascadeLayerMap(
-          new_style_sheets);
-    }
     if (resolver_)
       resolver_->InvalidateMatchedPropertiesCache();
 

third_party/blink/web_tests/external/wpt/css/css-cascade/layer-media-toggle.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<title>CSS Cascade Layers: Tests against a Chrome bug that modifying a sheet affects existing layers</title>
+<link rel="help" href="https://drafts.csswg.org/css-cascade-5/#layering">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1313357">
+<link rel="author" href="mailto:xiaochengh@chromium.org">
+<link rel="match" href="reference/ref-filled-green-100px-square.xht">
+
+<style>
+@layer foo, bar;
+@layer bar {
+  #target { background-color: green; }
+}
+@layer foo {
+  #target { background-color: red; }
+}
+</style>
+<style media="print" id="toggle">
+#target {
+  width: 100px;
+  height: 100px;
+}
+</style>
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div id="target"></div>
+
+<script>
+document.body.offsetWidth; // Force style calculation
+document.getElementById('toggle').media = 'all';
+</script>