[cascade] Ensure that !important :visited declaration win over animations

Consider a rule such as:

  a:visited {
    background-color: green !important;
  }

If 'background-color' on 'a' is currently undergoing a @keyframes
animation, and 'a' is a visited link, the -internal-visited-background-
color (i.e. what we paint) must be 'green', but the 'background-color'
as seen from JS must be the interpolated color.

Since unvisited and visited colors are applied together by the
interpolations machinery, we need to work around this manually. Whenever
we apply an interpolated color, we check the priority of the -internal-
visited- counterpart, and apply that again if its priority is higher.

Note that despite CSSCascade being enabled for stable at the moment,
this does not (yet) constitute a web-facing change, as the standard
property animations are still using their own StyleCascade object (see
ApplyAnimatedStandardProperties), and -internal-visited- properties are
never added to that cascade.

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

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

@@ -122,6 +122,15 @@ void StyleCascade::Analyze(const CascadeInterpolations& interpolations,
         continue;
 
       map_.Add(name, priority);
+
+      // Since an interpolation for an unvisited property also causes an
+      // interpolation of the visited property, add the visited property to
+      // the map as well.
+      // TODO(crbug.com/1062217): Interpolate visited colors separately
+      if (const CSSProperty* visited = property.GetVisitedProperty()) {
+        if (!filter.Rejects(*visited))
+          map_.Add(visited->GetCSSPropertyName(), priority);
+      }
     }
   }
 }
@@ -295,12 +304,13 @@ void StyleCascade::ApplyInterpolationMap(const ActiveInterpolationsMap& map,
     if (ResolveIfSurrogate(property, priority, resolver) == Surrogate::kSkip)
       continue;
 
-    ApplyInterpolation(property, entry.value, resolver);
+    ApplyInterpolation(property, priority, entry.value, resolver);
   }
 }
 
 void StyleCascade::ApplyInterpolation(
     const CSSProperty& property,
+    CascadePriority priority,
     const ActiveInterpolations& interpolations,
     Resolver& resolver) {
   const Interpolation& interpolation = *interpolations.front();
@@ -312,6 +322,24 @@ void StyleCascade::ApplyInterpolation(
   } else {
     To<TransitionInterpolation>(interpolation).Apply(state_);
   }
+
+  // Applying a color property interpolation will also unconditionally apply
+  // the -internal-visited- counterpart (see CSSColorInterpolationType::
+  // ApplyStandardPropertyValue). To make sure !important rules in :visited
+  // selectors win over animations, we re-apply the -internal-visited property
+  // if its priority is higher.
+  //
+  // TODO(crbug.com/1062217): Interpolate visited colors separately
+  if (const CSSProperty* visited = property.GetVisitedProperty()) {
+    CascadePriority* visited_priority =
+        map_.Find(visited->GetCSSPropertyName());
+    if (visited_priority && priority < *visited_priority) {
+      // Resetting generation to zero makes it possible to apply the
+      // visited property again.
+      *visited_priority = CascadePriority(*visited_priority, 0);
+      LookupAndApply(*visited, resolver);
+    }
+  }
 }
 
 void StyleCascade::LookupAndApply(const CSSPropertyName& name,
@@ -368,6 +396,12 @@ void StyleCascade::LookupAndApplyInterpolation(
     CascadePriority priority,
     const CascadeInterpolations& interpolations,
     Resolver& resolver) {
+  // Interpolations for -internal-visited properties are applied via the
+  // interpolation for the main (unvisited) property, so we don't need to
+  // apply it twice.
+  // TODO(crbug.com/1062217): Interpolate visited colors separately
+  if (property.IsVisited())
+    return;
   DCHECK(priority.GetOrigin() >= CascadeOrigin::kAnimation);
   size_t index = DecodeInterpolationIndex(priority.GetPosition());
   DCHECK_LE(index, interpolations.GetEntries().size());
@@ -375,7 +409,7 @@ void StyleCascade::LookupAndApplyInterpolation(
   PropertyHandle handle = ToPropertyHandle(property, priority);
   const auto& entry = map.find(handle);
   DCHECK_NE(entry, map.end());
-  ApplyInterpolation(property, entry->value, resolver);
+  ApplyInterpolation(property, priority, entry->value, resolver);
 }
 
 bool StyleCascade::IsRootElement() const {

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

@@ -230,6 +230,7 @@ class CORE_EXPORT StyleCascade {
                              size_t index,
                              Resolver&);
   void ApplyInterpolation(const CSSProperty&,
+                          CascadePriority,
                           const ActiveInterpolations&,
                           Resolver&);
 

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

@@ -99,7 +99,7 @@ class TestCascade {
   void Add(String block, CascadeOrigin origin = CascadeOrigin::kAuthor) {
     CSSParserMode mode =
         origin == CascadeOrigin::kUserAgent ? kUASheetMode : kHTMLStandardMode;
-    Add(ParseDeclarationBlock(block, mode), origin);
+    Add(ParseDeclarationBlock(block, mode), origin, CSSSelector::kMatchAll);
   }
 
   void Add(String name, String value, CascadeOrigin origin = Origin::kAuthor) {
@@ -107,11 +107,12 @@ class TestCascade {
   }
 
   void Add(const CSSPropertyValueSet* set,
-           CascadeOrigin origin = CascadeOrigin::kAuthor) {
+           CascadeOrigin origin = CascadeOrigin::kAuthor,
+           unsigned link_match_type = CSSSelector::kMatchAll) {
     DCHECK_LE(origin, CascadeOrigin::kAuthor) << "Animations not supported";
     DCHECK_LE(current_origin_, origin) << "Please add declarations in order";
     EnsureAtLeast(origin);
-    match_result_.AddMatchedProperties(set);
+    match_result_.AddMatchedProperties(set, link_match_type);
   }
 
   void Apply(CascadeFilter filter = CascadeFilter()) {
@@ -1750,6 +1751,68 @@ TEST_F(StyleCascadeTest, AnimateStandardShorthand) {
   EXPECT_EQ("15px", cascade.ComputedValue("margin-left"));
 }
 
+TEST_F(StyleCascadeTest, AnimatedVisitedImportantOverride) {
+  AppendSheet(R"HTML(
+     @keyframes test {
+        from { background-color: rgb(100, 100, 100); }
+        to { background-color: rgb(200, 200, 200); }
+     }
+    )HTML");
+
+  TestCascade cascade(GetDocument());
+  cascade.State().Style()->SetInsideLink(EInsideLink::kInsideVisitedLink);
+
+  cascade.Add(ParseDeclarationBlock("background-color:red !important"),
+              CascadeOrigin::kAuthor, CSSSelector::kMatchVisited);
+  cascade.Add("animation-name:test");
+  cascade.Add("animation-duration:10s");
+  cascade.Add("animation-timing-function:linear");
+  cascade.Add("animation-delay:-5s");
+  cascade.Apply();
+
+  cascade.AnalyzeAnimations();
+  cascade.Apply();
+  EXPECT_EQ("rgb(150, 150, 150)", cascade.ComputedValue("background-color"));
+
+  auto style = cascade.TakeStyle();
+
+  style->SetInsideLink(EInsideLink::kInsideVisitedLink);
+  EXPECT_EQ(Color(255, 0, 0),
+            style->VisitedDependentColor(GetCSSPropertyBackgroundColor()));
+
+  style->SetInsideLink(EInsideLink::kNotInsideLink);
+  EXPECT_EQ(Color(150, 150, 150),
+            style->VisitedDependentColor(GetCSSPropertyBackgroundColor()));
+}
+
+TEST_F(StyleCascadeTest, AnimatedVisitedHighPrio) {
+  AppendSheet(R"HTML(
+     @keyframes test {
+        from { color: rgb(100, 100, 100); }
+        to { color: rgb(200, 200, 200); }
+     }
+    )HTML");
+
+  TestCascade cascade(GetDocument());
+  cascade.Add("color:red");
+  cascade.Add("animation:test 10s -5s linear");
+  cascade.Apply();
+
+  cascade.AnalyzeAnimations();
+  cascade.Apply();
+  EXPECT_EQ("rgb(150, 150, 150)", cascade.ComputedValue("color"));
+
+  auto style = cascade.TakeStyle();
+
+  style->SetInsideLink(EInsideLink::kInsideVisitedLink);
+  EXPECT_EQ(Color(150, 150, 150),
+            style->VisitedDependentColor(GetCSSPropertyColor()));
+
+  style->SetInsideLink(EInsideLink::kNotInsideLink);
+  EXPECT_EQ(Color(150, 150, 150),
+            style->VisitedDependentColor(GetCSSPropertyColor()));
+}
+
 TEST_F(StyleCascadeTest, AnimatePendingSubstitutionValue) {
   RegisterProperty(GetDocument(), "--x", "<length>", "0px", false);