Merge pull request #2462 from atom/tray-event

More fixes of Tray

atom/browser/api/atom_api_menu.cc

@@ -20,7 +20,7 @@ namespace atom {
 namespace api {
 
 Menu::Menu()
-    : model_(new ui::SimpleMenuModel(this)),
+    : model_(new AtomMenuModel(this)),
       parent_(NULL) {
 }
 

atom/browser/api/atom_api_menu.h

@@ -8,9 +8,9 @@
 #include <string>
 
 #include "atom/browser/api/atom_api_window.h"
+#include "atom/browser/ui/atom_menu_model.h"
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
-#include "ui/base/models/simple_menu_model.h"
 #include "native_mate/wrappable.h"
 
 namespace atom {
@@ -18,7 +18,7 @@ namespace atom {
 namespace api {
 
 class Menu : public mate::Wrappable,
-             public ui::SimpleMenuModel::Delegate {
+             public AtomMenuModel::Delegate {
  public:
   static mate::Wrappable* Create();
 
@@ -33,7 +33,7 @@ class Menu : public mate::Wrappable,
   static void SendActionToFirstResponder(const std::string& action);
 #endif
 
-  ui::SimpleMenuModel* model() const { return model_.get(); }
+  AtomMenuModel* model() const { return model_.get(); }
 
  protected:
   Menu();
@@ -42,7 +42,7 @@ class Menu : public mate::Wrappable,
   // mate::Wrappable:
   void AfterInit(v8::Isolate* isolate) override;
 
-  // ui::SimpleMenuModel::Delegate implementations:
+  // ui::SimpleMenuModel::Delegate:
   bool IsCommandIdChecked(int command_id) const override;
   bool IsCommandIdEnabled(int command_id) const override;
   bool IsCommandIdVisible(int command_id) const override;
@@ -54,7 +54,7 @@ class Menu : public mate::Wrappable,
   virtual void Popup(Window* window) = 0;
   virtual void PopupAt(Window* window, int x, int y) = 0;
 
-  scoped_ptr<ui::SimpleMenuModel> model_;
+  scoped_ptr<AtomMenuModel> model_;
   Menu* parent_;
 
  private:
@@ -102,9 +102,9 @@ class Menu : public mate::Wrappable,
 namespace mate {
 
 template<>
-struct Converter<ui::SimpleMenuModel*> {
+struct Converter<atom::AtomMenuModel*> {
   static bool FromV8(v8::Isolate* isolate, v8::Local<v8::Value> val,
-                     ui::SimpleMenuModel** out) {
+                     atom::AtomMenuModel** out) {
     // null would be tranfered to NULL.
     if (val->IsNull()) {
       *out = nullptr;

atom/browser/api/atom_api_tray.cc

@@ -120,10 +120,10 @@ void Tray::DisplayBalloon(mate::Arguments* args,
   tray_icon_->DisplayBalloon(icon, title, content);
 }
 
-void Tray::PopContextMenu(mate::Arguments* args) {
+void Tray::PopUpContextMenu(mate::Arguments* args) {
   gfx::Point pos;
   args->GetNext(&pos);
-  tray_icon_->PopContextMenu(pos);
+  tray_icon_->PopUpContextMenu(pos);
 }
 
 void Tray::SetContextMenu(mate::Arguments* args, Menu* menu) {
@@ -151,7 +151,7 @@ void Tray::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setTitle", &Tray::SetTitle)
       .SetMethod("setHighlightMode", &Tray::SetHighlightMode)
       .SetMethod("displayBalloon", &Tray::DisplayBalloon)
-      .SetMethod("popContextMenu", &Tray::PopContextMenu)
+      .SetMethod("popUpContextMenu", &Tray::PopUpContextMenu)
       .SetMethod("_setContextMenu", &Tray::SetContextMenu);
 }
 

atom/browser/api/atom_api_tray.h

@@ -60,7 +60,7 @@ class Tray : public mate::EventEmitter,
   void SetTitle(mate::Arguments* args, const std::string& title);
   void SetHighlightMode(mate::Arguments* args, bool highlight);
   void DisplayBalloon(mate::Arguments* args, const mate::Dictionary& options);
-  void PopContextMenu(mate::Arguments* args);
+  void PopUpContextMenu(mate::Arguments* args);
   void SetContextMenu(mate::Arguments* args, Menu* menu);
 
  private:

atom/browser/api/lib/tray.coffee

@@ -3,8 +3,12 @@ bindings = process.atomBinding 'tray'
 
 Tray = bindings.Tray
 Tray::__proto__ = EventEmitter.prototype
+
 Tray::setContextMenu = (menu) ->
   @_setContextMenu menu
   @menu = menu  # Keep a strong reference of menu.
 
+# Keep compatibility with old APIs.
+Tray::popContextMenu = Tray::popUpContextMenu
+
 module.exports = Tray

atom/browser/ui/atom_menu_model.cc

@@ -0,0 +1,22 @@
+// Copyright (c) 2015 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/ui/atom_menu_model.h"
+
+namespace atom {
+
+AtomMenuModel::AtomMenuModel(Delegate* delegate)
+    : ui::SimpleMenuModel(delegate),
+      delegate_(delegate) {
+}
+
+AtomMenuModel::~AtomMenuModel() {
+}
+
+void AtomMenuModel::MenuClosed() {
+  ui::SimpleMenuModel::MenuClosed();
+  FOR_EACH_OBSERVER(Observer, observers_, MenuClosed());
+}
+
+}  // namespace atom

atom/browser/ui/atom_menu_model.h

@@ -0,0 +1,47 @@
+// Copyright (c) 2015 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_
+#define ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_
+
+#include "base/observer_list.h"
+#include "ui/base/models/simple_menu_model.h"
+
+namespace atom {
+
+class AtomMenuModel : public ui::SimpleMenuModel {
+ public:
+  class Delegate : public ui::SimpleMenuModel::Delegate {
+   public:
+    virtual ~Delegate() {}
+  };
+
+  class Observer {
+   public:
+    virtual ~Observer() {}
+
+    // Notifies the menu has been closed.
+    virtual void MenuClosed() {}
+  };
+
+  explicit AtomMenuModel(Delegate* delegate);
+  virtual ~AtomMenuModel();
+
+  void AddObserver(Observer* obs) { observers_.AddObserver(obs); }
+  void RemoveObserver(Observer* obs) { observers_.RemoveObserver(obs); }
+
+  // ui::SimpleMenuModel:
+  void MenuClosed() override;
+
+ private:
+  Delegate* delegate_;  // weak ref.
+
+  ObserverList<Observer> observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(AtomMenuModel);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_

atom/browser/ui/cocoa/atom_menu_controller.mm

@@ -5,12 +5,12 @@
 
 #import "atom/browser/ui/cocoa/atom_menu_controller.h"
 
+#include "atom/browser/ui/atom_menu_model.h"
 #include "base/logging.h"
 #include "base/strings/sys_string_conversions.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/platform_accelerator_cocoa.h"
 #include "ui/base/l10n/l10n_util_mac.h"
-#include "ui/base/models/simple_menu_model.h"
 #include "ui/events/cocoa/cocoa_event_utils.h"
 #include "ui/gfx/image/image.h"
 
@@ -120,8 +120,7 @@
     [item setTarget:nil];
     [item setAction:nil];
     ui::MenuModel* submenuModel = model->GetSubmenuModelAt(index);
-    NSMenu* submenu =
+    NSMenu* submenu = [self menuFromModel:submenuModel];
-        [self menuFromModel:(ui::SimpleMenuModel*)submenuModel];
     [submenu setTitle:[item title]];
     [item setSubmenu:submenu];
 

atom/browser/ui/tray_icon.cc

@@ -26,7 +26,7 @@ void TrayIcon::DisplayBalloon(const gfx::Image& icon,
                               const base::string16& contents) {
 }
 
-void TrayIcon::PopContextMenu(const gfx::Point& pos) {
+void TrayIcon::PopUpContextMenu(const gfx::Point& pos) {
 }
 
 void TrayIcon::NotifyClicked(const gfx::Rect& bounds, int modifiers) {

atom/browser/ui/tray_icon.h

@@ -47,7 +47,7 @@ class TrayIcon {
                               const base::string16& title,
                               const base::string16& contents);
 
-  virtual void PopContextMenu(const gfx::Point& pos);
+  virtual void PopUpContextMenu(const gfx::Point& pos);
 
   // Set the context menu for this icon.
   virtual void SetContextMenu(ui::SimpleMenuModel* menu_model) = 0;

atom/browser/ui/tray_icon_cocoa.h

@@ -9,6 +9,7 @@
 
 #include <string>
 
+#include "atom/browser/ui/atom_menu_model.h"
 #include "atom/browser/ui/tray_icon.h"
 #include "base/mac/scoped_nsobject.h"
 
@@ -17,7 +18,8 @@
 
 namespace atom {
 
-class TrayIconCocoa : public TrayIcon {
+class TrayIconCocoa : public TrayIcon,
+                      public AtomMenuModel::Observer {
  public:
   TrayIconCocoa();
   virtual ~TrayIconCocoa();
@@ -27,9 +29,13 @@ class TrayIconCocoa : public TrayIcon {
   void SetToolTip(const std::string& tool_tip) override;
   void SetTitle(const std::string& title) override;
   void SetHighlightMode(bool highlight) override;
-  void PopContextMenu(const gfx::Point& pos) override;
+  void PopUpContextMenu(const gfx::Point& pos) override;
   void SetContextMenu(ui::SimpleMenuModel* menu_model) override;
 
+ protected:
+  // AtomMenuModel::Observer:
+  void MenuClosed() override;
+
  private:
   // Atom custom view for NSStatusItem.
   base::scoped_nsobject<StatusItemView> status_item_view_;
@@ -37,6 +43,9 @@ class TrayIconCocoa : public TrayIcon {
   // Status menu shown when right-clicking the system icon.
   base::scoped_nsobject<AtomMenuController> menu_;
 
+  // Used for unregistering observer.
+  AtomMenuModel* menu_model_;  // weak ref.
+
   DISALLOW_COPY_AND_ASSIGN(TrayIconCocoa);
 };
 

atom/browser/ui/tray_icon_cocoa.mm

@@ -207,27 +207,30 @@ const CGFloat kVerticalTitleMargin = 2;
   }
   inMouseEventSequence_ = NO;
 
-  // Single click
+  // Show menu when single clicked on the icon.
-  if (event.clickCount == 1) {
+  if (event.clickCount == 1 && menuController_)
-    if (menuController_) {
+    [statusItem_ popUpStatusItemMenu:[menuController_ menu]];
-      [statusItem_ popUpStatusItemMenu:[menuController_ menu]];
-    }
 
+  // Don't emit click events when menu is showing.
+  if (menuController_)
+    return;
+
+  // Single click event.
+  if (event.clickCount == 1)
     trayIcon_->NotifyClicked(
         [self getBoundsFromEvent:event],
         ui::EventFlagsFromModifiers([event modifierFlags]));
-  }
 
-  // Double click
+  // Double click event.
-  if (event.clickCount == 2 && !menuController_) {
+  if (event.clickCount == 2)
     trayIcon_->NotifyDoubleClicked(
         [self getBoundsFromEvent:event],
         ui::EventFlagsFromModifiers([event modifierFlags]));
-  }
+
   [self setNeedsDisplay:YES];
 }
 
-- (void)popContextMenu {
+- (void)popUpContextMenu {
   if (menuController_ && ![menuController_ isMenuOpen]) {
     // Redraw the dray icon to show highlight if it is enabled.
     [self setNeedsDisplay:YES];
@@ -278,11 +281,13 @@ const CGFloat kVerticalTitleMargin = 2;
 
 namespace atom {
 
-TrayIconCocoa::TrayIconCocoa() {
+TrayIconCocoa::TrayIconCocoa() : menu_model_(nullptr) {
 }
 
 TrayIconCocoa::~TrayIconCocoa() {
   [status_item_view_ removeItem];
+  if (menu_model_)
+    menu_model_->RemoveObserver(this);
 }
 
 void TrayIconCocoa::SetImage(const gfx::Image& image) {
@@ -311,15 +316,25 @@ void TrayIconCocoa::SetHighlightMode(bool highlight) {
   [status_item_view_ setHighlight:highlight];
 }
 
-void TrayIconCocoa::PopContextMenu(const gfx::Point& pos) {
+void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos) {
-  [status_item_view_ popContextMenu];
+  [status_item_view_ popUpContextMenu];
 }
 
 void TrayIconCocoa::SetContextMenu(ui::SimpleMenuModel* menu_model) {
+  // Substribe to MenuClosed event.
+  if (menu_model_)
+    menu_model_->RemoveObserver(this);
+  static_cast<AtomMenuModel*>(menu_model)->AddObserver(this);
+
+  // Create native menu.
   menu_.reset([[AtomMenuController alloc] initWithModel:menu_model]);
   [status_item_view_ setMenuController:menu_.get()];
 }
 
+void TrayIconCocoa::MenuClosed() {
+  [status_item_view_ setNeedsDisplay:YES];
+}
+
 // static
 TrayIcon* TrayIcon::Create() {
   return new TrayIconCocoa;

atom/browser/ui/win/notify_icon.cc

@@ -86,8 +86,10 @@ void NotifyIcon::HandleClickEvent(const gfx::Point& cursor_pos,
       NotifyClicked(gfx::Rect(rect), modifiers);
     return;
   } else if (!double_button_click) {  // single right click
-    NotifyRightClicked(gfx::Rect(rect), modifiers);
+    if (menu_model_)
-    PopContextMenu(cursor_pos);
+      PopUpContextMenu(cursor_pos);
+    else
+      NotifyRightClicked(gfx::Rect(rect), modifiers);
   }
 }
 
@@ -161,7 +163,7 @@ void NotifyIcon::DisplayBalloon(const gfx::Image& icon,
     LOG(WARNING) << "Unable to create status tray balloon.";
 }
 
-void NotifyIcon::PopContextMenu(const gfx::Point& pos) {
+void NotifyIcon::PopUpContextMenu(const gfx::Point& pos) {
   // Returns if context menu isn't set.
   if (!menu_model_)
     return;

atom/browser/ui/win/notify_icon.h

@@ -52,7 +52,7 @@ class NotifyIcon : public TrayIcon {
   void DisplayBalloon(const gfx::Image& icon,
                       const base::string16& title,
                       const base::string16& contents) override;
-  void PopContextMenu(const gfx::Point& pos) override;
+  void PopUpContextMenu(const gfx::Point& pos) override;
   void SetContextMenu(ui::SimpleMenuModel* menu_model) override;
 
  private:

docs/api/tray.md

@@ -60,7 +60,7 @@ Creates a new tray icon associated with the `image`.
 
 Emitted when the tray icon is clicked.
 
-__Note:__ The `bounds` payload is only implemented on OS X and Windows 7 or newer.
+__Note:__ The `bounds` payload is only implemented on OS X and Windows.
 
 ### Event: 'right-clicked'
 
@@ -173,7 +173,7 @@ Displays a tray balloon.
 
 __Note:__ This is only implemented on Windows.
 
-### Tray.popContextMenu([position])
+### Tray.popUpContextMenu([position])
 
 * `position` Object - The pop position
   * `x` Integer

filenames.gypi

@@ -166,6 +166,8 @@
       'atom/browser/ui/accelerator_util.h',
       'atom/browser/ui/accelerator_util_mac.mm',
       'atom/browser/ui/accelerator_util_views.cc',
+      'atom/browser/ui/atom_menu_model.cc',
+      'atom/browser/ui/atom_menu_model.h',
       'atom/browser/ui/cocoa/atom_menu_controller.h',
       'atom/browser/ui/cocoa/atom_menu_controller.mm',
       'atom/browser/ui/cocoa/event_processing_window.h',