fix: destroy tray on current tick (#18196)

This code was originally added in #6448 to handle an edge case crash in 10.9, and we no longer support 10.9 and therefore no longer need to account for this case.

It addressed the crash, but also created a race condition whereby when a new tray is created the old tray's destroy wouldn't have been fully completed and therefore a new one would be spawned. This fixes that by destroying the tray on the current tick once more.

atom/browser/api/atom_api_tray.cc

@@ -60,11 +60,7 @@ Tray::Tray(v8::Isolate* isolate,
   InitWith(isolate, wrapper);
 }
 
-Tray::~Tray() {
+Tray::~Tray() = default;
-  // Destroy the native tray in next tick.
-  base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
-                                                  tray_icon_.release());
-}
 
 // static
 mate::WrappableBase* Tray::New(mate::Handle<NativeImage> image,

spec/api-tray-spec.js

@@ -9,12 +9,12 @@ describe('tray module', () => {
     tray = new Tray(nativeImage.createEmpty())
   })
 
-  afterEach(() => {
-    tray.destroy()
-    tray = null
-  })
-
   describe('tray.setContextMenu', () => {
+    afterEach(() => {
+      tray.destroy()
+      tray = null
+    })
+
     it('accepts menu instance', () => {
       tray.setContextMenu(new Menu())
     })
@@ -24,11 +24,24 @@ describe('tray module', () => {
     })
   })
 
+  describe('tray.destroy()', () => {
+    it('destroys a tray', () => {
+      expect(tray.isDestroyed()).to.be.false()
+      tray.destroy()
+
+      expect(tray.isDestroyed()).to.be.true()
+      tray = null
+    })
+  })
+
   describe('tray.popUpContextMenu', () => {
+    afterEach(() => {
+      tray.destroy()
+      tray = null
+    })
+
     before(function () {
-      if (process.platform !== 'win32') {
+      if (process.platform !== 'win32') this.skip()
-        this.skip()
-      }
     })
 
     it('can be called when menu is showing', (done) => {
@@ -44,20 +57,29 @@ describe('tray module', () => {
   describe('tray.setImage', () => {
     it('accepts empty image', () => {
       tray.setImage(nativeImage.createEmpty())
+
+      tray.destroy()
+      tray = null
     })
   })
 
   describe('tray.setPressedImage', () => {
     it('accepts empty image', () => {
       tray.setPressedImage(nativeImage.createEmpty())
+
+      tray.destroy()
+      tray = null
     })
   })
 
   describe('tray title get/set', () => {
     before(function () {
-      if (process.platform !== 'darwin') {
+      if (process.platform !== 'darwin') this.skip()
-        this.skip()
+    })
-      }
+
+    afterEach(() => {
+      tray.destroy()
+      tray = null
     })
 
     it('sets/gets non-empty title', () => {