MacViews: Clear mouse handler when showing context menus.

ui/views/controls/menu/menu_runner_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/menu/menu_runner.h"
 
 #include <stdint.h>
 
 #include <memory>
 
@@ skipping 236 matching lines @@
       internal::NativeWidgetPrivate::GetGlobalCapture(widget->GetNativeView()));
 
   // The menu should still be open.
   TestMenuDelegate* delegate = menu_delegate();
   EXPECT_TRUE(runner->IsRunning());
   EXPECT_EQ(0, delegate->on_menu_closed_called());
 
   widget->CloseNow();
 }
 
+// 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 shown the

[tapted, 2016/10/27 23:19:24]

nit: shown -> showed (or `has shown` - English grammar is weird :/)

[themblsha, 2016/10/29 13:47:19]

Done.

+// 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(MenuRunnerTest, ClearsMouseHandlerOnRun) {
+  Widget* widget = new Widget;
+  Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+  widget->Init(params);
+  widget->Show();
+  widget->SetSize(gfx::Size(200, 100));
+
+  // Create a view that will show a menu on press.
+  EventCountView* event_count_view = new EventCountView();
+  event_count_view->SetBounds(0, 0, 100, 100);
+  widget->GetRootView()->AddChildView(event_count_view);
+
+  InitMenuRunner(MenuRunner::ASYNC);

[tapted, 2016/10/27 23:19:24]

I think this will make a MenuRunnerImpl on Mac, so the test should already pass
without your fix. Passing MenuRunner::CONTEXT_MENU would use MenuRunnerImplCocoa
on Mac to test the bug being fixed. But then the menu runner won't be
asynchronous - that makes testing harder.

I think the coverage this provides is still good. The fix in menu_controller.cc
was added in  http://crrev.com/192688 without test coverage, so this helps
resolve that (thanks!).

To test the NSMenu flow, there are a bunch of tests in
menu_runner_cocoa_unittest.mm, but they're testing at a lower level (no
MenuRunner, just MenuRunnerImpls). I'm ok without extra stuff for
MenuRunnerImplCocoa - moving the logic to MenuRunner is aligning the contract of
what the MenuRunnerImpls need to fulfil.

[themblsha, 2016/10/29 13:47:19]

Yeah, it's non-Cocoa but tests a common code path now.

+  MenuRunner* runner = menu_runner();
+
+  MenuLauncherEventHandler consumer(runner, widget);
+  event_count_view->AddPostTargetHandler(&consumer);

[tapted, 2016/10/27 23:19:24]

everything above is copy-paste from WidgetDoesntTakeCapture . Also the
|generator| below - is there a neat way to share it?

Perhaps a `MenuRunnerWidgetTest` class

[themblsha, 2016/10/29 13:47:19]

Thanks for the name :-)

+
+  // Create a second view that's supposed to get the second mouse press.
+  EventCountView* second_event_count_view = new EventCountView();
+  second_event_count_view->SetBounds(100, 0, 100, 100);
+  widget->GetRootView()->AddChildView(second_event_count_view);
+
+  // Click on the first view to show the menu.
+  std::unique_ptr<ui::test::EventGenerator> generator(
+      new ui::test::EventGenerator(
+          IsMus() ? widget->GetNativeWindow() : GetContext(),
+          widget->GetNativeWindow()));
+  generator->MoveMouseTo(gfx::Point(50, 50));
+  generator->PressLeftButton();
+
+  // Pretend we dismissed the menu using normal means, as it doesn't matter.
+  EXPECT_TRUE(runner->IsRunning());
+  runner->Cancel();
+

[tapted, 2016/10/27 23:19:24]

EXPECT_FALSE(runner->IsRunning());

[themblsha, 2016/10/29 13:47:19]

Done.

+  // 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.reset(new ui::test::EventGenerator(
+      IsMus() ? widget->GetNativeWindow() : GetContext(),
+      widget->GetNativeWindow()));
+
+  generator->MoveMouseTo(gfx::Point(150, 50));
+  generator->PressLeftButton();
+  EXPECT_EQ(1, second_event_count_view->GetEventCount(ui::ET_MOUSE_PRESSED));
+
+  widget->CloseNow();
+}
+
 typedef MenuRunnerTest MenuRunnerImplTest;
 
 // Tests that when nested menu runners are destroyed out of order, that
 // MenuController is not accessed after it has been destroyed. This should not
 // crash on ASAN bots.
 TEST_F(MenuRunnerImplTest, NestedMenuRunnersDestroyedOutOfOrder) {
   internal::MenuRunnerImpl* menu_runner =
       new internal::MenuRunnerImpl(menu_item_view());
   EXPECT_EQ(MenuRunner::NORMAL_EXIT,
             menu_runner->RunMenuAt(owner(), nullptr, gfx::Rect(),
@@ skipping 18 matching lines @@
   menu_runner->OnMenuClosed(internal::MenuControllerDelegate::NOTIFY_DELEGATE,
                             nullptr, 0);
 
   // This should not access the destroyed MenuController
   menu_runner2->Release();
   menu_runner->Release();
 }
 
 }  // namespace test
 }  // namespace views