From 933c5ad88434ffb3549651834f8890b95bdef121 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Fri, 29 Oct 2021 19:58:24 +0000
Subject: [PATCH] Bug 1525107 - Move Canvas/Link color computation to C++-land.
 r=dholbert

This doesn't change behavior but will allow us to deduplicate some
logic given we compute the effective color-scheme in C++.

Differential Revision: https://phabricator.services.mozilla.com/D129744
---
 layout/style/GeckoBindings.cpp                | 26 ++++++++++++---
 layout/style/GeckoBindings.h                  | 10 +++---
 .../style/values/specified/color.rs           | 33 +++++++------------
 3 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/layout/style/GeckoBindings.cpp b/layout/style/GeckoBindings.cpp
index f4ba48f11c00..578036c9a978 100644
--- a/layout/style/GeckoBindings.cpp
+++ b/layout/style/GeckoBindings.cpp
@@ -687,14 +687,30 @@ bool Gecko_IsDocumentBody(const Element* aElement) {
   return doc && doc->GetBodyElement() == aElement;
 }
 
-nscolor Gecko_GetLookAndFeelSystemColor(int32_t aId, const Document* aDoc,
-                                        const StyleColorScheme* aStyle) {
-  auto colorId = static_cast<LookAndFeel::ColorID>(aId);
-  auto useStandins = LookAndFeel::ShouldUseStandins(*aDoc, colorId);
+nscolor Gecko_ComputeSystemColor(StyleSystemColor aColor, const Document* aDoc,
+                                 const StyleColorScheme* aStyle) {
+  const auto& colors = PreferenceSheet::PrefsFor(*aDoc).mColors;
+  // TODO: These should be color-scheme aware too.
+  switch (aColor) {
+    case StyleSystemColor::Canvastext:
+      return colors.mDefault;
+    case StyleSystemColor::Canvas:
+      return colors.mDefaultBackground;
+    case StyleSystemColor::Linktext:
+      return colors.mLink;
+    case StyleSystemColor::Activetext:
+      return colors.mActiveLink;
+    case StyleSystemColor::Visitedtext:
+      return colors.mVisitedLink;
+    default:
+      break;
+  }
+
   auto colorScheme = LookAndFeel::ColorSchemeForStyle(*aDoc, aStyle->bits);
+  auto useStandins = LookAndFeel::ShouldUseStandins(*aDoc, aColor);
 
   AutoWriteLock guard(*sServoFFILock);
-  return LookAndFeel::Color(colorId, colorScheme, useStandins);
+  return LookAndFeel::Color(aColor, colorScheme, useStandins);
 }
 
 int32_t Gecko_GetLookAndFeelInt(int32_t aId) {
diff --git a/layout/style/GeckoBindings.h b/layout/style/GeckoBindings.h
index 23b56013a8b7..07b231168045 100644
--- a/layout/style/GeckoBindings.h
+++ b/layout/style/GeckoBindings.h
@@ -502,12 +502,12 @@ void Gecko_StyleSheet_AddRef(const mozilla::StyleSheet* aSheet);
 void Gecko_StyleSheet_Release(const mozilla::StyleSheet* aSheet);
 bool Gecko_IsDocumentBody(const mozilla::dom::Element* element);
 
-// We use an int32_t here instead of a LookAndFeel::ColorID
-// because forward-declaring a nested enum/struct is impossible
-nscolor Gecko_GetLookAndFeelSystemColor(int32_t color_id,
-                                        const mozilla::dom::Document*,
-                                        const mozilla::StyleColorScheme*);
+nscolor Gecko_ComputeSystemColor(mozilla::StyleSystemColor,
+                                 const mozilla::dom::Document*,
+                                 const mozilla::StyleColorScheme*);
 
+// We use an int32_t here instead of a LookAndFeel::IntID/FloatID because
+// forward-declaring a nested enum/struct is impossible.
 int32_t Gecko_GetLookAndFeelInt(int32_t int_id);
 float Gecko_GetLookAndFeelFloat(int32_t float_id);
 
diff --git a/servo/components/style/values/specified/color.rs b/servo/components/style/values/specified/color.rs
index 0d32655999e7..790834047fd7 100644
--- a/servo/components/style/values/specified/color.rs
+++ b/servo/components/style/values/specified/color.rs
@@ -455,28 +455,17 @@ impl SystemColor {
     fn compute(&self, cx: &Context) -> ComputedColor {
         use crate::gecko_bindings::bindings;
 
-        let colors = &cx.device().pref_sheet_prefs().mColors;
-        let style_color_scheme = cx.style().get_inherited_ui().clone_color_scheme();
-
-        // TODO: At least Canvas / CanvasText should be color-scheme aware
-        // (probably the link colors too).
-        convert_nscolor_to_computedcolor(match *self {
-            SystemColor::Canvastext => colors.mDefault,
-            SystemColor::Canvas => colors.mDefaultBackground,
-            SystemColor::Linktext => colors.mLink,
-            SystemColor::Activetext => colors.mActiveLink,
-            SystemColor::Visitedtext => colors.mVisitedLink,
-
-            _ => {
-                let color = unsafe {
-                    bindings::Gecko_GetLookAndFeelSystemColor(*self as i32, cx.device().document(), &style_color_scheme)
-                };
-                if color == bindings::NS_SAME_AS_FOREGROUND_COLOR {
-                    return ComputedColor::currentcolor();
-                }
-                color
-            },
-        })
+        // TODO: We should avoid cloning here most likely, though it's
+        // cheap-ish.
+        let style_color_scheme =
+            cx.style().get_inherited_ui().clone_color_scheme();
+        let color = unsafe {
+            bindings::Gecko_ComputeSystemColor(*self, cx.device().document(), &style_color_scheme)
+        };
+        if color == bindings::NS_SAME_AS_FOREGROUND_COLOR {
+            return ComputedColor::currentcolor();
+        }
+        convert_nscolor_to_computedcolor(color)
     }
 }