Skip to content

Commit

Permalink
Use NSAppearance in color pipeline for Mac
Browse files Browse the repository at this point in the history
This updates the color pipeline and NativeThemeMac ways of getting
colors to stop relying on the NSApp level NSAppearance. Instead they
use the passed in color_scheme to set a (thread) local NSAppearance
value when they load system colors so they get the correct light/dark
color. This value is removed at the end of the function to clean up
the state.

Bug: 1071671
Change-Id: Ie8bc333bd955b64594fa2b0b538895d6749853e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2161318
Commit-Queue: Joan Barnett <johnsm@microsoft.com>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762617}
  • Loading branch information
John Smith authored and Commit Bot committed Apr 25, 2020
1 parent 586bec6 commit cb6ef92
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 24 deletions.
6 changes: 5 additions & 1 deletion ui/color/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ jumbo_component("mixers") {
sources += [ "linux/native_color_mixers.cc" ]
} else if (is_mac) {
libs = [ "AppKit.framework" ]
sources += [ "mac/native_color_mixers.mm" ]
sources += [
"mac/native_color_mixers.mm",
"mac/scoped_current_nsappearance.h",
"mac/scoped_current_nsappearance.mm",
]
} else if (is_win) {
sources += [ "win/native_color_mixers.cc" ]
}
Expand Down
23 changes: 14 additions & 9 deletions ui/color/mac/native_color_mixers.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,24 @@
#include "ui/color/color_provider.h"
#include "ui/color/color_recipe.h"
#include "ui/color/color_set.h"
#include "ui/color/mac/scoped_current_nsappearance.h"
#include "ui/gfx/color_palette.h"

namespace ui {

void AddNativeCoreColorMixer(ColorProvider* provider, bool dark_window) {
provider->AddMixer().AddSet({kColorSetNative,
{
{kColorTextSelectionBackground,
skia::NSSystemColorToSkColor(
[NSColor selectedTextBackgroundColor])},
}});
ScopedCurrentNSAppearance scoped_nsappearance(dark_window);
ColorMixer& mixer = provider->AddMixer();
mixer.AddSet({kColorSetNative,
{
{kColorTextSelectionBackground,
skia::NSSystemColorToSkColor(
[NSColor selectedTextBackgroundColor])},
}});
}

void AddNativeUiColorMixer(ColorProvider* provider, bool dark_window) {
ScopedCurrentNSAppearance scoped_nsappearance(dark_window);
ColorMixer& mixer = provider->AddMixer();
mixer.AddSet(
{kColorSetNative,
Expand Down Expand Up @@ -56,9 +60,10 @@ void AddNativeUiColorMixer(ColorProvider* provider, bool dark_window) {
NSColor.controlAlternatingRowBackgroundColors[1])};
}

dark_window
? mixer[kColorMenuSeparator] = {SkColorSetA(gfx::kGoogleGrey800, 0xCC)}
: mixer[kColorMenuSeparator] = {SkColorSetA(SK_ColorBLACK, 0x26)};
SkColor menu_separator_color = dark_window
? SkColorSetA(gfx::kGoogleGrey800, 0xCC)
: SkColorSetA(SK_ColorBLACK, 0x26);
mixer[kColorMenuSeparator] = {menu_separator_color};
}

} // namespace ui
28 changes: 28 additions & 0 deletions ui/color/mac/scoped_current_nsappearance.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2020 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 UI_COLOR_MAC_SCOPED_CURRENT_NSAPPEARANCE_H_
#define UI_COLOR_MAC_SCOPED_CURRENT_NSAPPEARANCE_H_

#include "base/component_export.h"

namespace ui {

// Class for handling changing the NSAppearance to get colors in a scoped way
// based on the desired light/dark colors scheme.
class COMPONENT_EXPORT(COLOR) ScopedCurrentNSAppearance {
public:
explicit ScopedCurrentNSAppearance(bool dark);

// There should be no reason to copy or move a ScopedCurrentNSAppearance.
ScopedCurrentNSAppearance(const ScopedCurrentNSAppearance&) = delete;
ScopedCurrentNSAppearance& operator=(const ScopedCurrentNSAppearance&) =
delete;

~ScopedCurrentNSAppearance();
};

} // namespace ui

#endif
23 changes: 23 additions & 0 deletions ui/color/mac/scoped_current_nsappearance.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2020 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.

#include "ui/color/mac/scoped_current_nsappearance.h"

#import <Cocoa/Cocoa.h>

namespace ui {
ScopedCurrentNSAppearance::ScopedCurrentNSAppearance(bool dark) {
if (@available(macOS 10.14, *)) {
NSAppearanceName appearance =
dark ? NSAppearanceNameDarkAqua : NSAppearanceNameAqua;
[NSAppearance
setCurrentAppearance:[NSAppearance appearanceNamed:appearance]];
}
}

ScopedCurrentNSAppearance::~ScopedCurrentNSAppearance() {
if (@available(macOS 10.14, *))
[NSAppearance setCurrentAppearance:nil];
}
}
6 changes: 6 additions & 0 deletions ui/native_theme/native_theme_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ class NATIVE_THEME_EXPORT NativeThemeMac : public NativeThemeBase {

void ConfigureWebInstance() override;

// Used by the GetSystem to run the switch for MacOS override colors that may
// use named NS system colors. This is a separate function from GetSystemColor
// to make sure the NSAppearance can be set in a scoped way.
base::Optional<SkColor> GetOSColor(ColorId color_id,
ColorScheme color_scheme) const;

base::scoped_nsobject<NativeThemeEffectiveAppearanceObserver>
appearance_observer_;
id high_contrast_notification_token_;
Expand Down
31 changes: 17 additions & 14 deletions ui/native_theme/native_theme_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#import "skia/ext/skia_utils_mac.h"
#include "ui/base/ui_base_features.h"
#include "ui/base/ui_base_switches.h"
#include "ui/color/mac/scoped_current_nsappearance.h"
#include "ui/gfx/color_palette.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/skia_util.h"
Expand Down Expand Up @@ -167,16 +168,6 @@ SkColor ColorToGrayscale(SkColor color) {
should_only_use_dark_colors_)
return NativeTheme::GetSystemColor(color_id, color_scheme);

// Empirically, currentAppearance is incorrect when switching
// appearances. It's unclear exactly why right now, so work
// around it for the time being by resynchronizing.
if (@available(macOS 10.14, *)) {
NSAppearance* effective_appearance = [NSApp effectiveAppearance];
if (![effective_appearance isEqual:[NSAppearance currentAppearance]]) {
[NSAppearance setCurrentAppearance:effective_appearance];
}
}

if (UsesHighContrastColors()) {
switch (color_id) {
case kColorId_SelectedMenuItemForegroundColor:
Expand All @@ -189,6 +180,21 @@ SkColor ColorToGrayscale(SkColor color) {
break;
}
}

base::Optional<SkColor> os_color = GetOSColor(color_id, color_scheme);
if (os_color.has_value())
return os_color.value();

return ApplySystemControlTint(
NativeTheme::GetSystemColor(color_id, color_scheme));
}

base::Optional<SkColor> NativeThemeMac::GetOSColor(
ColorId color_id,
ColorScheme color_scheme) const {
ScopedCurrentNSAppearance scoped_nsappearance(color_scheme ==
ColorScheme::kDark);

// Even with --secondary-ui-md, menus use the platform colors and styling, and
// Mac has a couple of specific color overrides, documented below.
switch (color_id) {
Expand Down Expand Up @@ -231,11 +237,8 @@ SkColor ColorToGrayscale(SkColor color) {
NSColor.controlAlternatingRowBackgroundColors[1]);

default:
break;
return base::nullopt;
}

return ApplySystemControlTint(
NativeTheme::GetSystemColor(color_id, color_scheme));
}

void NativeThemeMac::PaintMenuPopupBackground(
Expand Down

0 comments on commit cb6ef92

Please sign in to comment.