Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(914)

Unified Diff: content/browser/frame_host/popup_menu_helper_mac.mm

Issue 2888803002: Handle WebContentsViewMac being destroyed while a <select> menu is showing. (Closed)
Patch Set: respond to comments Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/frame_host/popup_menu_helper_mac.mm
diff --git a/content/browser/frame_host/popup_menu_helper_mac.mm b/content/browser/frame_host/popup_menu_helper_mac.mm
index 5cef607e10edba1fffeecf1eb65dbd4e12c2e647..c33a667e552a49f8b719dc771e0e6a0f6e967e3b 100644
--- a/content/browser/frame_host/popup_menu_helper_mac.mm
+++ b/content/browser/frame_host/popup_menu_helper_mac.mm
@@ -2,11 +2,9 @@
// 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/frame_host/popup_menu_helper_mac.h"
-#include "base/mac/scoped_nsobject.h"
+#import "base/mac/scoped_nsobject.h"
#import "base/mac/scoped_sending_event.h"
#include "base/message_loop/message_loop.h"
#include "content/browser/frame_host/frame_tree.h"
@@ -28,10 +26,13 @@ bool g_allow_showing_popup_menus = true;
} // namespace
-PopupMenuHelper::PopupMenuHelper(RenderFrameHost* render_frame_host)
- : render_frame_host_(static_cast<RenderFrameHostImpl*>(render_frame_host)),
+PopupMenuHelper::PopupMenuHelper(Delegate* delegate,
+ RenderFrameHost* render_frame_host)
+ : delegate_(delegate),
+ render_frame_host_(static_cast<RenderFrameHostImpl*>(render_frame_host)),
menu_runner_(nil),
- popup_was_hidden_(false) {
+ popup_was_hidden_(false),
+ weak_ptr_factory_(this) {
notification_registrar_.Add(
this, NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
Source<RenderWidgetHost>(
@@ -42,6 +43,10 @@ PopupMenuHelper::PopupMenuHelper(RenderFrameHost* render_frame_host)
render_frame_host->GetRenderViewHost()->GetWidget()));
}
+PopupMenuHelper::~PopupMenuHelper() {
+ Hide();
+}
+
void PopupMenuHelper::ShowPopupMenu(
const gfx::Rect& bounds,
int item_height,
@@ -66,9 +71,15 @@ void PopupMenuHelper::ShowPopupMenu(
[rwhvm->cocoa_view() retain]);
// Display the menu.
- menu_runner_ = [[WebMenuRunner alloc] initWithItems:items
- fontSize:item_font_size
- rightAligned:right_aligned];
+ base::scoped_nsobject<WebMenuRunner> runner([[WebMenuRunner alloc]
+ initWithItems:items
+ fontSize:item_font_size
+ rightAligned:right_aligned]);
+
+ // Take a weak reference so that Hide() can close the menu.
+ menu_runner_ = runner;
+
+ base::WeakPtr<PopupMenuHelper> weak_ptr(weak_ptr_factory_.GetWeakPtr());
{
// Make sure events can be pumped while the menu is up.
@@ -82,30 +93,27 @@ void PopupMenuHelper::ShowPopupMenu(
// 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];
+ // Now run a NESTED EVENT LOOP until the pop-up is finished.
+ [runner runMenuInView:cocoa_view
+ withBounds:[cocoa_view flipRectToNSRect:bounds]
+ initialIndex:selected_item];
}
- if (!render_frame_host_) {
- // Bad news, the RenderFrameHost got deleted while we were off running the
- // menu. Nothing to do.
- [menu_runner_ release];
- menu_runner_ = nil;
- return;
- }
+ if (!weak_ptr)
+ return; // Handle |this| being deleted.
+
+ menu_runner_ = nil;
- if (!popup_was_hidden_) {
- if ([menu_runner_ menuItemWasChosen]) {
- render_frame_host_->DidSelectPopupMenuItem(
- [menu_runner_ indexOfSelectedItem]);
- } else {
+ // The RenderFrameHost may be deleted while running the menu, or it may have
+ // requested the close. Don't notify in these cases.
+ if (render_frame_host_ && !popup_was_hidden_) {
+ if ([runner menuItemWasChosen])
+ render_frame_host_->DidSelectPopupMenuItem([runner indexOfSelectedItem]);
+ else
render_frame_host_->DidCancelPopupMenu();
- }
}
- [menu_runner_ release];
- menu_runner_ = nil;
+
+ delegate_->OnMenuClosed(); // May delete |this|.
}
void PopupMenuHelper::Hide() {
« no previous file with comments | « content/browser/frame_host/popup_menu_helper_mac.h ('k') | content/browser/web_contents/web_contents_view_mac.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698