Make color-scheme affect used system color.

We currently compute system colors as their rgba values based on
color-scheme. However, color-scheme was not a high priority property, so
changing color-scheme on the same element as the system color was set
did not always yield the correct result as the color-scheme might have
been applied too late.

There is an open issue[1] for having system colors computing to
themselves and resolve to an rgba value for the used value. That would
mean we will have to change the internal storage for colors in
ComputedStyle and possibly store a StyleColor with system color keywords
in addition to currentcolor which currently computes to the keyword.

For now, instead of introducing an even higher priority level, make
color-scheme a high priority property, and delay application of the
cascaded color property value to the ComputedStyle until we have applied
other high priority properties.

[1] https://github.com/w3c/csswg-drafts/issues/3847

Bug: 939811
Change-Id: I91ce04a55886507ee20aace9b2470a5ac5b43fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1773073
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693151}

third_party/blink/renderer/core/css/css_properties.json5

@@ -1536,6 +1536,7 @@
       type_name: "Vector<AtomicString>",
       default_value: "Vector<AtomicString, 0>()",
       field_template: "external",
+      priority: "High",
     },
     {
       name: "column-fill",
@@ -5210,11 +5211,7 @@
       default_value: "Color::kBlack",
       type_name: "Color",
       computed_style_custom_functions: ["getter", "setter"],
-      style_builder_custom_functions: ['value'],
-      style_builder_template: "visited_color",
-      style_builder_template_args: {
-        initial_color: "ComputedStyleInitialValues::InitialColor",
-      },
+      style_builder_custom_functions: ["initial", "inherit", "value"],
       priority: "High",
     },
     {

third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc

@@ -1401,11 +1401,21 @@ const CSSValue* Color::CSSValueFromComputedStyleInternal(
 }
 
 void Color::ApplyInitial(StyleResolverState& state) const {
+  if (!RuntimeEnabledFeatures::CSSCascadeEnabled()) {
+    state.SetCascadedColorValue(
+        CSSIdentifierValue::Create(CSSValueID::kInitial));
+    return;
+  }
   blink::Color color = ComputedStyleInitialValues::InitialColor();
   state.Style()->SetColor(color);
 }
 
 void Color::ApplyInherit(StyleResolverState& state) const {
+  if (!RuntimeEnabledFeatures::CSSCascadeEnabled()) {
+    state.SetCascadedColorValue(
+        CSSIdentifierValue::Create(CSSValueID::kInherit));
+    return;
+  }
   blink::Color color = state.ParentStyle()->GetColor();
   if (state.ParentStyle()->IsColorInternalText())
     state.Style()->SetIsColorInternalText(true);
@@ -1414,6 +1424,10 @@ void Color::ApplyInherit(StyleResolverState& state) const {
 }
 
 void Color::ApplyValue(StyleResolverState& state, const CSSValue& value) const {
+  if (!RuntimeEnabledFeatures::CSSCascadeEnabled()) {
+    state.SetCascadedColorValue(&value);
+    return;
+  }
   // As per the spec, 'color: currentColor' is treated as 'color: inherit'
   auto* identifier_value = DynamicTo<CSSIdentifierValue>(value);
   if (identifier_value &&
@@ -2821,8 +2835,32 @@ const CSSValue* InternalEffectiveZoom::ParseSingleValue(
   return css_property_parser_helpers::ConsumeNumber(range, value_range);
 }
 
+void InternalVisitedColor::ApplyInitial(StyleResolverState& state) const {
+  if (!RuntimeEnabledFeatures::CSSCascadeEnabled()) {
+    state.SetCascadedVisitedColorValue(
+        CSSIdentifierValue::Create(CSSValueID::kInitial));
+    return;
+  }
+  auto color = ComputedStyleInitialValues::InitialColor();
+  state.Style()->SetInternalVisitedColor(color);
+}
+
+void InternalVisitedColor::ApplyInherit(StyleResolverState& state) const {
+  if (!RuntimeEnabledFeatures::CSSCascadeEnabled()) {
+    state.SetCascadedVisitedColorValue(
+        CSSIdentifierValue::Create(CSSValueID::kInherit));
+    return;
+  }
+  auto color = state.ParentStyle()->GetColor();
+  state.Style()->SetInternalVisitedColor(color);
+}
+
 void InternalVisitedColor::ApplyValue(StyleResolverState& state,
                                       const CSSValue& value) const {
+  if (!RuntimeEnabledFeatures::CSSCascadeEnabled()) {
+    state.SetCascadedVisitedColorValue(&value);
+    return;
+  }
   auto* identifier_value = DynamicTo<CSSIdentifierValue>(value);
   if (identifier_value &&
       identifier_value->GetValueID() == CSSValueID::kCurrentcolor) {

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

@@ -87,7 +87,7 @@ inline CSSPropertyID CSSPropertyPriorityData<kHighPropertyPriority>::First() {
 template <>
 inline CSSPropertyID CSSPropertyPriorityData<kHighPropertyPriority>::Last() {
   static_assert(static_cast<int>(CSSPropertyID::kZoom) ==
-                    static_cast<int>(CSSPropertyID::kColor) + 25,
+                    static_cast<int>(CSSPropertyID::kColor) + 26,
                 "CSSPropertyID::kZoom should be the end of the high priority "
                 "property range");
   static_assert(static_cast<int>(CSSPropertyID::kWritingMode) ==

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

@@ -71,6 +71,7 @@
 #include "third_party/blink/renderer/core/css/resolver/selector_filter_parent_scope.h"
 #include "third_party/blink/renderer/core/css/resolver/style_adjuster.h"
 #include "third_party/blink/renderer/core/css/resolver/style_animator.h"
+#include "third_party/blink/renderer/core/css/resolver/style_builder_converter.h"
 #include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
 #include "third_party/blink/renderer/core/css/resolver/style_resolver_stats.h"
 #include "third_party/blink/renderer/core/css/resolver/style_rule_usage_tracker.h"
@@ -1886,6 +1887,8 @@ void StyleResolver::ApplyMatchedHighPriorityProperties(
     apply_inherited_only = false;
   }
 
+  ApplyCascadedColorValue(state);
+
   // If our font got dirtied, go ahead and update it now.
   UpdateFont(state);
 
@@ -2297,4 +2300,67 @@ bool StyleResolver::IsForcedColorsModeEnabled() const {
   return GetDocument().InForcedColorsMode();
 }
 
+void StyleResolver::ApplyCascadedColorValue(StyleResolverState& state) {
+  if (RuntimeEnabledFeatures::CSSCascadeEnabled())
+    return;
+
+  if (const CSSValue* color_value = state.GetCascadedColorValue()) {
+    state.SetCascadedColorValue(nullptr);
+    const auto* identifier_value = DynamicTo<CSSIdentifierValue>(color_value);
+    if (identifier_value) {
+      switch (identifier_value->GetValueID()) {
+        case CSSValueID::kCurrentcolor:
+          // As per the spec, 'color: currentColor' is treated as 'color:
+          // inherit'
+        case CSSValueID::kInherit:
+          if (state.ParentStyle()->IsColorInternalText())
+            state.Style()->SetIsColorInternalText(true);
+          else
+            state.Style()->SetColor(state.ParentStyle()->GetColor());
+          break;
+        case CSSValueID::kInitial:
+          state.Style()->SetColor(ComputedStyleInitialValues::InitialColor());
+          break;
+        default:
+          identifier_value = nullptr;
+          break;
+      }
+    }
+    if (!identifier_value) {
+      state.Style()->SetColor(
+          StyleBuilderConverter::ConvertColor(state, *color_value));
+    }
+  }
+
+  if (const CSSValue* visited_color_value =
+          state.GetCascadedVisitedColorValue()) {
+    state.SetCascadedVisitedColorValue(nullptr);
+    const auto* identifier_value =
+        DynamicTo<CSSIdentifierValue>(visited_color_value);
+    if (identifier_value) {
+      switch (identifier_value->GetValueID()) {
+        case CSSValueID::kCurrentcolor:
+          // As per the spec, 'color: currentColor' is treated as 'color:
+          // inherit'
+        case CSSValueID::kInherit:
+          state.Style()->SetInternalVisitedColor(
+              state.ParentStyle()->GetColor());
+          break;
+        case CSSValueID::kInitial:
+          state.Style()->SetInternalVisitedColor(
+              ComputedStyleInitialValues::InitialColor());
+          break;
+        default:
+          identifier_value = nullptr;
+          break;
+      }
+    }
+    if (!identifier_value) {
+      state.Style()->SetInternalVisitedColor(
+          StyleBuilderConverter::ConvertColor(state, *visited_color_value,
+                                              true));
+    }
+  }
+}
+
 }  // namespace blink

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

@@ -297,6 +297,8 @@ class CORE_EXPORT StyleResolver final
                         ValidPropertyFilter,
                         unsigned apply_mask);
 
+  void ApplyCascadedColorValue(StyleResolverState&);
+
   bool PseudoStyleForElementInternal(Element&,
                                      const PseudoStyleRequest&,
                                      StyleResolverState&);

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

@@ -174,6 +174,20 @@ class CORE_EXPORT StyleResolverState {
   void SetHasDirAutoAttribute(bool value) { has_dir_auto_attribute_ = value; }
   bool HasDirAutoAttribute() const { return has_dir_auto_attribute_; }
 
+  const CSSValue* GetCascadedColorValue() const {
+    return cascaded_color_value_;
+  }
+  const CSSValue* GetCascadedVisitedColorValue() const {
+    return cascaded_visited_color_value_;
+  }
+
+  void SetCascadedColorValue(const CSSValue* color) {
+    cascaded_color_value_ = color;
+  }
+  void SetCascadedVisitedColorValue(const CSSValue* color) {
+    cascaded_visited_color_value_ = color;
+  }
+
   HeapHashMap<CSSPropertyID, Member<const CSSValue>>&
   ParsedPropertiesForPendingSubstitutionCache(
       const cssvalue::CSSPendingSubstitutionValue&) const;
@@ -213,6 +227,9 @@ class CORE_EXPORT StyleResolverState {
 
   bool has_dir_auto_attribute_;
 
+  Member<const CSSValue> cascaded_color_value_;
+  Member<const CSSValue> cascaded_visited_color_value_;
+
   FontBuilder font_builder_;
 
   std::unique_ptr<CachedUAStyle> cached_ua_style_;

third_party/blink/renderer/core/layout/layout_theme_default.cc

@@ -73,8 +73,12 @@ Color LayoutThemeDefault::SystemColor(CSSValueID css_value_id,
   constexpr Color kDefaultMenuColor(0xfff7f7f7);
 
   if (css_value_id == CSSValueID::kButtonface) {
-    if (UseMockTheme())
-      return Color(0xc0, 0xc0, 0xc0);
+    if (UseMockTheme()) {
+      if (color_scheme == WebColorScheme::kLight)
+        return Color(0xc0, 0xc0, 0xc0);
+      else
+        return Color(0x80, 0x80, 0x80);
+    }
     return kDefaultButtonGrayColor;
   }
   if (css_value_id == CSSValueID::kMenu)

third_party/blink/renderer/core/layout/layout_theme_test.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/layout/layout_theme.h"
 
 #include <memory>
+#include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/core/css/style_engine.h"
 #include "third_party/blink/renderer/core/dom/document.h"
@@ -18,6 +19,7 @@
 #include "third_party/blink/renderer/core/testing/page_test_base.h"
 #include "third_party/blink/renderer/platform/graphics/color.h"
 #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
+#include "third_party/blink/renderer/platform/web_test_support.h"
 
 namespace blink {
 
@@ -26,6 +28,16 @@ class LayoutThemeTest : public PageTestBase,
  protected:
   LayoutThemeTest() : ScopedCSSColorSchemeForTest(true) {}
   void SetHtmlInnerHTML(const char* html_content);
+
+  void SetUp() override {
+    WebTestSupport::SetMockThemeEnabledForTest(true);
+    PageTestBase::SetUp();
+  }
+
+  void TearDown() override {
+    PageTestBase::TearDown();
+    WebTestSupport::SetMockThemeEnabledForTest(false);
+  }
 };
 
 void LayoutThemeTest::SetHtmlInnerHTML(const char* html_content) {
@@ -124,4 +136,38 @@ TEST_F(LayoutThemeTest, RootElementColorChange) {
             initial_style->VisitedDependentColor(GetCSSPropertyColor()));
 }
 
+// Mock theming is done on LayoutThemeDefault which is not a base class for
+// LayoutThemeMac.
+#if !defined(OS_MACOSX)
+TEST_F(LayoutThemeTest, SystemColorWithColorScheme) {
+  SetHtmlInnerHTML(R"HTML(
+    <style>
+      #dark {
+        color: buttonface;
+        color-scheme: dark;
+      }
+    </style>
+    <div id="dark"></div>
+  )HTML");
+
+  Element* dark_element = GetDocument().getElementById("dark");
+  ASSERT_TRUE(dark_element);
+
+  const ComputedStyle* style = dark_element->GetComputedStyle();
+  EXPECT_EQ(WebColorScheme::kLight, style->UsedColorScheme());
+  EXPECT_EQ(Color(0xc0, 0xc0, 0xc0),
+            style->VisitedDependentColor(GetCSSPropertyColor()));
+
+  // Change color scheme to dark.
+  GetDocument().GetSettings()->SetPreferredColorScheme(
+      PreferredColorScheme::kDark);
+  UpdateAllLifecyclePhasesForTest();
+
+  style = dark_element->GetComputedStyle();
+  EXPECT_EQ(WebColorScheme::kDark, style->UsedColorScheme());
+  EXPECT_EQ(Color(0x80, 0x80, 0x80),
+            style->VisitedDependentColor(GetCSSPropertyColor()));
+}
+#endif  // !defined(OS_MACOSX)
+
 }  // namespace blink