This patch added Observer in FaviconTabHelper, so the favicon retrieved by...

This patch added Observer in FaviconTabHelper, so the favicon retrieved by FaviconHandler could be used right away.

BUG=428218

Review URL: https://codereview.chromium.org/684983003

Cr-Commit-Position: refs/heads/master@{#302700}

chrome/browser/favicon/favicon_handler.cc

@@ -288,14 +288,19 @@ void FaviconHandler::SetFavicon(const GURL& url,
   if (client_->GetFaviconService() && ShouldSaveFavicon(url))
     SetHistoryFavicons(url, icon_url, icon_type, image);
 
-  if (UrlMatches(url, url_) && icon_type == favicon_base::FAVICON) {
-    if (!PageChangedSinceFaviconWasRequested())
-      SetFaviconOnActivePage(icon_url, image);
-  }
+  if (!UrlMatches(url, url_) || PageChangedSinceFaviconWasRequested())
+    return;
+
+  NotifyFaviconAvailable(
+      icon_url,
+      image,
+      icon_type == favicon_base::FAVICON && !download_largest_icon_);
 }
 
-void FaviconHandler::SetFaviconOnActivePage(const std::vector<
-    favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results) {
+void FaviconHandler::NotifyFaviconAvailable(
+    const std::vector<favicon_base::FaviconRawBitmapResult>&
+        favicon_bitmap_results,
+    bool is_active_favicon) {
   gfx::Image resized_image = favicon_base::SelectFaviconFramesFromPNGs(
       favicon_bitmap_results,
       favicon_base::GetFaviconScales(),
@@ -304,25 +309,17 @@ void FaviconHandler::SetFaviconOnActivePage(const std::vector<
   // not matter which result we get the |icon_url| from.
   const GURL icon_url = favicon_bitmap_results.empty() ?
       GURL() : favicon_bitmap_results[0].icon_url;
-  SetFaviconOnActivePage(icon_url, resized_image);
+  NotifyFaviconAvailable(icon_url, resized_image, is_active_favicon);
 }
 
-void FaviconHandler::SetFaviconOnActivePage(const GURL& icon_url,
-                                            const gfx::Image& image) {
-  // No matter what happens, we need to mark the favicon as being set.
-  driver_->SetActiveFaviconValidity(true);
-
-  bool icon_url_changed = driver_->GetActiveFaviconURL() != icon_url;
-  driver_->SetActiveFaviconURL(icon_url);
-
-  if (image.IsEmpty())
-    return;
-
+void FaviconHandler::NotifyFaviconAvailable(const GURL& icon_url,
+                                            const gfx::Image& image,
+                                            bool is_active_favicon) {
   gfx::Image image_with_adjusted_colorspace = image;
   favicon_base::SetFaviconColorSpace(&image_with_adjusted_colorspace);
 
-  driver_->SetActiveFaviconImage(image_with_adjusted_colorspace);
-  NotifyFaviconUpdated(icon_url_changed);
+  driver_->OnFaviconAvailable(
+      image_with_adjusted_colorspace, icon_url, is_active_favicon);
 }
 
 void FaviconHandler::OnUpdateFaviconURL(
@@ -510,10 +507,6 @@ bool FaviconHandler::ShouldSaveFavicon(const GURL& url) {
   return client_->IsBookmarked(url);
 }
 
-void FaviconHandler::NotifyFaviconUpdated(bool icon_url_changed) {
-  driver_->NotifyFaviconUpdated(icon_url_changed);
-}
-
 int FaviconHandler::GetMaximalIconSize(favicon_base::IconType icon_type) {
   switch (icon_type) {
     case favicon_base::FAVICON:
@@ -542,16 +535,18 @@ void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
   bool has_results = !favicon_bitmap_results.empty();
   favicon_expired_or_incomplete_ = has_results && HasExpiredOrIncompleteResult(
       preferred_icon_size(), favicon_bitmap_results);
+  bool has_valid_result = HasValidResult(favicon_bitmap_results);
+
   if (has_results && icon_types_ == favicon_base::FAVICON &&
       !driver_->GetActiveFaviconValidity() &&
       (!current_candidate() ||
        DoUrlsAndIconsMatch(*current_candidate(), favicon_bitmap_results))) {
-    if (HasValidResult(favicon_bitmap_results)) {
+    if (has_valid_result) {
       // The db knows the favicon (although it may be out of date) and the entry
       // doesn't have an icon. Set the favicon now, and if the favicon turns out
       // to be expired (or the wrong url) we'll fetch later on. This way the
       // user doesn't see a flash of the default favicon.
-      SetFaviconOnActivePage(favicon_bitmap_results);
+      NotifyFaviconAvailable(favicon_bitmap_results, !download_largest_icon_);
     } else {
       // If |favicon_bitmap_results| does not have any valid results, treat the
       // favicon as if it's expired.
@@ -579,6 +574,9 @@ void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
   }
   // else we haven't got the icon url. When we get it we'll ask the
   // renderer to download the icon.
+
+  if (has_valid_result && icon_types_ != favicon_base::FAVICON)
+    NotifyFaviconAvailable(favicon_bitmap_results, false);
 }
 
 void FaviconHandler::DownloadFaviconOrAskFaviconService(
@@ -619,13 +617,14 @@ void FaviconHandler::OnFaviconData(const std::vector<
   bool has_results = !favicon_bitmap_results.empty();
   bool has_expired_or_incomplete_result = HasExpiredOrIncompleteResult(
       preferred_icon_size(), favicon_bitmap_results);
+  bool has_valid_result = HasValidResult(favicon_bitmap_results);
 
   if (has_results && icon_types_ == favicon_base::FAVICON) {
-    if (HasValidResult(favicon_bitmap_results)) {
+    if (has_valid_result) {
       // There is a favicon, set it now. If expired we'll download the current
       // one again, but at least the user will get some icon instead of the
       // default and most likely the current one is fine anyway.
-      SetFaviconOnActivePage(favicon_bitmap_results);
+      NotifyFaviconAvailable(favicon_bitmap_results, !download_largest_icon_);
     }
     if (has_expired_or_incomplete_result) {
       // The favicon is out of date. Request the current one.
@@ -643,6 +642,9 @@ void FaviconHandler::OnFaviconData(const std::vector<
                      current_candidate()->icon_type);
   }
   history_results_ = favicon_bitmap_results;
+
+  if (has_valid_result && icon_types_ != favicon_base::FAVICON)
+    NotifyFaviconAvailable(favicon_bitmap_results, false);
 }
 
 int FaviconHandler::ScheduleDownload(const GURL& url,

chrome/browser/favicon/favicon_handler.h

@@ -148,11 +148,6 @@ class FaviconHandler {
   // Returns true if the favicon should be saved.
   virtual bool ShouldSaveFavicon(const GURL& url);
 
-  // Notifies the driver that the favicon for the active entry was updated.
-  // |icon_url_changed| is true if a favicon with a different icon URL has been
-  // selected since the previous call to NotifyFaviconUpdated().
-  virtual void NotifyFaviconUpdated(bool icon_url_changed);
-
  private:
   friend class TestFaviconHandler; // For testing
 
@@ -226,10 +221,15 @@ class FaviconHandler {
                   const gfx::Image& image,
                   favicon_base::IconType icon_type);
 
-  // Sets the favicon's data.
-  void SetFaviconOnActivePage(const std::vector<
-      favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results);
-  void SetFaviconOnActivePage(const GURL& icon_url, const gfx::Image& image);
+  // Notifies |driver_| favicon available. See
+  // FaviconDriver::NotifyFaviconAvailable() for |is_active_favicon| in detail.
+  void NotifyFaviconAvailable(
+      const std::vector<favicon_base::FaviconRawBitmapResult>&
+          favicon_bitmap_results,
+      bool is_active_favicon);
+  void NotifyFaviconAvailable(const GURL& icon_url,
+                              const gfx::Image& image,
+                              bool is_active_favicon);
 
   // Return the current candidate if any.
   favicon::FaviconURL* current_candidate() {

chrome/browser/favicon/favicon_handler_unittest.cc

@@ -189,7 +189,10 @@ class TestFaviconClient : public FaviconClient {
 
 class TestFaviconDriver : public FaviconDriver {
  public:
-  TestFaviconDriver() : favicon_validity_(false) {}
+  TestFaviconDriver()
+      : favicon_validity_(false),
+        num_favicon_available_(0),
+        update_active_favicon_(false) {}
 
   virtual ~TestFaviconDriver() {
   }
@@ -204,13 +207,11 @@ class TestFaviconDriver : public FaviconDriver {
 
   const GURL GetActiveURL() override { return url_; }
 
-  void SetActiveFaviconImage(gfx::Image image) override { image_ = image; }
+  void SetActiveFaviconImage(gfx::Image image) { image_ = image; }
 
-  void SetActiveFaviconURL(GURL favicon_url) override {
-    favicon_url_ = favicon_url;
-  }
+  void SetActiveFaviconURL(GURL favicon_url) { favicon_url_ = favicon_url; }
 
-  void SetActiveFaviconValidity(bool favicon_validity) override {
+  void SetActiveFaviconValidity(bool favicon_validity) {
     favicon_validity_ = favicon_validity;
   }
 
@@ -220,18 +221,45 @@ class TestFaviconDriver : public FaviconDriver {
     return -1;
   }
 
-  void NotifyFaviconUpdated(bool icon_url_changed) override {
-    ADD_FAILURE() << "TestFaviconDriver::NotifyFaviconUpdated() "
-                  << "should never be called in tests.";
+  void OnFaviconAvailable(const gfx::Image& image,
+                          const GURL& icon_url,
+                          bool update_active_favicon) override {
+    available_image_ = image;
+    available_icon_url_ = icon_url;
+    update_active_favicon_ = update_active_favicon;
+    if (!update_active_favicon)
+      return;
+
+    ++num_favicon_available_;
+    SetActiveFaviconURL(icon_url);
+    SetActiveFaviconValidity(true);
+    SetActiveFaviconImage(image);
   }
 
+  size_t num_favicon_available() const { return num_favicon_available_; }
+
+  void ResetNumFaviconAvailable() { num_favicon_available_ = 0; }
+
   void SetActiveURL(GURL url) { url_ = url; }
 
+  const gfx::Image available_favicon() { return available_image_; }
+
+  const GURL available_icon_url() { return available_icon_url_; }
+
+  bool update_active_favicon() { return update_active_favicon_; }
+
  private:
   GURL favicon_url_;
   GURL url_;
   gfx::Image image_;
   bool favicon_validity_;
+
+  // The number of times that NotifyFaviconAvailable() has been called.
+  size_t num_favicon_available_;
+  gfx::Image available_image_;
+  GURL available_icon_url_;
+  bool update_active_favicon_;
+
   DISALLOW_COPY_AND_ASSIGN(TestFaviconDriver);
 };
 
@@ -250,8 +278,7 @@ class TestFaviconHandler : public FaviconHandler {
                      Type type,
                      bool download_largest_icon)
       : FaviconHandler(client, driver, type, download_largest_icon),
-        download_id_(0),
-        num_favicon_updates_(0) {
+        download_id_(0) {
     driver->SetActiveURL(page_url);
     download_handler_.reset(new DownloadHandler(this));
   }
@@ -271,14 +298,6 @@ class TestFaviconHandler : public FaviconHandler {
     return download_handler_.get();
   }
 
-  size_t num_favicon_update_notifications() const {
-    return num_favicon_updates_;
-  }
-
-  void ResetNumFaviconUpdateNotifications() {
-    num_favicon_updates_ = 0;
-  }
-
   // Methods to access favicon internals.
   const std::vector<FaviconURL>& urls() {
     return image_urls_;
@@ -343,10 +362,6 @@ class TestFaviconHandler : public FaviconHandler {
 
   bool ShouldSaveFavicon(const GURL& url) override { return true; }
 
-  void NotifyFaviconUpdated(bool icon_url_changed) override {
-    ++num_favicon_updates_;
-  }
-
   GURL page_url_;
 
  private:
@@ -358,9 +373,6 @@ class TestFaviconHandler : public FaviconHandler {
   scoped_ptr<DownloadHandler> download_handler_;
   scoped_ptr<HistoryRequestHandler> history_handler_;
 
-  // The number of times that NotifyFaviconUpdated() has been called.
-  size_t num_favicon_updates_;
-
   DISALLOW_COPY_AND_ASSIGN(TestFaviconHandler);
 };
 
@@ -410,11 +422,13 @@ class FaviconHandlerTest : public ChromeRenderViewHostTestHarness {
   // - The favicons at |candidate_icons| have edge pixel sizes of
   //   |candidate_icon_sizes|.
   void DownloadTillDoneIgnoringHistory(
+      TestFaviconDriver* favicon_driver,
       TestFaviconHandler* favicon_handler,
       const GURL& page_url,
       const std::vector<FaviconURL>& candidate_icons,
       const int* candidate_icon_sizes) {
-    UpdateFaviconURL(favicon_handler, page_url, candidate_icons);
+    UpdateFaviconURL(
+        favicon_driver, favicon_handler, page_url, candidate_icons);
     EXPECT_EQ(candidate_icons.size(), favicon_handler->image_urls().size());
 
     DownloadHandler* download_handler = favicon_handler->download_handler();
@@ -429,16 +443,16 @@ class FaviconHandlerTest : public ChromeRenderViewHostTestHarness {
       download_handler->SetImageSizes(sizes);
       download_handler->InvokeCallback();
 
-      if (favicon_handler->num_favicon_update_notifications())
+      if (favicon_driver->num_favicon_available())
         return;
     }
   }
 
-  void UpdateFaviconURL(
-      TestFaviconHandler* favicon_handler,
-      const GURL& page_url,
-      const std::vector<FaviconURL>& candidate_icons) {
-    favicon_handler->ResetNumFaviconUpdateNotifications();
+  void UpdateFaviconURL(TestFaviconDriver* favicon_driver,
+                        TestFaviconHandler* favicon_handler,
+                        const GURL& page_url,
+                        const std::vector<FaviconURL>& candidate_icons) {
+    favicon_driver->ResetNumFaviconAvailable();
 
     favicon_handler->FetchFavicon(page_url);
     favicon_handler->history_handler()->InvokeCallback();
@@ -1070,7 +1084,8 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
   const int kSizes1[] = { 16, 24, 32, 48, 256 };
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSizes1));
-  DownloadTillDoneIgnoringHistory(&handler1, kPageURL, urls1, kSizes1);
+  DownloadTillDoneIgnoringHistory(
+      &driver1, &handler1, kPageURL, urls1, kSizes1);
 
   EXPECT_EQ(0u, handler1.image_urls().size());
   EXPECT_TRUE(driver1.GetActiveFaviconValidity());
@@ -1091,7 +1106,8 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
   const int kSizes2[] = { 16, 24, 48, 256 };
   std::vector<FaviconURL> urls2(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSizes2));
-  DownloadTillDoneIgnoringHistory(&handler2, kPageURL, urls2, kSizes2);
+  DownloadTillDoneIgnoringHistory(
+      &driver2, &handler2, kPageURL, urls2, kSizes2);
   EXPECT_TRUE(driver2.GetActiveFaviconValidity());
   expected_index = 0u;
   EXPECT_EQ(16, kSizes2[expected_index]);
@@ -1107,7 +1123,8 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
   const int kSizes3[] = { 256, 48 };
   std::vector<FaviconURL> urls3(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSizes3));
-  DownloadTillDoneIgnoringHistory(&handler3, kPageURL, urls3, kSizes3);
+  DownloadTillDoneIgnoringHistory(
+      &driver3, &handler3, kPageURL, urls3, kSizes3);
   EXPECT_TRUE(driver3.GetActiveFaviconValidity());
   expected_index = 1u;
   EXPECT_EQ(48, kSizes3[expected_index]);
@@ -1121,7 +1138,8 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
   const int kSizes4[] = { 17, 256 };
   std::vector<FaviconURL> urls4(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSizes4));
-  DownloadTillDoneIgnoringHistory(&handler4, kPageURL, urls4, kSizes4);
+  DownloadTillDoneIgnoringHistory(
+      &driver4, &handler4, kPageURL, urls4, kSizes4);
   EXPECT_TRUE(driver4.GetActiveFaviconValidity());
   expected_index = 0u;
   EXPECT_EQ(17, kSizes4[expected_index]);
@@ -1162,7 +1180,7 @@ TEST_F(FaviconHandlerTest, TestSortFavicon) {
       kPageURL, &client, &driver1, FaviconHandler::FAVICON, true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&handler1, kPageURL, urls1);
+  UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
 
   struct ExpectedResult {
     // The favicon's index in kSourceIconURLs.
@@ -1226,7 +1244,7 @@ TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
       kPageURL, &client, &driver1, FaviconHandler::FAVICON, true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&handler1, kPageURL, urls1);
+  UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
 
   // Simulate the download failed, to check whether the icons were requested
   // to download according their size.
@@ -1292,7 +1310,7 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
       kPageURL, &client, &driver1, FaviconHandler::FAVICON, true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&handler1, kPageURL, urls1);
+  UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
 
   ASSERT_EQ(2u, handler1.urls().size());
 
@@ -1330,6 +1348,11 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
   EXPECT_EQ(kSourceIconURLs[i].icon_url, handler1.history_handler()->icon_url_);
   EXPECT_EQ(kSourceIconURLs[i].icon_sizes[b],
             handler1.history_handler()->size_);
+  // Verify NotifyFaviconAvailable().
+  EXPECT_FALSE(driver1.update_active_favicon());
+  EXPECT_EQ(kSourceIconURLs[i].icon_url, driver1.available_icon_url());
+  EXPECT_EQ(kSourceIconURLs[i].icon_sizes[b],
+            driver1.available_favicon().Size());
 }
 
 TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
@@ -1355,7 +1378,7 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
       kPageURL, &client, &driver1, FaviconHandler::FAVICON, true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&handler1, kPageURL, urls1);
+  UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
 
   ASSERT_EQ(2u, handler1.urls().size());
 
@@ -1417,7 +1440,7 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
       kPageURL, &client, &driver1, FaviconHandler::FAVICON, true);
   std::vector<FaviconURL> urls1(kSourceIconURLs,
                                 kSourceIconURLs + arraysize(kSourceIconURLs));
-  UpdateFaviconURL(&handler1, kPageURL, urls1);
+  UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
   ASSERT_EQ(3u, handler1.urls().size());
 
   // Simulate no favicon from history.

chrome/browser/favicon/favicon_tab_helper.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/favicon/favicon_handler.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
+#include "chrome/browser/favicon/favicon_tab_helper_observer.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -136,6 +137,14 @@ void FaviconTabHelper::SaveFavicon() {
       entry->GetURL(), favicon.url, favicon_base::FAVICON, favicon.image);
 }
 
+void FaviconTabHelper::AddObserver(FaviconTabHelperObserver* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void FaviconTabHelper::RemoveObserver(FaviconTabHelperObserver* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
 int FaviconTabHelper::StartDownload(const GURL& url, int max_image_size) {
   FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
       profile_->GetOriginalProfile(), Profile::IMPLICIT_ACCESS);
@@ -151,14 +160,6 @@ int FaviconTabHelper::StartDownload(const GURL& url, int max_image_size) {
       base::Bind(&FaviconTabHelper::DidDownloadFavicon,base::Unretained(this)));
 }
 
-void FaviconTabHelper::NotifyFaviconUpdated(bool icon_url_changed) {
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_FAVICON_UPDATED,
-      content::Source<WebContents>(web_contents()),
-      content::Details<bool>(&icon_url_changed));
-  web_contents()->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_TAB);
-}
-
 bool FaviconTabHelper::IsOffTheRecord() {
   DCHECK(web_contents());
   return web_contents()->GetBrowserContext()->IsOffTheRecord();
@@ -195,6 +196,30 @@ void FaviconTabHelper::SetActiveFaviconValidity(bool validity) {
   GetFaviconStatus().valid = validity;
 }
 
+void FaviconTabHelper::OnFaviconAvailable(const gfx::Image& image,
+                                          const GURL& icon_url,
+                                          bool is_active_favicon) {
+  if (is_active_favicon) {
+    // No matter what happens, we need to mark the favicon as being set.
+    SetActiveFaviconValidity(true);
+    bool icon_url_changed = GetActiveFaviconURL() != icon_url;
+    SetActiveFaviconURL(icon_url);
+
+    if (image.IsEmpty())
+      return;
+
+    SetActiveFaviconImage(image);
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_FAVICON_UPDATED,
+        content::Source<WebContents>(web_contents()),
+        content::Details<bool>(&icon_url_changed));
+    web_contents()->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_TAB);
+  }
+  if (!image.IsEmpty())
+    FOR_EACH_OBSERVER(FaviconTabHelperObserver, observer_list_,
+                      OnFaviconAvailable(image));
+}
+
 content::FaviconStatus& FaviconTabHelper::GetFaviconStatus() {
   DCHECK(web_contents()->GetController().GetActiveEntry());
   return web_contents()->GetController().GetActiveEntry()->GetFavicon();

chrome/browser/favicon/favicon_tab_helper.h

@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/observer_list.h"
 #include "components/favicon/core/browser/favicon_client.h"
 #include "components/favicon/core/favicon_driver.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -25,6 +26,7 @@ struct FaviconStatus;
 
 class GURL;
 class FaviconHandler;
+class FaviconTabHelperObserver;
 class Profile;
 class SkBitmap;
 
@@ -70,17 +72,19 @@ class FaviconTabHelper : public content::WebContentsObserver,
   // Saves the favicon for the current page.
   void SaveFavicon();
 
+  void AddObserver(FaviconTabHelperObserver* observer);
+  void RemoveObserver(FaviconTabHelperObserver* observer);
+
   // FaviconDriver methods.
   int StartDownload(const GURL& url, int max_bitmap_size) override;
-  void NotifyFaviconUpdated(bool icon_url_changed) override;
   bool IsOffTheRecord() override;
   const gfx::Image GetActiveFaviconImage() override;
   const GURL GetActiveFaviconURL() override;
   bool GetActiveFaviconValidity() override;
   const GURL GetActiveURL() override;
-  void SetActiveFaviconImage(gfx::Image image) override;
-  void SetActiveFaviconURL(GURL url) override;
-  void SetActiveFaviconValidity(bool validity) override;
+  void OnFaviconAvailable(const gfx::Image& image,
+                          const GURL& url,
+                          bool is_active_favicon) override;
 
   // Favicon download callback.
   void DidDownloadFavicon(
@@ -102,6 +106,16 @@ class FaviconTabHelper : public content::WebContentsObserver,
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) override;
 
+  // Sets whether the page's favicon is valid (if false, the default favicon is
+  // being used). Requires GetActiveURL() to be valid.
+  void SetActiveFaviconValidity(bool validity);
+
+  // Sets the URL of the favicon's bitmap.
+  void SetActiveFaviconURL(GURL url);
+
+  // Sets the bitmap of the current page's favicon.
+  void SetActiveFaviconImage(gfx::Image image);
+
   // Helper method that returns the active navigation entry's favicon.
   content::FaviconStatus& GetFaviconStatus();
 
@@ -117,6 +131,8 @@ class FaviconTabHelper : public content::WebContentsObserver,
   // browser_defaults::kEnableTouchIcon is false.
   scoped_ptr<FaviconHandler> touch_icon_handler_;
 
+  ObserverList<FaviconTabHelperObserver> observer_list_;
+
   DISALLOW_COPY_AND_ASSIGN(FaviconTabHelper);
 };
 

chrome/browser/favicon/favicon_tab_helper_observer.h

+// Copyright 2014 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_TAB_HELPER_OBSERVER_H_
+#define CHROME_BROWSER_FAVICON_FAVICON_TAB_HELPER_OBSERVER_H_
+
+namespace gfx {
+class Image;
+}
+
+// An observer implemented by classes which are interested in envent in
+// FaviconTabHelper.
+class FaviconTabHelperObserver {
+ public:
+  // Called when favicon |image| is retrieved from either web site or history
+  // backend.
+  virtual void OnFaviconAvailable(const gfx::Image& image) = 0;
+
+ protected:
+  virtual ~FaviconTabHelperObserver() {}
+};
+
+#endif  // CHROME_BROWSER_FAVICON_FAVICON_TAB_HELPER_OBSERVER_H_

chrome/chrome_browser.gypi

@@ -398,6 +398,7 @@
       'browser/favicon/favicon_service_factory.h',
       'browser/favicon/favicon_tab_helper.cc',
       'browser/favicon/favicon_tab_helper.h',
+      'browser/favicon/favicon_tab_helper_observer.h',
       'browser/file_select_helper.cc',
       'browser/file_select_helper.h',
       'browser/file_select_helper_mac.mm',

components/favicon/core/favicon_driver.h

@@ -27,11 +27,6 @@ class FaviconDriver {
   // is the only result. A |max_bitmap_size| of 0 means unlimited.
   virtual int StartDownload(const GURL& url, int max_bitmap_size) = 0;
 
-  // Notifies the driver that the favicon for the active entry was updated.
-  // |icon_url_changed| is true if a favicon with a different icon URL has
-  // been selected since the previous call to NotifyFaviconUpdated().
-  virtual void NotifyFaviconUpdated(bool icon_url_changed) = 0;
-
   // Returns whether the user is operating in an off-the-record context.
   virtual bool IsOffTheRecord() = 0;
 
@@ -51,16 +46,13 @@ class FaviconDriver {
   // URL otherwise.
   virtual const GURL GetActiveURL() = 0;
 
-  // Sets the bitmap of the current page's favicon. Requires GetActiveURL() to
-  // be valid.
-  virtual void SetActiveFaviconImage(gfx::Image image) = 0;
-
-  // Sets the URL of the favicon's bitmap. Requires GetActiveURL() to be valid.
-  virtual void SetActiveFaviconURL(GURL url) = 0;
-
-  // Sets whether the page's favicon is valid (if false, the default favicon is
-  // being used). Requires GetActiveURL() to be valid.
-  virtual void SetActiveFaviconValidity(bool validity) = 0;
+  // Notifies the driver a favicon image is available. |image| is not
+  // necessarily 16x16. |icon_url| is the url the image is from. If
+  // |is_active_favicon| is true the image corresponds to the favicon
+  // (possibly empty) of the page.
+  virtual void OnFaviconAvailable(const gfx::Image& image,
+                                  const GURL& icon_url,
+                                  bool is_active_favicon) = 0;
 };
 
 #endif  // COMPONENTS_FAVICON_CORE_FAVICON_DRIVER_H_