Added support for color valued compositor keyframes

A new class CompositorKeyframeColor was added to accommodate color values
on the compositor.  Compositor keyframe value factory will now create a
CompositorKeyframeColor when the CSSValue can be casted to a
CSSColorValue.

Unit tests were changed to test color values on the compositor.

Bug: 883721
Change-Id: I771e0ece5d546baa345d9b9aa4d49c451ee899f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1695689
Commit-Queue: Adam Raine <asraine@google.com>
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676923}

third_party/blink/renderer/core/animation/BUILD.gn

@@ -28,6 +28,7 @@ blink_core_sources("animation") {
     "color_property_functions.h",
     "compositor_animations.cc",
     "compositor_animations.h",
+    "css/compositor_keyframe_color.h",
     "css/compositor_keyframe_double.h",
     "css/compositor_keyframe_filter_operations.h",
     "css/compositor_keyframe_transform.h",

third_party/blink/renderer/core/animation/compositor_animations.cc

@@ -271,7 +271,8 @@ CompositorAnimations::CheckCanStartEffectOnCompositor(
           // property types. Otherwise they are treated as unsupported.
           if (keyframe->GetCompositorKeyframeValue()) {
             DCHECK(RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled());
-            DCHECK(keyframe->GetCompositorKeyframeValue()->IsDouble());
+            DCHECK(keyframe->GetCompositorKeyframeValue()->IsDouble() ||
+                   keyframe->GetCompositorKeyframeValue()->IsColor());
           } else {
             // We skip the rest of the loop in this case for the same reason as
             // unsupported CSS properties - see below.

third_party/blink/renderer/core/animation/compositor_animations_test.cc

@@ -74,6 +74,7 @@
 #include "third_party/blink/renderer/platform/transforms/transform_operations.h"
 #include "third_party/blink/renderer/platform/transforms/translate_transform_operation.h"
 #include "third_party/blink/renderer/platform/wtf/hash_functions.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 namespace blink {
 
@@ -586,8 +587,10 @@ TEST_P(AnimationCompositorAnimationsTest,
   ScopedOffMainThreadCSSPaintForTest off_main_thread_css_paint(true);
   RegisterProperty(GetDocument(), "--foo", "<number>", "0", false);
   RegisterProperty(GetDocument(), "--bar", "<length>", "10px", false);
+  RegisterProperty(GetDocument(), "--loo", "<color>", "rgb(0, 0, 0)", false);
   SetCustomProperty("--foo", "10");
   SetCustomProperty("--bar", "10px");
+  SetCustomProperty("--loo", "rgb(0, 255, 0)");
 
   auto style = GetDocument().EnsureStyleResolver().StyleForElement(element_);
   EXPECT_TRUE(style->NonInheritedVariables());
@@ -597,11 +600,20 @@ TEST_P(AnimationCompositorAnimationsTest,
   EXPECT_TRUE(style->NonInheritedVariables()
                   ->GetData(AtomicString("--bar"))
                   .value_or(nullptr));
+  EXPECT_TRUE(style->NonInheritedVariables()
+                  ->GetData(AtomicString("--loo"))
+                  .value_or(nullptr));
 
   StringKeyframe* keyframe = CreateReplaceOpKeyframe("--foo", "10");
   EXPECT_EQ(DuplicateSingleKeyframeAndTestIsCandidateOnResult(keyframe),
             CompositorAnimations::kNoFailure);
 
+  // Color-valued properties are supported
+  StringKeyframe* color_keyframe =
+      CreateReplaceOpKeyframe("--loo", "rgb(0, 255, 0)");
+  EXPECT_EQ(DuplicateSingleKeyframeAndTestIsCandidateOnResult(color_keyframe),
+            CompositorAnimations::kNoFailure);
+
   // Length-valued properties are not compositable.
   StringKeyframe* non_animatable_keyframe =
       CreateReplaceOpKeyframe("--bar", "10px");

third_party/blink/renderer/core/animation/css/compositor_keyframe_color.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_ANIMATION_CSS_COMPOSITOR_KEYFRAME_COLOR_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_ANIMATION_CSS_COMPOSITOR_KEYFRAME_COLOR_H_
+
+#include "third_party/blink/renderer/core/animation/css/compositor_keyframe_value.h"
+#include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/skia/include/core/SkColor.h"
+
+namespace blink {
+
+class CORE_EXPORT CompositorKeyframeColor final
+    : public CompositorKeyframeValue {
+ public:
+  CompositorKeyframeColor(SkColor color) : color_(color) {}
+  ~CompositorKeyframeColor() override = default;
+
+  static CompositorKeyframeColor* Create(SkColor color) {
+    return MakeGarbageCollected<CompositorKeyframeColor>(color);
+  }
+
+  SkColor ToColor() const { return color_; }
+
+ private:
+  Type GetType() const override { return Type::kColor; }
+
+  SkColor color_;
+};
+
+DEFINE_COMPOSITOR_KEYFRAME_VALUE_TYPE_CASTS(CompositorKeyframeColor, IsColor());
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_ANIMATION_CSS_COMPOSITOR_KEYFRAME_COLOR_H_

third_party/blink/renderer/core/animation/css/compositor_keyframe_value.h

@@ -21,6 +21,7 @@ class CORE_EXPORT CompositorKeyframeValue
     return GetType() == Type::kFilterOperations;
   }
   bool IsTransform() const { return GetType() == Type::kTransform; }
+  bool IsColor() const { return GetType() == Type::kColor; }
 
   virtual void Trace(Visitor*) {}
 
@@ -29,6 +30,7 @@ class CORE_EXPORT CompositorKeyframeValue
     kDouble,
     kFilterOperations,
     kTransform,
+    kColor,
   };
 
  private:

third_party/blink/renderer/core/animation/css/compositor_keyframe_value_factory.cc

@@ -5,10 +5,12 @@
 #include "third_party/blink/renderer/core/animation/css/compositor_keyframe_value_factory.h"
 
 #include "third_party/blink/renderer/core/animation/compositor_animations.h"
+#include "third_party/blink/renderer/core/animation/css/compositor_keyframe_color.h"
 #include "third_party/blink/renderer/core/animation/css/compositor_keyframe_double.h"
 #include "third_party/blink/renderer/core/animation/css/compositor_keyframe_filter_operations.h"
 #include "third_party/blink/renderer/core/animation/css/compositor_keyframe_transform.h"
 #include "third_party/blink/renderer/core/animation/property_handle.h"
+#include "third_party/blink/renderer/core/css/css_color_value.h"
 #include "third_party/blink/renderer/core/css/properties/css_property.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
@@ -68,11 +70,22 @@ CompositorKeyframeValue* CompositorKeyframeValueFactory::Create(
       }
       const AtomicString& property_name = property.CustomPropertyName();
       const CSSValue* value = style.GetVariableValue(property_name);
+
       const auto* primitive_value = DynamicTo<CSSPrimitiveValue>(value);
-      if (!primitive_value || !primitive_value->IsNumber())
-        return nullptr;
+      if (primitive_value && primitive_value->IsNumber()) {
+        return CompositorKeyframeDouble::Create(
+            primitive_value->GetFloatValue());
+      }
 
-      return CompositorKeyframeDouble::Create(primitive_value->GetFloatValue());
+      // TODO: Add supported for interpolable color values from
+      // CSSIdentifierValue when given a value of currentcolor
+      if (const auto* color_value = DynamicTo<cssvalue::CSSColorValue>(value)) {
+        Color color = color_value->Value();
+        return CompositorKeyframeColor::Create(SkColorSetARGB(
+            color.Alpha(), color.Red(), color.Green(), color.Blue()));
+      }
+
+      return nullptr;
     }
     default:
       NOTREACHED();

third_party/blink/renderer/core/animation/keyframe_effect_model_test.cc

@@ -32,6 +32,9 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/core/animation/animation_test_helper.h"
+#include "third_party/blink/renderer/core/animation/css/compositor_keyframe_color.h"
+#include "third_party/blink/renderer/core/animation/css/compositor_keyframe_double.h"
+#include "third_party/blink/renderer/core/animation/css/compositor_keyframe_value_factory.h"
 #include "third_party/blink/renderer/core/animation/css_default_interpolation_type.h"
 #include "third_party/blink/renderer/core/animation/invalidatable_interpolation.h"
 #include "third_party/blink/renderer/core/animation/string_keyframe.h"
@@ -45,6 +48,7 @@
 #include "third_party/blink/renderer/core/testing/page_test_base.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 namespace blink {
 
@@ -131,6 +135,39 @@ StringKeyframeVector KeyframesAtZeroAndOne(
   return keyframes;
 }
 
+const PropertySpecificKeyframeVector& ConstructEffectAndGetKeyframes(
+    const AtomicString& property_name,
+    const AtomicString& type,
+    Document* document,
+    Element* element,
+    const String& zero_value,
+    const String& one_value,
+    ExceptionState& exception_state) {
+  PropertyDescriptor* property_descriptor = PropertyDescriptor::Create();
+  property_descriptor->setName(property_name);
+  property_descriptor->setSyntax(type);
+  property_descriptor->setInitialValue(zero_value);
+  property_descriptor->setInherits(false);
+  PropertyRegistration::registerProperty(document, property_descriptor,
+                                         exception_state);
+
+  StringKeyframeVector keyframes = KeyframesAtZeroAndOne(
+      property_name, document->GetPropertyRegistry(), zero_value, one_value);
+
+  element->style()->setProperty(document, property_name, zero_value,
+                                g_empty_string, exception_state);
+
+  auto* effect = MakeGarbageCollected<StringKeyframeEffectModel>(keyframes);
+
+  auto style = document->EnsureStyleResolver().StyleForElement(element);
+
+  // Snapshot should update first time after construction
+  EXPECT_TRUE(effect->SnapshotAllCompositorKeyframesIfNecessary(
+      *element, *style, nullptr));
+
+  return effect->GetPropertySpecificKeyframes(PropertyHandle(property_name));
+}
+
 void ExpectProperty(CSSPropertyID property,
                     Interpolation* interpolation_value) {
   InvalidatableInterpolation* interpolation =
@@ -666,39 +703,95 @@ TEST_F(AnimationKeyframeEffectModel,
 TEST_F(AnimationKeyframeEffectModel, CompositorSnapshotUpdateCustomProperty) {
   ScopedOffMainThreadCSSPaintForTest off_main_thread_css_paint(true);
   DummyExceptionStateForTesting exception_state;
-  PropertyDescriptor* property_descriptor = PropertyDescriptor::Create();
-  property_descriptor->setName("--foo");
-  property_descriptor->setSyntax("<number>");
-  property_descriptor->setInitialValue("0");
-  property_descriptor->setInherits(false);
-  PropertyRegistration::registerProperty(&GetDocument(), property_descriptor,
-                                         exception_state);
-  EXPECT_FALSE(exception_state.HadException());
 
-  StringKeyframeVector keyframes = KeyframesAtZeroAndOne(
-      AtomicString("--foo"), GetDocument().GetPropertyRegistry(), "0", "100");
-
-  element->style()->setProperty(&GetDocument(), "--foo", "0", g_empty_string,
-                                exception_state);
-  EXPECT_FALSE(exception_state.HadException());
-
-  auto* effect = MakeGarbageCollected<StringKeyframeEffectModel>(keyframes);
+  // Compositor keyframe value available after snapshot
+  const CompositorKeyframeValue* value =
+      ConstructEffectAndGetKeyframes("--foo", "<number>", &GetDocument(),
+                                     element, "0", "100", exception_state)[1]
+          ->GetCompositorKeyframeValue();
+  ASSERT_FALSE(exception_state.HadException());
 
-  auto style = GetDocument().EnsureStyleResolver().StyleForElement(element);
+  // Test value holds the correct number type
+  EXPECT_TRUE(value);
+  EXPECT_TRUE(value->IsDouble());
+  EXPECT_EQ(ToCompositorKeyframeDouble(value)->ToDouble(), 100);
+}
 
-  const CompositorKeyframeValue* value;
+TEST_F(AnimationKeyframeEffectModel, CompositorUpdateColorProperty) {
+  ScopedOffMainThreadCSSPaintForTest off_main_thread_css_paint(true);
+  DummyExceptionStateForTesting exception_state;
 
-  // Snapshot should update first time after construction
-  EXPECT_TRUE(effect->SnapshotAllCompositorKeyframesIfNecessary(
-      *element, *style, nullptr));
+  element->style()->setProperty(&GetDocument(), "color", "rgb(0, 255, 0)",
+                                g_empty_string, exception_state);
 
   // Compositor keyframe value available after snapshot
-  value = effect
-              ->GetPropertySpecificKeyframes(
-                  PropertyHandle(AtomicString("--foo")))[0]
-              ->GetCompositorKeyframeValue();
-  EXPECT_TRUE(value);
-  EXPECT_TRUE(value->IsDouble());
+  const CompositorKeyframeValue* value_rgb =
+      ConstructEffectAndGetKeyframes("--rgb", "<color>", &GetDocument(),
+                                     element, "rgb(0, 0, 0)", "rgb(0, 255, 0)",
+                                     exception_state)[1]
+          ->GetCompositorKeyframeValue();
+  const CompositorKeyframeValue* value_hsl =
+      ConstructEffectAndGetKeyframes("--hsl", "<color>", &GetDocument(),
+                                     element, "hsl(0, 0%, 0%)",
+                                     "hsl(120, 100%, 50%)", exception_state)[1]
+          ->GetCompositorKeyframeValue();
+  const CompositorKeyframeValue* value_name =
+      ConstructEffectAndGetKeyframes("--named", "<color>", &GetDocument(),
+                                     element, "black", "lime",
+                                     exception_state)[1]
+          ->GetCompositorKeyframeValue();
+  const CompositorKeyframeValue* value_hex =
+      ConstructEffectAndGetKeyframes("--hex", "<color>", &GetDocument(),
+                                     element, "#000000", "#00FF00",
+                                     exception_state)[1]
+          ->GetCompositorKeyframeValue();
+  const CompositorKeyframeValue* value_curr =
+      ConstructEffectAndGetKeyframes("--curr", "<color>", &GetDocument(),
+                                     element, "#000000", "currentcolor",
+                                     exception_state)[1]
+          ->GetCompositorKeyframeValue();
+  const PropertySpecificKeyframeVector& values_mixed =
+      ConstructEffectAndGetKeyframes("--mixed", "<color>", &GetDocument(),
+                                     element, "#000000", "lime",
+                                     exception_state);
+  ASSERT_FALSE(exception_state.HadException());
+
+  // Test rgb color input
+  EXPECT_TRUE(value_rgb);
+  EXPECT_TRUE(value_rgb->IsColor());
+  EXPECT_EQ(ToCompositorKeyframeColor(value_rgb)->ToColor(), SK_ColorGREEN);
+
+  // Test hsl color input
+  EXPECT_TRUE(value_hsl);
+  EXPECT_TRUE(value_hsl->IsColor());
+  EXPECT_EQ(ToCompositorKeyframeColor(value_hsl)->ToColor(), SK_ColorGREEN);
+
+  // Test named color input
+  EXPECT_TRUE(value_name);
+  EXPECT_TRUE(value_name->IsColor());
+  EXPECT_EQ(ToCompositorKeyframeColor(value_name)->ToColor(), SK_ColorGREEN);
+
+  // Test hex color input
+  EXPECT_TRUE(value_hex);
+  EXPECT_TRUE(value_hex->IsColor());
+  EXPECT_EQ(ToCompositorKeyframeColor(value_hex)->ToColor(), SK_ColorGREEN);
+
+  // currentcolor is a CSSIdentifierValue not a color
+  EXPECT_FALSE(value_curr);
+
+  // Ensure both frames are consistent when values are mixed
+  const CompositorKeyframeValue* value_mixed0 =
+      values_mixed[0]->GetCompositorKeyframeValue();
+  const CompositorKeyframeValue* value_mixed1 =
+      values_mixed[1]->GetCompositorKeyframeValue();
+
+  EXPECT_TRUE(value_mixed0);
+  EXPECT_TRUE(value_mixed0->IsColor());
+  EXPECT_EQ(ToCompositorKeyframeColor(value_mixed0)->ToColor(), SK_ColorBLACK);
+
+  EXPECT_TRUE(value_mixed1);
+  EXPECT_TRUE(value_mixed1->IsColor());
+  EXPECT_EQ(ToCompositorKeyframeColor(value_mixed1)->ToColor(), SK_ColorGREEN);
 }
 
 }  // namespace blink