Add Observer in FaviconTabHelper

chrome/browser/favicon/favicon_handler.cc

Index: chrome/browser/favicon/favicon_handler.cc
diff --git a/chrome/browser/favicon/favicon_handler.cc b/chrome/browser/favicon/favicon_handler.cc
index fcf89fe939638f967025ff89bcfbacbd64655fe9..a024df2b7de22e6dcbc98dc032abfec0c0dfa719 100644
--- a/chrome/browser/favicon/favicon_handler.cc
+++ b/chrome/browser/favicon/favicon_handler.cc
@@ -158,6 +158,16 @@ bool CompareIconSize(const FaviconURL& b1, const FaviconURL& b2) {
   return area1 > area2;
 }
 
+// Return the first |favicon_bitmap_results|'s icon_url if the result isn't
+// empty, otherwise empty GURL is returned.
+inline GURL GetIconURL(const std::vector<

[sky, 2014/10/30 03:34:30]

GetFirstIconURL

[sky, 2014/10/30 03:34:31]

Is it really necessary to inline here?

[michaelbai, 2014/10/30 19:22:50]

Function was removed

[michaelbai, 2014/10/30 19:22:50]

Function was removed

+    favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results) {
+  // The history service sends back results for a single icon URL, so it does
+  // not matter which result we get the |icon_url| from.
+  return favicon_bitmap_results.empty() ?
+      GURL() : favicon_bitmap_results[0].icon_url;
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -288,23 +298,20 @@ 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())
+  if (!UrlMatches(url, url_) || PageChangedSinceFaviconWasRequested())
+    return;
+
+  if (icon_type == favicon_base::FAVICON && !download_largest_icon_)
       SetFaviconOnActivePage(icon_url, image);

[sky, 2014/10/30 03:34:30]

nit: spacing is off now (use git cl format).

[michaelbai, 2014/10/30 19:22:50]

Acknowledged.

-  }
+  driver_->OnFaviconAvailable(image);
 }
 
-void FaviconHandler::SetFaviconOnActivePage(const std::vector<
+gfx::Image FaviconHandler::SelectFaviconFrames(const std::vector<
     favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results) {
-  gfx::Image resized_image = favicon_base::SelectFaviconFramesFromPNGs(
+  return favicon_base::SelectFaviconFramesFromPNGs(
       favicon_bitmap_results,
       favicon_base::GetFaviconScales(),
       preferred_icon_size());
-  // The history service sends back results for a single icon URL, so it does
-  // 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);
 }
 
 void FaviconHandler::SetFaviconOnActivePage(const GURL& icon_url,
@@ -542,16 +549,21 @@ 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);
+  gfx::Image image;
+  if (has_valid_result)
+    image = SelectFaviconFrames(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);
+      if (!download_largest_icon_)
+        SetFaviconOnActivePage(GetIconURL(favicon_bitmap_results), image);
     } else {
       // If |favicon_bitmap_results| does not have any valid results, treat the
       // favicon as if it's expired.
@@ -579,6 +591,10 @@ 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) {
+    driver_->OnFaviconAvailable(image);
+  }
 }
 
 void FaviconHandler::DownloadFaviconOrAskFaviconService(
@@ -619,13 +635,17 @@ 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);
+  gfx::Image image;
+  if (has_valid_result)
+    image = SelectFaviconFrames(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);
+      if (!download_largest_icon_)
+        SetFaviconOnActivePage(GetIconURL(favicon_bitmap_results), image);
     }
     if (has_expired_or_incomplete_result) {
       // The favicon is out of date. Request the current one.
@@ -643,6 +663,9 @@ void FaviconHandler::OnFaviconData(const std::vector<
                      current_candidate()->icon_type);
   }
   history_results_ = favicon_bitmap_results;
+
+  if (has_valid_result)
+    driver_->OnFaviconAvailable(image);
 }
 
 int FaviconHandler::ScheduleDownload(const GURL& url,