Revert 40275 - Maybe broke TabRestoreUITest.RestoreIntoSameWindow? - [Mac] Add favicons to the history menu

BUG=20464
TEST=Open History menu, see icons.

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

TBR=rsesek@chromium.org
Review URL: http://codereview.chromium.org/660277

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

6 files changed, 104 insertions, 323 deletions

diff --git a/chrome/browser/cocoa/history_menu_bridge.h b/chrome/browser/cocoa/history_menu_bridge.h
index 488909f..c67ba83 100644
--- a/chrome/browser/cocoa/history_menu_bridge.h
+++ b/chrome/browser/cocoa/history_menu_bridge.h
@@ -1,16 +1,32 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
+// C++ controller for the history menu; one per AppController (means there
+// is only one). This class observes various data sources, namely the
+// HistoryService and the TabRestoreService, and then updates the NSMenu when
+// there is new data.
+//
+// The history menu is broken up into sections: most visisted and recently
+// closed. The overall menu has a tag of IDC_HISTORY_MENU, with two sections
+// IDC_HISTORY_MENU_VISITED and IDC_HISTORY_MENU_CLOSED, which are used to
+// delineate the two sections. Items within these sections are assigned tags
+// within IDC_HISTORY_MENU_* + 1..99. Most Chromium Cocoa menu items are static
+// from a nib (e.g. New Tab), but may be enabled/disabled under certain
+// circumstances (e.g. Cut and Paste). In addition, most Cocoa menu items have
+// firstResponder: as a target. Unusually, bookmark menu items are created
+// dynamically. They also have a target of HistoryMenuCocoaController, not
+// firstResponder. See HistoryMenuBridge::AddItemToMenu()). Unlike most of our
+// Cocoa-Bridge classes, the HMCC is not at the root of the ownership model
+// because its only function is to respond to menu item actions; everything
+// else is done in this bridge.
+
 #ifndef CHROME_BROWSER_COCOA_HISTORY_MENU_BRIDGE_H_
 #define CHROME_BROWSER_COCOA_HISTORY_MENU_BRIDGE_H_
 
 #import <Cocoa/Cocoa.h>
 #include "base/scoped_nsobject.h"
-#include "base/scoped_ptr.h"
-#include "base/scoped_vector.h"
 #include "chrome/browser/cancelable_request.h"
-#import "chrome/browser/favicon_service.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/common/notification_observer.h"
@@ -22,56 +38,20 @@ class Profile;
 class TabNavigationEntry;
 @class HistoryMenuCocoaController;
 
-// C++ controller for the history menu; one per AppController (means there
-// is only one). This class observes various data sources, namely the
-// HistoryService and the TabRestoreService, and then updates the NSMenu when
-// there is new data.
-//
-// The history menu is broken up into sections: most visisted and recently
-// closed. The overall menu has a tag of IDC_HISTORY_MENU, with two sections
-// IDC_HISTORY_MENU_VISITED and IDC_HISTORY_MENU_CLOSED, which are used to
-// delineate the two sections. Items within these sections are assigned tags
-// within IDC_HISTORY_MENU_* + 1..99. Most Chromium Cocoa menu items are static
-// from a nib (e.g. New Tab), but may be enabled/disabled under certain
-// circumstances (e.g. Cut and Paste). In addition, most Cocoa menu items have
-// firstResponder: as a target. Unusually, history menu items are created
-// dynamically. They also have a target of HistoryMenuCocoaController, not
-// firstResponder. See HistoryMenuBridge::AddItemToMenu(). Unlike most of our
-// Cocoa-Bridge classes, the HMCC is not at the root of the ownership model
-// because its only function is to respond to menu item actions; everything
-// else is done in this bridge.
 class HistoryMenuBridge : public NotificationObserver,
                           public TabRestoreService::Observer {
  public:
   // This is a generalization of the data we store in the history menu because
   // we pull things from different sources with different data types.
   struct HistoryItem {
-   public:
-    HistoryItem() : icon_requested(false) {}
+    HistoryItem() {}
     ~HistoryItem() {}
 
-    // The title for the menu item.
     string16 title;
-    // The URL that will be navigated to if the user selects this item.
     GURL url;
-    // Favicon for the URL.
-    scoped_nsobject<NSImage> icon;
-
-    // If the icon is being requested from the FaviconService, |icon_requested|
-    // will be true and |icon_handle| will be non-NULL. If this is false, then
-    // |icon_handle| will be NULL.
-    bool icon_requested;
-    // 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;
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(HistoryItem);
   };
 
-  explicit HistoryMenuBridge(Profile* profile);
+  HistoryMenuBridge(Profile* profile);
   virtual ~HistoryMenuBridge();
 
   // Overriden from NotificationObserver.
@@ -87,8 +67,8 @@ class HistoryMenuBridge : public NotificationObserver,
   // to access model information when responding to actions.
   HistoryService* service();
   Profile* profile();
-  const ScopedVector<HistoryItem>* const visited_results();
-  const ScopedVector<HistoryItem>* const closed_results();
+  std::vector<HistoryItem>* visited_results();
+  std::vector<HistoryItem>* closed_results();
 
  protected:
   // Return the History menu.
@@ -100,7 +80,7 @@ class HistoryMenuBridge : public NotificationObserver,
   void ClearMenuSection(NSMenu* menu, NSInteger tag, unsigned int count);
 
   // Adds a given title and URL to HistoryMenu() with a certain tag and index.
-  void AddItemToMenu(HistoryItem* item,
+  void AddItemToMenu(const HistoryItem& item,
                      NSMenu* menu,
                      NSInteger tag,
                      NSInteger index);
@@ -122,24 +102,6 @@ class HistoryMenuBridge : public NotificationObserver,
   // to |closed_results_|. Return TRUE if the operation completed successfully.
   bool AddNavigationForTab(const TabRestoreService::Tab& entry);
 
-  // Helper function that sends an async request to the FaviconService to get
-  // an icon. The callback will update the NSMenuItem directly.
-  void GetFaviconForHistoryItem(HistoryItem* item);
-
-  // Callback for the FaviconService to return favicon image data when we
-  // request it. This decodes the raw data, updates the HistoryItem, and then
-  // sets the image on the menu. Called on the same same thread that
-  // GetFaviconForHistoryItem() was called on (UI thread).
-  void GotFaviconData(FaviconService::Handle handle,
-                      bool know_favicon,
-                      scoped_refptr<RefCountedBytes> data,
-                      bool expired,
-                      GURL url);
-
-  // Cancels a favicon load request for a given HistoryItem, if one is in
-  // progress.
-  void CancelFaviconRequest(HistoryItem* item);
-
  private:
   friend class HistoryMenuBridgeTest;
 
@@ -153,11 +115,8 @@ class HistoryMenuBridge : public NotificationObserver,
   CancelableRequestConsumer cancelable_request_consumer_;
 
   // The most recent results we've received.
-  ScopedVector<HistoryItem> visited_results_;
-  ScopedVector<HistoryItem> closed_results_;
-
-  // Maps HistoryItems to favicon request Handles.
-  CancelableRequestConsumerTSimple<HistoryItem*> favicon_consumer_;
+  std::vector<HistoryItem> visited_results_;
+  std::vector<HistoryItem> closed_results_;
 
   // We coalesce requests to re-create the menu. |create_in_progress_| is true
   // whenever we are either waiting for the history service to return query
@@ -166,9 +125,6 @@ class HistoryMenuBridge : public NotificationObserver,
   bool create_in_progress_;
   bool need_recreate_;
 
-  // The default favicon if a HistoryItem does not have one.
-  scoped_nsobject<NSImage> default_favicon_;
-
   DISALLOW_COPY_AND_ASSIGN(HistoryMenuBridge);
 };
 
diff --git a/chrome/browser/cocoa/history_menu_bridge.mm b/chrome/browser/cocoa/history_menu_bridge.mm
index 0cc64b6..30b4488 100644
--- a/chrome/browser/cocoa/history_menu_bridge.mm
+++ b/chrome/browser/cocoa/history_menu_bridge.mm
@@ -3,13 +3,9 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/cocoa/history_menu_bridge.h"
-
-#include "app/gfx/codec/png_codec.h"
-#include "app/resource_bundle.h"
 #include "base/callback.h"
-#include "base/stl_util-inl.h"
-#include "base/string_util.h"
 #include "base/sys_string_conversions.h"
+#include "base/string_util.h"
 #include "chrome/app/chrome_dll_resource.h"  // IDC_HISTORY_MENU
 #import "chrome/browser/app_controller_mac.h"
 #import "chrome/browser/cocoa/history_menu_cocoa_controller.h"
@@ -19,8 +15,6 @@
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/url_constants.h"
-#include "grit/app_resources.h"
-#include "skia/ext/skia_utils_mac.h"
 
 namespace {
 
@@ -66,9 +60,6 @@ HistoryMenuBridge::HistoryMenuBridge(Profile* profile)
       tab_restore_service_->AddObserver(this);
   }
 
-  ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-  default_favicon_.reset([rb.GetNSImageNamed(IDR_DEFAULT_FAVICON) retain]);
-
   // The service is not ready for use yet, so become notified when it does.
   if (!history_service_) {
     registrar_.Add(this,
@@ -90,15 +81,6 @@ HistoryMenuBridge::~HistoryMenuBridge() {
 
   if (tab_restore_service_)
     tab_restore_service_->RemoveObserver(this);
-
-  // Cancel any favicon requests.
-  ScopedVector<HistoryItem>::iterator it;
-  for (it = visited_results_.begin(); it != visited_results_.end(); ++it) {
-    CancelFaviconRequest(*it);
-  }
-  for (it = closed_results_.begin(); it != closed_results_.end(); ++it) {
-    CancelFaviconRequest(*it);
-  }
 }
 
 void HistoryMenuBridge::Observe(NotificationType type,
@@ -137,7 +119,7 @@ void HistoryMenuBridge::TabRestoreServiceChanged(TabRestoreService* service) {
   // what's cached, throw it away and rebuild it. It probably means
   // the browsing data was cleared by the user.
   if (entries.size() < closed_results_.size())
-    closed_results_.reset();
+    closed_results_.clear();
 
   unsigned int added_count = 0;
   for (TabRestoreService::Entries::const_iterator it = entries.begin();
@@ -166,15 +148,15 @@ void HistoryMenuBridge::TabRestoreServiceChanged(TabRestoreService* service) {
 
   // Remove extraneous/old results.
   if (closed_results_.size() > kRecentlyClosedCount)
-    STLDeleteContainerPointers(closed_results_.begin(),
-        closed_results_.end() - kRecentlyClosedCount);
+    closed_results_.erase(closed_results_.begin(),
+                          closed_results_.end() - kRecentlyClosedCount);
 
   NSInteger top_index = [menu indexOfItemWithTag:IDC_HISTORY_MENU_CLOSED] + 1;
 
   int i = 0;  // Count offsets for |tag| and |index| in AddItemToMenu().
-  for (ScopedVector<HistoryItem>::const_iterator it = closed_results_.begin();
+  for (std::vector<HistoryItem>::const_iterator it = closed_results_.begin();
       it != closed_results_.end(); ++it) {
-    HistoryItem* item = *it;
+    HistoryItem item = *it;
     NSInteger tag = IDC_HISTORY_MENU_CLOSED + 1 + i;
     AddItemToMenu(item, HistoryMenu(), tag, top_index + i);
     ++i;
@@ -194,12 +176,12 @@ Profile* HistoryMenuBridge::profile() {
   return profile_;
 }
 
-const ScopedVector<HistoryMenuBridge::HistoryItem>* const
+std::vector<HistoryMenuBridge::HistoryItem>*
     HistoryMenuBridge::visited_results() {
   return &visited_results_;
 }
 
-const ScopedVector<HistoryMenuBridge::HistoryItem>* const
+std::vector<HistoryMenuBridge::HistoryItem>*
     HistoryMenuBridge::closed_results() {
   return &closed_results_;
 }
@@ -228,14 +210,6 @@ void HistoryMenuBridge::ClearMenuSection(NSMenu* menu,
     NSMenuItem* menu_item = [menu itemAtIndex:index];
     item_tag = [menu_item tag];
     if ([menu_item action] == @selector(openHistoryMenuItem:)) {
-      // If there is a pending favicon request for this menu item, find and
-      // cancel it.
-      HistoryItem* item =
-          const_cast<HistoryItem*>([controller_ itemForTag:item_tag]);
-      if (item)
-        CancelFaviconRequest(item);
-
-      // Now remove it from the menu.
       [menu removeItemAtIndex:index];
       --count;
     }
@@ -245,12 +219,12 @@ void HistoryMenuBridge::ClearMenuSection(NSMenu* menu,
   }
 }
 
-void HistoryMenuBridge::AddItemToMenu(HistoryItem* item,
+void HistoryMenuBridge::AddItemToMenu(const HistoryItem& item,
                                       NSMenu* menu,
                                       NSInteger tag,
                                       NSInteger index) {
-  NSString* title = base::SysUTF16ToNSString(item->title);
-  std::string url_string = item->url.possibly_invalid_spec();
+  NSString* title = base::SysUTF16ToNSString(item.title);
+  std::string url_string = item.url.possibly_invalid_spec();
 
   // If we don't have a title, use the URL.
   if ([title isEqualToString:@""])
@@ -271,10 +245,6 @@ void HistoryMenuBridge::AddItemToMenu(HistoryItem* item,
   [menu_item setTarget:controller_];
   [menu_item setAction:@selector(openHistoryMenuItem:)];
   [menu_item setTag:tag];
-  if (item->icon.get())
-    [menu_item setImage:item->icon.get()];
-  else
-    [menu_item setImage:default_favicon_.get()];
 
   // Add a tooltip.
   NSString* tooltip = [NSString stringWithFormat:@"%@\n%s", full_title,
@@ -282,7 +252,6 @@ void HistoryMenuBridge::AddItemToMenu(HistoryItem* item,
   [menu_item setToolTip:tooltip];
 
   [menu insertItem:menu_item atIndex:index];
-  item->menu_item = menu_item;
 }
 
 void HistoryMenuBridge::Init() {
@@ -313,22 +282,16 @@ void HistoryMenuBridge::OnVisitedHistoryResults(
   NSInteger top_item = [menu indexOfItemWithTag:IDC_HISTORY_MENU_VISITED] + 1;
 
   ClearMenuSection(menu, IDC_HISTORY_MENU_VISITED, visited_results_.size());
-  visited_results_.reset();
+  visited_results_.clear();
 
   size_t count = results->size();
   for (size_t i = 0; i < count; ++i) {
     PageUsageData* history_item = (*results)[i];
 
-    HistoryItem* item = new HistoryItem();
-    item->title = history_item->GetTitle();
-    item->url = history_item->GetURL();
-    if (history_item->HasFavIcon()) {
-      const SkBitmap* icon = history_item->GetFavIcon();
-      item->icon.reset([gfx::SkBitmapToNSImage(*icon) retain]);
-    } else {
-      GetFaviconForHistoryItem(item);
-    }
-    visited_results_.push_back(item);  // ScopedVector takes ownership.
+    HistoryItem item;
+    item.title = history_item->GetTitle();
+    item.url = history_item->GetURL();
+    visited_results_.push_back(item);
 
     // Use the large gaps in tags assignment to create the tag for history menu
     // items.
@@ -353,62 +316,9 @@ bool HistoryMenuBridge::AddNavigationForTab(
   if (current_navigation.url() == GURL(chrome::kChromeUINewTabURL))
     return false;
 
-  HistoryItem* item = new HistoryItem();
-  item->title = current_navigation.title();
-  item->url = current_navigation.url();
-  closed_results_.push_back(item);  // ScopedVector takes ownership.
-
-  // Tab navigations don't come with icons, so we always have to request them.
-  GetFaviconForHistoryItem(item);
-
+  HistoryItem item;
+  item.title = current_navigation.title();
+  item.url = current_navigation.url();
+  closed_results_.push_back(item);
   return true;
 }
-
-void HistoryMenuBridge::GetFaviconForHistoryItem(HistoryItem* item) {
-  FaviconService* service =
-      profile_->GetFaviconService(Profile::EXPLICIT_ACCESS);
-  FaviconService::Handle handle = service->GetFaviconForURL(item->url,
-      &favicon_consumer_,
-      NewCallback(this, &HistoryMenuBridge::GotFaviconData));
-  favicon_consumer_.SetClientData(service, handle, item);
-  item->icon_handle = handle;
-  item->icon_requested = true;
-}
-
-void HistoryMenuBridge::GotFaviconData(FaviconService::Handle handle,
-                                       bool know_favicon,
-                                       scoped_refptr<RefCountedBytes> data,
-                                       bool expired,
-                                       GURL url) {
-  // Since we're going to do Cocoa-y things, make sure this is the main thread.
-  DCHECK([NSThread isMainThread]);
-
-  HistoryItem* item =
-      favicon_consumer_.GetClientData(
-          profile_->GetFaviconService(Profile::EXPLICIT_ACCESS), handle);
-  item->icon_requested = false;
-  item->icon_handle = NULL;
-
-  // Convert the raw data to Skia and then to a NSImage.
-  // TODO(rsesek): Is there an easier way to do this?
-  SkBitmap icon;
-  if (know_favicon && data.get() && data->size() &&
-      gfx::PNGCodec::Decode(data->front(), data->size(), &icon)) {
-    NSImage* image = gfx::SkBitmapToNSImage(icon);
-    if (image) {
-      // The conversion was successful.
-      item->icon.reset([image retain]);
-      [item->menu_item setImage:item->icon.get()];
-    }
-  }
-}
-
-void HistoryMenuBridge::CancelFaviconRequest(HistoryItem* item) {
-  if (item->icon_requested) {
-    FaviconService* service =
-        profile_->GetFaviconService(Profile::EXPLICIT_ACCESS);
-    service->CancelRequest(item->icon_handle);
-    item->icon_requested = false;
-    item->icon_handle = NULL;
-  }
-}
diff --git a/chrome/browser/cocoa/history_menu_bridge_unittest.mm b/chrome/browser/cocoa/history_menu_bridge_unittest.mm
index 84efb4f..a0e45e6 100644
--- a/chrome/browser/cocoa/history_menu_bridge_unittest.mm
+++ b/chrome/browser/cocoa/history_menu_bridge_unittest.mm
@@ -1,29 +1,19 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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 <Cocoa/Cocoa.h>
-#include <vector>
-
-#include "app/gfx/codec/png_codec.h"
-#include "base/ref_counted_memory.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser.h"
-#include "chrome/browser/cancelable_request.h"
-#include "chrome/browser/cocoa/cocoa_test_helper.h"
 #include "chrome/browser/cocoa/history_menu_bridge.h"
 #include "chrome/browser/cocoa/browser_test_helper.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
 
-class HistoryMenuBridgeTest : public CocoaTest {
+class HistoryMenuBridgeTest : public PlatformTest {
  public:
 
-  virtual void SetUp() {
-    CocoaTest::SetUp();
-    browser_test_helper_.profile()->CreateFaviconService();
-  }
-
   // We are a friend of HistoryMenuBridge (and have access to
   // protected methods), but none of the classes generated by TEST_F()
   // are. Wraps common commands.
@@ -35,7 +25,7 @@ class HistoryMenuBridgeTest : public CocoaTest {
   }
 
   void AddItemToBridgeMenu(HistoryMenuBridge* bridge,
-                           HistoryMenuBridge::HistoryItem* item,
+                           HistoryMenuBridge::HistoryItem& item,
                            NSMenu* menu,
                            NSInteger tag,
                            NSInteger index) {
@@ -55,65 +45,46 @@ class HistoryMenuBridgeTest : public CocoaTest {
     return item;
   }
 
-  HistoryMenuBridge::HistoryItem* CreateItem(const string16& title) {
-    HistoryMenuBridge::HistoryItem* item =
-        new HistoryMenuBridge::HistoryItem();
-    item->title = title;
-    item->url = GURL(title);
+  HistoryMenuBridge::HistoryItem CreateItem(const string16& title) {
+    HistoryMenuBridge::HistoryItem item;
+    item.title = title;
+    item.url = GURL("http://google.com");
     return item;
   }
 
-  void GetFaviconForHistoryItem(HistoryMenuBridge* bridge,
-                                HistoryMenuBridge::HistoryItem* item) {
-    bridge->GetFaviconForHistoryItem(item);
-  }
-
-  void GotFaviconData(HistoryMenuBridge* bridge,
-                      FaviconService::Handle handle,
-                      bool know_favicon,
-                      scoped_refptr<RefCountedBytes> data,
-                      bool expired,
-                      GURL url) {
-    bridge->GotFaviconData(handle, know_favicon, data, expired, url);
-  }
-
-  CancelableRequestConsumerTSimple<HistoryMenuBridge::HistoryItem*>&
-      favicon_consumer(HistoryMenuBridge* bridge) {
-    return bridge->favicon_consumer_;
-  }
-
   BrowserTestHelper browser_test_helper_;
 };
 
 // Edge case test for clearing until the end of a menu.
-TEST_F(HistoryMenuBridgeTest, ClearHistoryMenuUntilEnd) {
+TEST_F(HistoryMenuBridgeTest, TestClearHistoryMenuUntilEnd) {
   scoped_ptr<HistoryMenuBridge> bridge(
       new HistoryMenuBridge(browser_test_helper_.profile()));
   EXPECT_TRUE(bridge.get());
 
   NSMenu* menu = [[[NSMenu alloc] initWithTitle:@"history foo"] autorelease];
-  NSInteger tag = IDC_HISTORY_MENU_VISITED;
-  AddItemToMenu(menu, @"HEADER", NULL, tag);
+  NSInteger section_tag = 9990;
+  AddItemToMenu(menu, @"HEADER", NULL, section_tag);
+  NSInteger tag = section_tag;
 
   AddItemToMenu(menu, @"alpha", @selector(openHistoryMenuItem:), ++tag);
   AddItemToMenu(menu, @"bravo", @selector(openHistoryMenuItem:), ++tag);
   AddItemToMenu(menu, @"charlie", @selector(openHistoryMenuItem:), ++tag);
   AddItemToMenu(menu, @"delta", @selector(openHistoryMenuItem:), ++tag);
 
-  ClearMenuSection(bridge.get(), menu, IDC_HISTORY_MENU_VISITED, 4);
+  ClearMenuSection(bridge.get(), menu, section_tag, 4);
 
   EXPECT_EQ(1, [menu numberOfItems]);
-  EXPECT_EQ(@"HEADER", [[menu itemWithTag:IDC_HISTORY_MENU_VISITED] title]);
+  EXPECT_EQ(@"HEADER", [[menu itemWithTag:section_tag] title]);
 }
 
 // Skip menu items that are not hooked up to |-openHistoryMenuItem:|.
-TEST_F(HistoryMenuBridgeTest, ClearHistoryMenuSkipping) {
+TEST_F(HistoryMenuBridgeTest, TestClearHistoryMenuSkipping) {
   scoped_ptr<HistoryMenuBridge> bridge(
       new HistoryMenuBridge(browser_test_helper_.profile()));
   EXPECT_TRUE(bridge.get());
 
   NSMenu* menu = [[[NSMenu alloc] initWithTitle:@"history foo"] autorelease];
-  NSInteger section_tag = IDC_HISTORY_MENU_VISITED;
+  NSInteger section_tag = 9990;
   AddItemToMenu(menu, @"HEADER", NULL, section_tag);
   NSInteger tag = section_tag;
 
@@ -130,39 +101,44 @@ TEST_F(HistoryMenuBridgeTest, ClearHistoryMenuSkipping) {
 }
 
 // Edge case test for clearing an empty menu.
-TEST_F(HistoryMenuBridgeTest, ClearHistoryMenuEmpty) {
+TEST_F(HistoryMenuBridgeTest, TestClearHistoryMenuEmpty) {
   scoped_ptr<HistoryMenuBridge> bridge(
       new HistoryMenuBridge(browser_test_helper_.profile()));
   EXPECT_TRUE(bridge.get());
 
   NSMenu* menu = [[[NSMenu alloc] initWithTitle:@"history foo"] autorelease];
-  AddItemToMenu(menu, @"HEADER", NULL, IDC_HISTORY_MENU_VISITED);
+  NSInteger section_tag = 9990;
+  AddItemToMenu(menu, @"HEADER", NULL, section_tag);
 
-  ClearMenuSection(bridge.get(), menu, IDC_HISTORY_MENU_VISITED, 1);
+  ClearMenuSection(bridge.get(), menu, section_tag, 1);
 
   EXPECT_EQ(1, [menu numberOfItems]);
-  EXPECT_EQ(@"HEADER", [[menu itemWithTag:IDC_HISTORY_MENU_VISITED] title]);
+  EXPECT_EQ(@"HEADER", [[menu itemWithTag:section_tag] title]);
 }
 
 // Test that AddItemToMenu() properly adds HistoryItem objects as menus.
-TEST_F(HistoryMenuBridgeTest, AddItemToMenu) {
+TEST_F(HistoryMenuBridgeTest, TestAddItemToMenu) {
   scoped_ptr<HistoryMenuBridge> bridge(
       new HistoryMenuBridge(browser_test_helper_.profile()));
   EXPECT_TRUE(bridge.get());
 
   NSMenu* menu = [[[NSMenu alloc] initWithTitle:@"history foo"] autorelease];
 
-  const string16 short_url = ASCIIToUTF16("http://foo/");
-  const string16 long_url = ASCIIToUTF16("http://super-duper-long-url--."
-      "that.cannot.possibly.fit.even-in-80-columns"
-      "or.be.reasonably-displayed-in-a-menu"
-      "without.looking-ridiculous.com/"); // 140 chars total
+  string16 short_url = ASCIIToUTF16("http://foo/");
+  string16 long_url = ASCIIToUTF16("http://super-duper-long-url--."
+    "that.cannot.possibly.fit.even-in-80-columns"
+    "or.be.reasonably-displayed-in-a-menu"
+    "without.looking-ridiculous.com/"); // 140 chars total
 
-  scoped_ptr<HistoryMenuBridge::HistoryItem> item1(CreateItem(short_url));
-  AddItemToBridgeMenu(bridge.get(), item1.get(), menu, 100, 0);
+  HistoryMenuBridge::HistoryItem item1;
+  item1.title = short_url;
+  item1.url = GURL(short_url);
+  AddItemToBridgeMenu(bridge.get(), item1, menu, 100, 0);
 
-  scoped_ptr<HistoryMenuBridge::HistoryItem> item2(CreateItem(long_url));
-  AddItemToBridgeMenu(bridge.get(), item2.get(), menu, 101, 1);
+  HistoryMenuBridge::HistoryItem item2;
+  item2.title = long_url;
+  item2.url = GURL(long_url);
+  AddItemToBridgeMenu(bridge.get(), item2, menu, 101, 1);
 
   EXPECT_EQ(2, [menu numberOfItems]);
 
@@ -186,59 +162,3 @@ TEST_F(HistoryMenuBridgeTest, AddItemToMenu) {
   EXPECT_GE([[[menu itemAtIndex:0] toolTip] length], (2*short_url.length()-5));
   EXPECT_GE([[[menu itemAtIndex:1] toolTip] length], (2*long_url.length()-5));
 }
-
-// Tests that we properly request an icon from the FaviconService.
-TEST_F(HistoryMenuBridgeTest, GetFaviconForHistoryItem) {
-  TestingProfile* profile = browser_test_helper_.profile();
-  HistoryMenuBridge bridge(profile);
-
-  // Create a fake item.
-  HistoryMenuBridge::HistoryItem item;
-  item.title = ASCIIToUTF16("Title");
-  item.url = GURL("http://google.com");
-
-  // Request the icon.
-  GetFaviconForHistoryItem(&bridge, &item);
-
-  // Make sure that there is ClientData for the request.
-  std::vector<HistoryMenuBridge::HistoryItem*> data;
-  favicon_consumer(&bridge).GetAllClientData(&data);
-  ASSERT_EQ(data.size(), 1U);
-  EXPECT_EQ(&item, data[0]);
-
-  // Make sure the item was modified properly.
-  EXPECT_TRUE(item.icon_requested);
-  EXPECT_GT(item.icon_handle, 0);
-}
-
-TEST_F(HistoryMenuBridgeTest, GotFaviconData) {
-  TestingProfile* profile = browser_test_helper_.profile();
-  HistoryMenuBridge bridge(profile);
-
-  // Create a dummy bitmap.
-  SkBitmap bitmap;
-  bitmap.setConfig(SkBitmap::kARGB_8888_Config, 25, 25);
-  bitmap.allocPixels();
-  bitmap.eraseRGB(255, 0, 0);
-
-  // Convert it to raw PNG bytes. We totally ignore color order here because
-  // we just want to test the roundtrip through the Bridge, not that we can
-  // make icons look pretty.
-  std::vector<unsigned char> raw;
-  gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, true, &raw);
-  scoped_refptr<RefCountedBytes> bytes(new RefCountedBytes(raw));
-
-  // Set up the HistoryItem.
-  HistoryMenuBridge::HistoryItem item;
-  scoped_nsobject<NSMenuItem> menu_item([[NSMenuItem alloc] init]);
-  item.menu_item = menu_item.get();
-  GetFaviconForHistoryItem(&bridge, &item);
-
-  // Pretend to be called back.
-  GotFaviconData(&bridge, item.icon_handle, true, bytes, false, GURL());
-
-  // Make sure the callback works.
-  EXPECT_EQ(false, item.icon_requested);
-  EXPECT_TRUE(item.icon.get());
-  EXPECT_TRUE([item.menu_item image]);
-}
diff --git a/chrome/browser/cocoa/history_menu_cocoa_controller.h b/chrome/browser/cocoa/history_menu_cocoa_controller.h
index beb5576..32a7939 100644
--- a/chrome/browser/cocoa/history_menu_cocoa_controller.h
+++ b/chrome/browser/cocoa/history_menu_cocoa_controller.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -25,8 +25,8 @@
 @end  // HistoryMenuCocoaController
 
 @interface HistoryMenuCocoaController (ExposedForUnitTests)
-- (const HistoryMenuBridge::HistoryItem*)itemForTag:(NSInteger)tag;
-- (void)openURLForItem:(const HistoryMenuBridge::HistoryItem*)node;
+- (HistoryMenuBridge::HistoryItem)itemForTag:(NSInteger)tag;
+- (void)openURLForItem:(HistoryMenuBridge::HistoryItem&)node;
 @end  // HistoryMenuCocoaController (ExposedForUnitTests)
 
 #endif  // CHROME_BROWSER_COCOA_HISTORY_MENU_COCOA_CONTROLLER_H_
diff --git a/chrome/browser/cocoa/history_menu_cocoa_controller.mm b/chrome/browser/cocoa/history_menu_cocoa_controller.mm
index a0672b3..89dd2f8 100644
--- a/chrome/browser/cocoa/history_menu_cocoa_controller.mm
+++ b/chrome/browser/cocoa/history_menu_cocoa_controller.mm
@@ -1,8 +1,7 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
-#include "base/scoped_vector.h"
 #include "chrome/app/chrome_dll_resource.h"  // IDC_HISTORY_MENU
 #include "chrome/browser/browser.h"
 #import "chrome/browser/cocoa/history_menu_cocoa_controller.h"
@@ -25,16 +24,16 @@
 }
 
 // Open the URL of the given history item in the current tab.
-- (void)openURLForItem:(const HistoryMenuBridge::HistoryItem*)node {
+- (void)openURLForItem:(HistoryMenuBridge::HistoryItem&)node {
   Browser* browser = Browser::GetOrCreateTabbedBrowser(bridge_->profile());
   WindowOpenDisposition disposition =
       event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]);
-  browser->OpenURL(node->url, GURL(), disposition,
+  browser->OpenURL(node.url, GURL(), disposition,
                    PageTransition::AUTO_BOOKMARK);
 }
 
-- (const HistoryMenuBridge::HistoryItem*)itemForTag:(NSInteger)tag {
-  const ScopedVector<HistoryMenuBridge::HistoryItem>* results = NULL;
+- (HistoryMenuBridge::HistoryItem)itemForTag:(NSInteger)tag {
+  std::vector<HistoryMenuBridge::HistoryItem>* results = NULL;
   NSInteger tag_base = 0;
   if (tag > IDC_HISTORY_MENU_VISITED && tag < IDC_HISTORY_MENU_CLOSED) {
     results = bridge_->visited_results();
@@ -49,14 +48,14 @@
   DCHECK(tag > tag_base);
   size_t index = tag - tag_base - 1;
   if (index >= results->size())
-    return NULL;
+    return HistoryMenuBridge::HistoryItem();
   return (*results)[index];
 }
 
 - (IBAction)openHistoryMenuItem:(id)sender {
   NSInteger tag = [sender tag];
-  const HistoryMenuBridge::HistoryItem* item = [self itemForTag:tag];
-  DCHECK(item->url.is_valid());
+  HistoryMenuBridge::HistoryItem item = [self itemForTag:tag];
+  DCHECK(item.url.is_valid());
   [self openURLForItem:item];
 }
 
diff --git a/chrome/browser/cocoa/history_menu_cocoa_controller_unittest.mm b/chrome/browser/cocoa/history_menu_cocoa_controller_unittest.mm
index 6bd64f5..112f798 100644
--- a/chrome/browser/cocoa/history_menu_cocoa_controller_unittest.mm
+++ b/chrome/browser/cocoa/history_menu_cocoa_controller_unittest.mm
@@ -1,8 +1,7 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
-#include "base/scoped_vector.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser.h"
@@ -13,7 +12,6 @@
 @interface FakeHistoryMenuController : HistoryMenuCocoaController {
  @public
   BOOL opened_[2];
-  ScopedVector<HistoryMenuBridge::HistoryItem> items_;
 }
 @end
 
@@ -27,24 +25,22 @@
   return self;
 }
 
-- (HistoryMenuBridge::HistoryItem*)itemForTag:(NSInteger)tag {
-  HistoryMenuBridge::HistoryItem* item = new HistoryMenuBridge::HistoryItem();
+- (HistoryMenuBridge::HistoryItem)itemForTag:(NSInteger)tag {
+  HistoryMenuBridge::HistoryItem item;
   if (tag == 0) {
-    item->title = ASCIIToUTF16("uno");
-    item->url = GURL("http://google.com");
+    item.title = ASCIIToUTF16("uno");
+    item.url = GURL("http://google.com");
   } else if (tag == 1) {
-    item->title = ASCIIToUTF16("duo");
-    item->url = GURL("http://apple.com");
+    item.title = ASCIIToUTF16("duo");
+    item.url = GURL("http://apple.com");
   } else {
     NOTREACHED();
   }
-  // We push the item into a scoped vector that will delete it on destruction.
-  items_.push_back(item);
   return item;
 }
 
-- (void)openURLForItem:(HistoryMenuBridge::HistoryItem*)item {
-  std::string url = item->url.possibly_invalid_spec();
+- (void)openURLForItem:(HistoryMenuBridge::HistoryItem&)item {
+  std::string url = item.url.possibly_invalid_spec();
   if (url.find("http://google.com") != std::string::npos)
     opened_[0] = YES;
   if (url.find("http://apple.com") != std::string::npos)
@@ -53,8 +49,8 @@
 
 @end  // FakeHistoryMenuController
 
-TEST(HistoryMenuCocoaControllerTest, OpenURLForItem) {
-  FakeHistoryMenuController* c = [[FakeHistoryMenuController alloc] init];
+TEST(HistoryMenuCocoaControllerTest, TestOpenItem) {
+  FakeHistoryMenuController *c = [[FakeHistoryMenuController alloc] init];
   NSMenuItem* item = [[[NSMenuItem alloc] init] autorelease];
   for (int i = 0; i < 2; ++i) {
     [item setTag:i];