MacViews: Clear mouse handler when showing context menus.

ui/views/controls/menu/menu_runner_impl_cocoa.mm

 // Copyright 2014 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.
 
 #import "ui/views/controls/menu/menu_runner_impl_cocoa.h"
 
 #include "base/mac/sdk_forward_declarations.h"
 #import "ui/base/cocoa/menu_controller.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/events/base_event_utils.h"
@@ skipping 110 matching lines @@
                                                      MenuButton* button,
                                                      const gfx::Rect& bounds,
                                                      MenuAnchorPosition anchor,
                                                      int32_t run_types) {
   DCHECK(run_types & kNativeRunTypes);
   DCHECK(!IsRunning());
   DCHECK(parent);
   closing_event_time_ = base::TimeTicks();
   running_ = true;
 
+  // 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())

[tapted, 2016/10/26 02:15:31]

above there's a DCHECK(parent), so there's no need to check |parent| here as
well. And the RootView is only reset in Widget::~Widget(), so I don't think
there's a need to check GetRootView either. However, MenuRunner::RunMenuAt(..)
isn't as strict, so we may want to keep the checks if this fragment moves there.

+    parent->GetRootView()->SetMouseHandler(nullptr);

[tapted, 2016/10/26 02:15:31]

This is just copied from menu_controller.cc - can we move this step and the
comment into MenuRunner::RunMenuAt instead so that it's not duplicated?

It's possible there's a reason not to. E.g. MenuRunnerImpl::RunMenuAt may exit
early before invoking MenuController::Run, so the codepaths are not identical.
sky@'s amazing review powers may see something I've missed, but this approach
seems sensible if no other errors appear, e.g., in existing tests.

But also this fragment appears in a bunch of places. The one in
menu_controller.cc was added in r192688

There's another in combobox.cc that was "added" after that, in r223213. But
actually it was moved from some code added in r84253. Well, actually before that
even since that's also a move.  So the fragment in combobox.cc is probably
redundant with the code that appears in menu_controller.cc (which was added
after) and we can delete it too.

These fragments are also in OnMenuClosed handlers in toolbar_button.cc and
browser.cc -- those can probably be deleted as well.

[themblsha, 2016/10/26 15:49:14]

The fragment in browser.cc is from 2016, and it's for 'mash' which I'm not sure
how to test. Others seem safe, so your proposal sounds good to me.

+
   if (run_types & MenuRunner::CONTEXT_MENU) {
     [NSMenu popUpContextMenu:[menu_controller_ menu]
                    withEvent:[NSApp currentEvent]
                      forView:parent->GetNativeView()];
   } else if (run_types & MenuRunner::COMBOBOX) {
     NSMenuItem* checked_item = FirstCheckedItem(menu_controller_);
     base::scoped_nsobject<NSView> anchor_view(
         CreateMenuAnchorView(parent->GetNativeWindow(), bounds, checked_item));
     NSMenu* menu = [menu_controller_ menu];
     [menu setMinimumWidth:bounds.width() + kNativeCheckmarkWidth];
@@ skipping 22 matching lines @@
 
 base::TimeTicks MenuRunnerImplCocoa::GetClosingEventTime() const {
   return closing_event_time_;
 }
 
 MenuRunnerImplCocoa::~MenuRunnerImplCocoa() {
 }
 
 }  // namespace internal
 }  // namespace views