Skip to content

Commit

Permalink
Remove Dark Mode FeatureList Checks
Browse files Browse the repository at this point in the history
Dark mode support is now enabled by default on all supported platforms.

BUG=923516,927180

Change-Id: I3486b3714149c311b60f7114cb08883335129221
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1597930
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659339}
  • Loading branch information
Robert Liao authored and Commit Bot committed May 14, 2019
1 parent be23bae commit 6372111
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 64 deletions.
1 change: 0 additions & 1 deletion chrome/browser/chrome_browser_main_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ class ChromeBrowserMainPartsMac : public ChromeBrowserMainPartsPosix {

// BrowserParts overrides.
int PreEarlyInitialization() override;
void PostEarlyInitialization() override;
void PreMainMessageLoopStart() override;
void PostMainMessageLoopStart() override;
void PreProfileInit() override;
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/chrome_browser_main_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,6 @@ void RecordUpdateState() {
return ChromeBrowserMainPartsPosix::PreEarlyInitialization();
}

void ChromeBrowserMainPartsMac::PostEarlyInitialization() {
ui::NativeThemeMac::MaybeUpdateBrowserAppearance();
ChromeBrowserMainPartsPosix::PostEarlyInitialization();
}

void ChromeBrowserMainPartsMac::PreMainMessageLoopStart() {
MacStartupProfiler::GetInstance()->Profile(
MacStartupProfiler::PRE_MAIN_MESSAGE_LOOP_START);
Expand Down
15 changes: 0 additions & 15 deletions testing/variations/fieldtrial_testing_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -1534,21 +1534,6 @@
]
}
],
"DarkModeWindows": [
{
"platforms": [
"windows"
],
"experiments": [
{
"name": "DarkMode",
"enable_features": [
"DarkMode"
]
}
]
}
],
"DataCompressionProxyLoFi": [
{
"platforms": [
Expand Down
6 changes: 0 additions & 6 deletions ui/base/ui_base_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,6 @@ bool IsOzoneDrmMojo() {
IsMultiProcessMash();
}

#if defined(OS_MACOSX)
const base::Feature kDarkMode = {"DarkMode", base::FEATURE_ENABLED_BY_DEFAULT};
#else
const base::Feature kDarkMode = {"DarkMode", base::FEATURE_DISABLED_BY_DEFAULT};
#endif

#if defined(OS_CHROMEOS)
const base::Feature kHandwritingGesture = {"HandwritingGesture",
base::FEATURE_DISABLED_BY_DEFAULT};
Expand Down
4 changes: 0 additions & 4 deletions ui/base/ui_base_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,6 @@ COMPONENT_EXPORT(UI_BASE_FEATURES)
extern const base::Feature kEnableOzoneDrmMojo;
COMPONENT_EXPORT(UI_BASE_FEATURES) bool IsOzoneDrmMojo();

// Whether default UI should use a dark mode color scheme, if enabled on
// macOS Mojave/Windows 10.
COMPONENT_EXPORT(UI_BASE_FEATURES) extern const base::Feature kDarkMode;

#if defined(OS_CHROMEOS)
COMPONENT_EXPORT(UI_BASE_FEATURES)
extern const base::Feature kHandwritingGesture;
Expand Down
9 changes: 2 additions & 7 deletions ui/native_theme/native_theme_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ class NATIVE_THEME_EXPORT NativeThemeMac : public NativeThemeBase {
// an appropriate gray.
static SkColor ApplySystemControlTint(SkColor color);

// If the system is not running Mojave, or not forcing dark/light mode, do
// nothing. Otherwise, set the correct appearance on NSApp, adjusting for High
// Contrast if necessary.
// TODO(lgrey): Remove this when we're no longer suppressing dark mode by
// default.
static void MaybeUpdateBrowserAppearance();

// Overridden from NativeTheme:
SkColor GetSystemColor(ColorId color_id) const override;

Expand Down Expand Up @@ -82,6 +75,8 @@ class NATIVE_THEME_EXPORT NativeThemeMac : public NativeThemeBase {
void PaintSelectedMenuItem(cc::PaintCanvas* canvas,
const gfx::Rect& rect) const;

void InitializeDarkModeStateAndObserver();

base::scoped_nsobject<NativeThemeEffectiveAppearanceObserver>
appearance_observer_;
id high_contrast_notification_token_;
Expand Down
36 changes: 12 additions & 24 deletions ui/native_theme/native_theme_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,6 @@ SkColor ColorToGrayscale(SkColor color) {
return color;
}

// static
void NativeThemeMac::MaybeUpdateBrowserAppearance() {
if (@available(macOS 10.14, *)) {
if (!base::FeatureList::IsEnabled(features::kDarkMode)) {
NSAppearanceName new_appearance_name =
base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceDarkMode)
? NSAppearanceNameDarkAqua
: NSAppearanceNameAqua;

[NSApp setAppearance:[NSAppearance appearanceNamed:new_appearance_name]];
}
}
}

SkColor NativeThemeMac::GetSystemColor(ColorId color_id) const {
// Empirically, currentAppearance is incorrect when switching
// appearances. It's unclear exactly why right now, so work
Expand Down Expand Up @@ -277,15 +262,8 @@ SkColor ColorToGrayscale(SkColor color) {
}

NativeThemeMac::NativeThemeMac() {
if (base::FeatureList::IsEnabled(features::kDarkMode)) {
__block auto theme = this;
set_dark_mode(IsDarkMode());
appearance_observer_.reset(
[[NativeThemeEffectiveAppearanceObserver alloc] initWithHandler:^{
theme->set_dark_mode(IsDarkMode());
theme->NotifyObservers();
}]);
}
InitializeDarkModeStateAndObserver();

if (!IsForcedHighContrast()) {
set_high_contrast(IsHighContrast());
__block auto theme = this;
Expand Down Expand Up @@ -315,4 +293,14 @@ SkColor ColorToGrayscale(SkColor color) {
canvas->drawRect(gfx::RectToSkRect(rect), flags);
}

void NativeThemeMac::InitializeDarkModeStateAndObserver() {
__block auto theme = this;
set_dark_mode(IsDarkMode());
appearance_observer_.reset(
[[NativeThemeEffectiveAppearanceObserver alloc] initWithHandler:^{
theme->set_dark_mode(IsDarkMode());
theme->NotifyObservers();
}]);
}

} // namespace ui
8 changes: 6 additions & 2 deletions ui/native_theme/native_theme_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@

#include "base/bind.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "base/stl_util.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/win/scoped_gdi_object.h"
#include "base/win/scoped_hdc.h"
#include "base/win/scoped_select_object.h"
Expand Down Expand Up @@ -255,7 +255,11 @@ NativeThemeWin::NativeThemeWin()
}

if (!IsForcedDarkMode() && !IsForcedHighContrast() &&
base::FeatureList::IsEnabled(features::kDarkMode)) {
base::SequencedTaskRunnerHandle::IsSet()) {
// If there's no sequenced task runner handle, we can't be called back for
// dark mode changes. This generally happens in tests. As a result, ignore
// dark mode in this case.

// Dark Mode currently targets UWP apps, which means Win32 apps need to use
// alternate, less reliable means of detecting the state. The following
// can break in future Windows versions.
Expand Down

0 comments on commit 6372111

Please sign in to comment.