Save large icons to a new LARGE_ICON type

components/favicon/core/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 "components/favicon/core/favicon_handler.h"
 
 #include <algorithm>
 #include <cmath>
 #include <vector>
 
@@ skipping 207 matching lines @@
 
 FaviconHandler::~FaviconHandler() {
 }
 
 // static
 int FaviconHandler::GetIconTypesFromHandlerType(
     FaviconHandler::Type handler_type) {
   switch (handler_type) {
     case FAVICON:
       return favicon_base::FAVICON;
-    case TOUCH:  // Falls through.
+    case TOUCH:
+      return favicon_base::TOUCH_ICON | favicon_base::TOUCH_PRECOMPOSED_ICON;
     case LARGE:
-      return favicon_base::TOUCH_ICON | favicon_base::TOUCH_PRECOMPOSED_ICON;
+      return favicon_base::FAVICON | favicon_base::TOUCH_ICON |

[sky, 2015/05/06 19:50:42]

Why do we need FAVICON here too? If you use FAVICON here won't that mean you'll
have two FaviconHandlers attempting to download FAVICON?

[Roger McFarlane (Chromium), 2015/05/06 21:22:51]

Yes and no.

Attempts to get the existing FAVICON and TOUCH handlers to do all the work were
unsuccessful (see https://codereview.chromium.org/1118073002/) because the
handler seems to only capture a single representation of the icon. I.e., if you
tell the regular favicon handler to capture a large icon, it forgoes the 16x16
version which is needed for tab rendering.

If you have any suggestions on how to fix that, I'd be glad to hear them and go
back to that approach.

But, it's not quite as bad as it may seem...

The issue you've brought up occurs when the small and large icon are in the same
file (e.g., an ICO file that stores both the small and the large images or a
single one-size fits all icon). In that case, the downside is that the file will
be downloaded twice (because the image fetcher bypasses the cache) but the
handler will (potentially) extract different representations from it.

In the more general case, the small and large icon are in different files,
possibly across different icon types. In this case, multiple downloads and
handlers are exactly what is needed.

In both cases, you want the bitmaps and the mappings to be updated indepedently
for the small and large icons, hence their treatment as different icon types by
the backend.

+          favicon_base::TOUCH_PRECOMPOSED_ICON;
     default:
       NOTREACHED();
   }
   return 0;
 }
 
 void FaviconHandler::FetchFavicon(const GURL& url) {
   cancelable_task_tracker_.TryCancelAll();
 
   url_ = url;
@@ skipping 110 matching lines @@
 
   // TODO(davemoore) Should clear on empty url. Currently we ignore it.
   // This appears to be what FF does as well.
   if (!image_urls_.empty())
     ProcessCurrentUrl();
 }
 
 void FaviconHandler::ProcessCurrentUrl() {
   DCHECK(!image_urls_.empty());
 
+  // TODO(rogerm): To use fetch on demand for large icons: just add the Favicon
+  //     record and update the url mappings here, instead of downloading.
+  //     i.e., if (handler_type_ == LARGE) { Save(image_urls[0]); return; }
+
   // current_candidate() may return NULL if download_largest_icon_ is true and
   // all the sizes are larger than the max.
   if (PageChangedSinceFaviconWasRequested() || !current_candidate())
     return;
 
   if (current_candidate()->icon_type == favicon_base::FAVICON &&
       !download_largest_icon_) {
     if (!favicon_expired_or_incomplete_ &&
         driver_->GetActiveFaviconValidity() &&
         DoUrlAndIconMatch(*current_candidate(),
@@ skipping 69 matching lines @@
     }
   }
 
   if (request_next_icon && image_urls_.size() > 1) {
     // Remove the first member of image_urls_ and process the remaining.
     image_urls_.erase(image_urls_.begin());
     ProcessCurrentUrl();
   } else {
     // We have either found the ideal candidate or run out of candidates.
     if (best_favicon_candidate_.icon_type != favicon_base::INVALID_ICON) {
-      // No more icons to request, set the favicon from the candidate.
+      // If we're specifically handling large icons, save the icon using the
+      // virtual LARGE_ICON type, otherwise save the icon using its true type.
+      favicon_base::IconType saved_type = handler_type_ == LARGE ?
+          favicon_base::LARGE_ICON : best_favicon_candidate_.icon_type;
       SetFavicon(best_favicon_candidate_.url, best_favicon_candidate_.image_url,
-                 best_favicon_candidate_.image,
-                 best_favicon_candidate_.icon_type);
+                 best_favicon_candidate_.image, saved_type);
     }
     // Clear download related state.
     image_urls_.clear();
     download_requests_.clear();
     best_favicon_candidate_ = FaviconCandidate();
   }
 }
 
 bool FaviconHandler::HasPendingTasksForTest() {
   return !download_requests_.empty() ||
@@ skipping 69 matching lines @@
 
 bool FaviconHandler::ShouldSaveFavicon(const GURL& url) {
   if (!driver_->IsOffTheRecord())
     return true;
 
   // Always save favicon if the page is bookmarked.
   return driver_->IsBookmarked(url);
 }
 
 int FaviconHandler::GetMaximalIconSize(favicon_base::IconType icon_type) {
+  if (handler_type_ == FaviconHandler::LARGE) {
+    // If we're only concerned with saving large icons, return a fixed large
+    // icon size, unless something is wrong with the icon type.
+    return icon_type == favicon_base::INVALID_ICON ? 0 : 192;
+  }
+
   switch (icon_type) {
     case favicon_base::FAVICON:
 #if defined(OS_ANDROID)
       return 192;
 #else
       return gfx::ImageSkia::GetMaxSupportedScale() * gfx::kFaviconSize;
 #endif
     case favicon_base::TOUCH_ICON:
     case favicon_base::TOUCH_PRECOMPOSED_ICON:
       return kTouchIconSize;
+    case favicon_base::LARGE_ICON:
+      // This method is called to get the maximal size when we're about to
+      // request the download of a concrete icon type. LARGE_ICON is a virtual
+      // type, so we're not expecting a call to get a maximal size to downlaod.

[sky, 2015/05/06 19:50:42]

downlaod->download

[Roger McFarlane (Chromium), 2015/05/06 21:22:51]

Will do.

+      NOTREACHED();
+      return 192;
     case favicon_base::INVALID_ICON:
       return 0;
   }
   NOTREACHED();
   return 0;
 }
 
 void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
     const std::vector<favicon_base::FaviconRawBitmapResult>&
         favicon_bitmap_results) {
@@ skipping 143 matching lines @@
     gfx::Size largest =
         image_url.icon_sizes[GetLargestSizeIndex(image_url.icon_sizes)];
     image_url.icon_sizes.clear();
     image_url.icon_sizes.push_back(largest);
   }
   std::stable_sort(image_urls_.begin(), image_urls_.end(),
                    CompareIconSize);
 }
 
 }  // namespace favicon