Move favicon from TabContents to TabContentsWrapper.

BUG=71097
TEST=no visible change

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

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

5 files changed, 32 insertions, 127 deletions

diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index d1866ab..fc6d519 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -28,7 +28,6 @@
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_request_limiter.h"
 #include "chrome/browser/external_protocol_handler.h"
-#include "chrome/browser/favicon_service.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/history/history_types.h"
@@ -377,7 +376,6 @@ TabContents::~TabContents() {
 }
 
 void TabContents::AddObservers() {
-  favicon_helper_.reset(new FaviconHelper(this));
   desktop_notification_handler_.reset(
       new DesktopNotificationHandlerForTC(this, GetRenderProcessHost()));
   plugin_observer_.reset(new PluginObserver(this));
@@ -521,42 +519,6 @@ bool TabContents::ShouldDisplayURL() {
   return true;
 }
 
-SkBitmap TabContents::GetFavicon() const {
-  // Like GetTitle(), we also want to use the favicon for the last committed
-  // entry rather than a pending navigation entry.
-  NavigationEntry* entry = controller_.GetTransientEntry();
-  if (entry)
-    return entry->favicon().bitmap();
-
-  entry = controller_.GetLastCommittedEntry();
-  if (entry)
-    return entry->favicon().bitmap();
-  return SkBitmap();
-}
-
-bool TabContents::FaviconIsValid() const {
-  NavigationEntry* entry = controller_.GetTransientEntry();
-  if (entry)
-    return entry->favicon().is_valid();
-
-  entry = controller_.GetLastCommittedEntry();
-  if (entry)
-    return entry->favicon().is_valid();
-
-  return false;
-}
-
-bool TabContents::ShouldDisplayFavicon() {
-  // Always display a throbber during pending loads.
-  if (controller_.GetLastCommittedEntry() && controller_.pending_entry())
-    return true;
-
-  WebUI* web_ui = GetWebUIForCurrentState();
-  if (web_ui)
-    return !web_ui->hide_favicon();
-  return true;
-}
-
 void TabContents::AddObserver(TabContentsObserver* observer) {
   observers_.AddObserver(observer);
 }
@@ -708,15 +670,8 @@ bool TabContents::NavigateToEntry(
   }
 
   // Notify observers about navigation.
-  FOR_EACH_OBSERVER(TabContentsObserver, observers_, NavigateToPendingEntry());
-
-  if (reload_type != NavigationController::NO_RELOAD &&
-      !profile()->IsOffTheRecord()) {
-    FaviconService* favicon_service =
-        profile()->GetFaviconService(Profile::IMPLICIT_ACCESS);
-    if (favicon_service)
-      favicon_service->SetFaviconOutOfDateForPage(entry.url());
-  }
+  FOR_EACH_OBSERVER(TabContentsObserver, observers_,
+                    NavigateToPendingEntry(entry.url(), reload_type));
 
   return true;
 }
@@ -750,34 +705,6 @@ void TabContents::ShowPageInfo(const GURL& url,
   delegate_->ShowPageInfo(profile(), url, ssl, show_history);
 }
 
-void TabContents::SaveFavicon() {
-  NavigationEntry* entry = controller_.GetActiveEntry();
-  if (!entry || entry->url().is_empty())
-    return;
-
-  // Make sure the page is in history, otherwise adding the favicon does
-  // nothing.
-  HistoryService* history = profile()->GetOriginalProfile()->GetHistoryService(
-      Profile::IMPLICIT_ACCESS);
-  if (!history)
-    return;
-  history->AddPageNoVisitForBookmark(entry->url());
-
-  FaviconService* service = profile()->GetOriginalProfile()->GetFaviconService(
-      Profile::IMPLICIT_ACCESS);
-  if (!service)
-    return;
-  const NavigationEntry::FaviconStatus& favicon(entry->favicon());
-  if (!favicon.is_valid() || favicon.url().is_empty() ||
-      favicon.bitmap().empty()) {
-    return;
-  }
-  std::vector<unsigned char> image_data;
-  gfx::PNGCodec::EncodeBGRASkBitmap(favicon.bitmap(), false, &image_data);
-  service->SetFavicon(
-      entry->url(), favicon.url(), image_data, history::FAVICON);
-}
-
 ConstrainedWindow* TabContents::CreateConstrainedDialog(
       ConstrainedWindowDelegate* delegate) {
   ConstrainedWindow* window =
@@ -1565,9 +1492,6 @@ void TabContents::DidNavigateMainFramePostCommit(
   // Allow the new page to set the title again.
   received_page_title_ = false;
 
-  // Get the favicon, either from history or request it from the net.
-  favicon_helper_->FetchFavicon(details.entry->url());
-
   // Clear all page actions, blocked content notifications and browser actions
   // for this tab, unless this is an in-page navigation.
   if (!details.is_in_page) {
diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h
index 356d2b5..1ef7c86 100644
--- a/content/browser/tab_contents/tab_contents.h
+++ b/content/browser/tab_contents/tab_contents.h
@@ -16,7 +16,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/string16.h"
 #include "chrome/browser/download/save_package.h"
-#include "chrome/browser/favicon_helper.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/tab_contents/tab_specific_content_settings.h"
 #include "chrome/browser/ui/app_modal_dialogs/js_modal_dialog.h"
@@ -173,11 +172,6 @@ class TabContents : public PageNavigator,
     return view_.get();
   }
 
-  // Returns the FaviconHelper of this TabContents.
-  FaviconHelper& favicon_helper() {
-    return *favicon_helper_.get();
-  }
-
   // Tab navigation state ------------------------------------------------------
 
   // Returns the current navigation properties, which if a navigation is
@@ -206,18 +200,6 @@ class TabContents : public PageNavigator,
   // the user is invited to type into it.
   virtual bool ShouldDisplayURL();
 
-  // Returns the favicon for this tab, or an isNull() bitmap if the tab does not
-  // have a favicon. The default implementation uses the current navigation
-  // entry.
-  SkBitmap GetFavicon() const;
-
-  // Returns true if we are not using the default favicon.
-  bool FaviconIsValid() const;
-
-  // Returns whether the favicon should be displayed. If this returns false, no
-  // space is provided for the favicon, and the favicon is never displayed.
-  virtual bool ShouldDisplayFavicon();
-
   // Return whether this tab contents is loading a resource.
   bool is_loading() const { return is_loading_; }
 
@@ -348,9 +330,6 @@ class TabContents : public PageNavigator,
                     const NavigationEntry::SSLStatus& ssl,
                     bool show_history);
 
-  // Saves the favicon for the current page.
-  void SaveFavicon();
-
   // Window management ---------------------------------------------------------
 
   // Create a new window constrained to this TabContents' clip and visibility.
@@ -643,6 +622,10 @@ class TabContents : public PageNavigator,
   // Query the WebUIFactory for the TypeID for the current URL.
   WebUI::TypeID GetWebUITypeForCurrentState();
 
+  // Returns the WebUI for the current state of the tab. This will either be
+  // the pending WebUI, the committed WebUI, or NULL.
+  WebUI* GetWebUIForCurrentState();
+
  protected:
   friend class TabContentsObserver;
   friend class TabContentsObserver::Registrar;
@@ -752,10 +735,6 @@ class TabContents : public PageNavigator,
   void ExpireInfoBars(
       const NavigationController::LoadCommittedDetails& details);
 
-  // Returns the WebUI for the current state of the tab. This will either be
-  // the pending WebUI, the committed WebUI, or NULL.
-  WebUI* GetWebUIForCurrentState();
-
   // Navigation helpers --------------------------------------------------------
   //
   // These functions are helpers for Navigate() and DidNavigate().
@@ -981,9 +960,6 @@ class TabContents : public PageNavigator,
   // Handles drag and drop event forwarding to extensions.
   BookmarkDrag* bookmark_drag_;
 
-  // Handles downloading favicons.
-  scoped_ptr<FaviconHelper> favicon_helper_;
-
   // Cached web app icon.
   SkBitmap app_icon_;
 
@@ -1072,10 +1048,6 @@ class TabContents : public PageNavigator,
   // once.
   bool notify_disconnection_;
 
-  // Maps from handle to page_id.
-  typedef std::map<FaviconService::Handle, int32> HistoryRequestMap;
-  HistoryRequestMap history_requests_;
-
 #if defined(OS_WIN)
   // Handle to an event that's set when the page is showing a message box (or
   // equivalent constrained window).  Plugin processes check this to know if
diff --git a/content/browser/tab_contents/tab_contents_observer.cc b/content/browser/tab_contents/tab_contents_observer.cc
index d2eae1c..a391402 100644
--- a/content/browser/tab_contents/tab_contents_observer.cc
+++ b/content/browser/tab_contents/tab_contents_observer.cc
@@ -25,7 +25,9 @@ void TabContentsObserver::Registrar::Observe(TabContents* tab) {
     tab_->AddObserver(observer_);
 }
 
-void TabContentsObserver::NavigateToPendingEntry() {
+void TabContentsObserver::NavigateToPendingEntry(
+    const GURL& url,
+    NavigationController::ReloadType reload_type) {
 }
 
 void TabContentsObserver::DidNavigateMainFramePostCommit(
diff --git a/content/browser/tab_contents/tab_contents_observer.h b/content/browser/tab_contents/tab_contents_observer.h
index 69d371a..ad65247 100644
--- a/content/browser/tab_contents/tab_contents_observer.h
+++ b/content/browser/tab_contents/tab_contents_observer.h
@@ -15,7 +15,7 @@ struct ViewHostMsg_FrameNavigate_Params;
 class TabContentsObserver : public IPC::Channel::Listener,
                             public IPC::Message::Sender {
  public:
-  // Use this as a member variable in a class that uses the emptry constructor
+  // Use this as a member variable in a class that uses the empty constructor
   // version of this interface.
   class Registrar {
    public:
@@ -35,7 +35,9 @@ class TabContentsObserver : public IPC::Channel::Listener,
     DISALLOW_COPY_AND_ASSIGN(Registrar);
   };
 
-  virtual void NavigateToPendingEntry();
+  virtual void NavigateToPendingEntry(
+      const GURL& url,
+      NavigationController::ReloadType reload_type);
 
   virtual void DidNavigateMainFramePostCommit(
       const NavigationController::LoadCommittedDetails& details,
diff --git a/content/browser/webui/web_ui_unittest.cc b/content/browser/webui/web_ui_unittest.cc
index b81f3a8..e96642e 100644
--- a/content/browser/webui/web_ui_unittest.cc
+++ b/content/browser/webui/web_ui_unittest.cc
@@ -2,17 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/favicon_tab_helper.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/browser/ui/tab_contents/test_tab_contents_wrapper.h"
 #include "chrome/browser/ui/webui/new_tab_ui.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/testing_profile.h"
 #include "content/browser/browser_thread.h"
-#include "content/browser/renderer_host/test_render_view_host.h"
 #include "content/browser/site_instance.h"
 #include "content/browser/tab_contents/navigation_controller.h"
 #include "content/browser/tab_contents/test_tab_contents.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class WebUITest : public RenderViewHostTestHarness {
+class WebUITest : public TabContentsWrapperTestHarness {
  public:
   WebUITest() : ui_thread_(BrowserThread::UI, MessageLoop::current()) {}
 
@@ -20,7 +22,8 @@ class WebUITest : public RenderViewHostTestHarness {
   // state, through pending, committed, then another navigation. The first page
   // ID that we should use is passed as a parameter. We'll use the next two
   // values. This must be increasing for the life of the tests.
-  static void DoNavigationTest(TabContents* contents, int page_id) {
+  static void DoNavigationTest(TabContentsWrapper* wrapper, int page_id) {
+    TabContents* contents = wrapper->tab_contents();
     NavigationController* controller = &contents->controller();
 
     // Start a pending load.
@@ -33,7 +36,7 @@ class WebUITest : public RenderViewHostTestHarness {
 
     // Check the things the pending Web UI should have set.
     EXPECT_FALSE(contents->ShouldDisplayURL());
-    EXPECT_FALSE(contents->ShouldDisplayFavicon());
+    EXPECT_FALSE(wrapper->favicon_tab_helper()->ShouldDisplayFavicon());
     EXPECT_TRUE(contents->ShouldShowBookmarkBar());
     EXPECT_TRUE(contents->FocusLocationBarByDefault());
 
@@ -43,7 +46,7 @@ class WebUITest : public RenderViewHostTestHarness {
 
     // The same flags should be set as before now that the load has committed.
     EXPECT_FALSE(contents->ShouldDisplayURL());
-    EXPECT_FALSE(contents->ShouldDisplayFavicon());
+    EXPECT_FALSE(wrapper->favicon_tab_helper()->ShouldDisplayFavicon());
     EXPECT_TRUE(contents->ShouldShowBookmarkBar());
     EXPECT_TRUE(contents->FocusLocationBarByDefault());
 
@@ -54,7 +57,7 @@ class WebUITest : public RenderViewHostTestHarness {
     // Check the flags. Some should reflect the new page (URL, title), some
     // should reflect the old one (bookmark bar) until it has committed.
     EXPECT_TRUE(contents->ShouldDisplayURL());
-    EXPECT_TRUE(contents->ShouldDisplayFavicon());
+    EXPECT_TRUE(wrapper->favicon_tab_helper()->ShouldDisplayFavicon());
     EXPECT_TRUE(contents->ShouldShowBookmarkBar());
     EXPECT_FALSE(contents->FocusLocationBarByDefault());
 
@@ -74,7 +77,7 @@ class WebUITest : public RenderViewHostTestHarness {
 
     // The state should now reflect a regular page.
     EXPECT_TRUE(contents->ShouldDisplayURL());
-    EXPECT_TRUE(contents->ShouldDisplayFavicon());
+    EXPECT_TRUE(wrapper->favicon_tab_helper()->ShouldDisplayFavicon());
     EXPECT_FALSE(contents->ShouldShowBookmarkBar());
     EXPECT_FALSE(contents->FocusLocationBarByDefault());
   }
@@ -89,15 +92,16 @@ class WebUITest : public RenderViewHostTestHarness {
 // TabContents when we first navigate to a Web UI page, then to a standard
 // non-DOM-UI page.
 TEST_F(WebUITest, WebUIToStandard) {
-  DoNavigationTest(contents(), 1);
+  DoNavigationTest(contents_wrapper(), 1);
 
   // Test the case where we're not doing the initial navigation. This is
   // slightly different than the very-first-navigation case since the
   // SiteInstance will be the same (the original TabContents must still be
   // alive), which will trigger different behavior in RenderViewHostManager.
-  TestTabContents contents2(profile_.get(), NULL);
+  TestTabContents* contents2 = new TestTabContents(profile_.get(), NULL);
+  TabContentsWrapper wrapper2(contents2);
 
-  DoNavigationTest(&contents2, 101);
+  DoNavigationTest(&wrapper2, 101);
 }
 
 TEST_F(WebUITest, WebUIToWebUI) {
@@ -112,7 +116,8 @@ TEST_F(WebUITest, WebUIToWebUI) {
 
   // The flags should be the same as the non-pending state.
   EXPECT_FALSE(contents()->ShouldDisplayURL());
-  EXPECT_FALSE(contents()->ShouldDisplayFavicon());
+  EXPECT_FALSE(
+      contents_wrapper()->favicon_tab_helper()->ShouldDisplayFavicon());
   EXPECT_TRUE(contents()->ShouldShowBookmarkBar());
   EXPECT_TRUE(contents()->FocusLocationBarByDefault());
 }
@@ -125,14 +130,14 @@ TEST_F(WebUITest, StandardToWebUI) {
 
   // The state should now reflect the default.
   EXPECT_TRUE(contents()->ShouldDisplayURL());
-  EXPECT_TRUE(contents()->ShouldDisplayFavicon());
+  EXPECT_TRUE(contents_wrapper()->favicon_tab_helper()->ShouldDisplayFavicon());
   EXPECT_FALSE(contents()->ShouldShowBookmarkBar());
   EXPECT_FALSE(contents()->FocusLocationBarByDefault());
 
   // Commit the load, the state should be the same.
   rvh()->SendNavigate(1, std_url);
   EXPECT_TRUE(contents()->ShouldDisplayURL());
-  EXPECT_TRUE(contents()->ShouldDisplayFavicon());
+  EXPECT_TRUE(contents_wrapper()->favicon_tab_helper()->ShouldDisplayFavicon());
   EXPECT_FALSE(contents()->ShouldShowBookmarkBar());
   EXPECT_FALSE(contents()->FocusLocationBarByDefault());
 
@@ -140,7 +145,7 @@ TEST_F(WebUITest, StandardToWebUI) {
   GURL new_tab_url(chrome::kChromeUINewTabURL);
   controller().LoadURL(new_tab_url, GURL(), PageTransition::LINK);
   EXPECT_FALSE(contents()->ShouldDisplayURL());
-  EXPECT_TRUE(contents()->ShouldDisplayFavicon());
+  EXPECT_TRUE(contents_wrapper()->favicon_tab_helper()->ShouldDisplayFavicon());
   EXPECT_FALSE(contents()->ShouldShowBookmarkBar());
   EXPECT_TRUE(contents()->FocusLocationBarByDefault());