Skip to content

Commit 2bad83e

Browse files
rogerwangrogerwang
committed
Fix nwjs#5953: menu stopped working on Linux and Windows
1 parent f975f3e commit 2bad83e

File tree

6 files changed

+79
-6
lines changed

6 files changed

+79
-6
lines changed

‎src/api/menu/menu.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "content/nw/src/api/menuitem/menuitem.h"
2727
#include "content/public/browser/web_contents.h"
2828
#include "content/public/common/page_zoom.h"
29+
#include "ui/views/controls/menu/menu_runner.h"
2930

3031
namespace nw {
3132

‎src/api/menu/menu.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#ifndef CONTENT_NW_SRC_API_MENU_MENU_H_
2222
#define CONTENT_NW_SRC_API_MENU_MENU_H_
2323

24+
#include "base/callback.h"
2425
#include "base/compiler_specific.h"
2526
#include "content/nw/src/api/base/base.h"
2627

@@ -82,6 +83,10 @@ class RenderFrameHost;
8283
class RenderFrameHost;
8384
}
8485

86+
namespace views {
87+
class MenuRunner;
88+
}
89+
8590
namespace nw {
8691

8792
class MenuItem;
@@ -127,6 +132,8 @@ class Menu : public Base {
127132

128133
void UpdateStates();
129134

135+
void OnMenuClosed();
136+
130137
//**Never Try to free this pointer**
131138
//We get it from top widget
132139
views::FocusManager *focus_manager_;
@@ -138,6 +145,8 @@ class Menu : public Base {
138145

139146
std::unique_ptr<MenuDelegate> menu_delegate_;
140147
std::unique_ptr<ui::NwMenuModel> menu_model_;
148+
std::unique_ptr<views::MenuRunner> menu_runner_;
149+
base::Closure message_loop_quit_;
141150
#endif
142151

143152
DISALLOW_COPY_AND_ASSIGN(Menu);

‎src/api/menu/menu_views.cc

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include "content/nw/src/api/menu/menu.h"
2222

23+
#include "base/run_loop.h"
2324
#include "base/values.h"
2425
#include "base/strings/utf_string_conversions.h"
2526
#include "content/nw/src/api/object_manager.h"
@@ -33,6 +34,7 @@
3334
#include "ui/aura/client/screen_position_client.h"
3435
#include "ui/aura/window.h"
3536
#include "ui/aura/window_tree_host.h"
37+
#include "ui/events/platform/platform_event_source.h"
3638
#include "ui/views/controls/menu/menu_runner.h"
3739
#include "ui/views/widget/widget.h"
3840
#include "ui/views/focus/focus_manager.h"
@@ -131,18 +133,48 @@ void Menu::Popup(int x, int y, content::RenderFrameHost* rfh) {
131133
&screen_point);
132134
}
133135
set_delay_destruction(true);
134-
views::MenuRunner runner(menu_model_.get(), views::MenuRunner::CONTEXT_MENU);
136+
menu_runner_.reset(new views::MenuRunner(menu_model_.get(), views::MenuRunner::CONTEXT_MENU | views::MenuRunner::ASYNC,
137+
base::Bind(&Menu::OnMenuClosed, base::Unretained(this))));
135138
ignore_result(
136-
runner.RunMenuAt(top_level_widget,
137-
NULL,
139+
menu_runner_->RunMenuAt(top_level_widget,
140+
nullptr,
138141
gfx::Rect(screen_point, gfx::Size()),
139142
views::MENU_ANCHOR_TOPRIGHT,
140143
ui::MENU_SOURCE_NONE));
144+
// It is possible for the same MenuMessageLoopAura to start a nested
145+
// message-loop while it is already running a nested loop. So make
146+
// sure the quit-closure gets reset to the outer loop's quit-closure
147+
// once the innermost loop terminates.
148+
{
149+
base::AutoReset<base::Closure> reset_quit_closure(&message_loop_quit_,
150+
base::Closure());
151+
152+
base::MessageLoop* loop = base::MessageLoop::current();
153+
base::MessageLoop::ScopedNestableTaskAllower allow(loop);
154+
base::RunLoop run_loop;
155+
message_loop_quit_ = run_loop.QuitClosure();
156+
157+
run_loop.Run();
158+
}
141159
set_delay_destruction(false);
142160
if (pending_destruction())
143161
object_manager_->OnDeallocateObject(id_);
144162
}
145163

164+
void Menu::OnMenuClosed() {
165+
CHECK(!message_loop_quit_.is_null());
166+
message_loop_quit_.Run();
167+
168+
#if !defined(OS_WIN)
169+
// Ask PlatformEventSource to stop dispatching
170+
// events in this message loop
171+
// iteration. We want our menu's loop to return
172+
// before the next event.
173+
if (ui::PlatformEventSource::GetInstance())
174+
ui::PlatformEventSource::GetInstance()->StopCurrentEventStream();
175+
#endif
176+
}
177+
146178
void Menu::UpdateKeys(views::FocusManager *focus_manager){
147179
if (focus_manager == NULL){
148180
return ;

‎src/browser/menubar_controller.cc

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
#include "content/nw/src/browser/menubar_controller.h"
22

3+
#include "base/run_loop.h"
34
#include "base/stl_util.h"
45
#include "content/nw/src/browser/menubar_view.h"
56
#include "ui/base/models/menu_model.h"
7+
#include "ui/events/platform/platform_event_source.h"
68
#include "ui/views/controls/button/menu_button.h"
79
#include "ui/views/controls/menu/menu_item_view.h"
810
#include "ui/views/widget/widget.h"
@@ -15,10 +17,10 @@ MenuBarController* MenuBarController::master_;
1517
MenuBarController::MenuBarController(MenuBarView* menubar, ui::MenuModel* menu_model, MenuBarController* master)
1618
:MenuModelAdapter(menu_model), menubar_(menubar), active_menu_model_(menu_model) {
1719

18-
views::MenuItemView* menu = MenuBarController::CreateMenu(menubar, menu_model, this);
20+
MenuBarController::CreateMenu(menubar, menu_model, this);
1921
if (!master) {
2022
master_ = this;
21-
menu_runner_.reset(new views::MenuRunner(menu, views::MenuRunner::HAS_MNEMONICS));
23+
menu_runner_.reset(new views::MenuRunner(menu_model, views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::ASYNC, base::Bind(&MenuBarController::OnMenuClose, base::Unretained(this))));
2224
}
2325
}
2426

@@ -102,7 +104,32 @@ void MenuBarController::RunMenuAt(views::View* view, const gfx::Point& point) {
102104
bounds,
103105
views::MENU_ANCHOR_TOPLEFT,
104106
ui::MENU_SOURCE_NONE));
107+
{
108+
base::AutoReset<base::Closure> reset_quit_closure(&message_loop_quit_,
109+
base::Closure());
110+
111+
base::MessageLoop* loop = base::MessageLoop::current();
112+
base::MessageLoop::ScopedNestableTaskAllower allow(loop);
113+
base::RunLoop run_loop;
114+
message_loop_quit_ = run_loop.QuitClosure();
115+
116+
run_loop.Run();
117+
}
105118
delete this;
106119
}
107120

121+
void MenuBarController::OnMenuClose() {
122+
CHECK(!message_loop_quit_.is_null());
123+
message_loop_quit_.Run();
124+
125+
#if !defined(OS_WIN)
126+
// Ask PlatformEventSource to stop dispatching
127+
// events in this message loop
128+
// iteration. We want our menu's loop to return
129+
// before the next event.
130+
if (ui::PlatformEventSource::GetInstance())
131+
ui::PlatformEventSource::GetInstance()->StopCurrentEventStream();
132+
#endif
133+
}
134+
108135
} //namespace nw

‎src/browser/menubar_controller.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef NW_BROWSER_MENUBAR_CONTROLLER_H
22
#define NW_BROWSER_MENUBAR_CONTROLLER_H
33

4+
#include "base/callback.h"
45
#include "ui/views/controls/menu/menu_model_adapter.h"
56
#include "ui/views/controls/menu/menu_runner.h"
67
#include "ui/views/view.h"
@@ -32,6 +33,8 @@ class MenuBarController : public views::MenuModelAdapter {
3233
void ExecuteCommand(int id) override;
3334
void ExecuteCommand(int id, int mouse_event_flags) override;
3435

36+
void OnMenuClose();
37+
3538
private:
3639
typedef std::map<const ui::MenuModel*, views::MenuItemView*> ModelToMenuMap;
3740

@@ -41,6 +44,7 @@ class MenuBarController : public views::MenuModelAdapter {
4144
std::vector<MenuBarController*> controllers_;
4245
static ModelToMenuMap model_to_menu_map_;
4346
static MenuBarController* master_;
47+
base::Closure message_loop_quit_;
4448

4549
DISALLOW_COPY_AND_ASSIGN(MenuBarController);
4650
};

‎src/resources/api_nw_object.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ nw_binding.registerCustomHook(function(bindingsAPI) {
2121
// in order not to bring side effects before release when fix this for #4593, I introduce the
2222
// new method as a workaround. Please see removing it later.
2323
apiFunctions.setHandleRequest('callObjectMethodAsync', function() {
24-
return sendRequest.sendRequest(this.name, arguments, this.definition.parameters, {});
24+
return sendRequest.sendRequest(this.name, arguments, this.definition.parameters);
2525
});
2626
apiFunctions.setHandleRequest('callObjectMethodSync', function() {
2727
return sendRequest.sendRequestSync(this.name, arguments, this.definition.parameters, {})[0];

0 commit comments

Comments
 (0)