Load Mac system colors for both light and dark scheme.

System colors used in CSS should per spec depend on the used
color-scheme for that element. Since the used color-scheme may not be
the same as the preferred color-scheme, we need to provide the system
colors for both light and dark at the same time. This CL temporarily
sets the appearance to light and dark aqua to retrieve those colors.

TEST=StyleEngineTest.InitialColorChange

Bug: 929098
Change-Id: I4bb9553cf1b2fc4e98cd1c7c13041adce3e50353
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860023
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744630}

content/browser/theme_helper_mac.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_BROWSER_THEME_HELPER_MAC_H_
 #define CONTENT_BROWSER_THEME_HELPER_MAC_H_
 
+#include "base/containers/span.h"
 #include "base/macros.h"
 #include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/writable_shared_memory_region.h"
@@ -12,6 +13,7 @@
 #include "content/public/browser/notification_registrar.h"
 #include "third_party/blink/public/common/sandbox_support/sandbox_support_mac.h"
 #include "third_party/blink/public/platform/mac/web_scrollbar_theme.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 #if __OBJC__
 @class SystemThemeObserver;
@@ -39,9 +41,16 @@ class ThemeHelperMac : public NotificationObserver {
   ~ThemeHelperMac() override;
 
   // Looks up the blink::MacSystemColorID corresponding to the NSColor
-  // selector and stores them in the |writable_color_map_| table.
+  // selector and stores them in the |writable_color_map_| table. Looks up
+  // colors for both light and dark appearances.
   void LoadSystemColors();
 
+  // Looks up system colors for the current appearance, either a light or
+  // dark appearance and stores them in values. The values parameter is the part
+  // of writable_color_map_ where the colors for the current appearance should
+  // be stored.
+  void LoadSystemColorsForCurrentAppearance(base::span<SkColor> values);
+
   // Overridden from NotificationObserver:
   void Observe(int type,
                const NotificationSource& source,
@@ -50,7 +59,9 @@ class ThemeHelperMac : public NotificationObserver {
   // ObjC object that observes notifications from the system.
   SystemThemeObserver* theme_observer_;  // strong
 
-  // Writable and mapped array of SkColor values, indexed by MacSystemColorID.
+  // Writable and mapped array of SkColor values, indexed by MacSystemColorID
+  // for a light appearance. Colors for a dark appearance in indexed by
+  // MacSystemColorID starting at index MacSystemColorID::kCount.
   base::WritableSharedMemoryMapping writable_color_map_;
 
   // Read-only handle to the |writable_color_map_| that can be duplicated for

content/browser/theme_helper_mac.mm

@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/containers/span.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/sys_string_conversions.h"
@@ -300,7 +301,8 @@ ThemeHelperMac::DuplicateReadOnlyColorMapRegion() {
 ThemeHelperMac::ThemeHelperMac() {
   // Allocate a region for the SkColor value table and map it.
   auto writable_region = base::WritableSharedMemoryRegion::Create(
-      sizeof(SkColor) * blink::kMacSystemColorIDCount);
+      sizeof(SkColor) * blink::kMacSystemColorIDCount *
+      blink::kMacSystemColorSchemeCount);
   writable_color_map_ = writable_region.Map();
   // Downgrade the region to read-only after it has been mapped.
   read_only_color_map_ = base::WritableSharedMemoryRegion::ConvertToReadOnly(
@@ -321,20 +323,8 @@ ThemeHelperMac::~ThemeHelperMac() {
   [theme_observer_ release];
 }
 
-void ThemeHelperMac::LoadSystemColors() {
-  base::span<SkColor> values = writable_color_map_.GetMemoryAsSpan<SkColor>(
-      blink::kMacSystemColorIDCount);
-  // Ensure light mode appearance in web content even if the topchrome is in
-  // dark mode.
-  // TODO(lgrey): Add a second map for content dark mode for the
-  // `prefers-color-scheme` media query: https://crbug.com/889087.
-  NSAppearance* savedAppearance;
-  if (@available(macOS 10.14, *)) {
-    savedAppearance = [NSAppearance currentAppearance];
-    [NSAppearance
-        setCurrentAppearance:[NSAppearance
-                                 appearanceNamed:NSAppearanceNameAqua]];
-  }
+void ThemeHelperMac::LoadSystemColorsForCurrentAppearance(
+    base::span<SkColor> values) {
   for (size_t i = 0; i < blink::kMacSystemColorIDCount; ++i) {
     blink::MacSystemColorID color_id = static_cast<blink::MacSystemColorID>(i);
     switch (color_id) {
@@ -431,9 +421,39 @@ void ThemeHelperMac::LoadSystemColors() {
         break;
     }
   }
+}
+
+void ThemeHelperMac::LoadSystemColors() {
+  static_assert(blink::kMacSystemColorSchemeCount == 2,
+                "Light and dark color scheme system colors loaded.");
+  base::span<SkColor> values = writable_color_map_.GetMemoryAsSpan<SkColor>(
+      blink::kMacSystemColorIDCount * blink::kMacSystemColorSchemeCount);
+
+  NSAppearance* savedAppearance;
   if (@available(macOS 10.14, *)) {
-    [NSAppearance setCurrentAppearance:savedAppearance];
+    savedAppearance = [NSAppearance currentAppearance];
+    // Ensure light mode appearance in web content even if the topchrome is in
+    // dark mode.
+    [NSAppearance
+        setCurrentAppearance:[NSAppearance
+                                 appearanceNamed:NSAppearanceNameAqua]];
+  }
+
+  LoadSystemColorsForCurrentAppearance(
+      values.subspan(0, static_cast<size_t>(blink::MacSystemColorID::kCount)));
+
+  if (@available(macOS 10.14, *)) {
+    [NSAppearance
+        setCurrentAppearance:[NSAppearance
+                                 appearanceNamed:NSAppearanceNameDarkAqua]];
   }
+
+  LoadSystemColorsForCurrentAppearance(
+      values.subspan(static_cast<size_t>(blink::MacSystemColorID::kCount),
+                     static_cast<size_t>(blink::MacSystemColorID::kCount)));
+
+  if (@available(macOS 10.14, *))
+    [NSAppearance setCurrentAppearance:savedAppearance];
 }
 
 void ThemeHelperMac::Observe(int type,

content/child/child_process_sandbox_support_impl_mac.cc

@@ -61,14 +61,23 @@ bool WebSandboxSupportMac::LoadFont(
       out_descriptor);
 }
 
-SkColor WebSandboxSupportMac::GetSystemColor(blink::MacSystemColorID color_id) {
+SkColor WebSandboxSupportMac::GetSystemColor(
+    blink::MacSystemColorID color_id,
+    blink::WebColorScheme color_scheme) {
   if (!color_map_.IsValid()) {
     DLOG(ERROR) << "GetSystemColor does not have a valid color_map_";
     return SK_ColorMAGENTA;
   }
-  base::span<const SkColor> color_map =
-      color_map_.GetMemoryAsSpan<SkColor>(blink::kMacSystemColorIDCount);
-  return color_map[static_cast<size_t>(color_id)];
+  static_assert(blink::kMacSystemColorSchemeCount == 2,
+                "Light and dark color scheme system colors loaded.");
+  base::span<const SkColor> color_map = color_map_.GetMemoryAsSpan<SkColor>(
+      blink::kMacSystemColorIDCount * blink::kMacSystemColorSchemeCount);
+  base::span<const SkColor> color_map_for_scheme =
+      color_map.subspan(color_scheme == blink::WebColorScheme::kDark
+                            ? blink::kMacSystemColorIDCount
+                            : 0,
+                        blink::kMacSystemColorIDCount);
+  return color_map_for_scheme[static_cast<size_t>(color_id)];
 }
 
 void WebSandboxSupportMac::OnGotSystemColors(

content/child/child_process_sandbox_support_impl_mac.h

@@ -28,7 +28,8 @@ class WebSandboxSupportMac : public blink::WebSandboxSupport {
   bool LoadFont(CTFontRef font,
                 base::ScopedCFTypeRef<CTFontDescriptorRef>* out_descriptor,
                 uint32_t* font_id) override;
-  SkColor GetSystemColor(blink::MacSystemColorID color_id) override;
+  SkColor GetSystemColor(blink::MacSystemColorID color_id,
+                         blink::WebColorScheme color_scheme) override;
 
  private:
   void OnGotSystemColors(base::ReadOnlySharedMemoryRegion region);

third_party/blink/public/common/sandbox_support/sandbox_support_mac.h

@@ -40,6 +40,8 @@ enum class MacSystemColorID {
 constexpr size_t kMacSystemColorIDCount =
     static_cast<size_t>(MacSystemColorID::kCount);
 
+constexpr size_t kMacSystemColorSchemeCount = 2;
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_PUBLIC_COMMON_SANDBOX_SUPPORT_SANDBOX_SUPPORT_MAC_H_

third_party/blink/public/platform/mac/web_sandbox_support.h

@@ -33,6 +33,7 @@
 
 #include "base/mac/scoped_cftyperef.h"
 #include "third_party/blink/public/common/sandbox_support/sandbox_support_mac.h"
+#include "third_party/blink/public/platform/web_color_scheme.h"
 #include "third_party/skia/include/core/SkColor.h"
 
 typedef struct CGFont* CGFontRef;
@@ -56,7 +57,7 @@ class WebSandboxSupport {
       uint32_t* font_id) = 0;
 
   // Returns the system's preferred value for a named color.
-  virtual SkColor GetSystemColor(MacSystemColorID) = 0;
+  virtual SkColor GetSystemColor(MacSystemColorID, WebColorScheme) = 0;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/layout/layout_theme_mac.mm

@@ -180,13 +180,13 @@ bool FontSizeMatchesToControlSize(const ComputedStyle& style) {
   return false;
 }
 
-Color GetSystemColor(MacSystemColorID color_id) {
+Color GetSystemColor(MacSystemColorID color_id, WebColorScheme color_scheme) {
   // In tests, a WebSandboxSupport may not be set up. Just return a dummy
   // color, in this case, black.
   auto* sandbox_support = Platform::Current()->GetSandboxSupport();
   if (!sandbox_support)
     return Color();
-  return sandbox_support->GetSystemColor(color_id);
+  return sandbox_support->GetSystemColor(color_id, color_scheme);
 }
 
 // Helper functions used by a bunch of different control parts.
@@ -243,12 +243,14 @@ LayoutThemeMac::~LayoutThemeMac() {
 
 Color LayoutThemeMac::PlatformActiveSelectionBackgroundColor(
     WebColorScheme color_scheme) const {
-  return GetSystemColor(MacSystemColorID::kSelectedTextBackground);
+  return GetSystemColor(MacSystemColorID::kSelectedTextBackground,
+                        color_scheme);
 }
 
 Color LayoutThemeMac::PlatformInactiveSelectionBackgroundColor(
     WebColorScheme color_scheme) const {
-  return GetSystemColor(MacSystemColorID::kSecondarySelectedControl);
+  return GetSystemColor(MacSystemColorID::kSecondarySelectedControl,
+                        color_scheme);
 }
 
 Color LayoutThemeMac::PlatformActiveSelectionForegroundColor(
@@ -258,7 +260,8 @@ Color LayoutThemeMac::PlatformActiveSelectionForegroundColor(
 
 Color LayoutThemeMac::PlatformActiveListBoxSelectionBackgroundColor(
     WebColorScheme color_scheme) const {
-  return GetSystemColor(MacSystemColorID::kAlternateSelectedControl);
+  return GetSystemColor(MacSystemColorID::kAlternateSelectedControl,
+                        color_scheme);
 }
 
 Color LayoutThemeMac::PlatformActiveListBoxSelectionForegroundColor(
@@ -296,12 +299,14 @@ Color LayoutThemeMac::PlatformInactiveListBoxSelectionBackgroundColor(
 
 Color LayoutThemeMacRefresh::PlatformActiveSelectionBackgroundColor(
     WebColorScheme color_scheme) const {
-  return GetSystemColor(MacSystemColorID::kSelectedTextBackground);
+  return GetSystemColor(MacSystemColorID::kSelectedTextBackground,
+                        color_scheme);
 }
 
 Color LayoutThemeMacRefresh::PlatformInactiveSelectionBackgroundColor(
     WebColorScheme color_scheme) const {
-  return GetSystemColor(MacSystemColorID::kSecondarySelectedControl);
+  return GetSystemColor(MacSystemColorID::kSecondarySelectedControl,
+                        color_scheme);
 }
 
 Color LayoutThemeMacRefresh::PlatformActiveSelectionForegroundColor(
@@ -324,8 +329,11 @@ Color LayoutThemeMacRefresh::FocusRingColor() const {
                                      : kDefaultFocusRingColor;
   }
 
+  // TODO(crbug.com/929098) Need to pass an appropriate color scheme here.
+  WebColorScheme color_scheme = ComputedStyle::InitialStyle().UsedColorScheme();
+
   Color keyboard_focus_indicator =
-      GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator);
+      GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator, color_scheme);
   // Take the RGB values from the keyboard_focus_indicator color, but use a
   // different alpha value to avoid having a color too light.
   Color focus_ring =
@@ -338,9 +346,9 @@ Color LayoutThemeMacRefresh::FocusRingColor() const {
   // changed.
   if (@available(macOS 10.14, *)) {
     static const Color kControlBlueAccentColor =
-        GetSystemColor(MacSystemColorID::kControlAccentBlueColor);
+        GetSystemColor(MacSystemColorID::kControlAccentBlueColor, color_scheme);
     if (kControlBlueAccentColor ==
-        GetSystemColor(MacSystemColorID::kControlAccentColor)) {
+        GetSystemColor(MacSystemColorID::kControlAccentColor, color_scheme)) {
       return GetCustomFocusRingColor();
     }
   } else {
@@ -419,78 +427,84 @@ Color LayoutThemeMac::SystemColor(CSSValueID css_value_id,
                                   WebColorScheme color_scheme) const {
   switch (css_value_id) {
     case CSSValueID::kActiveborder:
-      return GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator);
+      return GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator,
+                            color_scheme);
     case CSSValueID::kActivecaption:
-      return GetSystemColor(MacSystemColorID::kWindowFrameText);
+      return GetSystemColor(MacSystemColorID::kWindowFrameText, color_scheme);
     case CSSValueID::kAppworkspace:
-      return GetSystemColor(MacSystemColorID::kHeader);
+      return GetSystemColor(MacSystemColorID::kHeader, color_scheme);
     case CSSValueID::kBackground:
       // Use theme independent default.
       break;
     case CSSValueID::kButtonface:
-      return GetSystemColor(MacSystemColorID::kControlBackground);
+      return GetSystemColor(MacSystemColorID::kControlBackground, color_scheme);
     case CSSValueID::kButtonhighlight:
-      return GetSystemColor(MacSystemColorID::kControlHighlight);
+      return GetSystemColor(MacSystemColorID::kControlHighlight, color_scheme);
     case CSSValueID::kButtonshadow:
-      return GetSystemColor(MacSystemColorID::kControlShadow);
+      return GetSystemColor(MacSystemColorID::kControlShadow, color_scheme);
     case CSSValueID::kButtontext:
-      return GetSystemColor(MacSystemColorID::kControlText);
+      return GetSystemColor(MacSystemColorID::kControlText, color_scheme);
     case CSSValueID::kCaptiontext:
-      return GetSystemColor(MacSystemColorID::kText);
+      return GetSystemColor(MacSystemColorID::kText, color_scheme);
     case CSSValueID::kField:
-      return GetSystemColor(MacSystemColorID::kControlBackground);
+      return GetSystemColor(MacSystemColorID::kControlBackground, color_scheme);
     case CSSValueID::kFieldtext:
-      return GetSystemColor(MacSystemColorID::kText);
+      return GetSystemColor(MacSystemColorID::kText, color_scheme);
     case CSSValueID::kGraytext:
-      return GetSystemColor(MacSystemColorID::kDisabledControlText);
+      return GetSystemColor(MacSystemColorID::kDisabledControlText,
+                            color_scheme);
     case CSSValueID::kHighlight:
-      return GetSystemColor(MacSystemColorID::kSelectedTextBackground);
+      return GetSystemColor(MacSystemColorID::kSelectedTextBackground,
+                            color_scheme);
     case CSSValueID::kHighlighttext:
-      return GetSystemColor(MacSystemColorID::kSelectedText);
+      return GetSystemColor(MacSystemColorID::kSelectedText, color_scheme);
     case CSSValueID::kInactiveborder:
-      return GetSystemColor(MacSystemColorID::kControlBackground);
+      return GetSystemColor(MacSystemColorID::kControlBackground, color_scheme);
     case CSSValueID::kInactivecaption:
-      return GetSystemColor(MacSystemColorID::kControlBackground);
+      return GetSystemColor(MacSystemColorID::kControlBackground, color_scheme);
     case CSSValueID::kInactivecaptiontext:
-      return GetSystemColor(MacSystemColorID::kText);
+      return GetSystemColor(MacSystemColorID::kText, color_scheme);
     case CSSValueID::kInfobackground:
       // There is no corresponding NSColor for this so we use a hard coded
       // value.
       return 0xFFFBFCC5;
     case CSSValueID::kInfotext:
-      return GetSystemColor(MacSystemColorID::kText);
+      return GetSystemColor(MacSystemColorID::kText, color_scheme);
     case CSSValueID::kMenu:
-      return GetSystemColor(MacSystemColorID::kMenuBackground);
+      return GetSystemColor(MacSystemColorID::kMenuBackground, color_scheme);
     case CSSValueID::kMenutext:
-      return GetSystemColor(MacSystemColorID::kSelectedMenuItemText);
+      return GetSystemColor(MacSystemColorID::kSelectedMenuItemText,
+                            color_scheme);
     case CSSValueID::kScrollbar:
-      return GetSystemColor(MacSystemColorID::kScrollBar);
+      return GetSystemColor(MacSystemColorID::kScrollBar, color_scheme);
     case CSSValueID::kText:
-      return GetSystemColor(MacSystemColorID::kText);
+      return GetSystemColor(MacSystemColorID::kText, color_scheme);
     case CSSValueID::kThreeddarkshadow:
-      return GetSystemColor(MacSystemColorID::kControlDarkShadow);
+      return GetSystemColor(MacSystemColorID::kControlDarkShadow, color_scheme);
     case CSSValueID::kThreedshadow:
-      return GetSystemColor(MacSystemColorID::kShadow);
+      return GetSystemColor(MacSystemColorID::kShadow, color_scheme);
     case CSSValueID::kThreedface:
       // We use this value instead of NSColor's controlColor to avoid website
       // incompatibilities. We may want to change this to use the NSColor in
       // future.
       return 0xFFC0C0C0;
     case CSSValueID::kThreedhighlight:
-      return GetSystemColor(MacSystemColorID::kHighlight);
+      return GetSystemColor(MacSystemColorID::kHighlight, color_scheme);
     case CSSValueID::kThreedlightshadow:
-      return GetSystemColor(MacSystemColorID::kControlLightHighlight);
+      return GetSystemColor(MacSystemColorID::kControlLightHighlight,
+                            color_scheme);
     case CSSValueID::kWebkitFocusRingColor:
-      return GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator);
+      return GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator,
+                            color_scheme);
     case CSSValueID::kWindow:
     case CSSValueID::kCanvas:
-      return GetSystemColor(MacSystemColorID::kWindowBackground);
+      return GetSystemColor(MacSystemColorID::kWindowBackground, color_scheme);
     case CSSValueID::kWindowframe:
-      return GetSystemColor(MacSystemColorID::kWindowFrame);
+      return GetSystemColor(MacSystemColorID::kWindowFrame, color_scheme);
     case CSSValueID::kWindowtext:
-      return GetSystemColor(MacSystemColorID::kWindowFrameText);
+      return GetSystemColor(MacSystemColorID::kWindowFrameText, color_scheme);
     case CSSValueID::kCanvastext:
-      return GetSystemColor(MacSystemColorID::kText);
+      return GetSystemColor(MacSystemColorID::kText, color_scheme);
     default:
       break;
   }