Introduce dedicated enum value for icons from Web Manifests

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 <utility>
 #include <vector>
@@ skipping 187 matching lines @@
 }
 
 // static
 int FaviconHandler::GetIconTypesFromHandlerType(
     FaviconDriverObserver::NotificationIconType handler_type) {
   switch (handler_type) {
     case FaviconDriverObserver::NON_TOUCH_16_DIP:
     case FaviconDriverObserver::NON_TOUCH_LARGEST:
       return favicon_base::FAVICON;
     case FaviconDriverObserver::TOUCH_LARGEST:
-      return favicon_base::TOUCH_ICON | favicon_base::TOUCH_PRECOMPOSED_ICON;
+      return favicon_base::TOUCH_ICON | favicon_base::TOUCH_PRECOMPOSED_ICON |
+             favicon_base::WEB_MANIFEST_ICON;
   }
   return 0;
 }
 
 void FaviconHandler::FetchFavicon(const GURL& url) {
   cancelable_task_tracker_for_page_url_.TryCancelAll();
   cancelable_task_tracker_for_candidates_.TryCancelAll();
 
   url_ = url;
 
@@ skipping 130 matching lines @@
 
   // If no manifest available, proceed with the regular candidates only.
   if (manifest_url_.is_empty()) {
     OnGotFinalIconURLCandidates(candidates);
     return;
   }
 
   // See if there is a cached favicon for the manifest. This will update the DB
   // mappings only if the manifest URL is cached.
   GetFaviconAndUpdateMappingsUnlessIncognito(
-      /*icon_url=*/manifest_url_, favicon_base::FAVICON,
+      /*icon_url=*/manifest_url_, favicon_base::WEB_MANIFEST_ICON,
       base::Bind(&FaviconHandler::OnFaviconDataForManifestFromFaviconService,
                  base::Unretained(this)));
 }
 
 void FaviconHandler::OnFaviconDataForManifestFromFaviconService(
     const std::vector<favicon_base::FaviconRawBitmapResult>&
         favicon_bitmap_results) {
   // The database lookup for the page URL is guaranteed to be completed because
   // the HistoryBackend uses a SequencedTaskRunner, and we also know that
   // FetchFavicon() was called before OnUpdateCandidates().
   DCHECK(got_favicon_from_history_);
 
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
   bool has_expired_or_incomplete_result =
       !has_valid_result || HasExpiredOrIncompleteResult(preferred_icon_size(),
                                                         favicon_bitmap_results);
 
-  if (has_valid_result && (notification_icon_url_ != manifest_url_ ||
-                           notification_icon_type_ != favicon_base::FAVICON)) {
+  if (has_valid_result &&
+      (notification_icon_url_ != manifest_url_ ||
+       notification_icon_type_ != favicon_base::WEB_MANIFEST_ICON)) {
     // There is a valid favicon. Notify any observers. It is useful to notify
     // the observers even if the favicon is expired or incomplete (incorrect
     // size) because temporarily showing the user an expired favicon or
     // streched favicon is preferable to showing the user the default favicon.
     NotifyFaviconUpdated(favicon_bitmap_results);
   }
 
   if (has_expired_or_incomplete_result) {
     manifest_download_request_.Reset(base::Bind(
         &FaviconHandler::OnDidDownloadManifest, base::Unretained(this)));
     delegate_->DownloadManifest(manifest_url_,
                                 manifest_download_request_.callback());
   }
 }
 
 void FaviconHandler::OnDidDownloadManifest(
     const std::vector<FaviconURL>& candidates) {

[pkotwicz, 2017/05/24 14:27:44]

Shouldn't |candidates| be of type favicon_base::WEB_MANIFEST_ICON?

I am referring to ExtractManifestIcons() in content_favicon_driver.cc

[mastiz, 2017/05/25 11:14:16]

Done, changed. It's certainly cleaner this way, although the field is ignored
altogether.

   // Mark manifest download as finished.
   manifest_download_request_.Cancel();
 
   if (!candidates.empty()) {
     OnGotFinalIconURLCandidates(candidates);
     return;
   }
 
   // If either the downloading of the manifest failed, OR the manifest contains
   // no icons, proceed with the list of icons listed in the HTML.
@@ skipping 113 matching lines @@
     // OnDidDownloadFavicon() can only be called after requesting a download, so
     // |num_image_download_requests_| can never be 0.
     RecordDownloadAttemptsForHandlerType(handler_type_,
                                          num_image_download_requests_);
     // 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. The
       // manifest URL, if available, is used instead of the icon URL.
       SetFavicon(manifest_url_.is_empty() ? best_favicon_.candidate.icon_url
                                           : manifest_url_,
-                 best_favicon_.image, best_favicon_.candidate.icon_type);
+                 best_favicon_.image,
+                 manifest_url_.is_empty() ? best_favicon_.candidate.icon_type
+                                          : favicon_base::WEB_MANIFEST_ICON);

[pkotwicz, 2017/05/24 14:27:44]

You need to update HistoryBackend::SetFaviconMappingsForPage() so that saving an
icon of type WEB_MANIFEST_ICON deletes icons of type TOUCH_ICON and
TOUCH_PRECOMPOSED_ICON

[mastiz, 2017/05/25 11:14:16]

Done.

     }
     // Clear download related state.
     current_candidate_index_ = candidates_.size();
     num_image_download_requests_ = 0;
     best_favicon_ = DownloadedFavicon();
   }
 }
 
 const std::vector<GURL> FaviconHandler::GetIconURLs() const {
   std::vector<GURL> icon_urls;
@@ skipping 123 matching lines @@
   // A max bitmap size is specified to avoid receiving huge bitmaps in
   // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
   // for more details about the max bitmap size.
   const int download_id =
       delegate_->DownloadImage(image_url, GetMaximalIconSize(handler_type_),
                                image_download_request_.callback());
   DCHECK_NE(download_id, 0);
 }
 
 }  // namespace favicon