Browse Source

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

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

* 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

* chore: update native theme source patch for linux
Samuel Attard 5 years ago
parent
commit
3ba4a35c9c

+ 2 - 2
patches/chromium/can_create_window.patch

@@ -5,10 +5,10 @@ Subject: can_create_window.patch
 
 
 diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
-index bbd42aad3d42311dfff47e725a5deddb4ed04134..797ee4073cfc7b78038d0c007922783a55b89683 100644
+index 050531a82b48797012adbf6fe49ab5741a38d989..8a02ff83c8e40d641530211ef944b62b09a03ac8 100644
 --- a/content/browser/frame_host/render_frame_host_impl.cc
 +++ b/content/browser/frame_host/render_frame_host_impl.cc
-@@ -4375,6 +4375,7 @@ void RenderFrameHostImpl::CreateNewWindow(
+@@ -4379,6 +4379,7 @@ void RenderFrameHostImpl::CreateNewWindow(
            last_committed_origin_, params->window_container_type,
            params->target_url, params->referrer.To<Referrer>(),
            params->frame_name, params->disposition, *params->features,

+ 30 - 7
patches/chromium/feat_add_set_theme_source_to_allow_apps_to.patch

@@ -26,10 +26,10 @@ index d9175ae9661218b58e9b587ad3a9dbd540bfd384..ed178ea7b3d7f8039f47402bc08cae20
  }
  
 diff --git a/ui/native_theme/native_theme.h b/ui/native_theme/native_theme.h
-index 205181053803813b64497884bc93cfd857254866..bb9e6fba1cb0dd734c7aeb8dcd2eeadcd4b5d2da 100644
+index 205181053803813b64497884bc93cfd857254866..7b14086b43eeb86baa0f8471e15e7f6f19193889 100644
 --- a/ui/native_theme/native_theme.h
 +++ b/ui/native_theme/native_theme.h
-@@ -431,6 +431,22 @@ class NATIVE_THEME_EXPORT NativeTheme {
+@@ -431,6 +431,38 @@ class NATIVE_THEME_EXPORT NativeTheme {
        ColorId color_id,
        ColorScheme color_scheme = ColorScheme::kDefault) const = 0;
  
@@ -44,15 +44,31 @@ index 205181053803813b64497884bc93cfd857254866..bb9e6fba1cb0dd734c7aeb8dcd2eeadc
 +  }
 +
 +  void set_theme_source(ThemeSource theme_source) {
-+    bool original = ShouldUseDarkColors();
++    bool original_dark_colors = ShouldUseDarkColors();
++    PreferredColorScheme original_color_scheme = GetPreferredColorScheme();
++
 +    theme_source_ = theme_source;
-+    if (ShouldUseDarkColors() != original) NotifyObservers();
++
++    bool changed = false;
++    if (ShouldUseDarkColors() != original_dark_colors) {
++      changed = true;
++    }
++
++    PreferredColorScheme new_color_scheme = CalculatePreferredColorScheme();
++    if (new_color_scheme != original_color_scheme) {
++      set_preferred_color_scheme(new_color_scheme);
++      changed = true;
++    }
++
++    if (changed) {
++      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
-@@ -555,6 +571,8 @@ class NATIVE_THEME_EXPORT NativeTheme {
+@@ -555,6 +587,8 @@ class NATIVE_THEME_EXPORT NativeTheme {
    PreferredColorScheme preferred_color_scheme_ =
        PreferredColorScheme::kNoPreference;
  
@@ -62,10 +78,10 @@ index 205181053803813b64497884bc93cfd857254866..bb9e6fba1cb0dd734c7aeb8dcd2eeadc
  };
  
 diff --git a/ui/native_theme/native_theme_dark_aura.cc b/ui/native_theme/native_theme_dark_aura.cc
-index a8fbfee3b13672902aac05fd5a65fa8ee81f9f7e..1be6369acf0b7c02a6f862636c2b2de1fbf8cb5a 100644
+index a8fbfee3b13672902aac05fd5a65fa8ee81f9f7e..80770f0e46641f0d4ea81c6fa6602a68c1f9b9a8 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,
+@@ -20,12 +20,14 @@ SkColor NativeThemeDarkAura::GetSystemColor(ColorId color_id,
  }
  
  bool NativeThemeDarkAura::ShouldUseDarkColors() const {
@@ -74,6 +90,13 @@ index a8fbfee3b13672902aac05fd5a65fa8ee81f9f7e..1be6369acf0b7c02a6f862636c2b2de1
    return true;
  }
  
+ NativeTheme::PreferredColorScheme NativeThemeDarkAura::GetPreferredColorScheme()
+     const {
+-  return NativeTheme::PreferredColorScheme::kDark;
++  return ShouldUseDarkColors() ? NativeTheme::PreferredColorScheme::kDark : NativeTheme::PreferredColorScheme::kLight;
+ }
+ 
+ NativeThemeDarkAura::NativeThemeDarkAura() : NativeThemeAura(false) {}
 diff --git a/ui/native_theme/native_theme_win.cc b/ui/native_theme/native_theme_win.cc
 index cff7681a4e0023a67edda37bf9865ab073b32549..83507081e0734b53fd37133014256243dda6864d 100644
 --- a/ui/native_theme/native_theme_win.cc

+ 6 - 7
patches/chromium/feat_allow_embedders_to_add_observers_on_created_hunspell.patch

@@ -4,12 +4,11 @@ Date: Sun, 1 Mar 2020 16:33:55 -0800
 Subject: feat: allow embedders to add observers on created hunspell
  dictionaries
 
-
 diff --git a/chrome/browser/spellchecker/spellcheck_service.cc b/chrome/browser/spellchecker/spellcheck_service.cc
-index 0dc509bafcfab2637aab4ea55769cd06ad3492c9..3b1f02e354c5c5a85e9193859ca8e7497f02cf86 100644
+index 6a508168067b70822fabee5d50a5be7ac2ed15a5..5afbef4fb24441a850c45703b2d7a66b8f92abb1 100644
 --- a/chrome/browser/spellchecker/spellcheck_service.cc
 +++ b/chrome/browser/spellchecker/spellcheck_service.cc
-@@ -284,6 +284,9 @@ void SpellcheckService::LoadHunspellDictionaries() {
+@@ -266,6 +266,9 @@ void SpellcheckService::LoadHunspellDictionaries() {
          std::make_unique<SpellcheckHunspellDictionary>(dictionary, context_,
                                                         this));
      hunspell_dictionaries_.back()->AddObserver(this);
@@ -19,7 +18,7 @@ index 0dc509bafcfab2637aab4ea55769cd06ad3492c9..3b1f02e354c5c5a85e9193859ca8e749
      hunspell_dictionaries_.back()->Load();
    }
  
-@@ -297,6 +300,20 @@ SpellcheckService::GetHunspellDictionaries() {
+@@ -279,6 +282,20 @@ SpellcheckService::GetHunspellDictionaries() {
    return hunspell_dictionaries_;
  }
  
@@ -41,10 +40,10 @@ index 0dc509bafcfab2637aab4ea55769cd06ad3492c9..3b1f02e354c5c5a85e9193859ca8e749
                                                 std::string locale,
                                                 std::string path,
 diff --git a/chrome/browser/spellchecker/spellcheck_service.h b/chrome/browser/spellchecker/spellcheck_service.h
-index 557a0a2a91821a595181481f92b2a2a06dcb1f50..59e24da4be927303df8c4aac87f50778c1c208b0 100644
+index d5af9d5829c362743e64a9f2fefdf58a340c4b9f..8c00b16996192a4b65f4794e0018b50559bbe1c1 100644
 --- a/chrome/browser/spellchecker/spellcheck_service.h
 +++ b/chrome/browser/spellchecker/spellcheck_service.h
-@@ -116,6 +116,8 @@ class SpellcheckService : public KeyedService,
+@@ -114,6 +114,8 @@ class SpellcheckService : public KeyedService,
    const std::vector<std::unique_ptr<SpellcheckHunspellDictionary>>&
    GetHunspellDictionaries();
  
@@ -53,7 +52,7 @@ index 557a0a2a91821a595181481f92b2a2a06dcb1f50..59e24da4be927303df8c4aac87f50778
    // Load a dictionary from a given path. Format specifies how the dictionary
    // is stored. Return value is true if successful.
    bool LoadExternalDictionary(std::string language,
-@@ -213,6 +215,8 @@ class SpellcheckService : public KeyedService,
+@@ -195,6 +197,8 @@ class SpellcheckService : public KeyedService,
    // A pointer to the BrowserContext which this service refers to.
    content::BrowserContext* context_;
  

+ 1 - 1
patches/chromium/os_metrics_mac.patch

@@ -8,7 +8,7 @@ This patch fixes the memory_instrumentation module returning 0.
 Backport https://chromium-review.googlesource.com/c/chromium/src/+/1967436.
 
 diff --git a/services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics_mac.cc b/services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics_mac.cc
-index a20418240d52..487336bbd484 100644
+index a20418240d52d65a0b905df7ff8754c50d38c0fc..487336bbd48440edde753d03fc8c79b2a4dbc299 100644
 --- a/services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics_mac.cc
 +++ b/services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics_mac.cc
 @@ -50,8 +50,34 @@ struct ChromeTaskVMInfo {

+ 4 - 4
patches/chromium/port_aria_tree_fix_for_macos_voiceover.patch

@@ -9,7 +9,7 @@ correctly announce tree items.
 CL: https://chromium-review.googlesource.com/c/chromium/src/+/2062913
 
 diff --git a/content/browser/accessibility/browser_accessibility_cocoa.h b/content/browser/accessibility/browser_accessibility_cocoa.h
-index a7e81072194c00baa0aa3159a6bfe374aaffa54f..f029ffdd0980b0d199d8e8d870255bd44118b1a5 100644
+index a5529ac8841b79f230f0fa8eae2b3cb226beb7d7..2e9ca33ccce50e7ca2552991e55878ca33e59bac 100644
 --- a/content/browser/accessibility/browser_accessibility_cocoa.h
 +++ b/content/browser/accessibility/browser_accessibility_cocoa.h
 @@ -75,6 +75,8 @@ struct AXTextEdit {
@@ -22,10 +22,10 @@ index a7e81072194c00baa0aa3159a6bfe374aaffa54f..f029ffdd0980b0d199d8e8d870255bd4
  - (NSString*)methodNameForAttribute:(NSString*)attribute;
  
 diff --git a/content/browser/accessibility/browser_accessibility_cocoa.mm b/content/browser/accessibility/browser_accessibility_cocoa.mm
-index ac2343f648ba3db19ffcb1fc1503e5d68cdb5135..eda671d3a04263fe4c5895c1bcb1359edc11f65b 100644
+index 917660269257bb51716e902d5ad70578793a4bb1..225d333884302310c8c4c0289043d8caad406c84 100644
 --- a/content/browser/accessibility/browser_accessibility_cocoa.mm
 +++ b/content/browser/accessibility/browser_accessibility_cocoa.mm
-@@ -2012,7 +2012,9 @@ - (NSArray*)rows {
+@@ -2090,7 +2090,9 @@ - (NSArray*)rows {
      return nil;
    NSMutableArray* ret = [[[NSMutableArray alloc] init] autorelease];
  
@@ -36,7 +36,7 @@ index ac2343f648ba3db19ffcb1fc1503e5d68cdb5135..eda671d3a04263fe4c5895c1bcb1359e
      for (BrowserAccessibilityCocoa* child in [self children]) {
        if ([[child role] isEqualToString:NSAccessibilityRowRole])
          [ret addObject:child];
-@@ -2465,6 +2467,19 @@ - (id)window {
+@@ -2509,6 +2511,19 @@ - (id)window {
    return manager->GetWindow();
  }
  

+ 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 mate::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 mate::EventEmitter<NativeTheme>,
   void OnNativeThemeUpdatedOnUI();
 
  private:
-  ui::NativeTheme* theme_;
+  ui::NativeTheme* ui_theme_;
+  ui::NativeTheme* web_theme_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeTheme);
 };

+ 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 RepositionTrafficLights() = 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 RepositionTrafficLights();
+  void RepositionTrafficLights() 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

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

+ 33 - 9
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,7 +20,9 @@ describe('nativeTheme module', () => {
       nativeTheme.themeSource = 'system'
       // Wait for any pending events to emit
       await delay(20)
-    })
+
+      closeAllWindows()
+    });
 
     it('is system by default', () => {
       expect(nativeTheme.themeSource).to.equal('system')
@@ -56,13 +60,33 @@ describe('nativeTheme module', () => {
 
     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')
-      })
-    })
-  })
+        nativeTheme.themeSource = 'dark';
+        expect(systemPreferences.appLevelAppearance).to.equal('dark');
+        nativeTheme.themeSource = 'light';
+        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', () => {
     it('returns a boolean', () => {