Turn RootWindowController Menus Async

ui/views/widget/widget.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/widget/widget.h"
 
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 1156 matching lines @@
     case ui::ET_MOUSE_PRESSED: {
       last_mouse_event_was_move_ = false;
 
       // We may get deleted by the time we return from OnMousePressed. So we
       // use an observer to make sure we are still alive.
       WidgetDeletionObserver widget_deletion_observer(this);
 
       // Make sure we're still visible before we attempt capture as the mouse
       // press processing may have made the window hide (as happens with menus).
 
-      // It is possible for a View to show a context menu on mouse-press. Since
-      // the menu does a capture and starts a nested message-loop, the release
-      // would go to the menu. The next click (i.e. both mouse-press and release
-      // events) also go to the menu. The menu (and the nested message-loop)
-      // gets closed after this second release event. The code then resumes from
-      // here. So make sure that the mouse-button is still down before doing a
-      // capture.
+      // It is possible for a View to show a context menu on mouse-press, which
+      // takes capture.
+      //
+      // Menus which use a nested message-loop, receive the release. The next
+      // click (i.e. both mouse-press and release events) also go to the menu.
+      // The menu (and the nested message-loop) gets closed after this second
+      // release event. The code then resumes from here. So make sure that the
+      // mouse-button is still down before doing a capture.
+      //
+      // Menus which do not use a nested message-loop will return here, holding
+      // capture. If there is an active MenuController do not reassign capture
+      // to |native_widget_|.
       if (root_view && root_view->OnMousePressed(*event) &&
           widget_deletion_observer.IsWidgetAlive() && IsVisible() &&
-          internal::NativeWidgetPrivate::IsMouseButtonDown()) {
+          internal::NativeWidgetPrivate::IsMouseButtonDown() &&
+          !MenuController::GetActiveInstance()) {

[sky, 2016/05/05 16:34:39]

I don't like having to special case the check for menus here. Would instead
checking if capture changed between the before calling OnMousePressed and after
and assume if there is capture we shouldn't change it be enough?

[jonross, 2016/05/05 17:50:42]

I believe that could account for this. I'm not aware of other actions which
would swap capture, without the native widget becoming the new source of
capture.

Though looking at how we track capture, that would require importing
aura::client::CaptureClient, and I don't know if we want aura code popping into
widget.

I could expand NativeWidgetPrivate to do the lookup....

Thoughts?

         is_mouse_button_pressed_ = true;
         if (!native_widget_->HasCapture())
           native_widget_->SetCapture();
         event->SetHandled();
       }
       return;
     }
 
     case ui::ET_MOUSE_RELEASED:
       last_mouse_event_was_move_ = false;
@@ skipping 286 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // internal::NativeWidgetPrivate, NativeWidget implementation:
 
 internal::NativeWidgetPrivate* NativeWidgetPrivate::AsNativeWidgetPrivate() {
   return this;
 }
 
 }  // namespace internal
 }  // namespace views