From 47e9deeb9ab13f0e6b13f6210df1caf11f33adf3 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Thu, 4 Jun 2015 15:32:33 +0800
Subject: [PATCH] Remove Menu::AttachToWindow

It makes the logic more complex without any benefit
---
 atom/browser/api/atom_api_menu.cc          |  5 -----
 atom/browser/api/atom_api_menu.h           | 24 +++++++++++++++++++++-
 atom/browser/api/atom_api_window.cc        |  6 ++++++
 atom/browser/api/atom_api_window.h         |  5 +++++
 atom/browser/api/lib/browser-window.coffee |  5 +----
 5 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc
index c0bfff3f050c..c0704a22d052 100644
--- a/atom/browser/api/atom_api_menu.cc
+++ b/atom/browser/api/atom_api_menu.cc
@@ -70,10 +70,6 @@ void Menu::MenuWillShow(ui::SimpleMenuModel* source) {
   menu_will_show_.Run();
 }
 
-void Menu::AttachToWindow(Window* window) {
-  window->window()->SetMenu(model_.get());
-}
-
 void Menu::InsertItemAt(
     int index, int command_id, const base::string16& label) {
   model_->InsertItemAt(index, command_id, label);
@@ -168,7 +164,6 @@ void Menu::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("isItemCheckedAt", &Menu::IsItemCheckedAt)
       .SetMethod("isEnabledAt", &Menu::IsEnabledAt)
       .SetMethod("isVisibleAt", &Menu::IsVisibleAt)
-      .SetMethod("attachToWindow", &Menu::AttachToWindow)
       .SetMethod("_popup", &Menu::Popup)
       .SetMethod("_popupAt", &Menu::PopupAt);
 }
diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h
index 37e544a82e09..0c2743878f78 100644
--- a/atom/browser/api/atom_api_menu.h
+++ b/atom/browser/api/atom_api_menu.h
@@ -51,7 +51,6 @@ class Menu : public mate::Wrappable,
   void ExecuteCommand(int command_id, int event_flags) override;
   void MenuWillShow(ui::SimpleMenuModel* source) override;
 
-  virtual void AttachToWindow(Window* window);
   virtual void Popup(Window* window) = 0;
   virtual void PopupAt(Window* window, int x, int y) = 0;
 
@@ -99,4 +98,27 @@ class Menu : public mate::Wrappable,
 
 }  // namespace atom
 
+
+namespace mate {
+
+template<>
+struct Converter<ui::SimpleMenuModel*> {
+  static bool FromV8(v8::Isolate* isolate, v8::Local<v8::Value> val,
+                     ui::SimpleMenuModel** out) {
+    // null would be tranfered to NULL.
+    if (val->IsNull()) {
+      *out = nullptr;
+      return true;
+    }
+
+    atom::api::Menu* menu;
+    if (!Converter<atom::api::Menu*>::FromV8(isolate, val, &menu))
+      return false;
+    *out = menu->model();
+    return true;
+  }
+};
+
+}  // namespace mate
+
 #endif  // ATOM_BROWSER_API_ATOM_API_MENU_H_
diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc
index a016c16da012..f689af1288b7 100644
--- a/atom/browser/api/atom_api_window.cc
+++ b/atom/browser/api/atom_api_window.cc
@@ -4,6 +4,7 @@
 
 #include "atom/browser/api/atom_api_window.h"
 
+#include "atom/browser/api/atom_api_menu.h"
 #include "atom/browser/api/atom_api_web_contents.h"
 #include "atom/browser/browser.h"
 #include "atom/browser/native_window.h"
@@ -437,6 +438,10 @@ void Window::SetOverlayIcon(const gfx::Image& overlay,
   window_->SetOverlayIcon(overlay, description);
 }
 
+void Window::SetMenu(ui::SimpleMenuModel* menu) {
+  window_->SetMenu(menu);
+}
+
 void Window::SetAutoHideMenuBar(bool auto_hide) {
   window_->SetAutoHideMenuBar(auto_hide);
 }
@@ -535,6 +540,7 @@ void Window::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("print", &Window::Print)
       .SetMethod("setProgressBar", &Window::SetProgressBar)
       .SetMethod("setOverlayIcon", &Window::SetOverlayIcon)
+      .SetMethod("_setMenu", &Window::SetMenu)
       .SetMethod("setAutoHideMenuBar", &Window::SetAutoHideMenuBar)
       .SetMethod("isMenuBarAutoHide", &Window::IsMenuBarAutoHide)
       .SetMethod("setMenuBarVisibility", &Window::SetMenuBarVisibility)
diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h
index 71de91df6913..72d2c842d8e8 100644
--- a/atom/browser/api/atom_api_window.h
+++ b/atom/browser/api/atom_api_window.h
@@ -25,6 +25,10 @@ class Arguments;
 class Dictionary;
 }
 
+namespace ui {
+class SimpleMenuModel;
+}
+
 namespace atom {
 
 class NativeWindow;
@@ -134,6 +138,7 @@ class Window : public mate::EventEmitter,
   void SetProgressBar(double progress);
   void SetOverlayIcon(const gfx::Image& overlay,
                       const std::string& description);
+  void SetMenu(ui::SimpleMenuModel* menu);
   void SetAutoHideMenuBar(bool auto_hide);
   bool IsMenuBarAutoHide();
   void SetMenuBarVisibility(bool visible);
diff --git a/atom/browser/api/lib/browser-window.coffee b/atom/browser/api/lib/browser-window.coffee
index 5042b78a3adc..7e4711e817c4 100644
--- a/atom/browser/api/lib/browser-window.coffee
+++ b/atom/browser/api/lib/browser-window.coffee
@@ -70,13 +70,10 @@ BrowserWindow::getDevToolsWebContents = ->
   wrapWebContents @_getDevToolsWebContents()
 
 BrowserWindow::setMenu = (menu) ->
-  if process.platform is 'darwin'
-    throw new Error('BrowserWindow.setMenu is not available on OS X')
-
   throw new TypeError('Invalid menu') unless menu is null or menu?.constructor?.name is 'Menu'
 
   @menu = menu  # Keep a reference of menu in case of GC.
-  @menu.attachToWindow this
+  @_setMenu menu
 
 BrowserWindow.getAllWindows = ->
   windows = BrowserWindow.windows