Merge r32364 to 249 branch.

RenderWidgetHostViewMac should release its owning RenderWidgetHostViewCocoa
in Destroy when the widget was a native (external) popup menu.

This change depends on WebKit r51102
(https://bugs.webkit.org/show_bug.cgi?id=31609).

BUG=27723, 26876
TEST=1. Following the steps in bug 27723 comment 5 should not result in a sad
        tab:
         a. Open Google Calendar
         b. Bring up a recurring event
         c. Click the "Edit" link on the "Repeat" line beneath "When"
         d. Change the value of the "Repeat Every" dropdown menu
     2. Also, web content popup menus should work more generally.
     3. After using a web content popup menu and then closing all windows,
        there should not be any renderer processes running.
Review URL: http://codereview.chromium.org/397039

TBR=mark@chromium.org
Review URL: http://codereview.chromium.org/404032

git-svn-id: svn://svn.chromium.org/chrome/branches/249/src@32380 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 48 insertions, 41 deletions

diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.h b/chrome/browser/renderer_host/render_widget_host_view_mac.h
index 67c8abf..0e65cef 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.h
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.h
@@ -32,7 +32,7 @@ class RWHVMEditCommandHelper;
 @interface RenderWidgetHostViewCocoa
     : BaseView <RenderWidgetHostViewMacOwner, NSTextInput, NSChangeSpelling> {
  @private
-  RenderWidgetHostViewMac* renderWidgetHostView_;  // Owned by us.
+  scoped_ptr<RenderWidgetHostViewMac> renderWidgetHostView_;
   BOOL canBeKeyView_;
   BOOL closeOnDeactivate_;
   scoped_ptr<RWHVMEditCommandHelper> editCommand_helper_;
@@ -41,7 +41,7 @@ class RWHVMEditCommandHelper;
   id trackingRectOwner_;              // (not retained)
   void *trackingRectUserData_;
   NSTrackingRectTag lastToolTipTag_;
-  NSString* toolTip_;
+  scoped_nsobject<NSString> toolTip_;
 
   BOOL ignoreKeyEvents_;
   scoped_nsobject<NSEvent> lastKeyPressedEvent_;
@@ -195,8 +195,13 @@ class RenderWidgetHostViewMac : public RenderWidgetHostView {
   // invoke it from the message loop.
   void ShutdownHost();
 
-  // The associated view.
-  RenderWidgetHostViewCocoa* cocoa_view_;  // WEAK
+  // The associated view. This is weak and is inserted into the view hierarchy
+  // to own this RenderWidgetHostViewMac object unless is_popup_menu_ is true.
+  // In that case, cocoa_view_ is never inserted into the view hierarchy, so
+  // the RenderWidgetHostViewMac will treat it as a strong reference and will
+  // release it when told to destroy (for example, because a pop-up menu has
+  // closed).
+  RenderWidgetHostViewCocoa* cocoa_view_;
 
   // The cursor for the page. This is passed up from the renderer.
   WebCursor current_cursor_;
@@ -207,6 +212,10 @@ class RenderWidgetHostViewMac : public RenderWidgetHostView {
   // true if the View is not visible.
   bool is_hidden_;
 
+  // True if the widget is a native popup menu.  The renderer code calls this
+  // an "external popup."
+  bool is_popup_menu_;
+
   // The text to be shown in the tooltip, supplied by the renderer.
   std::wstring tooltip_text_;
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.mm b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
index 1c5469c..1d4121c 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
@@ -68,6 +68,7 @@ RenderWidgetHostViewMac::RenderWidgetHostViewMac(RenderWidgetHost* widget)
       im_composing_(false),
       is_loading_(false),
       is_hidden_(false),
+      is_popup_menu_(false),
       shutdown_factory_(this),
       parent_view_(NULL) {
   // |cocoa_view_| owns us and we will be deleted when |cocoa_view_| goes away.
@@ -296,16 +297,30 @@ void RenderWidgetHostViewMac::Destroy() {
   // time Destroy() was called. On the Mac we have to destroy all the popups
   // ourselves.
 
-  // Depth-first destroy all popups. Use ShutdownHost() to enforce deepest-first
-  // ordering.
-  for (RenderWidgetHostViewCocoa* subview in [cocoa_view_ subviews]) {
-    [subview renderWidgetHostViewMac]->ShutdownHost();
-  }
+  if (!is_popup_menu_) {
+    // Depth-first destroy all popups. Use ShutdownHost() to enforce
+    // deepest-first ordering.
+    for (RenderWidgetHostViewCocoa* subview in [cocoa_view_ subviews]) {
+      [subview renderWidgetHostViewMac]->ShutdownHost();
+    }
+
+    // We've been told to destroy.
+    [cocoa_view_ retain];
+    [cocoa_view_ removeFromSuperview];
+    [cocoa_view_ autorelease];
+  } else {
+    // From the renderer's perspective, the pop-up menu is represented by a
+    // RenderWidget. The actual Mac implementation uses a native pop-up menu
+    // and doesn't actually make use of the RenderWidgetHostViewCocoa that
+    // was allocated to own it in its constructor.  When the pop-up menu goes
+    // away, free the RenderWidgetHostViewCocoa. Its deallocation will result
+    // in this object's destruction.
 
-  // We've been told to destroy.
-  [cocoa_view_ retain];
-  [cocoa_view_ removeFromSuperview];
-  [cocoa_view_ autorelease];
+    DCHECK([[cocoa_view_ subviews] count] == 0);
+    DCHECK([cocoa_view_ superview] == nil);
+
+    [cocoa_view_ autorelease];
+  }
 
   // We get this call just before |render_widget_host_| deletes
   // itself.  But we are owned by |cocoa_view_|, which may be retained
@@ -346,6 +361,8 @@ void RenderWidgetHostViewMac::ShowPopupWithItems(
     int item_height,
     int selected_item,
     const std::vector<WebMenuItem>& items) {
+  is_popup_menu_ = true;
+
   NSRect view_rect = [cocoa_view_ bounds];
   NSRect parent_rect = [parent_view_ bounds];
   int y_offset = bounds.y() + bounds.height();
@@ -377,19 +394,6 @@ void RenderWidgetHostViewMac::ShowPopupWithItems(
     NativeWebKeyboardEvent keyboard_event(event);
     render_widget_host_->ForwardKeyboardEvent(keyboard_event);
   }
-
-  // From the renderer's perspective, the pop-up menu is represented by a
-  // RenderWidget.  The actual Mac implementation uses a native pop-up menu
-  // and doesn't actually make use of the RenderWidgetHostViewCocoa that
-  // was allocated to own it in its constructor.  When the pop-up menu goes
-  // away, shut down the RenderWidgetHost (and RenderWidget), and then free
-  // the RenderWidgetHostViewCocoa, which will result in the destruction of
-  // this object.
-  //
-  // TODO(mark): This is a little bit dirty.  Figure out a more proper
-  // ownership model.  http://crbug.com/26876
-  ShutdownHost();
-  [cocoa_view_ release];
 }
 
 void RenderWidgetHostViewMac::KillSelf() {
@@ -527,20 +531,13 @@ void RenderWidgetHostViewMac::SetBackground(const SkBitmap& background) {
     editCommand_helper_.reset(new RWHVMEditCommandHelper);
     editCommand_helper_->AddEditingSelectorsToClass([self class]);
 
-    renderWidgetHostView_ = r;
+    renderWidgetHostView_.reset(r);
     canBeKeyView_ = YES;
     closeOnDeactivate_ = NO;
   }
   return self;
 }
 
-- (void)dealloc {
-  delete renderWidgetHostView_;
-  [toolTip_ release];
-
-  [super dealloc];
-}
-
 - (void)setCanBeKeyView:(BOOL)can {
   canBeKeyView_ = can;
 }
@@ -884,7 +881,7 @@ void RenderWidgetHostViewMac::SetBackground(const SkBitmap& background) {
 }
 
 - (RenderWidgetHostViewMac*)renderWidgetHostViewMac {
-  return renderWidgetHostView_;
+  return renderWidgetHostView_.get();
 }
 
 // Determine whether we should autohide the cursor (i.e., hide it until mouse
@@ -1103,16 +1100,17 @@ static const NSTrackingRectTag kTrackingRectTag = 0xBADFACE;
 // appears after a delay.) Pass null to remove the tooltip.
 - (void)setToolTipAtMousePoint:(NSString *)string {
   NSString *toolTip = [string length] == 0 ? nil : string;
-  NSString *oldToolTip = toolTip_;
-  if ((toolTip == nil || oldToolTip == nil) ? toolTip == oldToolTip
-                                    : [toolTip isEqualToString:oldToolTip]) {
+  if ((toolTip && toolTip_ && [toolTip isEqualToString:toolTip_]) ||
+      (!toolTip && !toolTip_)) {
     return;
   }
-  if (oldToolTip) {
+
+  if (toolTip_) {
     [self _sendToolTipMouseExited];
-    [oldToolTip release];
   }
-  toolTip_ = [toolTip copy];
+
+  toolTip_.reset([toolTip copy]);
+
   if (toolTip) {
     // See radar 3500217 for why we remove all tooltips
     // rather than just the single one we created.