Mac: ExternalPopupMenu::close should hide a showing popup menu.

content/browser/renderer_host/popup_menu_helper_mac.mm

 // 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.
 
 #import <Carbon/Carbon.h>
 
 #include "content/browser/renderer_host/popup_menu_helper_mac.h"
 
 #include "base/mac/scoped_nsobject.h"
 #import "base/mac/scoped_sending_event.h"
 #include "base/message_loop/message_loop.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_mac.h"
 #include "content/browser/renderer_host/webmenurunner_mac.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #import "ui/base/cocoa/base_view.h"
 
 namespace content {
 
 namespace {
 
 bool g_allow_showing_popup_menus = true;
 
 }  // namespace
 
 PopupMenuHelper::PopupMenuHelper(RenderViewHost* render_view_host)
-    : render_view_host_(static_cast<RenderViewHostImpl*>(render_view_host)) {
+    : render_view_host_(static_cast<RenderViewHostImpl*>(render_view_host)),

[Robert Sesek, 2014/03/20 15:06:04]

menu_runner_ is uninitialized here.

[tkent, 2014/03/21 13:55:26]

Done.

+      popup_was_hidden_(false) {
   notification_registrar_.Add(
       this, NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
       Source<RenderWidgetHost>(render_view_host));
 }
 
 void PopupMenuHelper::ShowPopupMenu(
     const gfx::Rect& bounds,
     int item_height,
     double item_font_size,
     int selected_item,
     const std::vector<MenuItem>& items,
     bool right_aligned,
     bool allow_multiple_selection) {
   // Only single selection list boxes show a popup on Mac.
   DCHECK(!allow_multiple_selection);
 
   if (!g_allow_showing_popup_menus)
     return;
 
   // Retain the Cocoa view for the duration of the pop-up so that it can't be
   // dealloced if my Destroy() method is called while the pop-up's up (which
   // would in turn delete me, causing a crash once the -runMenuInView
   // call returns. That's what was happening in <http://crbug.com/33250>).
   RenderWidgetHostViewMac* rwhvm =
       static_cast<RenderWidgetHostViewMac*>(GetRenderWidgetHostView());
   base::scoped_nsobject<RenderWidgetHostViewCocoa> cocoa_view(
       [rwhvm->cocoa_view() retain]);
 
   // Display the menu.
-  base::scoped_nsobject<WebMenuRunner> menu_runner;
-   menu_runner.reset([[WebMenuRunner alloc] initWithItems:items
-                                                 fontSize:item_font_size
-                                             rightAligned:right_aligned]);
+  menu_runner_ = [[WebMenuRunner alloc] initWithItems:items
+                                             fontSize:item_font_size
+                                         rightAligned:right_aligned];
 
   {
     // Make sure events can be pumped while the menu is up.
     base::MessageLoop::ScopedNestableTaskAllower allow(
         base::MessageLoop::current());
 
     // One of the events that could be pumped is |window.close()|.
     // User-initiated event-tracking loops protect against this by
     // setting flags in -[CrApplication sendEvent:], but since
     // web-content menus are initiated by IPC message the setup has to
     // be done manually.
     base::mac::ScopedSendingEvent sending_event_scoper;
 
     // Now run a SYNCHRONOUS NESTED EVENT LOOP until the pop-up is finished.
-    [menu_runner runMenuInView:cocoa_view
-                    withBounds:[cocoa_view flipRectToNSRect:bounds]
-                  initialIndex:selected_item];
+    [menu_runner_ runMenuInView:cocoa_view
+                     withBounds:[cocoa_view flipRectToNSRect:bounds]
+                   initialIndex:selected_item];
   }
 
   if (!render_view_host_) {
     // Bad news, the RenderViewHost got deleted while we were off running the
     // menu. Nothing to do.
+    [menu_runner_ release];
+    menu_runner_ = NULL;

[Robert Sesek, 2014/03/20 15:06:04]

nil instead of NULL for ObjC objects, and on line 98.

[tkent, 2014/03/21 13:55:26]

Done.

     return;
   }
 
-  if ([menu_runner menuItemWasChosen]) {
-    render_view_host_->DidSelectPopupMenuItem(
-        [menu_runner indexOfSelectedItem]);
-  } else {
-    render_view_host_->DidCancelPopupMenu();
+  if (!popup_was_hidden_) {
+    if ([menu_runner_ menuItemWasChosen]) {
+      render_view_host_->DidSelectPopupMenuItem(
+          [menu_runner_ indexOfSelectedItem]);
+    } else {
+      render_view_host_->DidCancelPopupMenu();
+    }
   }
+  [menu_runner_ release];
+  menu_runner_ = NULL;
+}
+
+void PopupMenuHelper::Hide() {
+  if (menu_runner_)
+    [menu_runner_ hide];
+  popup_was_hidden_ = true;
 }
 
 // static
 void PopupMenuHelper::DontShowPopupMenuForTesting() {
   g_allow_showing_popup_menus = false;
 }
 
 RenderWidgetHostViewMac* PopupMenuHelper::GetRenderWidgetHostView() const {
   return static_cast<RenderWidgetHostViewMac*>(render_view_host_->GetView());
 }
 
 void PopupMenuHelper::Observe(int type,
                               const NotificationSource& source,
                               const NotificationDetails& details) {
   DCHECK(type == NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED);
   DCHECK(Source<RenderWidgetHost>(source).ptr() == render_view_host_);
   render_view_host_ = NULL;
 }
 
 }  // namespace content