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 6ff45ea2d872c6959ecbe4423688856ba238e0ca..f58e9e767739e205e0907e42e418e792974a92b4 100644
--- a/chrome/browser/favicon_helper.cc
+++ b/chrome/browser/favicon_helper.cc
@@ -20,13 +20,18 @@
 #include "content/browser/tab_contents/tab_contents.h"
 #include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
-#include "ui/gfx/favicon_size.h"
 
-FaviconHelper::FaviconHelper(TabContents* tab_contents)
-    : TabContentsObserver(tab_contents),
-      got_favicon_url_(false),
+FaviconHelper::FaviconHelper(TabContents* tab_contents,
+                             int supported_icon_types,
+                             FaviconDelegate* delegate)
+    : TabContentsObserver(tab_contents)
       got_favicon_from_history_(false),
-      favicon_expired_(false) {
+      favicon_expired_(false),
+      supported_icon_types_(supported_icon_types),
+      message_handled_(false),
+      favicon_delegate_(delegate),
+      preferred_icon_size_(favicon_delegate_.get() ?
+          favicon_delegate_->GetIconSize() : 0 ) {
 }
 
 FaviconHelper::~FaviconHelper() {
@@ -41,18 +46,20 @@ FaviconHelper::~FaviconHelper() {
   }
 }
 
-void FaviconHelper::FetchFavicon(const GURL& url) {
+void FaviconHelper::FetchFavicon(const GURL& url, int icon_types) {
   cancelable_consumer_.CancelAllRequests();
 
   url_ = url;
 
-  favicon_expired_ = got_favicon_from_history_ = got_favicon_url_ = false;
+  favicon_expired_ = got_favicon_from_history_ = false;
+  current_candidate_idx_ = 0;
+  favicon_candidates_.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 +67,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,79 +84,69 @@ 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 = (!favicon_delegate_.get()) ?
+      image : favicon_delegate_->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_) {
     NavigationEntry* entry = GetEntry();
-    if (entry)
-      UpdateFavicon(entry, sized_image);
+    if (entry && favicon_delegate_.get())
+      favicon_delegate_->UpdateFaviconImageData(entry, sized_image);
   }
 }
 
-void FaviconHelper::UpdateFavicon(NavigationEntry* entry,
-                                  scoped_refptr<RefCountedMemory> data) {
-  SkBitmap image;
-  gfx::PNGCodec::Decode(data->front(), data->size(), &image);
-  UpdateFavicon(entry, image);
-}
-
-void FaviconHelper::UpdateFavicon(NavigationEntry* entry,
-                                  const SkBitmap& image) {
-  // No matter what happens, we need to mark the favicon as being set.
-  entry->favicon().set_is_valid(true);
-
-  if (image.empty())
-    return;
+void FaviconHelper::OnUpdateFaviconURL(
+    int32 page_id,
+    std::vector<FaviconCandidate>& candidates) {
+  std::vector<FaviconCandidate> my_candidates;
+  for (std::vector<FaviconCandidate>::iterator i = candidates.begin();
+       i != candidates.end(); +i) {

[sky, 2011/03/18 17:33:37]

++i

[michaelbai, 2011/03/22 18:14:13]

Done.

+    if (!i->icon_url.is_empty() && (i->icon_type & supported_icon_types_)) {
+      my_candidates.push_back(*i);
+      // Remove the candidate we handled.
+      candidates.erase(i);
+    }
+  }
 
-  entry->favicon().set_bitmap(image);
-  tab_contents()->NotifyNavigationStateChanged(TabContents::INVALIDATE_TAB);
-}
+  message_handled_ = (candidates.size() == 0);
 
-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())
+  if (my_candidates.size() == 0)

[sky, 2011/03/18 17:33:37]

How come you have both my_candidates and favicon_candidates_. Can't you just use
favicon_candidates_?

[michaelbai, 2011/03/22 18:14:13]

I thought the coming icons might not the one this FaviconHelper could handle.
But, it seems I could get ride of it

     return;
 
   NavigationEntry* entry = GetEntry();
   if (!entry)
     return;
 
-  got_favicon_url_ = true;
-
-  if (!GetFaviconService())
-    return;
-
-  if (!favicon_expired_ && entry->favicon().is_valid() &&
-      entry->favicon().url() == icon_url) {
-    // We already have the icon, no need to proceed.
-    return;
-  }
-
-  entry->favicon().set_url(icon_url);
-
-  if (got_favicon_from_history_)
-    DownloadFaviconOrAskHistory(entry);
+  favicon_candidates_ = my_candidates;
+  current_candidate_idx_ = 0;
+
+  // DownloadFaviconOrAskHistory() only if we have gotten FaviconData from
+  // history and it's url or type is not same as current Candidate.
+  if (GetFaviconService() && got_favicon_from_history_ &&
+      (!history_icon_.is_valid() ||
+          !Candidate()->is_same(history_icon_.icon_url,
+                                history_icon_.icon_type)))
+    DownloadFaviconOrAskHistory(entry->url(), Candidate()->icon_url,
+                                Candidate()->icon_type);
 }
 
 bool FaviconHelper::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
+  message_handled_ = true;

[sky, 2011/03/18 17:33:37]

YUCK! There has to be a better way to handle this. Talk to John (jabdelmalek),
he's the IPC master.

[jam, 2011/03/18 18:40:45]

There isn't a super clean way of doing this (it's pretty rare).  I think it's
better to avoid a member variable.  What we've done in these places is to take
that message out of the message map and then afterwards do:

if (message.type()) == OnUpdateFaviconURL::ID)
and then unpack the message manually (i don't have the syntax infront of me, can
look it up later if we need it).

one question: where is this message handled if your return false here?  can't
the sender send different messages depending on where they want it to go?

[michaelbai, 2011/03/18 19:18:38]

Favicon is a TabContentsObserver. TabContents just forwards the message to each
observers until the message is handled(TabContents::OnMessageReceived).

I think TabContent shouldn't unpack the message (might split into 2 message) and
forward to the proper FaviconHelper.

Do you think it might a little over skill to unpack message manually for
avoiding a member variable. I am sorry if I feel about this for I am not
familiar with IPC.

   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 +163,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_candidate_idx_ < favicon_candidates_.size()

[sky, 2011/03/18 17:33:37]

&& should be on this line.

[michaelbai, 2011/03/22 18:14:13]

Done.

+      && GetEntry()) {
+      FetchFavicon(Candidate()->icon_url, Candidate()->icon_type);

[sky, 2011/03/18 17:33:37]

indentation is wrong.

[michaelbai, 2011/03/22 18:14:13]

Done.

   }
-
   download_requests_.erase(i);
 }
 
@@ -182,59 +182,62 @@ 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);
+  NavigationEntry::FaviconStatus& favicon_status = entry->GetFavicon(
+      favicon.icon_type);
 
-  if (favicon.known_icon && !entry->favicon().is_valid() &&
-      (!got_favicon_url_ || entry->favicon().url() == favicon.icon_url)) {
+  if (favicon.known_icon && favicon_delegate_.get() && (!Candidate() ||
+      Candidate()->is_same(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
     // user doesn't see a flash of the default favicon.
-    entry->favicon().set_url(favicon.icon_url);
-    if (favicon.is_valid())
-      UpdateFavicon(entry, favicon.image_data);
-    entry->favicon().set_is_valid(true);
+    favicon_delegate_->UpdateFaviconImageData(entry, favicon.icon_url,
+                                              favicon.image_data);
   }
 
   if (favicon.known_icon && !favicon.expired) {
-    if (got_favicon_url_ && entry->favicon().url() != favicon.icon_url) {
+    if (Candidate() && ( Candidate()->icon_url != favicon.icon_url ||
+        Candidate()->icon_type() != 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(), Candidate()->icon_url,
+                                  Candidate()->icon_type);
     }
-  } else if (got_favicon_url_) {
+  } else if (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(), Candidate()->icon_url,
+                                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 +247,44 @@ 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(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_delegate_.get()) {
     // 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);
+    favicon_delegate_->UpdateFaviconImageData(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 ||
+      !Candidate()->is_same(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(), Candidate()->icon_url, preferred_icon_size_,
+                     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,24 +292,13 @@ 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;
 }
 
-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);
-    return skia::ImageOperations::Resize(
-          image, skia::ImageOperations::RESIZE_LANCZOS3,
-          width, height);
-  }
-  return image;
-}
-
 bool FaviconHelper::ShouldSaveFavicon(const GURL& url) {
   if (!profile()->IsOffTheRecord())
     return true;