From 5a6ff0f80d3a36ad7aa2434377560580a1a36d4d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 2 Oct 2013 21:24:21 +0800 Subject: [PATCH 01/18] win: Add BrowserWindow.setMenu API. --- browser/api/atom_api_menu.cc | 4 ++++ browser/api/atom_api_menu.h | 4 +++- browser/api/atom_api_menu_win.cc | 19 +++++++++++++++++++ browser/api/lib/browser-window.coffee | 8 ++++++++ browser/default_app/default_app.js | 2 ++ browser/native_window_win.cc | 6 ++++++ browser/native_window_win.h | 12 ++++++++++++ 7 files changed, 54 insertions(+), 1 deletion(-) diff --git a/browser/api/atom_api_menu.cc b/browser/api/atom_api_menu.cc index a057f307eb98..06ac6b2542c0 100644 --- a/browser/api/atom_api_menu.cc +++ b/browser/api/atom_api_menu.cc @@ -351,6 +351,10 @@ void Menu::Initialize(v8::Handle target) { NODE_SET_PROTOTYPE_METHOD(t, "popup", Popup); +#if defined(OS_WIN) + NODE_SET_PROTOTYPE_METHOD(t, "attachToWindow", AttachToWindow); +#endif + target->Set(v8::String::NewSymbol("Menu"), t->GetFunction()); #if defined(OS_MACOSX) diff --git a/browser/api/atom_api_menu.h b/browser/api/atom_api_menu.h index afba525f5758..bae25aa0573a 100644 --- a/browser/api/atom_api_menu.h +++ b/browser/api/atom_api_menu.h @@ -68,7 +68,9 @@ class Menu : public EventEmitter, static v8::Handle Popup(const v8::Arguments &args); -#if defined(OS_MACOSX) +#if defined(OS_WIN) + static v8::Handle AttachToWindow(const v8::Arguments &args); +#elif defined(OS_MACOSX) static v8::Handle SetApplicationMenu(const v8::Arguments &args); static v8::Handle SendActionToFirstResponder( const v8::Arguments &args); diff --git a/browser/api/atom_api_menu_win.cc b/browser/api/atom_api_menu_win.cc index 05d133356650..019d011df077 100644 --- a/browser/api/atom_api_menu_win.cc +++ b/browser/api/atom_api_menu_win.cc @@ -4,7 +4,9 @@ #include "browser/api/atom_api_menu_win.h" +#include "browser/native_window_win.h" #include "browser/ui/win/menu_2.h" +#include "common/v8_conversions.h" #include "ui/gfx/point.h" namespace atom { @@ -23,6 +25,23 @@ void MenuWin::Popup(NativeWindow* native_window) { menu_->RunContextMenuAt(gfx::Point(0, 0)); } +// static +v8::Handle Menu::AttachToWindow(const v8::Arguments& args) { + v8::HandleScope scope; + + Menu* self = ObjectWrap::Unwrap(args.This()); + if (self == NULL) + return node::ThrowError("Menu is already destroyed"); + + NativeWindow* native_window; + if (!FromV8Arguments(args, &native_window)) + return node::ThrowTypeError("Bad argument"); + + static_cast(native_window)->SetMenu(self->model_.get()); + + return v8::Undefined(); +} + // static Menu* Menu::Create(v8::Handle wrapper) { return new MenuWin(wrapper); diff --git a/browser/api/lib/browser-window.coffee b/browser/api/lib/browser-window.coffee index 452151c7bcd7..56d03e997c66 100644 --- a/browser/api/lib/browser-window.coffee +++ b/browser/api/lib/browser-window.coffee @@ -17,6 +17,14 @@ BrowserWindow::toggleDevTools = -> BrowserWindow::restart = -> @loadUrl(@getUrl()) +BrowserWindow::setMenu = (menu) -> + throw new Error('BrowserWindow.setMenu is only available on Windows') unless process.platform is 'win32' + + throw new TypeError('Invalid menu') unless menu?.constructor?.name is 'Menu' + + @menu = menu # Keep a reference of menu in case of GC. + @menu.attachToWindow this + BrowserWindow.getFocusedWindow = -> windows = objectsRegistry.getAllWindows() return window for window in windows when window.isFocused() diff --git a/browser/default_app/default_app.js b/browser/default_app/default_app.js index 7be7f5bfc4c9..a40e876c43cc 100644 --- a/browser/default_app/default_app.js +++ b/browser/default_app/default_app.js @@ -159,6 +159,8 @@ app.on('finish-launching', function() { if (process.platform == 'darwin') Menu.setApplicationMenu(menu); + else + mainWindow.setMenu(menu); ipc.on('message', function(processId, routingId, type) { console.log(type); diff --git a/browser/native_window_win.cc b/browser/native_window_win.cc index c8ac370e0c8d..57c5edf27e44 100644 --- a/browser/native_window_win.cc +++ b/browser/native_window_win.cc @@ -6,6 +6,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/values.h" +#include "browser/ui/win/menu_2.h" #include "common/draggable_region.h" #include "common/options_switches.h" #include "content/public/browser/native_web_keyboard_event.h" @@ -325,6 +326,11 @@ gfx::NativeWindow NativeWindowWin::GetNativeWindow() { return window_->GetNativeView(); } +void NativeWindowWin::SetMenu(ui::MenuModel* menu_model) { + menu_.reset(new atom::Menu2(menu_model, true)); + ::SetMenu(GetNativeWindow(), menu_->GetNativeMenu()); +} + void NativeWindowWin::UpdateDraggableRegions( const std::vector& regions) { if (has_frame_) diff --git a/browser/native_window_win.h b/browser/native_window_win.h index 810b3eeb55ed..e5cdad1eca83 100644 --- a/browser/native_window_win.h +++ b/browser/native_window_win.h @@ -12,6 +12,10 @@ #include "ui/gfx/size.h" #include "ui/views/widget/widget_delegate.h" +namespace ui { +class MenuModel; +} + namespace views { class WebView; class Widget; @@ -19,6 +23,8 @@ class Widget; namespace atom { +class Menu2; + class NativeWindowWin : public NativeWindow, public views::WidgetDelegate { public: @@ -60,6 +66,9 @@ class NativeWindowWin : public NativeWindow, virtual bool IsKiosk() OVERRIDE; virtual gfx::NativeWindow GetNativeWindow() OVERRIDE; + // Set the native window menu. + void SetMenu(ui::MenuModel* menu_model); + SkRegion* draggable_region() { return draggable_region_.get(); } protected: @@ -89,6 +98,9 @@ class NativeWindowWin : public NativeWindow, scoped_ptr window_; views::WebView* web_view_; // managed by window_. + // The window menu. + scoped_ptr menu_; + scoped_ptr draggable_region_; bool resizable_; From 5c8566e0d4fedc22c9fa6e6bf7e9ca340fe30e5e Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 2 Oct 2013 21:43:52 +0800 Subject: [PATCH 02/18] win: Show popup menu at right place. --- browser/api/atom_api_menu_win.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/browser/api/atom_api_menu_win.cc b/browser/api/atom_api_menu_win.cc index 019d011df077..3e41ab18d1f6 100644 --- a/browser/api/atom_api_menu_win.cc +++ b/browser/api/atom_api_menu_win.cc @@ -8,6 +8,7 @@ #include "browser/ui/win/menu_2.h" #include "common/v8_conversions.h" #include "ui/gfx/point.h" +#include "ui/gfx/screen.h" namespace atom { @@ -21,8 +22,9 @@ MenuWin::~MenuWin() { } void MenuWin::Popup(NativeWindow* native_window) { + gfx::Point cursor = gfx::Screen::GetNativeScreen()->GetCursorScreenPoint(); menu_.reset(new atom::Menu2(model_.get())); - menu_->RunContextMenuAt(gfx::Point(0, 0)); + menu_->RunContextMenuAt(cursor); } // static From 6748573dee92ffc8b549c20e61e780a0f7d48dd4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 2 Oct 2013 21:51:19 +0800 Subject: [PATCH 03/18] Show a simple menu in default_app on Windows. --- browser/default_app/default_app.js | 277 ++++++++++++++++------------- 1 file changed, 156 insertions(+), 121 deletions(-) diff --git a/browser/default_app/default_app.js b/browser/default_app/default_app.js index a40e876c43cc..dd43d8296dcb 100644 --- a/browser/default_app/default_app.js +++ b/browser/default_app/default_app.js @@ -31,7 +31,6 @@ app.on('finish-launching', function() { }); mainWindow.on('closed', function() { - console.log('closed'); mainWindow = null; }); @@ -39,131 +38,167 @@ app.on('finish-launching', function() { console.log('unresponsive'); }); - var template = [ - { - label: 'Atom Shell', - submenu: [ - { - label: 'About Atom Shell', - selector: 'orderFrontStandardAboutPanel:' - }, - { - type: 'separator' - }, - { - label: 'Hide Atom Shell', - accelerator: 'Command+H', - selector: 'hide:' - }, - { - label: 'Hide Others', - accelerator: 'Command+Shift+H', - selector: 'hideOtherApplications:' - }, - { - label: 'Show All', - selector: 'unhideAllApplications:' - }, - { - type: 'separator' - }, - { - label: 'Quit', - accelerator: 'Command+Q', - click: function() { app.quit(); } - }, - ] - }, - { - label: 'Edit', - submenu: [ - { - label: 'Undo', - accelerator: 'Command+Z', - selector: 'undo:' - }, - { - label: 'Redo', - accelerator: 'Shift+Command+Z', - selector: 'redo:' - }, - { - type: 'separator' - }, - { - label: 'Cut', - accelerator: 'Command+X', - selector: 'cut:' - }, - { - label: 'Copy', - accelerator: 'Command+C', - selector: 'copy:' - }, - { - label: 'Paste', - accelerator: 'Command+V', - selector: 'paste:' - }, - { - label: 'Select All', - accelerator: 'Command+A', - selector: 'selectAll:' - }, - ] - }, - { - label: 'View', - submenu: [ - { - label: 'Reload', - accelerator: 'Command+R', - click: function() { BrowserWindow.getFocusedWindow().restart(); } - }, - { - label: 'Enter Fullscreen', - click: function() { BrowserWindow.getFocusedWindow().setFullscreen(true); } - }, - { - label: 'Toggle DevTools', - accelerator: 'Alt+Command+I', - click: function() { BrowserWindow.getFocusedWindow().toggleDevTools(); } - }, - ] - }, - { - label: 'Window', - submenu: [ - { - label: 'Minimize', - accelerator: 'Command+M', - selector: 'performMiniaturize:' - }, - { - label: 'Close', - accelerator: 'Command+W', - selector: 'performClose:' - }, - { - type: 'separator' - }, - { - label: 'Bring All to Front', - selector: 'arrangeInFront:' - }, - ] - }, - ]; + if (process.platform == 'darwin') { + var template = [ + { + label: 'Atom Shell', + submenu: [ + { + label: 'About Atom Shell', + selector: 'orderFrontStandardAboutPanel:' + }, + { + type: 'separator' + }, + { + label: 'Hide Atom Shell', + accelerator: 'Command+H', + selector: 'hide:' + }, + { + label: 'Hide Others', + accelerator: 'Command+Shift+H', + selector: 'hideOtherApplications:' + }, + { + label: 'Show All', + selector: 'unhideAllApplications:' + }, + { + type: 'separator' + }, + { + label: 'Quit', + accelerator: 'Command+Q', + click: function() { app.quit(); } + }, + ] + }, + { + label: 'Edit', + submenu: [ + { + label: 'Undo', + accelerator: 'Command+Z', + selector: 'undo:' + }, + { + label: 'Redo', + accelerator: 'Shift+Command+Z', + selector: 'redo:' + }, + { + type: 'separator' + }, + { + label: 'Cut', + accelerator: 'Command+X', + selector: 'cut:' + }, + { + label: 'Copy', + accelerator: 'Command+C', + selector: 'copy:' + }, + { + label: 'Paste', + accelerator: 'Command+V', + selector: 'paste:' + }, + { + label: 'Select All', + accelerator: 'Command+A', + selector: 'selectAll:' + }, + ] + }, + { + label: 'View', + submenu: [ + { + label: 'Reload', + accelerator: 'Command+R', + click: function() { BrowserWindow.getFocusedWindow().restart(); } + }, + { + label: 'Enter Fullscreen', + click: function() { BrowserWindow.getFocusedWindow().setFullscreen(true); } + }, + { + label: 'Toggle DevTools', + accelerator: 'Alt+Command+I', + click: function() { BrowserWindow.getFocusedWindow().toggleDevTools(); } + }, + ] + }, + { + label: 'Window', + submenu: [ + { + label: 'Minimize', + accelerator: 'Command+M', + selector: 'performMiniaturize:' + }, + { + label: 'Close', + accelerator: 'Command+W', + selector: 'performClose:' + }, + { + type: 'separator' + }, + { + label: 'Bring All to Front', + selector: 'arrangeInFront:' + }, + ] + }, + ]; - menu = Menu.buildFromTemplate(template); - - if (process.platform == 'darwin') + menu = Menu.buildFromTemplate(template); Menu.setApplicationMenu(menu); - else + } else { + var template = [ + { + label: 'File', + submenu: [ + { + label: 'Open', + accelerator: 'Command+O', + }, + { + label: 'Close', + accelerator: 'Command+W', + click: function() { mainWindow.close(); } + }, + ] + }, + { + label: 'View', + submenu: [ + { + label: 'Reload', + accelerator: 'Command+R', + click: function() { mainWindow.restart(); } + }, + { + label: 'Enter Fullscreen', + click: function() { mainWindow.setFullscreen(true); } + }, + { + label: 'Toggle DevTools', + accelerator: 'Alt+Command+I', + click: function() { mainWindow.toggleDevTools(); } + }, + ] + }, + ]; + + menu = Menu.buildFromTemplate(template); mainWindow.setMenu(menu); + } ipc.on('message', function(processId, routingId, type) { - console.log(type); if (type == 'menu') menu.popup(mainWindow); }); From a2f679e4bd24935fe283f3f54d53098023e07bf1 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 3 Oct 2013 23:34:42 +0800 Subject: [PATCH 04/18] win: Respond to events of window menu. --- browser/native_window_win.cc | 37 +++++++++++++++++++++++++++++++ browser/native_window_win.h | 3 +++ browser/ui/win/menu_2.h | 1 + browser/ui/win/native_menu_win.cc | 32 ++++++++++++++------------ browser/ui/win/native_menu_win.h | 3 +++ 5 files changed, 62 insertions(+), 14 deletions(-) diff --git a/browser/native_window_win.cc b/browser/native_window_win.cc index 57c5edf27e44..4bacb0b0ba7c 100644 --- a/browser/native_window_win.cc +++ b/browser/native_window_win.cc @@ -7,6 +7,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/values.h" #include "browser/ui/win/menu_2.h" +#include "browser/ui/win/native_menu_win.h" #include "common/draggable_region.h" #include "common/options_switches.h" #include "content/public/browser/native_web_keyboard_event.h" @@ -17,6 +18,7 @@ #include "ui/gfx/path.h" #include "ui/views/controls/webview/webview.h" #include "ui/views/widget/widget.h" +#include "ui/views/widget/native_widget_win.h" #include "ui/views/window/client_view.h" #include "ui/views/window/native_frame_view.h" @@ -27,6 +29,35 @@ namespace { const int kResizeInsideBoundsSize = 5; const int kResizeAreaCornerSize = 16; +// Wrapper of NativeWidgetWin to handle WM_MENUCOMMAND messages, which are +// triggered by window menus. +class MenuCommandNativeWidget : public views::NativeWidgetWin { + public: + explicit MenuCommandNativeWidget(NativeWindowWin* delegate) + : views::NativeWidgetWin(delegate->window()), + delegate_(delegate) {} + virtual ~MenuCommandNativeWidget() {} + + protected: + virtual bool PreHandleMSG(UINT message, + WPARAM w_param, + LPARAM l_param, + LRESULT* result) OVERRIDE { + if (message == WM_MENUCOMMAND) { + delegate_->OnMenuCommand(w_param, reinterpret_cast(l_param)); + *result = 0; + return true; + } else { + return false; + } + } + + private: + NativeWindowWin* delegate_; + + DISALLOW_COPY_AND_ASSIGN(MenuCommandNativeWidget); +}; + class NativeWindowClientView : public views::ClientView { public: NativeWindowClientView(views::Widget* widget, @@ -174,6 +205,7 @@ NativeWindowWin::NativeWindowWin(content::WebContents* web_contents, resizable_(true) { views::Widget::InitParams params(views::Widget::InitParams::TYPE_WINDOW); params.delegate = this; + params.native_widget = new MenuCommandNativeWidget(this); params.remove_standard_frame = !has_frame_; params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; window_->set_frame_type(views::Widget::FRAME_TYPE_FORCE_NATIVE); @@ -326,6 +358,11 @@ gfx::NativeWindow NativeWindowWin::GetNativeWindow() { return window_->GetNativeView(); } +void NativeWindowWin::OnMenuCommand(int position, HMENU menu) { + DCHECK(menu_); + menu_->wrapper()->OnMenuCommand(position, menu); +} + void NativeWindowWin::SetMenu(ui::MenuModel* menu_model) { menu_.reset(new atom::Menu2(menu_model, true)); ::SetMenu(GetNativeWindow(), menu_->GetNativeMenu()); diff --git a/browser/native_window_win.h b/browser/native_window_win.h index e5cdad1eca83..a4406fc01b2d 100644 --- a/browser/native_window_win.h +++ b/browser/native_window_win.h @@ -66,9 +66,12 @@ class NativeWindowWin : public NativeWindow, virtual bool IsKiosk() OVERRIDE; virtual gfx::NativeWindow GetNativeWindow() OVERRIDE; + void OnMenuCommand(int position, HMENU menu); + // Set the native window menu. void SetMenu(ui::MenuModel* menu_model); + views::Widget* window() const { return window_.get(); } SkRegion* draggable_region() { return draggable_region_.get(); } protected: diff --git a/browser/ui/win/menu_2.h b/browser/ui/win/menu_2.h index 8877cdad0c17..67a80e28de11 100644 --- a/browser/ui/win/menu_2.h +++ b/browser/ui/win/menu_2.h @@ -77,6 +77,7 @@ class Menu2 { // Accessors. ui::MenuModel* model() const { return model_; } + NativeMenuWin* wrapper() const { return wrapper_.get(); } // Sets the minimum width of the menu. void SetMinimumWidth(int width); diff --git a/browser/ui/win/native_menu_win.cc b/browser/ui/win/native_menu_win.cc index f4c2369fb699..5a9bf640dcfd 100644 --- a/browser/ui/win/native_menu_win.cc +++ b/browser/ui/win/native_menu_win.cc @@ -92,6 +92,20 @@ class NativeMenuWin::MenuHostWindow { DestroyWindow(hwnd_); } + // Called when the user selects a specific item. + void OnMenuCommand(int position, HMENU menu) { + NativeMenuWin* menu_win = GetNativeMenuWinFromHMENU(menu); + ui::MenuModel* model = menu_win->model_; + NativeMenuWin* root_menu = menu_win; + while (root_menu->parent_) + root_menu = root_menu->parent_; + + // Only notify the model if it didn't already send out notification. + // See comment in MenuMessageHook for details. + if (root_menu->menu_action_ == MENU_ACTION_NONE) + model->ActivatedAt(position); + } + HWND hwnd() const { return hwnd_; } private: @@ -146,20 +160,6 @@ class NativeMenuWin::MenuHostWindow { return reinterpret_cast(item_data); } - // Called when the user selects a specific item. - void OnMenuCommand(int position, HMENU menu) { - NativeMenuWin* menu_win = GetNativeMenuWinFromHMENU(menu); - ui::MenuModel* model = menu_win->model_; - NativeMenuWin* root_menu = menu_win; - while (root_menu->parent_) - root_menu = root_menu->parent_; - - // Only notify the model if it didn't already send out notification. - // See comment in MenuMessageHook for details. - if (root_menu->menu_action_ == MENU_ACTION_NONE) - model->ActivatedAt(position); - } - // Called as the user moves their mouse or arrows through the contents of the // menu. void OnMenuSelect(WPARAM w_param, HMENU menu) { @@ -529,6 +529,10 @@ void NativeMenuWin::SetMinimumWidth(int width) { NOTIMPLEMENTED(); } +void NativeMenuWin::OnMenuCommand(int position, HMENU menu) { + host_window_->OnMenuCommand(position, menu); +} + //////////////////////////////////////////////////////////////////////////////// // NativeMenuWin, private: diff --git a/browser/ui/win/native_menu_win.h b/browser/ui/win/native_menu_win.h index 078bfac84d90..b74c654e0c67 100644 --- a/browser/ui/win/native_menu_win.h +++ b/browser/ui/win/native_menu_win.h @@ -56,6 +56,9 @@ class NativeMenuWin { void RemoveMenuListener(views::MenuListener* listener); void SetMinimumWidth(int width); + // Called by user to generate a menu command event. + void OnMenuCommand(int position, HMENU menu); + // Flag to create a window menu instead of popup menu. void set_create_as_window_menu(bool flag) { create_as_window_menu_ = flag; } bool create_as_window_menu() const { return create_as_window_menu_; } From 4e2d3f3d126d39c02006e7c3966484477c33a764 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 4 Oct 2013 08:52:45 +0800 Subject: [PATCH 05/18] win: Make native window a views::View. --- browser/native_window_win.cc | 19 ++++++++++++++++++- browser/native_window_win.h | 9 ++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/browser/native_window_win.cc b/browser/native_window_win.cc index 4bacb0b0ba7c..9219afdcef97 100644 --- a/browser/native_window_win.cc +++ b/browser/native_window_win.cc @@ -401,6 +401,23 @@ void NativeWindowWin::HandleKeyboardEvent( event.os_event.wParam, event.os_event.lParam); } +void NativeWindowWin::Layout() { + DCHECK(web_view_); + web_view_->SetBounds(0, 0, width(), height()); + OnViewWasResized(); +} + +void NativeWindowWin::ViewHierarchyChanged(bool is_add, + views::View* parent, + views::View* child) { + if (is_add && child == this) + AddChildView(web_view_); +} + +void NativeWindowWin::DeleteDelegate() { + // Do nothing, window is managed by users. +} + bool NativeWindowWin::CanResize() const { return resizable_; } @@ -430,7 +447,7 @@ const views::Widget* NativeWindowWin::GetWidget() const { } views::ClientView* NativeWindowWin::CreateClientView(views::Widget* widget) { - return new NativeWindowClientView(widget, web_view_, this); + return new NativeWindowClientView(widget, this, this); } views::NonClientFrameView* NativeWindowWin::CreateNonClientFrameView( diff --git a/browser/native_window_win.h b/browser/native_window_win.h index a4406fc01b2d..499cee3f1e1b 100644 --- a/browser/native_window_win.h +++ b/browser/native_window_win.h @@ -26,7 +26,7 @@ namespace atom { class Menu2; class NativeWindowWin : public NativeWindow, - public views::WidgetDelegate { + public views::WidgetDelegateView { public: explicit NativeWindowWin(content::WebContents* web_contents, base::DictionaryValue* options); @@ -83,7 +83,14 @@ class NativeWindowWin : public NativeWindow, content::WebContents*, const content::NativeWebKeyboardEvent&) OVERRIDE; + // Overridden from views::View: + virtual void Layout() OVERRIDE; + virtual void ViewHierarchyChanged(bool is_add, + views::View* parent, + views::View* child) OVERRIDE; + // Overridden from views::WidgetDelegate: + virtual void DeleteDelegate() OVERRIDE; virtual bool CanResize() const OVERRIDE; virtual bool CanMaximize() const OVERRIDE; virtual string16 GetWindowTitle() const OVERRIDE; From ae98d9c8b6d1fd2e00e299eaf6bf5907ff6b2359 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 4 Oct 2013 09:04:32 +0800 Subject: [PATCH 06/18] :lipstick: NativeWindowClientView no longer needs a extra content view. --- browser/native_window_win.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/browser/native_window_win.cc b/browser/native_window_win.cc index 9219afdcef97..da7978fb740c 100644 --- a/browser/native_window_win.cc +++ b/browser/native_window_win.cc @@ -61,21 +61,17 @@ class MenuCommandNativeWidget : public views::NativeWidgetWin { class NativeWindowClientView : public views::ClientView { public: NativeWindowClientView(views::Widget* widget, - views::View* contents_view, - NativeWindowWin* shell) - : views::ClientView(widget, contents_view), - shell_(shell) { + NativeWindowWin* contents_view) + : views::ClientView(widget, contents_view) { } virtual ~NativeWindowClientView() {} virtual bool CanClose() OVERRIDE { - shell_->CloseWebContents(); + static_cast(contents_view())->CloseWebContents(); return false; } private: - NativeWindowWin* shell_; - DISALLOW_COPY_AND_ASSIGN(NativeWindowClientView); }; @@ -447,7 +443,7 @@ const views::Widget* NativeWindowWin::GetWidget() const { } views::ClientView* NativeWindowWin::CreateClientView(views::Widget* widget) { - return new NativeWindowClientView(widget, this, this); + return new NativeWindowClientView(widget, this); } views::NonClientFrameView* NativeWindowWin::CreateNonClientFrameView( From 32432cc770eb49a612d37d89fee2e98fb0ba6422 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 4 Oct 2013 22:59:05 +0800 Subject: [PATCH 07/18] Convert "Command" to "Ctrl" in accelerators on non-Mac. --- browser/ui/accelerator_util.cc | 9 +++------ docs/api/browser/menu-item.md | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/browser/ui/accelerator_util.cc b/browser/ui/accelerator_util.cc index 438149ee21ff..88a510143d9d 100644 --- a/browser/ui/accelerator_util.cc +++ b/browser/ui/accelerator_util.cc @@ -15,10 +15,9 @@ namespace accelerator_util { namespace { -// For Mac, we convert "Ctrl" to "Command" and "MacCtrl" to "Ctrl". Other -// platforms leave the shortcut untouched. +// Convert "Command" to "Ctrl" on non-Mac std::string NormalizeShortcutSuggestion(const std::string& suggestion) { -#if !defined(OS_MACOSX) +#if defined(OS_MACOSX) return suggestion; #endif @@ -26,9 +25,7 @@ std::string NormalizeShortcutSuggestion(const std::string& suggestion) { std::vector tokens; base::SplitString(suggestion, '+', &tokens); for (size_t i = 0; i < tokens.size(); i++) { - if (tokens[i] == "Ctrl") - tokens[i] = "Command"; - else if (tokens[i] == "MacCtrl") + if (tokens[i] == "Command") tokens[i] = "Ctrl"; } return JoinString(tokens, '+'); diff --git a/docs/api/browser/menu-item.md b/docs/api/browser/menu-item.md index ec54743d9713..df922e14f68b 100644 --- a/docs/api/browser/menu-item.md +++ b/docs/api/browser/menu-item.md @@ -23,5 +23,5 @@ ## Notes on accelerator -On OS X, the `Ctrl` would automatically translated to `Command`, if you really -want `Ctrl` on OS X, you should use `MacCtrl`. +On Linux and Windows, the `Command` would be translated to `Ctrl`, so usually +you can use `Command` for most of the commands. From d86172cc87663e2fafe087ee335dfa6025be5a9f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 4 Oct 2013 23:36:31 +0800 Subject: [PATCH 08/18] win: Make window menu's accelerators work. --- browser/native_window_win.cc | 63 ++++++++++++++++++++++++++++++++++++ browser/native_window_win.h | 17 ++++++++++ 2 files changed, 80 insertions(+) diff --git a/browser/native_window_win.cc b/browser/native_window_win.cc index da7978fb740c..d4f225e0d6e4 100644 --- a/browser/native_window_win.cc +++ b/browser/native_window_win.cc @@ -4,8 +4,10 @@ #include "browser/native_window_win.h" +#include "base/stl_util.h" #include "base/strings/utf_string_conversions.h" #include "base/values.h" +#include "browser/api/atom_api_menu.h" #include "browser/ui/win/menu_2.h" #include "browser/ui/win/native_menu_win.h" #include "common/draggable_region.h" @@ -16,6 +18,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_view.h" #include "ui/gfx/path.h" +#include "ui/base/models/simple_menu_model.h" #include "ui/views/controls/webview/webview.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/native_widget_win.h" @@ -362,6 +365,7 @@ void NativeWindowWin::OnMenuCommand(int position, HMENU menu) { void NativeWindowWin::SetMenu(ui::MenuModel* menu_model) { menu_.reset(new atom::Menu2(menu_model, true)); ::SetMenu(GetNativeWindow(), menu_->GetNativeMenu()); + RegisterAccelerators(); } void NativeWindowWin::UpdateDraggableRegions( @@ -391,6 +395,16 @@ void NativeWindowWin::UpdateDraggableRegions( void NativeWindowWin::HandleKeyboardEvent( content::WebContents*, const content::NativeWebKeyboardEvent& event) { + if (event.type == WebKit::WebInputEvent::KeyUp) { + ui::Accelerator accelerator( + static_cast(event.windowsKeyCode), + content::GetModifiersFromNativeWebKeyboardEvent(event)); + + if (GetFocusManager()->ProcessAccelerator(accelerator)) { + return; + } + } + // Any unhandled keyboard/character messages should be defproced. // This allows stuff like F10, etc to work correctly. DefWindowProc(event.os_event.hwnd, event.os_event.message, @@ -410,6 +424,17 @@ void NativeWindowWin::ViewHierarchyChanged(bool is_add, AddChildView(web_view_); } +bool NativeWindowWin::AcceleratorPressed( + const ui::Accelerator& accelerator) { + if (ContainsKey(accelerator_table_, accelerator)) { + const MenuItem& item = accelerator_table_[accelerator]; + item.model->ActivatedAt(item.position); + return true; + } else { + return false; + } +} + void NativeWindowWin::DeleteDelegate() { // Do nothing, window is managed by users. } @@ -484,6 +509,44 @@ void NativeWindowWin::OnViewWasResized() { web_contents->GetRenderViewHost()->GetView()->SetClickthroughRegion(rgn); } +void NativeWindowWin::RegisterAccelerators() { + views::FocusManager* focus_manager = GetFocusManager(); + accelerator_table_.clear(); + focus_manager->UnregisterAccelerators(this); + + GenerateAcceleratorTable(); + for (AcceleratorTable::const_iterator iter = accelerator_table_.begin(); + iter != accelerator_table_.end(); ++iter) { + focus_manager->RegisterAccelerator( + iter->first, ui::AcceleratorManager::kNormalPriority, this); + } +} + +void NativeWindowWin::GenerateAcceleratorTable() { + DCHECK(menu_); + ui::SimpleMenuModel* model = static_cast( + menu_->model()); + FillAcceleratorTable(&accelerator_table_, model); +} + +void NativeWindowWin::FillAcceleratorTable(AcceleratorTable* table, + ui::MenuModel* model) { + int count = model->GetItemCount(); + for (int i = 0; i < count; ++i) { + ui::MenuModel::ItemType type = model->GetTypeAt(i); + if (type == ui::MenuModel::TYPE_SUBMENU) { + ui::MenuModel* submodel = model->GetSubmenuModelAt(i); + FillAcceleratorTable(table, submodel); + } else { + ui::Accelerator accelerator; + if (model->GetAcceleratorAt(i, &accelerator)) { + MenuItem item = { i, model }; + (*table)[accelerator] = item; + } + } + } +} + // static NativeWindow* NativeWindow::Create(content::WebContents* web_contents, base::DictionaryValue* options) { diff --git a/browser/native_window_win.h b/browser/native_window_win.h index 499cee3f1e1b..6004ec21f1fb 100644 --- a/browser/native_window_win.h +++ b/browser/native_window_win.h @@ -88,6 +88,7 @@ class NativeWindowWin : public NativeWindow, virtual void ViewHierarchyChanged(bool is_add, views::View* parent, views::View* child) OVERRIDE; + virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE; // Overridden from views::WidgetDelegate: virtual void DeleteDelegate() OVERRIDE; @@ -103,14 +104,30 @@ class NativeWindowWin : public NativeWindow, views::Widget* widget) OVERRIDE; private: + typedef struct { int position; ui::MenuModel* model; } MenuItem; + typedef std::map AcceleratorTable; + void OnViewWasResized(); + // Register accelerators supported by the menu model. + void RegisterAccelerators(); + + // Generate a table that contains memu model's accelerators and command ids. + void GenerateAcceleratorTable(); + + // Helper to fill the accelerator table from the model. + void FillAcceleratorTable(AcceleratorTable* table, + ui::MenuModel* model); + scoped_ptr window_; views::WebView* web_view_; // managed by window_. // The window menu. scoped_ptr menu_; + // Map from accelerator to menu item's command id. + AcceleratorTable accelerator_table_; + scoped_ptr draggable_region_; bool resizable_; From 55a35d473dac37c4bfa156b0937c171888f4d5dc Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 4 Oct 2013 23:38:37 +0800 Subject: [PATCH 09/18] win: Make web view focused initialy. --- browser/native_window_win.cc | 4 ++++ browser/native_window_win.h | 1 + 2 files changed, 5 insertions(+) diff --git a/browser/native_window_win.cc b/browser/native_window_win.cc index d4f225e0d6e4..f338b8ccf877 100644 --- a/browser/native_window_win.cc +++ b/browser/native_window_win.cc @@ -439,6 +439,10 @@ void NativeWindowWin::DeleteDelegate() { // Do nothing, window is managed by users. } +views::View* NativeWindowWin::GetInitiallyFocusedView() { + return web_view_; +} + bool NativeWindowWin::CanResize() const { return resizable_; } diff --git a/browser/native_window_win.h b/browser/native_window_win.h index 6004ec21f1fb..c4e46c16c0cf 100644 --- a/browser/native_window_win.h +++ b/browser/native_window_win.h @@ -92,6 +92,7 @@ class NativeWindowWin : public NativeWindow, // Overridden from views::WidgetDelegate: virtual void DeleteDelegate() OVERRIDE; + virtual views::View* GetInitiallyFocusedView() OVERRIDE; virtual bool CanResize() const OVERRIDE; virtual bool CanMaximize() const OVERRIDE; virtual string16 GetWindowTitle() const OVERRIDE; From defb6c9882bed6bb3038730586679d83e6e3701c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 4 Oct 2013 23:39:34 +0800 Subject: [PATCH 10/18] Don't wait for request in http spec. This spec is used to crash atom-shell, we don't care if http request succeeds. --- spec/web/http.coffee | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/spec/web/http.coffee b/spec/web/http.coffee index 896f3cf3b8b9..c7db44700651 100644 --- a/spec/web/http.coffee +++ b/spec/web/http.coffee @@ -1,7 +1,4 @@ describe 'http', -> describe 'sending request of http protocol urls', -> - it 'should not crash', (done) -> - $.ajax - url: 'http://127.0.0.1' - success: -> done() - error: -> done() + it 'should not crash', -> + $.get 'https://api.github.com/zen' From 587484a5d0cb17efca26bb75e5b6ce9874bdb4a8 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 12:56:30 +0800 Subject: [PATCH 11/18] Make sure application menu always get referenced. --- browser/api/lib/menu.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/browser/api/lib/menu.coffee b/browser/api/lib/menu.coffee index 14c5b8db15ff..0a1bb0714ef4 100644 --- a/browser/api/lib/menu.coffee +++ b/browser/api/lib/menu.coffee @@ -43,8 +43,10 @@ Menu::insert = (pos, item) -> @items.splice pos, 0, item @commandsMap[item.commandId] = item +applicationMenu = null Menu.setApplicationMenu = (menu) -> throw new TypeError('Invalid menu') unless menu?.constructor is Menu + applicationMenu = menu # Keep a reference. bindings.setApplicationMenu menu Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder From 61cc0bba251c4247e62d79692a948e01f74c62a3 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 13:03:08 +0800 Subject: [PATCH 12/18] :lipstick: fix typo in doc. --- docs/api/browser/app.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/browser/app.md b/docs/api/browser/app.md index 092eb3882521..1375218e9ef3 100644 --- a/docs/api/browser/app.md +++ b/docs/api/browser/app.md @@ -83,8 +83,8 @@ Returns the version of current bundle or executable. Append a switch [with optional value] to Chromium's command line. -**Note:** This will not affect `process.argv`, and is mainly used by -**developers to control some low-level Chromium behaviors. +**Note:** This will not affect `process.argv`, and is mainly used by developers +to control some low-level Chromium behaviors. ## app.commandLine.appendArgument(value) From 666f6b3a01a052bdc70ff4a2f095c834ca420032 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 13:13:04 +0800 Subject: [PATCH 13/18] Add app.getBrowserWindows() API. --- browser/api/lib/app.coffee | 4 ++++ browser/api/lib/browser-window.coffee | 6 +++--- docs/api/browser/app.md | 4 ++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/browser/api/lib/app.coffee b/browser/api/lib/app.coffee index 0960db29e3b7..6f386f14c11b 100644 --- a/browser/api/lib/app.coffee +++ b/browser/api/lib/app.coffee @@ -1,4 +1,5 @@ bindings = process.atomBinding 'app' +objectsRegistry = require '../../atom/objects-registry.js' EventEmitter = require('events').EventEmitter Application = bindings.Application @@ -9,6 +10,9 @@ app = new Application app.getHomeDir = -> process.env[if process.platform is 'win32' then 'USERPROFILE' else 'HOME'] +app.getBrowserWindows = -> + objectsRegistry.getAllWindows() + app.commandLine = appendSwitch: bindings.appendSwitch, appendArgument: bindings.appendArgument diff --git a/browser/api/lib/browser-window.coffee b/browser/api/lib/browser-window.coffee index 56d03e997c66..a016883aa0a7 100644 --- a/browser/api/lib/browser-window.coffee +++ b/browser/api/lib/browser-window.coffee @@ -1,6 +1,6 @@ EventEmitter = require('events').EventEmitter +app = require 'app' v8Util = process.atomBinding 'v8_util' -objectsRegistry = require '../../atom/objects-registry.js' BrowserWindow = process.atomBinding('window').BrowserWindow BrowserWindow::__proto__ = EventEmitter.prototype @@ -26,11 +26,11 @@ BrowserWindow::setMenu = (menu) -> @menu.attachToWindow this BrowserWindow.getFocusedWindow = -> - windows = objectsRegistry.getAllWindows() + windows = app.getBrowserWindows() return window for window in windows when window.isFocused() BrowserWindow.fromProcessIdAndRoutingId = (processId, routingId) -> - windows = objectsRegistry.getAllWindows() + windows = app.getBrowserWindows() return window for window in windows when window.getProcessId() == processId and window.getRoutingId() == routingId diff --git a/docs/api/browser/app.md b/docs/api/browser/app.md index 1375218e9ef3..c6e16a8875bf 100644 --- a/docs/api/browser/app.md +++ b/docs/api/browser/app.md @@ -79,6 +79,10 @@ code will not run. Returns the version of current bundle or executable. +## app.getBrowserWindows() + +Returns an array of all browser windows. + ## app.commandLine.appendSwitch(switch, [value]) Append a switch [with optional value] to Chromium's command line. From 93f1a3dbd5a0b185fbfbf5039106ba23a3fba945 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 13:46:48 +0800 Subject: [PATCH 14/18] Make app.setApplicationMenu an alias to Menu.setApplicationMenu. --- browser/api/lib/app.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/browser/api/lib/app.coffee b/browser/api/lib/app.coffee index 6f386f14c11b..b76f8f39495c 100644 --- a/browser/api/lib/app.coffee +++ b/browser/api/lib/app.coffee @@ -13,6 +13,9 @@ app.getHomeDir = -> app.getBrowserWindows = -> objectsRegistry.getAllWindows() +app.setApplicationMenu = (menu) -> + require('menu').setApplicationMenu menu + app.commandLine = appendSwitch: bindings.appendSwitch, appendArgument: bindings.appendArgument From 1524ced816668f3b5cafc4ca2e03a6b3d3a2c1bc Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 14:31:30 +0800 Subject: [PATCH 15/18] Set application menu would set menu for all windows on Windows. --- browser/api/lib/menu-item.coffee | 4 +++- browser/api/lib/menu.coffee | 10 ++++++-- spec/main.js | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/browser/api/lib/menu-item.coffee b/browser/api/lib/menu-item.coffee index 02f863886e42..ec38963b433a 100644 --- a/browser/api/lib/menu-item.coffee +++ b/browser/api/lib/menu-item.coffee @@ -1,3 +1,5 @@ +BrowserWindow = require 'browser-window' + nextCommandId = 0 class MenuItem @@ -26,7 +28,7 @@ class MenuItem @commandId = ++nextCommandId @click = => if typeof click is 'function' - click.apply this, arguments + click this, BrowserWindow.getFocusedWindow() else if typeof @selector is 'string' Menu.sendActionToFirstResponder @selector diff --git a/browser/api/lib/menu.coffee b/browser/api/lib/menu.coffee index 0a1bb0714ef4..5d00504da9ff 100644 --- a/browser/api/lib/menu.coffee +++ b/browser/api/lib/menu.coffee @@ -3,6 +3,7 @@ EventEmitter = require('events').EventEmitter IDWeakMap = require 'id-weak-map' MenuItem = require 'menu-item' +app = require 'app' bindings = process.atomBinding 'menu' Menu = bindings.Menu @@ -39,7 +40,7 @@ Menu::insert = (pos, item) -> getAcceleratorForCommandId: (commandId) => @commandsMap[commandId]?.accelerator executeCommand: (commandId) => activeItem = @commandsMap[commandId] - activeItem.click(activeItem) if activeItem? + activeItem.click() if activeItem? @items.splice pos, 0, item @commandsMap[item.commandId] = item @@ -47,7 +48,12 @@ applicationMenu = null Menu.setApplicationMenu = (menu) -> throw new TypeError('Invalid menu') unless menu?.constructor is Menu applicationMenu = menu # Keep a reference. - bindings.setApplicationMenu menu + + if process.platform is 'darwin' + bindings.setApplicationMenu menu + else + windows = app.getBrowserWindows() + w.setMenu menu for w in windows Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder diff --git a/spec/main.js b/spec/main.js index cf2453c5a11f..0c2f64dc4194 100644 --- a/spec/main.js +++ b/spec/main.js @@ -1,6 +1,7 @@ var app = require('app'); var ipc = require('ipc'); var BrowserWindow = require('browser-window'); +var Menu = require('menu'); var window = null; @@ -39,6 +40,42 @@ app.on('window-all-closed', function() { }); app.on('finish-launching', function() { + var template = [ + { + label: 'File', + submenu: [ + { + label: 'Open', + accelerator: 'Command+O', + }, + { + label: 'Close', + accelerator: 'Command+W', + click: function(item, window) { window.close(); } + }, + ] + }, + { + label: 'View', + submenu: [ + { + label: 'Reload', + accelerator: 'Command+R', + click: function(item, window) { window.restart(); } + }, + { + label: 'Enter Fullscreen', + click: function(item, window) { window.setFullScreen(true); } + }, + { + label: 'Toggle DevTools', + accelerator: 'Alt+Command+I', + click: function(item, window) { window.toggleDevTools(); } + }, + ] + }, + ]; + // Test if using protocol module would crash. require('protocol').registerProtocol('test-if-crashes', function() {}); @@ -49,4 +86,7 @@ app.on('finish-launching', function() { height: 600 }); window.loadUrl('file://' + __dirname + '/index.html'); + + var menu = Menu.buildFromTemplate(template); + app.setApplicationMenu(menu); }); From 1e1fec15b656dfc5531120762795c6ecbf1808dd Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 20:21:57 +0800 Subject: [PATCH 16/18] Make objects registry code more structured. --- browser/api/lib/app.coffee | 3 +- browser/atom/objects-registry.coffee | 80 ++++++++++++++++------------ 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/browser/api/lib/app.coffee b/browser/api/lib/app.coffee index b76f8f39495c..5e0d7e9a0050 100644 --- a/browser/api/lib/app.coffee +++ b/browser/api/lib/app.coffee @@ -1,5 +1,4 @@ bindings = process.atomBinding 'app' -objectsRegistry = require '../../atom/objects-registry.js' EventEmitter = require('events').EventEmitter Application = bindings.Application @@ -11,7 +10,7 @@ app.getHomeDir = -> process.env[if process.platform is 'win32' then 'USERPROFILE' else 'HOME'] app.getBrowserWindows = -> - objectsRegistry.getAllWindows() + require('../../atom/objects-registry.js').getAllWindows() app.setApplicationMenu = (menu) -> require('menu').setApplicationMenu menu diff --git a/browser/atom/objects-registry.coffee b/browser/atom/objects-registry.coffee index c60093167bdc..b107d0f156aa 100644 --- a/browser/atom/objects-registry.coffee +++ b/browser/atom/objects-registry.coffee @@ -1,7 +1,9 @@ BrowserWindow = require 'browser-window' +EventEmitter = require('events').EventEmitter IDWeakMap = require 'id-weak-map' v8Util = process.atomBinding 'v8_util' +# Class to reference all objects. class ObjectsStore @stores = {} @@ -37,46 +39,56 @@ class ObjectsStore key = "#{processId}_#{routingId}" delete @stores[key] -# Objects in weak map will be not referenced (so we won't leak memory), and -# every object created in browser will have a unique id in weak map. -objectsWeakMap = new IDWeakMap -objectsWeakMap.add = (obj) -> - id = IDWeakMap::add.call this, obj - v8Util.setHiddenValue obj, 'atomId', id - id +class ObjectsRegistry + constructor: -> + # Objects in weak map will be not referenced (so we won't leak memory), and + # every object created in browser will have a unique id in weak map. + @objectsWeakMap = new IDWeakMap + @objectsWeakMap.add = (obj) -> + id = IDWeakMap::add.call this, obj + v8Util.setHiddenValue obj, 'atomId', id + id -windowsWeakMap = new IDWeakMap + # Remember all windows in the weak map. + @windowsWeakMap = new IDWeakMap + process.on 'ATOM_BROWSER_INTERNAL_NEW', (obj) => + if obj.constructor is BrowserWindow + id = @windowsWeakMap.add obj + obj.on 'destroyed', => @windowsWeakMap.remove id -process.on 'ATOM_BROWSER_INTERNAL_NEW', (obj) -> - # Remember all windows. - if obj.constructor is BrowserWindow - id = windowsWeakMap.add obj - obj.on 'destroyed', -> - windowsWeakMap.remove id + # Register a new object, the object would be kept referenced until you release + # it explicitly. + add: (processId, routingId, obj) -> + # Some native objects may already been added to objectsWeakMap, be care not + # to add it twice. + @objectsWeakMap.add obj unless v8Util.getHiddenValue obj, 'atomId' + id = v8Util.getHiddenValue obj, 'atomId' -exports.add = (processId, routingId, obj) -> - # Some native objects may already been added to objectsWeakMap, be care not - # to add it twice. - objectsWeakMap.add obj unless v8Util.getHiddenValue obj, 'atomId' - id = v8Util.getHiddenValue obj, 'atomId' + # Store and reference the object, then return the storeId which points to + # where the object is stored. The caller can later dereference the object + # with the storeId. + # We use a difference key because the same object can be referenced for + # multiple times by the same renderer view. + store = ObjectsStore.forRenderView processId, routingId + storeId = store.add obj - # Store and reference the object, then return the storeId which points to - # where the object is stored. The caller can later dereference the object - # with the storeId. - store = ObjectsStore.forRenderView processId, routingId - storeId = store.add obj + [id, storeId] - [id, storeId] + # Get an object according to its id. + get: (id) -> + @objectsWeakMap.get id -exports.get = (id) -> - objectsWeakMap.get id + # Remove an object according to its storeId. + remove: (processId, routingId, storeId) -> + ObjectsStore.forRenderView(processId, routingId).remove storeId -exports.getAllWindows = () -> - keys = windowsWeakMap.keys() - windowsWeakMap.get key for key in keys + # Clear all references to objects from renderer view. + clear: (processId, routingId) -> + ObjectsStore.releaseForRenderView processId, routingId -exports.remove = (processId, routingId, storeId) -> - ObjectsStore.forRenderView(processId, routingId).remove storeId + # Return an array of all browser windows. + getAllWindows: -> + keys = @windowsWeakMap.keys() + @windowsWeakMap.get key for key in keys -exports.clear = (processId, routingId) -> - ObjectsStore.releaseForRenderView processId, routingId +module.exports = new ObjectsRegistry From caaab22841ed99120efc441831aabd70d12b60c2 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 20:53:31 +0800 Subject: [PATCH 17/18] Do not dereference remote callback if its renderer view is released. --- browser/atom/objects-registry.coffee | 3 ++- browser/atom/rpc-server.coffee | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/browser/atom/objects-registry.coffee b/browser/atom/objects-registry.coffee index b107d0f156aa..df2ad535244d 100644 --- a/browser/atom/objects-registry.coffee +++ b/browser/atom/objects-registry.coffee @@ -39,7 +39,7 @@ class ObjectsStore key = "#{processId}_#{routingId}" delete @stores[key] -class ObjectsRegistry +class ObjectsRegistry extends EventEmitter constructor: -> # Objects in weak map will be not referenced (so we won't leak memory), and # every object created in browser will have a unique id in weak map. @@ -84,6 +84,7 @@ class ObjectsRegistry # Clear all references to objects from renderer view. clear: (processId, routingId) -> + @emit "release-renderer-view-#{processId}-#{routingId}" ObjectsStore.releaseForRenderView processId, routingId # Return an array of all browser windows. diff --git a/browser/atom/rpc-server.coffee b/browser/atom/rpc-server.coffee index 04d5fe2c9484..6b56bfca2063 100644 --- a/browser/atom/rpc-server.coffee +++ b/browser/atom/rpc-server.coffee @@ -52,9 +52,15 @@ unwrapArgs = (processId, routingId, args) -> returnValue = metaToValue meta.value -> returnValue when 'function' + rendererReleased = false + objectsRegistry.once "release-renderer-view-#{processId}-#{routingId}", -> + rendererReleased = true + ret = -> + throw new Error('Calling a callback of released renderer view') if rendererReleased ipc.sendChannel processId, routingId, 'ATOM_RENDERER_CALLBACK', meta.id, valueToMeta(processId, routingId, arguments) v8Util.setDestructor ret, -> + return if rendererReleased ipc.sendChannel processId, routingId, 'ATOM_RENDERER_RELEASE_CALLBACK', meta.id ret else throw new TypeError("Unknown type: #{meta.type}") From a182de20a42a683bc18ca48789f5c94703883b14 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 5 Oct 2013 21:05:59 +0800 Subject: [PATCH 18/18] win: Newly created window should also be aware of application menu. --- browser/api/atom_api_window.cc | 3 +++ browser/api/lib/app.coffee | 3 +++ browser/api/lib/browser-window.coffee | 6 ++++++ browser/api/lib/menu.coffee | 2 ++ spec/main.js | 6 +++--- 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/browser/api/atom_api_window.cc b/browser/api/atom_api_window.cc index 8d1cd0a66749..b0753491bf3c 100644 --- a/browser/api/atom_api_window.cc +++ b/browser/api/atom_api_window.cc @@ -101,6 +101,9 @@ v8::Handle Window::New(const v8::Arguments &args) { new Window(args.This(), static_cast(options.get())); + // Give js code a chance to do initialization. + node::MakeCallback(args.This(), "_init", 0, NULL); + return args.This(); } diff --git a/browser/api/lib/app.coffee b/browser/api/lib/app.coffee index 5e0d7e9a0050..9fc9664ed423 100644 --- a/browser/api/lib/app.coffee +++ b/browser/api/lib/app.coffee @@ -15,6 +15,9 @@ app.getBrowserWindows = -> app.setApplicationMenu = (menu) -> require('menu').setApplicationMenu menu +app.getApplicationMenu = -> + require('menu').getApplicationMenu() + app.commandLine = appendSwitch: bindings.appendSwitch, appendArgument: bindings.appendArgument diff --git a/browser/api/lib/browser-window.coffee b/browser/api/lib/browser-window.coffee index a016883aa0a7..cd768404641e 100644 --- a/browser/api/lib/browser-window.coffee +++ b/browser/api/lib/browser-window.coffee @@ -5,6 +5,12 @@ v8Util = process.atomBinding 'v8_util' BrowserWindow = process.atomBinding('window').BrowserWindow BrowserWindow::__proto__ = EventEmitter.prototype +BrowserWindow::_init = -> + # Simulate the application menu on platforms other than OS X. + if process.platform isnt 'darwin' + menu = app.getApplicationMenu() + @setMenu menu if menu? + BrowserWindow::toggleDevTools = -> opened = v8Util.getHiddenValue this, 'devtoolsOpened' if opened diff --git a/browser/api/lib/menu.coffee b/browser/api/lib/menu.coffee index 5d00504da9ff..17c768e956eb 100644 --- a/browser/api/lib/menu.coffee +++ b/browser/api/lib/menu.coffee @@ -55,6 +55,8 @@ Menu.setApplicationMenu = (menu) -> windows = app.getBrowserWindows() w.setMenu menu for w in windows +Menu.getApplicationMenu = -> applicationMenu + Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder Menu.buildFromTemplate = (template) -> diff --git a/spec/main.js b/spec/main.js index 0c2f64dc4194..d5bad1457131 100644 --- a/spec/main.js +++ b/spec/main.js @@ -76,6 +76,9 @@ app.on('finish-launching', function() { }, ]; + var menu = Menu.buildFromTemplate(template); + app.setApplicationMenu(menu); + // Test if using protocol module would crash. require('protocol').registerProtocol('test-if-crashes', function() {}); @@ -86,7 +89,4 @@ app.on('finish-launching', function() { height: 600 }); window.loadUrl('file://' + __dirname + '/index.html'); - - var menu = Menu.buildFromTemplate(template); - app.setApplicationMenu(menu); });