feat: add nativeTheme.themeSource to allow apps to override Chromiums theme choice (electron#19960)

* feat: add nativeTheme.shouldUseDarkColorsOverride to allow apps to override Chromiums theme choice

* spec: add tests for shouldUseDarkColorsOverride

* chore: add missing forward declarations

* refactor: rename overrideShouldUseDarkColors to themeSource

* chore: only run appLevelAppearance specs on Mojave and up

* chore: update patch with more info and no define

* Update spec-main/api-native-theme-spec.ts

Co-Authored-By: Jeremy Apthorp <[email protected]>

* Update api-native-theme-spec.ts

* Update api-native-theme-spec.ts

* Update api-native-theme-spec.ts

Author: MarshallOfSound

Diff for: docs/api/native-theme.md

@@ -22,7 +22,38 @@ The `nativeTheme` module has the following properties:
 ### `nativeTheme.shouldUseDarkColors` _Readonly_
 
 A `Boolean` for if the OS / Chromium currently has a dark mode enabled or is
-being instructed to show a dark-style UI.
+being instructed to show a dark-style UI.  If you want to modify this value you
+should use `themeSource` below.
+
+### `nativeTheme.themeSource`
+
+A `String` property that can be `system`, `light` or `dark`.  It is used to override and supercede
+the value that Chromium has chosen to use internally.
+
+Setting this property to `system` will remove the override and
+everything will be reset to the OS default.  By default `themeSource` is `system`.
+
+Settings this property to `dark` will have the following effects:
+* `nativeTheme.shouldUseDarkColors` will be `true` when accessed
+* Any UI Electron renders on Linux and Windows including context menus, devtools, etc. will use the dark UI.
+* Any UI the OS renders on macOS including menus, window frames, etc. will use the dark UI.
+* The [`prefers-color-scheme`](https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme) CSS query will match `dark` mode.
+* The `updated` event will be emitted
+
+Settings this property to `light` will have the following effects:
+* `nativeTheme.shouldUseDarkColors` will be `false` when accessed
+* Any UI Electron renders on Linux and Windows including context menus, devtools, etc. will use the light UI.
+* Any UI the OS renders on macOS including menus, window frames, etc. will use the light UI.
+* The [`prefers-color-scheme`](https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme) CSS query will match `light` mode.
+* The `updated` event will be emitted
+
+The usage of this property should align with a classic "dark mode" state machine in your application
+where the user has three options.
+* `Follow OS` --> `themeSource = 'system'`
+* `Dark Mode` --> `themeSource = 'dark'`
+* `Light Mode` --> `themeSource = 'light'`
+
+Your application should then always use `shouldUseDarkColors` to determine what CSS to apply.
 
 ### `nativeTheme.shouldUseHighContrastColors` _macOS_ _Windows_ _Readonly_
 

Diff for: filenames.gni

@@ -429,6 +429,7 @@ filenames = {
     "shell/common/api/atom_api_native_image_mac.mm",
     "shell/common/api/atom_api_native_theme.cc",
     "shell/common/api/atom_api_native_theme.h",
+    "shell/common/api/atom_api_native_theme_mac.mm",
     "shell/common/api/atom_api_shell.cc",
     "shell/common/api/atom_api_v8_util.cc",
     "shell/common/api/electron_bindings.cc",

Diff for: package.json

@@ -16,6 +16,7 @@
     "@types/fs-extra": "^5.0.5",
     "@types/mocha": "^5.2.6",
     "@types/node": "^12.0.10",
+    "@types/semver": "^6.0.1",
     "@types/send": "^0.14.5",
     "@types/split": "^1.0.0",
     "@types/webpack": "^4.4.32",
@@ -130,4 +131,4 @@
       "git add filenames.auto.gni"
     ]
   }
-}
+}

Diff for: patches/chromium/.patches

@@ -77,3 +77,4 @@ picture-in-picture.patch
 disable_compositor_recycling.patch
 allow_new_privileges_in_unsandboxed_child_processes.patch
 expose_setuseragent_on_networkcontext.patch
+feat_add_set_theme_source_to_allow_apps_to.patch

Diff for: patches/chromium/feat_add_set_theme_source_to_allow_apps_to.patch

@@ -0,0 +1,89 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Samuel Attard <[email protected]>
+Date: Mon, 26 Aug 2019 14:32:41 -0700
+Subject: feat: add set_theme_source to allow apps to override chromiums
+ internal theme choice
+
+This patch is required as Chromium doesn't currently let folks using
+//ui override the theme choice in NativeTheme.  It defaults to
+respecting the OS theme choice and some apps don't always want to do
+that.  With this patch we can override the theme value that Chromium
+uses internally for things like menus and devtools.
+
+We can remove this patch once it has in some shape been upstreamed.
+
+diff --git a/ui/native_theme/native_theme.cc b/ui/native_theme/native_theme.cc
+index 2370d15332c8c6c7dc7e3403b38891c885704d9f..171214379437f319d3feccc289a5d91e74b77f9e 100644
+--- a/ui/native_theme/native_theme.cc
++++ b/ui/native_theme/native_theme.cc
+@@ -40,6 +40,8 @@ NativeTheme::NativeTheme()
+ NativeTheme::~NativeTheme() = default;
+ 
+ bool NativeTheme::ShouldUseDarkColors() const {
++  if (theme_source() == ThemeSource::kForcedLight) return false;
++  if (theme_source() == ThemeSource::kForcedDark) return true;
+   return should_use_dark_colors_;
+ }
+ 
+diff --git a/ui/native_theme/native_theme.h b/ui/native_theme/native_theme.h
+index 70389e0245993faa2c17e9deefeb000280ef2368..cef1c0d4706e7e720a4004ca54765a39fc29c5e8 100644
+--- a/ui/native_theme/native_theme.h
++++ b/ui/native_theme/native_theme.h
+@@ -429,6 +429,22 @@ class NATIVE_THEME_EXPORT NativeTheme {
+       ColorId color_id,
+       ColorScheme color_scheme = ColorScheme::kDefault) const = 0;
+ 
++  enum ThemeSource {
++    kSystem,
++    kForcedDark,
++    kForcedLight,
++  };
++
++  ThemeSource theme_source() const {
++    return theme_source_;
++  }
++
++  void set_theme_source(ThemeSource theme_source) {
++    bool original = ShouldUseDarkColors();
++    theme_source_ = theme_source;
++    if (ShouldUseDarkColors() != original) NotifyObservers();
++  }
++
+   // Returns a shared instance of the native theme that should be used for web
+   // rendering. Do not use it in a normal application context (i.e. browser).
+   // The returned object should not be deleted by the caller. This function is
+@@ -547,6 +563,8 @@ class NATIVE_THEME_EXPORT NativeTheme {
+   PreferredColorScheme preferred_color_scheme_ =
+       PreferredColorScheme::kNoPreference;
+ 
++  ThemeSource theme_source_ = ThemeSource::kSystem;
++
+   DISALLOW_COPY_AND_ASSIGN(NativeTheme);
+ };
+ 
+diff --git a/ui/native_theme/native_theme_dark_aura.cc b/ui/native_theme/native_theme_dark_aura.cc
+index a8fbfee3b13672902aac05fd5a65fa8ee81f9f7e..1be6369acf0b7c02a6f862636c2b2de1fbf8cb5a 100644
+--- a/ui/native_theme/native_theme_dark_aura.cc
++++ b/ui/native_theme/native_theme_dark_aura.cc
+@@ -20,6 +20,8 @@ SkColor NativeThemeDarkAura::GetSystemColor(ColorId color_id,
+ }
+ 
+ bool NativeThemeDarkAura::ShouldUseDarkColors() const {
++  if (theme_source() == ThemeSource::kForcedLight) return false;
++  if (theme_source() == ThemeSource::kForcedDark) return true;
+   return true;
+ }
+ 
+diff --git a/ui/native_theme/native_theme_win.cc b/ui/native_theme/native_theme_win.cc
+index 3003643bfb78cec2f5e84fc9e1471e1ef54aae41..06f2cbc84401958d49445f4ce6acb1b2fef0aa04 100644
+--- a/ui/native_theme/native_theme_win.cc
++++ b/ui/native_theme/native_theme_win.cc
+@@ -611,6 +611,8 @@ bool NativeThemeWin::ShouldUseDarkColors() const {
+   // ...unless --force-dark-mode was specified in which case caveat emptor.
+   if (UsesHighContrastColors() && !IsForcedDarkMode())
+     return false;
++  if (theme_source() == ThemeSource::kForcedLight) return false;
++  if (theme_source() == ThemeSource::kForcedDark) return true;
+   return NativeTheme::ShouldUseDarkColors();
+ }
+ 

Diff for: shell/common/api/atom_api_native_theme.cc

@@ -4,6 +4,8 @@
 
 #include "shell/common/api/atom_api_native_theme.h"
 
+#include <string>
+
 #include "native_mate/dictionary.h"
 #include "native_mate/object_template_builder.h"
 #include "shell/common/node_includes.h"
@@ -28,6 +30,20 @@ void NativeTheme::OnNativeThemeUpdated(ui::NativeTheme* theme) {
   Emit("updated");
 }
 
+void NativeTheme::SetThemeSource(ui::NativeTheme::ThemeSource override) {
+  theme_->set_theme_source(override);
+#if defined(OS_MACOSX)
+  // Update the macOS appearance setting for this new override value
+  UpdateMacOSAppearanceForOverrideValue(override);
+#endif
+  // TODO(MarshallOfSound): Update all existing browsers windows to use GTK dark
+  // theme
+}
+
+ui::NativeTheme::ThemeSource NativeTheme::GetThemeSource() const {
+  return theme_->theme_source();
+}
+
 bool NativeTheme::ShouldUseDarkColors() {
   return theme_->ShouldUseDarkColors();
 }
@@ -68,6 +84,8 @@ void NativeTheme::BuildPrototype(v8::Isolate* isolate,
   prototype->SetClassName(mate::StringToV8(isolate, "NativeTheme"));
   mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
       .SetProperty("shouldUseDarkColors", &NativeTheme::ShouldUseDarkColors)
+      .SetProperty("themeSource", &NativeTheme::GetThemeSource,
+                   &NativeTheme::SetThemeSource)
       .SetProperty("shouldUseHighContrastColors",
                    &NativeTheme::ShouldUseHighContrastColors)
       .SetProperty("shouldUseInvertedColorScheme",
@@ -94,4 +112,42 @@ void Initialize(v8::Local<v8::Object> exports,
 
 }  // namespace
 
+namespace mate {
+
+v8::Local<v8::Value> Converter<ui::NativeTheme::ThemeSource>::ToV8(
+    v8::Isolate* isolate,
+    const ui::NativeTheme::ThemeSource& val) {
+  switch (val) {
+    case ui::NativeTheme::ThemeSource::kForcedDark:
+      return mate::ConvertToV8(isolate, "dark");
+    case ui::NativeTheme::ThemeSource::kForcedLight:
+      return mate::ConvertToV8(isolate, "light");
+    case ui::NativeTheme::ThemeSource::kSystem:
+    default:
+      return mate::ConvertToV8(isolate, "system");
+  }
+}
+
+bool Converter<ui::NativeTheme::ThemeSource>::FromV8(
+    v8::Isolate* isolate,
+    v8::Local<v8::Value> val,
+    ui::NativeTheme::ThemeSource* out) {
+  std::string theme_source;
+  if (mate::ConvertFromV8(isolate, val, &theme_source)) {
+    if (theme_source == "dark") {
+      *out = ui::NativeTheme::ThemeSource::kForcedDark;
+    } else if (theme_source == "light") {
+      *out = ui::NativeTheme::ThemeSource::kForcedLight;
+    } else if (theme_source == "system") {
+      *out = ui::NativeTheme::ThemeSource::kSystem;
+    } else {
+      return false;
+    }
+    return true;
+  }
+  return false;
+}
+
+}  // namespace mate
+
 NODE_LINKED_MODULE_CONTEXT_AWARE(atom_common_native_theme, Initialize)

Diff for: shell/common/api/atom_api_native_theme.h

@@ -7,6 +7,7 @@
 
 #include "native_mate/handle.h"
 #include "shell/browser/api/event_emitter.h"
+#include "ui/native_theme/native_theme.h"
 #include "ui/native_theme/native_theme_observer.h"
 
 namespace electron {
@@ -25,6 +26,12 @@ class NativeTheme : public mate::EventEmitter<NativeTheme>,
   NativeTheme(v8::Isolate* isolate, ui::NativeTheme* theme);
   ~NativeTheme() override;
 
+  void SetThemeSource(ui::NativeTheme::ThemeSource override);
+#if defined(OS_MACOSX)
+  void UpdateMacOSAppearanceForOverrideValue(
+      ui::NativeTheme::ThemeSource override);
+#endif
+  ui::NativeTheme::ThemeSource GetThemeSource() const;
   bool ShouldUseDarkColors();
   bool ShouldUseHighContrastColors();
   bool ShouldUseInvertedColorScheme();
@@ -42,4 +49,17 @@ class NativeTheme : public mate::EventEmitter<NativeTheme>,
 
 }  // namespace electron
 
+namespace mate {
+
+template <>
+struct Converter<ui::NativeTheme::ThemeSource> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const ui::NativeTheme::ThemeSource& val);
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     ui::NativeTheme::ThemeSource* out);
+};
+
+}  // namespace mate
+
 #endif  // SHELL_COMMON_API_ATOM_API_NATIVE_THEME_H_

Diff for: shell/common/api/atom_api_native_theme_mac.mm

@@ -0,0 +1,37 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/common/api/atom_api_native_theme.h"
+
+#include "base/mac/sdk_forward_declarations.h"
+#include "shell/browser/mac/atom_application.h"
+
+namespace electron {
+
+namespace api {
+
+void NativeTheme::UpdateMacOSAppearanceForOverrideValue(
+    ui::NativeTheme::ThemeSource override) {
+  if (@available(macOS 10.14, *)) {
+    NSAppearance* new_appearance;
+    switch (override) {
+      case ui::NativeTheme::ThemeSource::kForcedDark:
+        new_appearance =
+            [NSAppearance appearanceNamed:NSAppearanceNameDarkAqua];
+        break;
+      case ui::NativeTheme::ThemeSource::kForcedLight:
+        new_appearance = [NSAppearance appearanceNamed:NSAppearanceNameAqua];
+        break;
+      case ui::NativeTheme::ThemeSource::kSystem:
+      default:
+        new_appearance = nil;
+        break;
+    }
+    [[NSApplication sharedApplication] setAppearance:new_appearance];
+  }
+}
+
+}  // namespace api
+
+}  // namespace electron

Diff for: spec-main/api-native-theme-spec.ts

@@ -1,5 +1,9 @@
 import { expect } from 'chai'
-import { nativeTheme } from 'electron'
+import { nativeTheme, systemPreferences } from 'electron'
+import * as os from 'os'
+import * as semver from 'semver'
+
+import { ifdescribe } from './spec-helpers'
 
 describe('nativeTheme module', () => {
   describe('nativeTheme.shouldUseDarkColors', () => {
@@ -8,6 +12,52 @@ describe('nativeTheme module', () => {
     })
   })
 
+  describe('nativeTheme.themeSource', () => {
+    afterEach(() => {
+      nativeTheme.themeSource = 'system'
+    })
+
+    it('is system by default', () => {
+      expect(nativeTheme.themeSource).to.equal('system')
+    })
+
+    it('should override the value of shouldUseDarkColors', () => {
+      nativeTheme.themeSource = 'dark'
+      expect(nativeTheme.shouldUseDarkColors).to.equal(true)
+      nativeTheme.themeSource = 'light'
+      expect(nativeTheme.shouldUseDarkColors).to.equal(false)
+    })
+
+    it('should emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value changes', () => {
+      nativeTheme.themeSource = 'dark'
+      let called = false
+      nativeTheme.once('updated', () => {
+        called = true
+      })
+      nativeTheme.themeSource = 'light'
+      expect(called).to.equal(true)
+    })
+
+    it('should not emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value is the same', () => {
+      nativeTheme.themeSource = 'dark'
+      let called = false
+      nativeTheme.once('updated', () => {
+        called = true
+      })
+      nativeTheme.themeSource = 'dark'
+      expect(called).to.equal(false)
+    })
+
+    ifdescribe(process.platform === 'darwin' && semver.gte(os.release(), '18.0.0'))('on macOS 10.14', () => {
+      it('should update appLevelAppearance when set', () => {
+        nativeTheme.themeSource = 'dark'
+        expect(systemPreferences.appLevelAppearance).to.equal('dark')
+        nativeTheme.themeSource = 'light'
+        expect(systemPreferences.appLevelAppearance).to.equal('light')
+      })
+    })
+  })
+
   describe('nativeTheme.shouldUseInvertedColorScheme', () => {
     it('returns a boolean', () => {
       expect(nativeTheme.shouldUseInvertedColorScheme).to.be.a('boolean')

Diff for: yarn.lock

@@ -261,6 +261,11 @@
   resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.3.tgz#7ee330ba7caafb98090bece86a5ee44115904c2c"
   integrity sha512-ewFXqrQHlFsgc09MK5jP5iR7vumV/BYayNC6PgJO2LPe8vrnNFyjQjSppfEngITi0qvfKtzFvgKymGheFM9UOA==
 
+"@types/semver@^6.0.1":
+  version "6.0.1"
+  resolved "https://registry.yarnpkg.com/@types/semver/-/semver-6.0.1.tgz#a984b405c702fa5a7ec6abc56b37f2ba35ef5af6"
+  integrity sha512-ffCdcrEE5h8DqVxinQjo+2d1q+FV5z7iNtPofw3JsrltSoSVlOGaW0rY8XxtO9XukdTn8TaCGWmk2VFGhI70mg==
+
 "@types/send@^0.14.5":
   version "0.14.5"
   resolved "https://registry.yarnpkg.com/@types/send/-/send-0.14.5.tgz#653f7d25b93c3f7f51a8994addaf8a229de022a7"