MacViews: Clear mouse handler when showing context menus.

ui/views/controls/menu/menu_runner.cc

 // Copyright (c) 2012 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 <utility>
 
 #include "ui/views/controls/menu/menu_runner_handler.h"
 #include "ui/views/controls/menu/menu_runner_impl.h"
+#include "ui/views/widget/widget.h"
 
 namespace views {
 
 MenuRunner::MenuRunner(ui::MenuModel* menu_model, int32_t run_types)
     : run_types_(run_types),
       impl_(internal::MenuRunnerImplInterface::Create(menu_model, run_types)) {}
 
 MenuRunner::MenuRunner(MenuItemView* menu_view, int32_t run_types)
     : run_types_(run_types), impl_(new internal::MenuRunnerImpl(menu_view)) {}
 
 MenuRunner::~MenuRunner() {
   impl_->Release();
 }
 
 MenuRunner::RunResult MenuRunner::RunMenuAt(Widget* parent,
                                             MenuButton* button,
                                             const gfx::Rect& bounds,
                                             MenuAnchorPosition anchor,
                                             ui::MenuSourceType source_type) {
+  // If we are shown on mouse press, we will eat the subsequent mouse down and
+  // the parent widget will not be able to reset its state (it might have mouse
+  // capture from the mouse down). So we clear its state here.
+  if (parent && parent->GetRootView())
+    parent->GetRootView()->SetMouseHandler(nullptr);
+
   if (runner_handler_.get()) {
     return runner_handler_->RunMenuAt(
         parent, button, bounds, anchor, source_type, run_types_);
   }
 
   // The parent of the nested menu will have created a DisplayChangeListener, so
   // we avoid creating a DisplayChangeListener if nested. Drop menus are
   // transient, so we don't cancel in that case.
   if ((run_types_ & (IS_NESTED | FOR_DROP)) == 0 && parent) {
     display_change_listener_.reset(
         internal::DisplayChangeListener::Create(parent, this));
   }
 
   if (run_types_ & CONTEXT_MENU) {
     switch (source_type) {
       case ui::MENU_SOURCE_NONE:
       case ui::MENU_SOURCE_KEYBOARD:
       case ui::MENU_SOURCE_MOUSE:
         anchor = MENU_ANCHOR_TOPLEFT;
         break;
       case ui::MENU_SOURCE_TOUCH:
       case ui::MENU_SOURCE_TOUCH_EDIT_MENU:
         anchor = MENU_ANCHOR_BOTTOMCENTER;
         break;
       default:
         break;
     }
   }
 
   return impl_->RunMenuAt(parent, button, bounds, anchor, run_types_);

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

(answering for themblsha :), This line may call

MenuRunnerImplCocoa::RunMenuAt(..)

which is using the NSMenu menu controller (ui/base/cocoa/menu_contoller.mm),
rather than a views::MenuController

Patchset 1 copied the logic verbatim from menu_controller.cc to
menu_runner_impl_cocoa.mm, but that code fragment was already scattered around a
bunch of places, so I suggested we move it here at
https://codereview.chromium.org/2450903002/diff/1/ui/views/controls/menu/menu...
. (maybe it's nicer to duplicate the code - sky will know!)

(themblsha: It's generally good to capture stuff like this in the CL description
- I don't want to write all your CL descriptions for you, so feel free to go
into more detail. When describing a fix, it's good not just to say "how" but
also "why" -- e.g. by moving the code in MenuController to MenuRunner we allow
all implementors of MenuRunnerImplInterface to benefit from the fix.

and, as earlier in the review for this CL and in other places, I keep pointing
yandex folks to
https://www.chromium.org/developers/contributing-code#TOC-Writing-change-list...
. Please read it all. Specifically in this case "Long lines should be wrapped to
80 columns for easier log message viewing in
terminals.")

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

Thanks for your tremendous patience with helping us improve the submitted CLs.
We're not really coordinated, but personally I'll try to put more effort into my
descriptions.

80 columns is understandable, but what about the TEST line? I left it in a
single line because it was like this in the provided example.

 }
 
 bool MenuRunner::IsRunning() const {
   return impl_->IsRunning();
 }
 
 void MenuRunner::Cancel() {
   impl_->Cancel();
 }
 
 base::TimeTicks MenuRunner::closing_event_time() const {
   return impl_->GetClosingEventTime();
 }
 
 void MenuRunner::SetRunnerHandler(
     std::unique_ptr<MenuRunnerHandler> runner_handler) {
   runner_handler_ = std::move(runner_handler);
 }
 
 }  // namespace views