Support touch icon in FaviconHelper

chrome/browser/favicon_helper.cc

Index: chrome/browser/favicon_helper.cc
diff --git a/chrome/browser/favicon_helper.cc b/chrome/browser/favicon_helper.cc
index 7ad421ca0381a5c057d12b406763cb916c1f2329..5ad86b46b50b8374d996b634710c9a04466504a2 100644
--- a/chrome/browser/favicon_helper.cc
+++ b/chrome/browser/favicon_helper.cc
@@ -22,11 +22,15 @@
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
 
-FaviconHelper::FaviconHelper(TabContents* tab_contents)
+FaviconHelper::FaviconHelper(TabContents* tab_contents,
+                             Types icon_type)
     : TabContentsObserver(tab_contents),
-      got_favicon_url_(false),
       got_favicon_from_history_(false),
-      favicon_expired_(false) {
+      favicon_expired_(false),
+      icon_types_(icon_type == FAVICON ? history::FAVICON :
+          history::TOUCH_ICON | history::TOUCH_PRECOMPOSED_ICON),
+      current_url_index_(0),
+      preferred_icon_size_(icon_type == FAVICON ? kFaviconSize : 0) {
 }
 
 FaviconHelper::~FaviconHelper() {
@@ -46,13 +50,15 @@ void FaviconHelper::FetchFavicon(const GURL& url) {
 
   url_ = url;
 
-  favicon_expired_ = got_favicon_from_history_ = got_favicon_url_ = false;
+  favicon_expired_ = got_favicon_from_history_ = false;
+  current_url_index_ = 0;
+  urls_.empty();
 
   // Request the favicon from the history service. In parallel to this the
   // renderer is going to notify us (well TabContents) when the favicon url is
   // available.
   if (GetFaviconService()) {
-    GetFaviconService()->GetFaviconForURL(url_, history::FAVICON,
+    GetFaviconService()->GetFaviconForURL(url_, icon_types_,
         &cancelable_consumer_,
         NewCallback(this, &FaviconHelper::OnFaviconDataForInitialURL));
   }
@@ -60,9 +66,10 @@ void FaviconHelper::FetchFavicon(const GURL& url) {
 
 int FaviconHelper::DownloadImage(const GURL& image_url,
                                  int image_size,
+                                 history::IconType icon_type,
                                  ImageDownloadCallback* callback) {
   DCHECK(callback);  // Must provide a callback.
-  return ScheduleDownload(GURL(), image_url, image_size, callback);
+  return ScheduleDownload(GURL(), image_url, image_size, icon_type, callback);
 }
 
 Profile* FaviconHelper::profile() {
@@ -76,19 +83,20 @@ FaviconService* FaviconHelper::GetFaviconService() {
 void FaviconHelper::SetFavicon(
     const GURL& url,
     const GURL& image_url,
-    const SkBitmap& image) {
-  const SkBitmap& sized_image =
-      (image.width() == kFaviconSize && image.height() == kFaviconSize)
-      ? image : ConvertToFaviconSize(image);
+    const SkBitmap& 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);
 
   if (GetFaviconService() && ShouldSaveFavicon(url)) {
     std::vector<unsigned char> image_data;
     gfx::PNGCodec::EncodeBGRASkBitmap(sized_image, false, &image_data);
-    GetFaviconService()->SetFavicon(url, image_url, image_data,
-                                    history::FAVICON);
+    GetFaviconService()->SetFavicon(url, image_url, image_data, icon_type);
   }
 
-  if (url == url_) {
+  if (url == url_ && icon_type == history::FAVICON) {
     NavigationEntry* entry = GetEntry();
     if (entry)
       UpdateFavicon(entry, sized_image);
@@ -114,41 +122,64 @@ void FaviconHelper::UpdateFavicon(NavigationEntry* entry,
   tab_contents()->NotifyNavigationStateChanged(TabContents::INVALIDATE_TAB);
 }
 
-void FaviconHelper::OnUpdateFaviconURL(int32 page_id, const GURL& icon_url) {
-  // TODO(davemoore) Should clear on empty url. Currently we ignore it.
-  // This appears to be what FF does as well.
-  if (icon_url.is_empty())
-    return;
-
+void FaviconHelper::OnUpdateFaviconURL(
+    int32 page_id,
+    const std::vector<FaviconURL>& candidates) {
   NavigationEntry* entry = GetEntry();
   if (!entry)
     return;
 
-  got_favicon_url_ = true;
+  bool got_favicon_url_update = false;
+  for (std::vector<FaviconURL>::const_iterator i = candidates.begin();
+       i != candidates.end(); ++i) {
+    if (!i->icon_url.is_empty() && (i->icon_type & icon_types_)) {
+      if (!got_favicon_url_update) {
+        got_favicon_url_update = true;
+        urls_.empty();
+        current_url_index_ = 0;
+      }
+      urls_.push_back(*i);
+    }
+  }
+
+  // TODO(davemoore) Should clear on empty url. Currently we ignore it.
+  // This appears to be what FF does as well.
+  // No URL was added.
+  if (!got_favicon_url_update)
+    return;
 
   if (!GetFaviconService())
     return;
 
-  if (!favicon_expired_ && entry->favicon().is_valid() &&
-      entry->favicon().url() == icon_url) {
-    // We already have the icon, no need to proceed.
+  // For FAVICON.
+  if (!favicon_expired_ && current_candidate()->icon_type == ::FAVICON &&
+      entry->favicon().is_valid() &&
+      do_url_and_icon_match(*current_candidate(), entry->favicon().url(),
+                            history::FAVICON))
     return;
-  }
 
-  entry->favicon().set_url(icon_url);
+  if (current_candidate()->icon_type == ::FAVICON)
+    entry->favicon().set_url(current_candidate()->icon_url);
+
+  // For the icons other than FAVICON.
+  if (!favicon_expired_ && current_candidate()->icon_type != ::FAVICON &&
+      got_favicon_from_history_ && history_icon_.is_valid() &&
+      do_url_and_icon_match(*current_candidate(), history_icon_.icon_url,
+                            history_icon_.icon_type))
+    return;
 
   if (got_favicon_from_history_)
-    DownloadFaviconOrAskHistory(entry);
+    DownloadFaviconOrAskHistory(entry->url(), current_candidate()->icon_url,
+         static_cast<history::IconType>(current_candidate()->icon_type));
 }
 
 bool FaviconHelper::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
+  bool message_handled = true;
   IPC_BEGIN_MESSAGE_MAP(FaviconHelper, message)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateFaviconURL, OnUpdateFaviconURL)
     IPC_MESSAGE_HANDLER(ViewHostMsg_DidDownloadFavicon, OnDidDownloadFavicon)
-    IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_MESSAGE_UNHANDLED(message_handled = false)
   IPC_END_MESSAGE_MAP()
-  return handled;
+  return message_handled;
 }
 
 void FaviconHelper::OnDidDownloadFavicon(int id,
@@ -165,9 +196,11 @@ void FaviconHelper::OnDidDownloadFavicon(int id,
   if (i->second.callback) {
     i->second.callback->Run(id, errored, image);
   } else if (!errored) {
-    SetFavicon(i->second.url, image_url, image);
+    SetFavicon(i->second.url, image_url, image, i->second.icon_type);
+  } else if (++current_url_index_ < urls_.size() &&

[sky, 2011/03/23 22:49:31]

What happens if between the time we ask for an icon the set of icons changes
(OnUpdateFaviconURL). Wouldn't we incorrectly increment current_url_index_?

[michaelbai, 2011/03/23 23:26:49]

Fixed, thanks very much!

+        GetEntry()) {
+      FetchFavicon(current_candidate()->icon_url);

[sky, 2011/03/23 22:49:31]

If we get here you've incremented current_url_index_, but FetchFavicon resets it
and clears out the list.

[michaelbai, 2011/03/23 23:26:49]

Sorry, I should pass the page_url in.

   }
-
   download_requests_.erase(i);
 }
 
@@ -182,19 +215,21 @@ NavigationEntry* FaviconHelper::GetEntry() {
   return NULL;
 }
 
-void FaviconHelper::OnFaviconDataForInitialURL(
-    FaviconService::Handle handle,
-    history::FaviconData favicon) {
+void FaviconHelper::OnFaviconDataForInitialURL(FaviconService::Handle handle,
+                                               history::FaviconData favicon) {
   NavigationEntry* entry = GetEntry();
   if (!entry)
     return;
 
   got_favicon_from_history_ = true;
+  history_icon_ = favicon;
 
   favicon_expired_ = (favicon.known_icon && favicon.expired);
 
-  if (favicon.known_icon && !entry->favicon().is_valid() &&
-      (!got_favicon_url_ || entry->favicon().url() == favicon.icon_url)) {
+  if (favicon.known_icon && favicon.icon_type == history::FAVICON &&
+      entry->favicon().is_valid() &&
+      (!current_candidate() || do_url_and_icon_match(*current_candidate(),
+                                  favicon.icon_url, favicon.icon_type))) {
     // 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
@@ -206,35 +241,37 @@ void FaviconHelper::OnFaviconDataForInitialURL(
   }
 
   if (favicon.known_icon && !favicon.expired) {
-    if (got_favicon_url_ && entry->favicon().url() != favicon.icon_url) {
-      // Mapping in the database is wrong. DownloadFaviconOrAskHistory will
+    if (current_candidate() && !do_url_and_icon_match(*current_candidate(),
+                                   favicon.icon_url, favicon.icon_type)) {
+      // Mapping in the database is wrong. DownloadFavIconOrAskHistory will
       // update the mapping for this url and download the favicon if we don't
       // already have it.
-      DownloadFaviconOrAskHistory(entry);
+      DownloadFaviconOrAskHistory(entry->url(), current_candidate()->icon_url,
+          static_cast<history::IconType>(current_candidate()->icon_type));
     }
-  } else if (got_favicon_url_) {
+  } else if (current_candidate()) {
     // We know the official url for the favicon, by either don't have the
     // favicon or its expired. Continue on to DownloadFaviconOrAskHistory to
     // either download or check history again.
-    DownloadFaviconOrAskHistory(entry);
+    DownloadFaviconOrAskHistory(entry->url(), current_candidate()->icon_url,
+        static_cast<history::IconType>(current_candidate()->icon_type));
   }
   // else we haven't got the icon url. When we get it we'll ask the
   // renderer to download the icon.
 }
 
-void FaviconHelper::DownloadFaviconOrAskHistory(NavigationEntry* entry) {
-  DCHECK(entry);  // We should only get here if entry is valid.
+void FaviconHelper::DownloadFaviconOrAskHistory(const GURL& page_url,
+                                                const GURL& icon_url,
+                                                history::IconType icon_type) {
   if (favicon_expired_) {
     // We have the mapping, but the favicon is out of date. Download it now.
-    ScheduleDownload(entry->url(), entry->favicon().url(), kFaviconSize, NULL);
+    ScheduleDownload(page_url, icon_url, preferred_icon_size_, icon_type, NULL);
   } else if (GetFaviconService()) {
     // 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 (profile()->IsOffTheRecord()) {
-      GetFaviconService()->GetFavicon(
-          entry->favicon().url(),
-          history::FAVICON,
+      GetFaviconService()->GetFavicon(icon_url, icon_type,
           &cancelable_consumer_,
           NewCallback(this, &FaviconHelper::OnFaviconData));
     } else {
@@ -244,42 +281,46 @@ void FaviconHelper::DownloadFaviconOrAskHistory(NavigationEntry* entry) {
       //    include the mapping between the page url and the favicon url.
       // This is asynchronous. The history service will call back when done.
       // Issue the request and associate the current page ID with it.
-      GetFaviconService()->UpdateFaviconMappingAndFetch(
-          entry->url(),
-          entry->favicon().url(),
-          history::FAVICON,
-          &cancelable_consumer_,
+      GetFaviconService()->UpdateFaviconMappingAndFetch(page_url, icon_url,
+          icon_type, &cancelable_consumer_,
           NewCallback(this, &FaviconHelper::OnFaviconData));
     }
   }
 }
 
-void FaviconHelper::OnFaviconData(
-    FaviconService::Handle handle,
-    history::FaviconData favicon) {
+void FaviconHelper::OnFaviconData(FaviconService::Handle handle,
+                                  history::FaviconData favicon) {
+  // We shouldn't be here if there is no candidate.
+  DCHECK(current_candidate());
   NavigationEntry* entry = GetEntry();
   if (!entry)
     return;
 
   // No need to update the favicon url. By the time we get here
   // UpdateFaviconURL will have set the favicon url.
-
-  if (favicon.is_valid()) {
+  if (favicon.is_valid() && favicon.icon_type == history::FAVICON) {
     // 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.
     UpdateFavicon(entry, favicon.image_data);
   }
 
-  if (!favicon.known_icon || favicon.expired) {
-    // We don't know the favicon, or it is out of date. Request the current one.
-    ScheduleDownload(entry->url(), entry->favicon().url(), kFaviconSize, NULL);
+  if (!favicon.known_icon || favicon.expired ||
+      !do_url_and_icon_match(*current_candidate(), favicon.icon_url,
+                             favicon.icon_type)) {
+    // We don't know the favicon, it is out of date or its type is not same as
+    // one got from page. Request the current one.
+    ScheduleDownload(entry->url(), current_candidate()->icon_url,
+        preferred_icon_size_,
+        static_cast<history::IconType>(current_candidate()->icon_type), NULL);
   }
+  history_icon_ = favicon;
 }
 
 int FaviconHelper::ScheduleDownload(const GURL& url,
                                     const GURL& image_url,
                                     int image_size,
+                                    history::IconType icon_type,
                                     ImageDownloadCallback* callback) {
   const int download_id = tab_contents()->render_view_host()->DownloadFavicon(
       image_url, image_size);
@@ -287,7 +328,8 @@ int FaviconHelper::ScheduleDownload(const GURL& url,
   if (download_id) {
     // Download ids should be unique.
     DCHECK(download_requests_.find(download_id) == download_requests_.end());
-    download_requests_[download_id] = DownloadRequest(url, image_url, callback);
+    download_requests_[download_id] =
+        DownloadRequest(url, image_url, callback, icon_type);
   }
 
   return download_id;
@@ -297,7 +339,7 @@ SkBitmap FaviconHelper::ConvertToFaviconSize(const SkBitmap& image) {
   int width = image.width();
   int height = image.height();
   if (width > 0 && height > 0) {
-    calc_favicon_target_size(&width, &height);
+    calc_favicon_target_size(preferred_icon_size_, &width, &height);
     return skia::ImageOperations::Resize(
           image, skia::ImageOperations::RESIZE_LANCZOS3,
           width, height);