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..eec1a380178a0e8292d649443c2aafc69ebc38e8 100644
--- a/chrome/browser/favicon/favicon_handler.cc
+++ b/chrome/browser/favicon/favicon_handler.cc
@@ -288,9 +288,11 @@ 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()) {
+    if (icon_type == favicon_base::FAVICON && !download_largest_icon_)
       SetFaviconOnActivePage(icon_url, image);
+    if (!image.IsEmpty())
+      NotifyFaviconUpdated(icon_url);

[sky, 2014/10/29 15:50:55]

Sorry, this is wrong. We shouldn't need the invalidate anything if the favicon
hasn't changed. Who is listening for the changes that needs that you need to
update? Seems like they should instead be listening for history change, if that
is what they care about, right?

   }
 }
 
@@ -311,8 +313,6 @@ 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())
@@ -322,7 +322,6 @@ void FaviconHandler::SetFaviconOnActivePage(const GURL& icon_url,
   favicon_base::SetFaviconColorSpace(&image_with_adjusted_colorspace);
 
   driver_->SetActiveFaviconImage(image_with_adjusted_colorspace);
-  NotifyFaviconUpdated(icon_url_changed);
 }
 
 void FaviconHandler::OnUpdateFaviconURL(
@@ -510,7 +509,14 @@ bool FaviconHandler::ShouldSaveFavicon(const GURL& url) {
   return client_->IsBookmarked(url);
 }
 
-void FaviconHandler::NotifyFaviconUpdated(bool icon_url_changed) {
+void FaviconHandler::NotifyFaviconUpdated(const GURL& icon_url) {
+  bool icon_url_changed = false;
+  if (icon_types_ == favicon_base::FAVICON) {
+    icon_url_changed = driver_->GetActiveFaviconURL() != icon_url;
+  } else {
+    icon_url_changed = icon_url != notified_touch_icon_url_;
+    notified_touch_icon_url_ = icon_url;
+  }
   driver_->NotifyFaviconUpdated(icon_url_changed);
 }
 
@@ -551,7 +557,9 @@ void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
       // 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(favicon_bitmap_results);
+      NotifyFaviconUpdated(favicon_bitmap_results[0].icon_url);
     } else {
       // If |favicon_bitmap_results| does not have any valid results, treat the
       // favicon as if it's expired.
@@ -579,6 +587,13 @@ void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
   }
   // else we haven't got the icon url. When we get it we'll ask the
   // renderer to download the icon.
+
+  // Notify the TOUCH_ICON or TOUCH_PRECOMPOSED_ICON available in
+  // HistoryBackend.
+  if (icon_types_ != favicon_base::FAVICON &&
+      HasValidResult(favicon_bitmap_results)) {
+    NotifyFaviconUpdated(favicon_bitmap_results[0].icon_url);
+  }

[michaelbai, 2014/10/29 04:30:32]

I just simply added code here because this is touch icon specific and will be
deleted when we remove the icon type concept.

 }
 
 void FaviconHandler::DownloadFaviconOrAskFaviconService(
@@ -625,7 +640,9 @@ void FaviconHandler::OnFaviconData(const std::vector<
       // 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(favicon_bitmap_results);
+      NotifyFaviconUpdated(favicon_bitmap_results[0].icon_url);
     }
     if (has_expired_or_incomplete_result) {
       // The favicon is out of date. Request the current one.
@@ -642,6 +659,12 @@ void FaviconHandler::OnFaviconData(const std::vector<
                      current_candidate()->icon_url,
                      current_candidate()->icon_type);
   }
+  // Notify the TOUCH_ICON or TOUCH_PRECOMPOSED_ICON available in
+  // HistoryBackend.
+  if (icon_types_ != favicon_base::FAVICON &&
+      HasValidResult(favicon_bitmap_results)) {
+    NotifyFaviconUpdated(favicon_bitmap_results[0].icon_url);
+  }
   history_results_ = favicon_bitmap_results;
 }