Browse Source

fix: propagate preferred color scheme to the renderer (#22900)

* fix: do not crash if the window is closed syncronously with a nativeTheme change

* fix: propogate preferred color scheme to the renderer and keep it up to date

Co-authored-by: Samuel Attard <[email protected]>
trop[bot] 5 years ago
parent
commit
3f6227c61a

+ 16 - 10
shell/browser/api/electron_api_native_theme.cc

@@ -21,14 +21,16 @@ namespace electron {
 
 namespace api {
 
-NativeTheme::NativeTheme(v8::Isolate* isolate, ui::NativeTheme* theme)
-    : theme_(theme) {
-  theme_->AddObserver(this);
+NativeTheme::NativeTheme(v8::Isolate* isolate,
+                         ui::NativeTheme* ui_theme,
+                         ui::NativeTheme* web_theme)
+    : ui_theme_(ui_theme), web_theme_(web_theme) {
+  ui_theme_->AddObserver(this);
   Init(isolate);
 }
 
 NativeTheme::~NativeTheme() {
-  theme_->RemoveObserver(this);
+  ui_theme_->RemoveObserver(this);
 }
 
 void NativeTheme::OnNativeThemeUpdatedOnUI() {
@@ -42,7 +44,8 @@ void NativeTheme::OnNativeThemeUpdated(ui::NativeTheme* theme) {
 }
 
 void NativeTheme::SetThemeSource(ui::NativeTheme::ThemeSource override) {
-  theme_->set_theme_source(override);
+  ui_theme_->set_theme_source(override);
+  web_theme_->set_theme_source(override);
 #if defined(OS_MACOSX)
   // Update the macOS appearance setting for this new override value
   UpdateMacOSAppearanceForOverrideValue(override);
@@ -52,15 +55,15 @@ void NativeTheme::SetThemeSource(ui::NativeTheme::ThemeSource override) {
 }
 
 ui::NativeTheme::ThemeSource NativeTheme::GetThemeSource() const {
-  return theme_->theme_source();
+  return ui_theme_->theme_source();
 }
 
 bool NativeTheme::ShouldUseDarkColors() {
-  return theme_->ShouldUseDarkColors();
+  return ui_theme_->ShouldUseDarkColors();
 }
 
 bool NativeTheme::ShouldUseHighContrastColors() {
-  return theme_->UsesHighContrastColors();
+  return ui_theme_->UsesHighContrastColors();
 }
 
 #if defined(OS_MACOSX)
@@ -85,8 +88,11 @@ bool NativeTheme::ShouldUseInvertedColorScheme() {
 
 // static
 v8::Local<v8::Value> NativeTheme::Create(v8::Isolate* isolate) {
-  ui::NativeTheme* theme = ui::NativeTheme::GetInstanceForNativeUi();
-  return gin::CreateHandle(isolate, new NativeTheme(isolate, theme)).ToV8();
+  ui::NativeTheme* ui_theme = ui::NativeTheme::GetInstanceForNativeUi();
+  ui::NativeTheme* web_theme = ui::NativeTheme::GetInstanceForWeb();
+  return gin::CreateHandle(isolate,
+                           new NativeTheme(isolate, ui_theme, web_theme))
+      .ToV8();
 }
 
 // static

+ 5 - 2
shell/browser/api/electron_api_native_theme.h

@@ -22,7 +22,9 @@ class NativeTheme : public gin_helper::EventEmitter<NativeTheme>,
                              v8::Local<v8::FunctionTemplate> prototype);
 
  protected:
-  NativeTheme(v8::Isolate* isolate, ui::NativeTheme* theme);
+  NativeTheme(v8::Isolate* isolate,
+              ui::NativeTheme* ui_theme,
+              ui::NativeTheme* web_theme);
   ~NativeTheme() override;
 
   void SetThemeSource(ui::NativeTheme::ThemeSource override);
@@ -40,7 +42,8 @@ class NativeTheme : public gin_helper::EventEmitter<NativeTheme>,
   void OnNativeThemeUpdatedOnUI();
 
  private:
-  ui::NativeTheme* theme_;
+  ui::NativeTheme* ui_theme_;
+  ui::NativeTheme* web_theme_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeTheme);
 };

+ 5 - 0
shell/browser/electron_browser_client.cc

@@ -523,6 +523,11 @@ void ElectronBrowserClient::OverrideWebkitPrefs(
   prefs->picture_in_picture_enabled = false;
 #endif
 
+  ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForNativeUi();
+  prefs->preferred_color_scheme = native_theme->ShouldUseDarkColors()
+                                      ? blink::PreferredColorScheme::kDark
+                                      : blink::PreferredColorScheme::kLight;
+
   SetFontDefaults(prefs);
 
   // Custom preferences of guest page.

+ 1 - 0
shell/browser/native_window.h

@@ -198,6 +198,7 @@ class NativeWindow : public base::SupportsUserData,
 #if defined(OS_MACOSX)
   virtual void SetTrafficLightPosition(const gfx::Point& position) = 0;
   virtual gfx::Point GetTrafficLightPosition() const = 0;
+  virtual void RedrawTrafficLights() = 0;
 #endif
 
   // Touchbar API

+ 1 - 1
shell/browser/native_window_mac.h

@@ -151,7 +151,7 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver {
   void SetWindowLevel(int level);
 
   // Custom traffic light positioning
-  void RedrawTrafficLights();
+  void RedrawTrafficLights() override;
   void SetExitingFullScreen(bool flag);
   void SetTrafficLightPosition(const gfx::Point& position) override;
   gfx::Point GetTrafficLightPosition() const override;

+ 3 - 3
shell/browser/native_window_mac.mm

@@ -710,9 +710,9 @@ void NativeWindowMac::SetExitingFullScreen(bool flag) {
 }
 
 void NativeWindowMac::OnNativeThemeUpdated(ui::NativeTheme* observed_theme) {
-  base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                 base::BindOnce(&NativeWindowMac::RedrawTrafficLights,
-                                base::Unretained(this)));
+  base::PostTask(
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(&NativeWindow::RedrawTrafficLights, GetWeakPtr()));
 }
 
 bool NativeWindowMac::IsEnabled() {

+ 25 - 1
spec-main/api-native-theme-spec.ts

@@ -1,10 +1,12 @@
 import { expect } from 'chai';
-import { nativeTheme, systemPreferences } from 'electron';
+import { nativeTheme, systemPreferences, BrowserWindow } from 'electron';
 import * as os from 'os';
+import * as path from 'path';
 import * as semver from 'semver';
 
 import { delay, ifdescribe } from './spec-helpers';
 import { emittedOnce } from './events-helpers';
+import { closeAllWindows } from './window-helpers';
 
 describe('nativeTheme module', () => {
   describe('nativeTheme.shouldUseDarkColors', () => {
@@ -18,6 +20,8 @@ describe('nativeTheme module', () => {
       nativeTheme.themeSource = 'system';
       // Wait for any pending events to emit
       await delay(20);
+
+      closeAllWindows();
     });
 
     it('is system by default', () => {
@@ -62,6 +66,26 @@ describe('nativeTheme module', () => {
         expect(systemPreferences.appLevelAppearance).to.equal('light');
       });
     });
+
+    const getPrefersColorSchemeIsDark = async (w: Electron.BrowserWindow) => {
+      const isDark: boolean = await w.webContents.executeJavaScript(
+        'matchMedia("(prefers-color-scheme: dark)").matches'
+      );
+      return isDark;
+    };
+
+    it('should override the result of prefers-color-scheme CSS media query', async () => {
+      const w = new BrowserWindow({ show: false });
+      await w.loadFile(path.resolve(__dirname, 'fixtures', 'blank.html'));
+      const originalSystemIsDark = await getPrefersColorSchemeIsDark(w);
+      nativeTheme.themeSource = 'dark';
+      expect(await getPrefersColorSchemeIsDark(w)).to.equal(true);
+      nativeTheme.themeSource = 'light';
+      expect(await getPrefersColorSchemeIsDark(w)).to.equal(false);
+      nativeTheme.themeSource = 'system';
+      expect(await getPrefersColorSchemeIsDark(w)).to.equal(originalSystemIsDark);
+      w.close();
+    });
   });
 
   describe('nativeTheme.shouldUseInvertedColorScheme', () => {