Browse Source

fix: don't crash on tray.setContextMenu(null) (#14330)

trop[bot] 6 years ago
parent
commit
7ed6e2b909
4 changed files with 38 additions and 6 deletions
  1. 1 1
      atom/browser/api/atom_api_tray.cc
  2. 11 4
      atom/browser/ui/tray_icon_cocoa.mm
  3. 1 1
      docs/api/tray.md
  4. 25 0
      spec/api-tray-spec.js

+ 1 - 1
atom/browser/api/atom_api_tray.cc

@@ -206,7 +206,7 @@ void Tray::PopUpContextMenu(mate::Arguments* args) {
 
 void Tray::SetContextMenu(v8::Isolate* isolate, mate::Handle<Menu> menu) {
   menu_.Reset(isolate, menu.ToV8());
-  tray_icon_->SetContextMenu(menu->model());
+  tray_icon_->SetContextMenu(menu.IsEmpty() ? nullptr : menu->model());
 }
 
 gfx::Rect Tray::GetBounds() {

+ 11 - 4
atom/browser/ui/tray_icon_cocoa.mm

@@ -454,11 +454,18 @@ void TrayIconCocoa::SetContextMenu(AtomMenuModel* menu_model) {
   // Substribe to MenuClosed event.
   if (menu_model_)
     menu_model_->RemoveObserver(this);
-  menu_model->AddObserver(this);
 
-  // Create native menu.
-  menu_.reset([[AtomMenuController alloc] initWithModel:menu_model
-                                  useDefaultAccelerator:NO]);
+  menu_model_ = menu_model;
+
+  if (menu_model) {
+    menu_model->AddObserver(this);
+    // Create native menu.
+    menu_.reset([[AtomMenuController alloc] initWithModel:menu_model
+                                    useDefaultAccelerator:NO]);
+  } else {
+    menu_.reset();
+  }
+
   [status_item_view_ setMenuController:menu_.get()];
 }
 

+ 1 - 1
docs/api/tray.md

@@ -262,7 +262,7 @@ The `position` is only available on Windows, and it is (0, 0) by default.
 
 #### `tray.setContextMenu(menu)`
 
-* `menu` Menu
+* `menu` Menu | null
 
 Sets the context menu for this icon.
 

+ 25 - 0
spec/api-tray-spec.js

@@ -0,0 +1,25 @@
+const {remote} = require('electron')
+const {Menu, Tray, nativeImage} = remote
+
+describe('tray module', () => {
+  describe('tray.setContextMenu', () => {
+    let tray
+
+    beforeEach(() => {
+      tray = new Tray(nativeImage.createEmpty())
+    })
+
+    afterEach(() => {
+      tray.destroy()
+      tray = null
+    })
+
+    it('accepts menu instance', () => {
+      tray.setContextMenu(new Menu())
+    })
+
+    it('accepts null', () => {
+      tray.setContextMenu(null)
+    })
+  })
+})