fix: emit updated on NativeTheme on the UI thread to avoid DCHECK (#20137)

* fix: emit updated on NativeTheme on the UI thread to avoid DCHECK

* Update atom_api_native_theme.cc

* spec: wait a few ticks for async events to emit so that test events do not leak into each other

shell/browser/api/atom_api_native_theme.cc

@@ -6,6 +6,9 @@
 
 #include <string>
 
+#include "base/task/post_task.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "native_mate/dictionary.h"
 #include "native_mate/object_template_builder.h"
 #include "shell/common/node_includes.h"
@@ -26,10 +29,17 @@ NativeTheme::~NativeTheme() {
   theme_->RemoveObserver(this);
 }
 
-void NativeTheme::OnNativeThemeUpdated(ui::NativeTheme* theme) {
+void NativeTheme::OnNativeThemeUpdatedOnUI() {
   Emit("updated");
 }
 
+void NativeTheme::OnNativeThemeUpdated(ui::NativeTheme* theme) {
+  base::PostTaskWithTraits(
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(&NativeTheme::OnNativeThemeUpdatedOnUI,
+                     base::Unretained(this)));
+}
+
 void NativeTheme::SetThemeSource(ui::NativeTheme::ThemeSource override) {
   theme_->set_theme_source(override);
 #if defined(OS_MACOSX)

shell/browser/api/atom_api_native_theme.h

@@ -38,6 +38,7 @@ class NativeTheme : public mate::EventEmitter<NativeTheme>,
 
   // ui::NativeThemeObserver:
   void OnNativeThemeUpdated(ui::NativeTheme* theme) override;
+  void OnNativeThemeUpdatedOnUI();
 
  private:
   ui::NativeTheme* theme_;

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

@@ -3,9 +3,10 @@ import { nativeTheme, systemPreferences } from 'electron'
 import * as os from 'os'
 import * as semver from 'semver'
 
-import { ifdescribe } from './spec-helpers'
+import { delay, ifdescribe } from './spec-helpers'
+import { emittedOnce } from './events-helpers';
 
-describe('nativeTheme module', () => {
+describe.only('nativeTheme module', () => {
   describe('nativeTheme.shouldUseDarkColors', () => {
     it('returns a boolean', () => {
       expect(nativeTheme.shouldUseDarkColors).to.be.a('boolean')
@@ -13,8 +14,10 @@ describe('nativeTheme module', () => {
   })
 
   describe('nativeTheme.themeSource', () => {
-    afterEach(() => {
+    afterEach(async () => {
       nativeTheme.themeSource = 'system'
+      // Wait for any pending events to emit
+      await delay(20)
     })
 
     it('is system by default', () => {
@@ -28,23 +31,26 @@ describe('nativeTheme module', () => {
       expect(nativeTheme.shouldUseDarkColors).to.equal(false)
     })
 
-    it('should emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value changes', () => {
+    it('should emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value changes', async () => {
+      let updatedEmitted = emittedOnce(nativeTheme, 'updated')
       nativeTheme.themeSource = 'dark'
-      let called = false
-      nativeTheme.once('updated', () => {
-        called = true
-      })
+      await updatedEmitted
+      updatedEmitted = emittedOnce(nativeTheme, 'updated')
       nativeTheme.themeSource = 'light'
-      expect(called).to.equal(true)
+      await updatedEmitted
     })
 
-    it('should not emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value is the same', () => {
+    it('should not emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value is the same', async () => {
       nativeTheme.themeSource = 'dark'
+      // Wait a few ticks to allow an async events to flush
+      await delay(20)
       let called = false
       nativeTheme.once('updated', () => {
         called = true
       })
       nativeTheme.themeSource = 'dark'
+      // Wait a few ticks to allow an async events to flush
+      await delay(20)
       expect(called).to.equal(false)
     })
 

spec-main/spec-helpers.ts

@@ -1,2 +1,4 @@
 export const ifit = (condition: boolean) => (condition ? it : it.skip)
-export const ifdescribe = (condition: boolean) => (condition ? describe : describe.skip)
+export const ifdescribe = (condition: boolean) => (condition ? describe : describe.skip)
+
+export const delay = (time: number) => new Promise(r => setTimeout(r, time))