Refactor FaviconHandler to move code into delegate

methods.

BUG=None
TEST=None

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

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

6 files changed, 194 insertions, 85 deletions

diff --git a/chrome/browser/favicon/favicon_handler.cc b/chrome/browser/favicon/favicon_handler.cc
index 66d2b47..835dc98 100644
--- a/chrome/browser/favicon/favicon_handler.cc
+++ b/chrome/browser/favicon/favicon_handler.cc
@@ -13,13 +13,11 @@
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/icon_messages.h"
-#include "content/browser/renderer_host/render_view_host.h"
-#include "content/browser/tab_contents/navigation_controller.h"
 #include "content/browser/tab_contents/navigation_entry.h"
-#include "content/browser/tab_contents/tab_contents_delegate.h"
-#include "content/browser/tab_contents/tab_contents.h"
 #include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_util.h"
 
 namespace {
 
@@ -65,23 +63,26 @@ FaviconHandler::DownloadRequest::DownloadRequest(
       icon_type(icon_type) {
 }
 
-FaviconHandler::FaviconHandler(TabContents* tab_contents, Type icon_type)
+FaviconHandler::FaviconHandler(Profile* profile,
+                               FaviconHandlerDelegate* delegate,
+                               Type icon_type)
     : got_favicon_from_history_(false),
       favicon_expired_(false),
       icon_types_(icon_type == FAVICON ? history::FAVICON :
           history::TOUCH_ICON | history::TOUCH_PRECOMPOSED_ICON),
       current_url_index_(0),
-      tab_contents_(tab_contents) {
+      profile_(profile),
+      delegate_(delegate) {
+  DCHECK(profile_);
+  DCHECK(delegate_);
 }
 
 FaviconHandler::~FaviconHandler() {
-  SkBitmap empty_image;
-
   // Call pending download callbacks with error to allow caller to clean up.
   for (DownloadRequests::iterator i = download_requests_.begin();
        i != download_requests_.end(); ++i) {
     if (i->second.callback) {
-      i->second.callback->Run(i->first, true, empty_image);
+      i->second.callback->Run(i->first, true, SkBitmap());
     }
   }
 }
@@ -114,49 +115,50 @@ int FaviconHandler::DownloadImage(
 }
 
 FaviconService* FaviconHandler::GetFaviconService() {
-  return tab_contents()->profile()->GetFaviconService(Profile::EXPLICIT_ACCESS);
+  return profile_->GetFaviconService(Profile::EXPLICIT_ACCESS);
 }
 
 void FaviconHandler::SetFavicon(
     const GURL& url,
     const GURL& image_url,
-    const SkBitmap& image,
+    const gfx::Image& image,
     history::IconType icon_type) {
-  const SkBitmap& sized_image = (preferred_icon_size() == 0 ||
-      (preferred_icon_size() == image.width() &&
-       preferred_icon_size() == image.height())) ?
-      image : ConvertToFaviconSize(image);
+  const SkBitmap& bitmap = image;
+  const gfx::Image& sized_image = (preferred_icon_size() == 0 ||
+      (preferred_icon_size() == bitmap.width() &&
+       preferred_icon_size() == bitmap.height())) ?
+      image : ResizeFaviconIfNeeded(image);
 
   if (GetFaviconService() && ShouldSaveFavicon(url)) {
     std::vector<unsigned char> image_data;
-    gfx::PNGCodec::EncodeBGRASkBitmap(sized_image, false, &image_data);
-    SetHistoryFavicon(url, image_url, image_data, icon_type);
+    if (gfx::PNGEncodedDataFromImage(sized_image, &image_data))
+      SetHistoryFavicon(url, image_url, image_data, icon_type);
   }
 
   if (url == url_ && icon_type == history::FAVICON) {
     NavigationEntry* entry = GetEntry();
     if (entry)
-      UpdateFavicon(entry, sized_image);
+      UpdateFavicon(entry, &sized_image);
   }
 }
 
 void FaviconHandler::UpdateFavicon(NavigationEntry* entry,
                                   scoped_refptr<RefCountedMemory> data) {
-  SkBitmap image;
-  gfx::PNGCodec::Decode(data->front(), data->size(), &image);
-  UpdateFavicon(entry, image);
+  scoped_ptr<gfx::Image> image(gfx::ImageFromPNGEncodedData(data->front(),
+                                                            data->size()));
+  UpdateFavicon(entry, image.get());
 }
 
 void FaviconHandler::UpdateFavicon(NavigationEntry* entry,
-                                  const SkBitmap& image) {
+                                   const gfx::Image* image) {
   // No matter what happens, we need to mark the favicon as being set.
   entry->favicon().set_is_valid(true);
 
-  if (image.empty())
+  if (!image)
     return;
 
-  entry->favicon().set_bitmap(image);
-  tab_contents()->NotifyNavigationStateChanged(TabContents::INVALIDATE_TAB);
+  entry->favicon().set_bitmap(*image);
+  delegate_->NotifyFaviconUpdated();
 }
 
 void FaviconHandler::OnUpdateFaviconURL(
@@ -210,9 +212,9 @@ void FaviconHandler::OnUpdateFaviconURL(
 }
 
 void FaviconHandler::OnDidDownloadFavicon(int id,
-                                         const GURL& image_url,
-                                         bool errored,
-                                         const SkBitmap& image) {
+                                          const GURL& image_url,
+                                          bool errored,
+                                          const gfx::Image& image) {
   DownloadRequests::iterator i = download_requests_.find(id);
   if (i == download_requests_.end()) {
     // Currently TabContents notifies us of ANY downloads so that it is
@@ -221,7 +223,7 @@ void FaviconHandler::OnDidDownloadFavicon(int id,
   }
 
   if (i->second.callback) {
-    i->second.callback->Run(id, errored, image);
+    i->second.callback->Run(id, errored, *(&image));
   } else if (current_candidate() &&
              DoUrlAndIconMatch(*current_candidate(), image_url,
                                i->second.icon_type)) {
@@ -240,11 +242,10 @@ void FaviconHandler::OnDidDownloadFavicon(int id,
 }
 
 NavigationEntry* FaviconHandler::GetEntry() {
-  NavigationEntry* entry = tab_contents()->controller().GetActiveEntry();
-  if (entry && entry->url() == url_ &&
-      tab_contents()->IsActiveEntry(entry->page_id())) {
+  NavigationEntry* entry = delegate_->GetActiveEntry();
+  if (entry && entry->url() == url_)
     return entry;
-  }
+
   // If the URL has changed out from under us (as will happen with redirects)
   // return NULL.
   return NULL;
@@ -257,9 +258,7 @@ int FaviconHandler::DownloadFavicon(const GURL& image_url, int image_size) {
   }
   static int next_id = 1;
   int id = next_id++;
-  RenderViewHost* host = tab_contents()->render_view_host();
-  host->Send(new IconMsg_DownloadFavicon(
-      host->routing_id(), id, image_url, image_size));
+  delegate_->StartDownload(id, image_url, image_size);
   return id;
 }
 
@@ -299,11 +298,11 @@ void FaviconHandler::SetHistoryFavicon(
 }
 
 bool FaviconHandler::ShouldSaveFavicon(const GURL& url) {
-  if (!tab_contents()->profile()->IsOffTheRecord())
+  if (!profile_->IsOffTheRecord())
     return true;
 
   // Otherwise store the favicon if the page is bookmarked.
-  BookmarkModel* bookmark_model = tab_contents()->profile()->GetBookmarkModel();
+  BookmarkModel* bookmark_model = profile_->GetBookmarkModel();
   return bookmark_model && bookmark_model->IsBookmarked(url);
 }
 
@@ -367,7 +366,7 @@ void FaviconHandler::DownloadFaviconOrAskHistory(
     // We don't know the favicon, but we may have previously downloaded the
     // favicon for another page that shares the same favicon. Ask for the
     // favicon given the favicon URL.
-    if (tab_contents()->profile()->IsOffTheRecord()) {
+    if (profile_->IsOffTheRecord()) {
       GetFavicon(icon_url, icon_type, &cancelable_consumer_,
           NewCallback(this, &FaviconHandler::OnFaviconData));
     } else {
@@ -435,14 +434,18 @@ int FaviconHandler::ScheduleDownload(
   return download_id;
 }
 
-SkBitmap FaviconHandler::ConvertToFaviconSize(const SkBitmap& image) {
-  int width = image.width();
-  int height = image.height();
+gfx::Image FaviconHandler::ResizeFaviconIfNeeded(const gfx::Image& image) {
+  // Get an SkBitmap from the gfx::Image.
+  const SkBitmap& bitmap = image;
+  int width = bitmap.width();
+  int height = bitmap.height();
   if (width > 0 && height > 0) {
     calc_favicon_target_size(&width, &height);
-    return skia::ImageOperations::Resize(
-          image, skia::ImageOperations::RESIZE_LANCZOS3,
-          width, height);
+    return gfx::Image(new SkBitmap(
+                          skia::ImageOperations::Resize(
+                              bitmap, skia::ImageOperations::RESIZE_LANCZOS3,
+                              width, height)));
   }
+
   return image;
 }
diff --git a/chrome/browser/favicon/favicon_handler.h b/chrome/browser/favicon/favicon_handler.h
index e47ace3..b2a462d 100644
--- a/chrome/browser/favicon/favicon_handler.h
+++ b/chrome/browser/favicon/favicon_handler.h
@@ -19,12 +19,17 @@
 #include "googleurl/src/gurl.h"
 #include "ui/gfx/favicon_size.h"
 
+class FaviconHandlerDelegate;
 class NavigationEntry;
 class Profile;
 class RefCountedMemory;
 class SkBitmap;
 class TabContents;
 
+namespace gfx {
+class Image;
+}
+
 // FaviconHandler works with FaviconTabHelper to fetch the specific type of
 // favicon.
 //
@@ -75,7 +80,9 @@ class FaviconHandler {
     TOUCH,
   };
 
-  FaviconHandler(TabContents* tab_contents, Type icon_type);
+  FaviconHandler(Profile* profile,
+                 FaviconHandlerDelegate* delegate,
+                 Type icon_type);
   virtual ~FaviconHandler();
 
   // Initiates loading the favicon for the specified url.
@@ -101,7 +108,7 @@ class FaviconHandler {
   void OnDidDownloadFavicon(int id,
                             const GURL& image_url,
                             bool errored,
-                            const SkBitmap& image);
+                            const gfx::Image& image);
 
  protected:
   // These virtual methods make FaviconHandler testable and are overridden by
@@ -188,7 +195,7 @@ class FaviconHandler {
   // we request the TabContents to download the favicon.
   void SetFavicon(const GURL& url,
                   const GURL& icon_url,
-                  const SkBitmap& image,
+                  const gfx::Image& image,
                   history::IconType icon_type);
 
   // Converts the FAVICON's image data to an SkBitmap and sets it on the
@@ -197,11 +204,12 @@ class FaviconHandler {
   // (INVALIDATE_FAVICON).
   void UpdateFavicon(NavigationEntry* entry,
                      scoped_refptr<RefCountedMemory> data);
-  void UpdateFavicon(NavigationEntry* entry, const SkBitmap& image);
+  void UpdateFavicon(NavigationEntry* entry, const gfx::Image* image);
 
-  // Scales the image such that either the width and/or height is 16 pixels
-  // wide. Does nothing if the image is empty.
-  SkBitmap ConvertToFaviconSize(const SkBitmap& image);
+  // If the image is not already at its preferred size, scales the image such
+  // that either the width and/or height is 16 pixels wide. Does nothing if the
+  // image is empty.
+  gfx::Image ResizeFaviconIfNeeded(const gfx::Image& image);
 
   void FetchFaviconInternal();
 
@@ -217,10 +225,6 @@ class FaviconHandler {
     return icon_types_ == history::FAVICON ? kFaviconSize : 0;
   }
 
-  TabContents* tab_contents() {
-    return tab_contents_;
-  }
-
   // Used for history requests.
   CancelableRequestConsumer cancelable_consumer_;
 
@@ -253,7 +257,11 @@ class FaviconHandler {
   // The FaviconData from history.
   history::FaviconData history_icon_;
 
-  TabContents* tab_contents_;
+  // The Profile associated with this handler.
+  Profile* profile_;
+
+  // This handler's delegate.
+  FaviconHandlerDelegate* delegate_;  // weak
 
   DISALLOW_COPY_AND_ASSIGN(FaviconHandler);
 };
diff --git a/chrome/browser/favicon/favicon_handler_delegate.h b/chrome/browser/favicon/favicon_handler_delegate.h
new file mode 100644
index 0000000..83089e2
--- /dev/null
+++ b/chrome/browser/favicon/favicon_handler_delegate.h
@@ -0,0 +1,28 @@
+// Copyright (c) 2011 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.
+
+#ifndef CHROME_BROWSER_FAVICON_FAVICON_HANDLER_DELEGATE_H_
+#define CHROME_BROWSER_FAVICON_FAVICON_HANDLER_DELEGATE_H_
+#pragma once
+
+class GURL;
+class NavigationEntry;
+
+// This class provides a delegate interface for a FaviconHandler.  It allows the
+// FaviconHandler to ask its delegate for information or notify its delegate
+// about changes.
+class FaviconHandlerDelegate {
+ public:
+  // Returns the current NavigationEntry.
+  virtual NavigationEntry* GetActiveEntry() = 0;
+
+  // Starts the download for the given favicon.  When finished, the delegate
+  // will call |OnDidDownloadFavicon()| with the results.
+  virtual void StartDownload(int id, const GURL& url, int image_size) = 0;
+
+  // Notifies the delegate that the favicon for the active entry was updated.
+  virtual void NotifyFaviconUpdated() = 0;
+};
+
+#endif  // CHROME_BROWSER_FAVICON_FAVICON_HANDLER_DELEGATE_H_
diff --git a/chrome/browser/favicon/favicon_handler_unittest.cc b/chrome/browser/favicon/favicon_handler_unittest.cc
index dc26ef5..c2cc69a 100644
--- a/chrome/browser/favicon/favicon_handler_unittest.cc
+++ b/chrome/browser/favicon/favicon_handler_unittest.cc
@@ -8,6 +8,7 @@
 #include "content/browser/tab_contents/test_tab_contents.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
+#include "ui/gfx/image/image.h"
 
 class TestFaviconHandler;
 
@@ -121,17 +122,48 @@ class HistoryRequestHandler {
 
 }  // namespace
 
+
+// This class is used as a temporary hack to provide working implementations of
+// the various delegate methods.  Most of these methods are actually never
+// called.
+// TODO(rohitrao): Refactor the tests to override these delegate methods instead
+// of subclassing.
+class TestFaviconHandlerDelegate : public FaviconHandlerDelegate {
+ public:
+  explicit TestFaviconHandlerDelegate(TabContents* tab_contents)
+      : tab_contents_(tab_contents) {
+  }
+
+  virtual NavigationEntry* GetActiveEntry() {
+    ADD_FAILURE() << "TestFaviconHandlerDelegate::GetActiveEntry() "
+                  << "should never be called in tests.";
+    return NULL;
+  }
+
+  virtual void StartDownload(int id, const GURL& url, int image_size) {
+    ADD_FAILURE() << "TestFaviconHandlerDelegate::StartDownload() "
+                  << "should never be called in tests.";
+  }
+
+  virtual void NotifyFaviconUpdated() {
+    tab_contents_->NotifyNavigationStateChanged(TabContents::INVALIDATE_TAB);
+  }
+
+ private:
+  TabContents* tab_contents_;  // weak
+};
+
 // This class is used to catch the FaviconHandler's download and history
 // request, and also provide the methods to access the FaviconHandler internal.
 class TestFaviconHandler : public FaviconHandler {
  public:
   TestFaviconHandler(const GURL& page_url,
-                    TabContents* tab_contents,
-                    Type type)
-    : FaviconHandler(tab_contents, type),
-      download_image_size_(0),
-      download_id_(0),
-      tab_contents_(tab_contents){
+                     Profile* profile,
+                     FaviconHandlerDelegate* delegate,
+                     Type type)
+      : FaviconHandler(profile, delegate, type),
+        download_image_size_(0),
+        download_id_(0) {
     entry_.set_url(page_url);
   }
 
@@ -176,7 +208,7 @@ class TestFaviconHandler : public FaviconHandler {
   void OnDidDownloadFavicon(int id,
                             const GURL& image_url,
                             bool errored,
-                            const SkBitmap& image) {
+                            gfx::Image& image) {
     FaviconHandler::OnDidDownloadFavicon(id, image_url, errored, image);
   }
 
@@ -245,7 +277,6 @@ class TestFaviconHandler : public FaviconHandler {
   // FaviconHandler.
   int download_id_;
 
-  TabContents* tab_contents_;
   scoped_ptr<DownloadHandler> download_handler_;
   scoped_ptr<HistoryRequestHandler> history_handler_;
 
@@ -264,8 +295,9 @@ void DownloadHandler::UpdateFaviconURL(const std::vector<FaviconURL> urls) {
 }
 
 void DownloadHandler::InvokeCallback() {
+  gfx::Image image(new SkBitmap(bitmap_));
   favicon_helper_->OnDidDownloadFavicon(download_id_, image_url_, failed_,
-                                        bitmap_);
+                                        image);
 }
 
 void HistoryRequestHandler::InvokeCallback() {
@@ -279,7 +311,9 @@ TEST_F(FaviconHandlerTest, GetFaviconFromHistory) {
   const GURL page_url("http://www.google.com");
   const GURL icon_url("http://www.google.com/favicon");
 
-  TestFaviconHandler helper(page_url, contents(), FaviconHandler::FAVICON);
+  TestFaviconHandlerDelegate delegate(contents());
+  TestFaviconHandler helper(page_url, contents()->profile(),
+                            &delegate, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -324,7 +358,9 @@ TEST_F(FaviconHandlerTest, DownloadFavicon) {
   const GURL page_url("http://www.google.com");
   const GURL icon_url("http://www.google.com/favicon");
 
-  TestFaviconHandler helper(page_url, contents(), FaviconHandler::FAVICON);
+  TestFaviconHandlerDelegate delegate(contents());
+  TestFaviconHandler helper(page_url, contents()->profile(),
+                            &delegate, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -388,7 +424,9 @@ TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) {
   const GURL icon_url("http://www.google.com/favicon");
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
-  TestFaviconHandler helper(page_url, contents(), FaviconHandler::FAVICON);
+  TestFaviconHandlerDelegate delegate(contents());
+  TestFaviconHandler helper(page_url, contents()->profile(),
+                            &delegate, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -473,7 +511,9 @@ TEST_F(FaviconHandlerTest, UpdateFavicon) {
   const GURL icon_url("http://www.google.com/favicon");
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
-  TestFaviconHandler helper(page_url, contents(), FaviconHandler::FAVICON);
+  TestFaviconHandlerDelegate delegate(contents());
+  TestFaviconHandler helper(page_url, contents()->profile(),
+                            &delegate, FaviconHandler::FAVICON);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -544,7 +584,9 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
   const GURL icon_url("http://www.google.com/favicon");
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
-  TestFaviconHandler helper(page_url, contents(), FaviconHandler::TOUCH);
+  TestFaviconHandlerDelegate delegate(contents());
+  TestFaviconHandler helper(page_url, contents()->profile(),
+                            &delegate, FaviconHandler::TOUCH);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
@@ -658,7 +700,9 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
   const GURL icon_url("http://www.google.com/favicon");
   const GURL new_icon_url("http://www.google.com/new_favicon");
 
-  TestFaviconHandler helper(page_url, contents(), FaviconHandler::TOUCH);
+  TestFaviconHandlerDelegate delegate(contents());
+  TestFaviconHandler helper(page_url, contents()->profile(),
+                            &delegate, FaviconHandler::TOUCH);
 
   helper.FetchFavicon(page_url);
   HistoryRequestHandler* history_handler = helper.history_handler();
diff --git a/chrome/browser/favicon/favicon_tab_helper.cc b/chrome/browser/favicon/favicon_tab_helper.cc
index 2601984..0200e47 100644
--- a/chrome/browser/favicon/favicon_tab_helper.cc
+++ b/chrome/browser/favicon/favicon_tab_helper.cc
@@ -9,17 +9,21 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/icon_messages.h"
+#include "content/browser/renderer_host/render_view_host.h"
+#include "content/browser/tab_contents/navigation_controller.h"
 #include "content/browser/tab_contents/navigation_details.h"
+#include "content/browser/tab_contents/tab_contents_delegate.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/browser/webui/web_ui.h"
 #include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/image/image.h"
 
 FaviconTabHelper::FaviconTabHelper(TabContents* tab_contents)
     : TabContentsObserver(tab_contents) {
-  favicon_handler_.reset(new FaviconHandler(tab_contents,
+  favicon_handler_.reset(new FaviconHandler(tab_contents->profile(), this,
                                             FaviconHandler::FAVICON));
   if (chrome::kEnableTouchIcon)
-    touch_icon_handler_.reset(new FaviconHandler(tab_contents,
+    touch_icon_handler_.reset(new FaviconHandler(tab_contents->profile(), this,
                                                  FaviconHandler::TOUCH));
 }
 
@@ -112,6 +116,28 @@ int FaviconTabHelper::DownloadImage(const GURL& image_url,
   return 0;
 }
 
+void FaviconTabHelper::OnUpdateFaviconURL(
+    int32 page_id,
+    const std::vector<FaviconURL>& candidates) {
+  favicon_handler_->OnUpdateFaviconURL(page_id, candidates);
+  if (touch_icon_handler_.get())
+    touch_icon_handler_->OnUpdateFaviconURL(page_id, candidates);
+}
+
+NavigationEntry* FaviconTabHelper::GetActiveEntry() {
+  return tab_contents()->controller().GetActiveEntry();
+}
+
+void FaviconTabHelper::StartDownload(int id, const GURL& url, int image_size) {
+  RenderViewHost* host = tab_contents()->render_view_host();
+  host->Send(new IconMsg_DownloadFavicon(
+                 host->routing_id(), id, url, image_size));
+}
+
+void FaviconTabHelper::NotifyFaviconUpdated() {
+  tab_contents()->NotifyNavigationStateChanged(TabContents::INVALIDATE_TAB);
+}
+
 void FaviconTabHelper::NavigateToPendingEntry(
     const GURL& url,
     NavigationController::ReloadType reload_type) {
@@ -145,15 +171,8 @@ void FaviconTabHelper::OnDidDownloadFavicon(int id,
                                             const GURL& image_url,
                                             bool errored,
                                             const SkBitmap& image) {
-  favicon_handler_->OnDidDownloadFavicon(id, image_url, errored, image);
+  gfx::Image favicon(new SkBitmap(image));
+  favicon_handler_->OnDidDownloadFavicon(id, image_url, errored, favicon);
   if (touch_icon_handler_.get())
-    touch_icon_handler_->OnDidDownloadFavicon(id, image_url, errored, image);
-}
-
-void FaviconTabHelper::OnUpdateFaviconURL(
-    int32 page_id,
-    const std::vector<FaviconURL>& candidates) {
-  favicon_handler_->OnUpdateFaviconURL(page_id, candidates);
-  if (touch_icon_handler_.get())
-    touch_icon_handler_->OnUpdateFaviconURL(page_id, candidates);
+    touch_icon_handler_->OnDidDownloadFavicon(id, image_url, errored, favicon);
 }
diff --git a/chrome/browser/favicon/favicon_tab_helper.h b/chrome/browser/favicon/favicon_tab_helper.h
index ab555b9..e10005c 100644
--- a/chrome/browser/favicon/favicon_tab_helper.h
+++ b/chrome/browser/favicon/favicon_tab_helper.h
@@ -8,6 +8,7 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "chrome/browser/favicon/favicon_handler_delegate.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/common/favicon_url.h"
 #include "content/browser/tab_contents/tab_contents_observer.h"
@@ -28,7 +29,8 @@ class TabContents;
 // DownloadImage downloads the specified icon and returns it through the given
 // callback.
 //
-class FaviconTabHelper : public TabContentsObserver {
+class FaviconTabHelper : public TabContentsObserver,
+                         public FaviconHandlerDelegate {
  public:
   explicit FaviconTabHelper(TabContents* tab_contents);
   virtual ~FaviconTabHelper();
@@ -70,6 +72,11 @@ class FaviconTabHelper : public TabContentsObserver {
   void OnUpdateFaviconURL(int32 page_id,
                           const std::vector<FaviconURL>& candidates);
 
+  // FaviconHandlerDelegate methods.
+  virtual NavigationEntry* GetActiveEntry() OVERRIDE;
+  virtual void StartDownload(int id, const GURL& url, int image_size) OVERRIDE;
+  virtual void NotifyFaviconUpdated() OVERRIDE;
+
  private:
   // TabContentsObserver overrides.
   virtual void NavigateToPendingEntry(