Prioritize smaller favicons over larger ones for tabs, etc.

chrome/browser/favicon/favicon_handler.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/favicon/favicon_handler.h"
 
 #include "build/build_config.h"
 
 #include <vector>
 
@@ skipping 33 matching lines @@
 
 bool DoUrlAndIconMatch(const FaviconURL& favicon_url,
                        const GURL& url,
                        history::IconType icon_type) {
   return favicon_url.icon_url == url &&
       favicon_url.icon_type == static_cast<FaviconURL::IconType>(icon_type);
 }
 
 }  // namespace
 
+////////////////////////////////////////////////////////////////////////////////
+
 FaviconHandler::DownloadRequest::DownloadRequest()
     : icon_type(history::INVALID_ICON) {
 }
 
 FaviconHandler::DownloadRequest::~DownloadRequest() {
 }
 
 FaviconHandler::DownloadRequest::DownloadRequest(
     const GURL& url,
     const GURL& image_url,
     const FaviconTabHelper::ImageDownloadCallback& callback,
     history::IconType icon_type)
     : url(url),
       image_url(image_url),
       callback(callback),
       icon_type(icon_type) {
 }
 
+////////////////////////////////////////////////////////////////////////////////
+
+FaviconHandler::FaviconCandidate::FaviconCandidate()
+    : icon_type(history::INVALID_ICON) {
+}
+
+FaviconHandler::FaviconCandidate::~FaviconCandidate() {
+}
+
+FaviconHandler::FaviconCandidate::FaviconCandidate(
+    const GURL& url,
+    const GURL& image_url,
+    const gfx::Image& image,
+    history::IconType icon_type)
+    : url(url),
+      image_url(image_url),
+      image(image),
+      icon_type(icon_type) {
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
 FaviconHandler::FaviconHandler(Profile* profile,
                                FaviconHandlerDelegate* delegate,
                                Type icon_type)
     : got_favicon_from_history_(false),
       favicon_expired_(false),
       icon_types_(icon_type == FAVICON ? history::FAVICON :
           history::TOUCH_ICON | history::TOUCH_PRECOMPOSED_ICON),
       current_url_index_(0),
       profile_(profile),
       delegate_(delegate) {
@@ skipping 12 matching lines @@
 }
 
 void FaviconHandler::FetchFavicon(const GURL& url) {
   cancelable_consumer_.CancelAllRequests();
 
   url_ = url;
 
   favicon_expired_ = got_favicon_from_history_ = false;
   current_url_index_ = 0;
   urls_.clear();
+  favicon_candidates_.clear();
 
   // 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()) {
     GetFaviconForURL(url_, icon_types_, &cancelable_consumer_,
         base::Bind(&FaviconHandler::OnFaviconDataForInitialURL,
                    base::Unretained(this)));
   }
 }
 
 int FaviconHandler::DownloadImage(
     const GURL& image_url,
     int image_size,
     history::IconType icon_type,
     const FaviconTabHelper::ImageDownloadCallback& callback) {
   return ScheduleDownload(GURL(), image_url, image_size, icon_type, callback);
 }
 
 FaviconService* FaviconHandler::GetFaviconService() {
   return profile_->GetFaviconService(Profile::EXPLICIT_ACCESS);
 }
 
+void FaviconHandler::SetFaviconFromCandidates() {
+  if (favicon_candidates_.empty())
+    return;
+  const int preferred_size = preferred_icon_size();
+  FaviconCandidates::iterator match;
+  if (preferred_size == 0) {
+    // No size preference, use the first icon.
+    match = favicon_candidates_.begin();
+  } else {
+    // Use the icon closest to preferred_size, favoring sizes >= preferred_size.
+    int match_size = -1;
+    for (FaviconCandidates::iterator iter = favicon_candidates_.begin();
+         iter != favicon_candidates_.end(); ++iter) {
+      FaviconCandidate& candidate = *iter;
+      SkBitmap bitmap = *candidate.image.ToSkBitmap();
+      int bitmap_size = std::max(bitmap.width(), bitmap.height());
+      if (match_size < 0 ||
+          (bitmap_size >= preferred_size && bitmap_size < match_size) ||
+          (match_size < preferred_size && bitmap_size > match_size)) {
+        match = iter;
+        match_size = bitmap_size;
+        if (bitmap_size == preferred_size)
+          break;
+      }
+    }
+  }
+  SetFavicon(match->url, match->image_url, match->image, match->icon_type);
+  favicon_candidates_.clear();
+}
+
 void FaviconHandler::SetFavicon(
     const GURL& url,
     const GURL& image_url,
     const gfx::Image& image,
     history::IconType icon_type) {
-  const SkBitmap& bitmap = image;
+  SkBitmap bitmap = *image.ToSkBitmap();
   const gfx::Image& sized_image = (preferred_icon_size() == 0 ||
       (preferred_icon_size() == bitmap.width() &&
        preferred_icon_size() == bitmap.height())) ?
       image : ResizeFaviconIfNeeded(image);
 
   if (GetFaviconService() && ShouldSaveFavicon(url)) {
     std::vector<unsigned char> image_data;
     if (gfx::PNGEncodedDataFromImage(sized_image, &image_data))
       SetHistoryFavicon(url, image_url, image_data, icon_type);
   }
 
   if (url == url_ && icon_type == history::FAVICON) {
     NavigationEntry* entry = GetEntry();
-    if (entry)
+    if (entry) {
+      entry->GetFavicon().url = image_url;
       UpdateFavicon(entry, &sized_image);
+    }
   }
 }
 
 void FaviconHandler::UpdateFavicon(NavigationEntry* entry,
                                    scoped_refptr<RefCountedMemory> data) {
   scoped_ptr<gfx::Image> image(gfx::ImageFromPNGEncodedData(data->front(),
                                                             data->size()));
   UpdateFavicon(entry, image.get());
 }
 
@@ skipping 71 matching lines @@
   }
 
   if (!i->second.callback.is_null()) {
     i->second.callback.Run(id, errored, *(&image));
   } else if (current_candidate() &&
              DoUrlAndIconMatch(*current_candidate(), image_url,
                                i->second.icon_type)) {
     // The downloaded icon is still valid when there is no FaviconURL update
     // during the downloading.
     if (!errored) {
-      SetFavicon(i->second.url, image_url, image, i->second.icon_type);
-    } else if (GetEntry() && ++current_url_index_ < urls_.size()) {
-      // Copies all candidate except first one and notifies the FaviconHandler,
+      favicon_candidates_.push_back(FaviconCandidate(
+          i->second.url, image_url, image, i->second.icon_type));

[michaelbai, 2012/03/14 22:34:14]

Would you like stop fetch the rest of icons if the preferred_size == 
bitmap_size?

It will reduce the network access, and the right favicon could be stored before
the next page loaded.

[stevenjb, 2012/03/15 00:45:53]

Yes that would be better. I thought of this but didn't want to convert from
gfx::Image yet again. I think what I really need to do is cache the SkBitmap in
the Candidate.

Done.

[michaelbai, 2012/03/15 01:12:00]

It seemed you don't need favicon_candidates_ vector, just store the currently
best matched one while you downloading the icon

On 2012/03/15 00:45:53, stevenjb (chromium) wrote:

[stevenjb, 2012/03/15 21:05:10]

That's a fair point. I did another round of re-factoring.

+    }
+    if ((errored || preferred_icon_size() != 0) &&
+        (GetEntry() && ++current_url_index_ < urls_.size())) {
+      // Copies all candidates except the first and notifies the FaviconHandler,
       // so the next candidate can be processed.
       std::vector<FaviconURL> new_candidates(urls_.begin() + 1, urls_.end());
       OnUpdateFaviconURL(0, new_candidates);
+    } else {
+      // We have all candidates, choose one and set the favicon.
+      SetFaviconFromCandidates();
     }
   }
   download_requests_.erase(i);
 }
 
 NavigationEntry* FaviconHandler::GetEntry() {
   NavigationEntry* entry = delegate_->GetActiveEntry();
   if (entry && entry->GetURL() == url_)
     return entry;
 
@@ skipping 196 matching lines @@
   if (width > 0 && height > 0) {
     gfx::CalculateFaviconTargetSize(&width, &height);
     return gfx::Image(new SkBitmap(
                           skia::ImageOperations::Resize(
                               bitmap, skia::ImageOperations::RESIZE_LANCZOS3,
                               width, height)));
   }
 
   return image;
 }