MacViews: Clear mouse handler when showing context menus.

ui/views/controls/menu/menu_runner_unittest.cc

Index: ui/views/controls/menu/menu_runner_unittest.cc
diff --git a/ui/views/controls/menu/menu_runner_unittest.cc b/ui/views/controls/menu/menu_runner_unittest.cc
index 6d76f7e92e2e718e7045cec79f8d6581bfb3712a..b77e385d6cade710ef993b05e3b00929f51fcf5c 100644
--- a/ui/views/controls/menu/menu_runner_unittest.cc
+++ b/ui/views/controls/menu/menu_runner_unittest.cc
@@ -215,43 +215,112 @@ class MenuLauncherEventHandler : public ui::EventHandler {
 
 }  // namespace
 
-// Tests that when a mouse press launches a menu, that the target widget does
-// not take explicit capture, nor closes the menu.
-TEST_F(MenuRunnerTest, WidgetDoesntTakeCapture) {
-  Widget* widget = new Widget;
-  Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
-  widget->Init(params);
-  widget->Show();
-  widget->SetSize(gfx::Size(300, 300));
+class MenuRunnerWidgetTest : public MenuRunnerTest {

[tapted, 2016/10/30 23:25:53]

nit: brief comment like
// Test harness that includes a parent Widget and View invoking the menu.

[themblsha, 2016/10/31 12:19:22]

Done.

+ public:
+  MenuRunnerWidgetTest() {}
 
-  EventCountView* event_count_view = new EventCountView();
-  event_count_view->SetBounds(0, 0, 300, 300);
-  widget->GetRootView()->AddChildView(event_count_view);
+  Widget* widget() { return widget_; }
+  EventCountView* event_count_view() { return event_count_view_; }
 
-  InitMenuRunner(MenuRunner::ASYNC);
-  MenuRunner* runner = menu_runner();
+  std::unique_ptr<ui::test::EventGenerator> EventGeneratorForWidget(
+      Widget* widget) {
+    return base::MakeUnique<ui::test::EventGenerator>(
+        IsMus() ? widget->GetNativeWindow() : GetContext(),
+        widget->GetNativeWindow());
+  }
+
+  void AddMenuLauncherEventHandler(Widget* widget) {
+    consumer_ =
+        base::MakeUnique<MenuLauncherEventHandler>(menu_runner(), widget);
+    event_count_view_->AddPostTargetHandler(consumer_.get());
+  }
+
+  // ViewsTestBase:
+  void SetUp() override {
+    MenuRunnerTest::SetUp();
+    widget_ = new Widget;
+    Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+    widget_->Init(params);
+    widget_->Show();
+    widget_->SetSize(gfx::Size(300, 300));
+
+    event_count_view_ = new EventCountView();
+    event_count_view_->SetBounds(0, 0, 300, 300);
+    widget_->GetRootView()->AddChildView(event_count_view_);
+
+    InitMenuRunner(MenuRunner::ASYNC);
+  }
+
+  void TearDown() override {
+    widget_->CloseNow();
+    MenuRunnerTest::TearDown();
+  }
+
+ private:
+  Widget* widget_;

[tapted, 2016/10/30 23:25:53]

nit: = nullptr;, same below. (although it's not critical now, it's hard to
predict whether a future refactoring may alter SetUp(), and someone may assume
this is initialized)

[themblsha, 2016/10/31 12:19:23]

Hopefully this would make possible future crashes simpler to debug, so added
explicit intialization, thanks!

+  EventCountView* event_count_view_;
+  std::unique_ptr<MenuLauncherEventHandler> consumer_;
+
+  DISALLOW_COPY_AND_ASSIGN(MenuRunnerWidgetTest);
+};
+
+// Tests that when a mouse press launches a menu, that the target widget does
+// not take explicit capture, nor closes the menu.
+TEST_F(MenuRunnerWidgetTest, WidgetDoesntTakeCapture) {
+  AddMenuLauncherEventHandler(owner());
 
-  MenuLauncherEventHandler consumer(runner, owner());
-  event_count_view->AddPostTargetHandler(&consumer);
   EXPECT_EQ(nullptr, internal::NativeWidgetPrivate::GetGlobalCapture(
-                         widget->GetNativeView()));
-  std::unique_ptr<ui::test::EventGenerator> generator(
-      new ui::test::EventGenerator(
-          IsMus() ? widget->GetNativeWindow() : GetContext(),
-          widget->GetNativeWindow()));
+                         widget()->GetNativeView()));
+  auto generator(EventGeneratorForWidget(widget()));
   // Implicit capture should not be held by |widget|.
   generator->PressLeftButton();
-  EXPECT_EQ(1, event_count_view->GetEventCount(ui::ET_MOUSE_PRESSED));
-  EXPECT_NE(
-      widget->GetNativeView(),
-      internal::NativeWidgetPrivate::GetGlobalCapture(widget->GetNativeView()));
+  EXPECT_EQ(1, event_count_view()->GetEventCount(ui::ET_MOUSE_PRESSED));
+  EXPECT_NE(widget()->GetNativeView(),
+            internal::NativeWidgetPrivate::GetGlobalCapture(
+                widget()->GetNativeView()));
 
   // The menu should still be open.
   TestMenuDelegate* delegate = menu_delegate();
-  EXPECT_TRUE(runner->IsRunning());
+  EXPECT_TRUE(menu_runner()->IsRunning());
   EXPECT_EQ(0, delegate->on_menu_closed_called());
+}
+
+// Tests that after showing a menu on mouse press, that the subsequent mouse
+// will be delivered to the correct view, and not to the one that showed the
+// menu.
+//
+// The original bug is reproducible only when showing the menu on mouse press,
+// as RootView::OnMouseReleased() doesn't have the same behavior.
+TEST_F(MenuRunnerWidgetTest, ClearsMouseHandlerOnRun) {
+  AddMenuLauncherEventHandler(widget());
+
+  // Create a second view that's supposed to get the second mouse press.
+  EventCountView* second_event_count_view = new EventCountView();
+  widget()->GetRootView()->AddChildView(second_event_count_view);
+
+  widget()->SetSize(gfx::Size(200, 100));
+  event_count_view()->SetBounds(0, 0, 100, 100);
+  second_event_count_view->SetBounds(100, 0, 100, 100);
+
+  // Click on the first view to show the menu.
+  auto generator(EventGeneratorForWidget(widget()));
+  generator->MoveMouseTo(event_count_view()->bounds().CenterPoint());
+  generator->PressLeftButton();
 
-  widget->CloseNow();
+  // Pretend we dismissed the menu using normal means, as it doesn't matter.
+  EXPECT_TRUE(menu_runner()->IsRunning());
+  menu_runner()->Cancel();
+
+  // EventGenerator won't allow us to re-send the left button press without
+  // releasing it first. We can't send the release event using the same
+  // generator as it would be handled by the RootView in the main Widget.
+  // In actual application the RootView doesn't see the release event.
+  generator.reset();
+  generator = EventGeneratorForWidget(widget());
+
+  generator->MoveMouseTo(second_event_count_view->bounds().CenterPoint());
+  generator->PressLeftButton();
+  EXPECT_EQ(1, second_event_count_view->GetEventCount(ui::ET_MOUSE_PRESSED));
 }
 
 typedef MenuRunnerTest MenuRunnerImplTest;