Mac: Return to the old behavior when updating a <select> while inside a click handler.

Blink core reuses the PopupMenu of an element and first invokes Hide() over
IPC if a menu is already showing. Attempting to show a new menu while the
old menu is fading out confuses AppKit, since we're sting in the NESTED
EVENT LOOP of ShowPopupMenu(). Disable pumping of events in the fade
animation of the old menu in this case so that it closes synchronously.

We still want to pump messages in normal/interactive menu fades, otherwise
web content animations / videos will pause for the duration of the AppKit
fade animation. So this only affects explicit calls to PopupMenuHelper::Hide().

r503112 added the logic to unblock animating content on menu fade out, but it
broke a corner case when a <select> menu updates itself in a click handler.

Bug: 812260
Test: See test case file in bug.
Change-Id: I06a63870126383e44ae2862ebeb133680df2fd8d
Reviewed-on: https://chromium-review.googlesource.com/1006238Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549966}

content/browser/frame_host/popup_menu_helper_mac.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_BROWSER_FRAME_HOST_POPUP_MENU_HELPER_MAC_H_
 #define CONTENT_BROWSER_FRAME_HOST_POPUP_MENU_HELPER_MAC_H_
 
+#include <memory>
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ -21,6 +22,10 @@
 class WebMenuRunner;
 #endif
 
+namespace base {
+class ScopedPumpMessagesInPrivateModes;
+}
+
 namespace content {
 
 class RenderFrameHost;
@@ -71,6 +76,9 @@ class PopupMenuHelper : public NotificationObserver {
   WebMenuRunner* menu_runner_;
   bool popup_was_hidden_;
 
+  // Controls whether messages can be pumped during the menu fade.
+  std::unique_ptr<base::ScopedPumpMessagesInPrivateModes> pump_in_fade_;
+
   base::WeakPtrFactory<PopupMenuHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PopupMenuHelper);

content/browser/frame_host/popup_menu_helper_mac.mm

@@ -7,6 +7,7 @@
 #import "base/mac/scoped_nsobject.h"
 #import "base/mac/scoped_sending_event.h"
 #include "base/message_loop/message_loop.h"
+#import "base/message_loop/message_pump_mac.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
@@ -94,6 +95,9 @@ void PopupMenuHelper::ShowPopupMenu(
     // be done manually.
     base::mac::ScopedSendingEvent sending_event_scoper;
 
+    // Ensure the UI can update while the menu is fading out.
+    pump_in_fade_ = std::make_unique<base::ScopedPumpMessagesInPrivateModes>();
+
     // Now run a NESTED EVENT LOOP until the pop-up is finished.
     [runner runMenuInView:cocoa_view
                withBounds:[cocoa_view flipRectToNSRect:bounds]
@@ -103,6 +107,7 @@ void PopupMenuHelper::ShowPopupMenu(
   if (!weak_ptr)
     return;  // Handle |this| being deleted.
 
+  pump_in_fade_ = nullptr;
   menu_runner_ = nil;
 
   // The RenderFrameHost may be deleted while running the menu, or it may have
@@ -118,6 +123,14 @@ void PopupMenuHelper::ShowPopupMenu(
 }
 
 void PopupMenuHelper::Hide() {
+  // Blink core reuses the PopupMenu of an element and first invokes Hide() over
+  // IPC if a menu is already showing. Attempting to show a new menu while the
+  // old menu is fading out confuses AppKit, since we're still in the NESTED
+  // EVENT LOOP of ShowPopupMenu(). Disable pumping of events in the fade
+  // animation of the old menu in this case so that it closes synchronously.
+  // See http://crbug.com/812260.
+  pump_in_fade_ = nullptr;
+
   if (menu_runner_)
     [menu_runner_ hide];
   popup_was_hidden_ = true;

content/browser/renderer_host/webmenurunner_mac.mm

@@ -6,7 +6,6 @@
 
 #include <stddef.h>
 
-#import "base/message_loop/message_pump_mac.h"
 #include "base/strings/sys_string_conversions.h"
 
 @interface WebMenuRunner (PrivateAPI)
@@ -156,11 +155,7 @@
 
   // Display the menu, and set a flag if a menu item was chosen.
   [cell attachPopUpWithFrame:[dummyView bounds] inView:dummyView];
-  {
-    // Ensure the UI can update while the menu is fading out.
-    base::ScopedPumpMessagesInPrivateModes pump_private;
-    [cell performClickWithFrame:[dummyView bounds] inView:dummyView];
-  }
+  [cell performClickWithFrame:[dummyView bounds] inView:dummyView];
 
   [dummyView removeFromSuperview];