Make the HistoryMenuBridge::HistoryItem co-own the NSMenuItem. This hopefully fixes a top-crash.

BUG=46289
TEST=Open 3 windows, with 10-12 tabs in each. Cmd+W rapidly. Chrome doesn't crash. See bug for details.

Review URL: http://codereview.chromium.org/2836008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50134 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 17 insertions, 15 deletions

diff --git a/chrome/browser/cocoa/history_menu_bridge.h b/chrome/browser/cocoa/history_menu_bridge.h
index ef84031..92be247 100644
--- a/chrome/browser/cocoa/history_menu_bridge.h
+++ b/chrome/browser/cocoa/history_menu_bridge.h
@@ -88,8 +88,12 @@ class HistoryMenuBridge : public NotificationObserver,
     // The Handle given to us by the FaviconService for the icon fetch request.
     FaviconService::Handle icon_handle;
 
-    // The pointer to the item after it has been created. Weak.
-    NSMenuItem* menu_item;
+    // The pointer to the item after it has been created. Strong; NSMenu also
+    // retains this. During a rebuild flood (if the user closes a lot of tabs
+    // quickly), the NSMenu can release the item before the HistoryItem has
+    // been fully deleted. If this were a weak pointer, it would result in a
+    // zombie.
+    scoped_nsobject<NSMenuItem> menu_item;
 
     // This ID is unique for a browser session and can be passed to the
     // TabRestoreService to re-open the closed window or tab that this
diff --git a/chrome/browser/cocoa/history_menu_bridge.mm b/chrome/browser/cocoa/history_menu_bridge.mm
index 093ab40..fb40e9b 100644
--- a/chrome/browser/cocoa/history_menu_bridge.mm
+++ b/chrome/browser/cocoa/history_menu_bridge.mm
@@ -298,28 +298,27 @@ NSMenuItem* HistoryMenuBridge::AddItemToMenu(HistoryItem* item,
                [title substringFromIndex:([title length] -
                                           kMenuTrimSizeInChars)]];
   }
-  scoped_nsobject<NSMenuItem> menu_item(
+  item->menu_item.reset(
       [[NSMenuItem alloc] initWithTitle:title
                                  action:nil
                           keyEquivalent:@""]);
-  [menu_item setTarget:controller_];
-  [menu_item setAction:@selector(openHistoryMenuItem:)];
-  [menu_item setTag:tag];
+  [item->menu_item setTarget:controller_];
+  [item->menu_item setAction:@selector(openHistoryMenuItem:)];
+  [item->menu_item setTag:tag];
   if (item->icon.get())
-    [menu_item setImage:item->icon.get()];
+    [item->menu_item setImage:item->icon.get()];
   else if (!item->tabs.size())
-    [menu_item setImage:default_favicon_.get()];
+    [item->menu_item setImage:default_favicon_.get()];
 
   // Add a tooltip.
   NSString* tooltip = [NSString stringWithFormat:@"%@\n%s", full_title,
                                 url_string.c_str()];
-  [menu_item setToolTip:tooltip];
+  [item->menu_item setToolTip:tooltip];
 
-  [menu insertItem:menu_item atIndex:index];
-  item->menu_item = menu_item.get();
-  menu_item_map_.insert(std::make_pair(menu_item.get(), item));
+  [menu insertItem:item->menu_item.get() atIndex:index];
+  menu_item_map_.insert(std::make_pair(item->menu_item.get(), item));
 
-  return menu_item;
+  return item->menu_item.get();
 }
 
 void HistoryMenuBridge::Init() {
diff --git a/chrome/browser/cocoa/history_menu_bridge_unittest.mm b/chrome/browser/cocoa/history_menu_bridge_unittest.mm
index 0d23906..1fd47a2 100644
--- a/chrome/browser/cocoa/history_menu_bridge_unittest.mm
+++ b/chrome/browser/cocoa/history_menu_bridge_unittest.mm
@@ -369,8 +369,7 @@ TEST_F(HistoryMenuBridgeTest, GotFaviconData) {
 
   // Set up the HistoryItem.
   HistoryMenuBridge::HistoryItem item;
-  scoped_nsobject<NSMenuItem> menu_item([[NSMenuItem alloc] init]);
-  item.menu_item = menu_item.get();
+  item.menu_item.reset([[NSMenuItem alloc] init]);
   GetFaviconForHistoryItem(&item);
 
   // Pretend to be called back.