Move WasUnableToDownload logic to FaviconHandler

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 308 matching lines @@
 
 void FaviconHandler::SetFavicon(const GURL& icon_url,
                                 const gfx::Image& image,
                                 favicon_base::IconType icon_type) {
   if (ShouldSaveFavicon())
     SetHistoryFavicons(url_, icon_url, icon_type, image);
 
   NotifyFaviconUpdated(icon_url, icon_type, image);
 }
 
+void FaviconHandler::CurrentCandidateFinished(bool exact_match_found) {
+  if (!exact_match_found && current_candidate_index_ + 1 < image_urls_.size()) {
+    // Process the next candidate.
+    ++current_candidate_index_;
+    DownloadCurrentCandidateOrAskFaviconService();
+  } 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.
+      SetFavicon(best_favicon_candidate_.image_url,
+                 best_favicon_candidate_.image,
+                 best_favicon_candidate_.icon_type);
+    }
+    // Clear download related state.
+    download_requests_.clear();
+    current_candidate_index_ = image_urls_.size();
+    best_favicon_candidate_ = FaviconCandidate();
+  }
+}
+
 void FaviconHandler::NotifyFaviconUpdated(
     const std::vector<favicon_base::FaviconRawBitmapResult>&
         favicon_bitmap_results) {
   if (favicon_bitmap_results.empty())
     return;
 
   gfx::Image resized_image = favicon_base::SelectFaviconFramesFromPNGs(
       favicon_bitmap_results,
       favicon_base::GetFaviconScales(),
       preferred_icon_size());
@@ skipping 96 matching lines @@
   DownloadRequest download_request = i->second;
   download_requests_.erase(i);
 
   if (!current_candidate() ||
       !DoUrlAndIconMatch(*current_candidate(),
                          image_url,
                          download_request.icon_type)) {
     return;
   }
 
-  bool request_next_icon = true;
+  bool exact_match_found = false;
   if (!bitmaps.empty()) {
     float score = 0.0f;
     gfx::ImageSkia image_skia;
     if (download_largest_icon_) {
       int index = -1;
       // Use the largest bitmap if FaviconURL doesn't have sizes attribute.
       if (current_candidate()->icon_sizes.empty()) {
         index = GetLargestSizeIndex(original_bitmap_sizes);
       } else {
         index = GetIndexBySize(original_bitmap_sizes,
                                current_candidate()->icon_sizes[0]);
         // Find largest bitmap if there is no one exactly matched.
         if (index == -1)
           index = GetLargestSizeIndex(original_bitmap_sizes);
       }
       image_skia = gfx::ImageSkia(gfx::ImageSkiaRep(bitmaps[index], 1));
     } else {
       image_skia = CreateFaviconImageSkia(bitmaps,
                                           original_bitmap_sizes,
                                           preferred_icon_size(),
                                           &score);
     }
 
     if (!image_skia.isNull()) {
       gfx::Image image(image_skia);
       // The downloaded icon is still valid when there is no FaviconURL update
       // during the downloading.
-      request_next_icon = !UpdateFaviconCandidate(image_url, image, score,
-                                                  download_request.icon_type);
+      exact_match_found = UpdateFaviconCandidate(image_url, image, score,
+                                                 download_request.icon_type);
     }
   }
 
-  if (request_next_icon && current_candidate_index_ + 1 < image_urls_.size()) {
-    // Process the next candidate.
-    ++current_candidate_index_;
-    DownloadCurrentCandidateOrAskFaviconService();
-  } 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.
-      SetFavicon(best_favicon_candidate_.image_url,
-                 best_favicon_candidate_.image,
-                 best_favicon_candidate_.icon_type);
-    }
-    // Clear download related state.
-    download_requests_.clear();
-    current_candidate_index_ = image_urls_.size();
-    best_favicon_candidate_ = FaviconCandidate();
-  }
+  CurrentCandidateFinished(exact_match_found);
 }
 
 bool FaviconHandler::HasPendingTasksForTest() {
   return !download_requests_.empty() ||
          cancelable_task_tracker_.HasTrackedTasks();
 }
 
 void FaviconHandler::UpdateFaviconMappingAndFetch(
     const GURL& page_url,
     const GURL& icon_url,
@@ skipping 149 matching lines @@
 
   if (has_expired_or_incomplete_result) {
     ScheduleDownload(current_candidate()->icon_url,
                      current_candidate()->icon_type);
   }
 }
 
 void FaviconHandler::ScheduleDownload(const GURL& image_url,
                                       favicon_base::IconType icon_type) {
   DCHECK(image_url.is_valid());
+  if (service_ && service_->WasUnableToDownloadFavicon(image_url)) {

[pkotwicz, 2017/03/03 18:31:16]

Can you call OnDidDownloadFavicon() instead?

This would reduce the number of possible code paths. That way ScheduleDownload()
always calls OnDidDownloadFavicon().

[mastiz, 2017/03/03 20:37:01]

Done.

+    DVLOG(1) << "Skip Failed FavIcon: " << image_url;
+    CurrentCandidateFinished(/*found_exact_match=*/false);
+    return;
+  }
   // 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(icon_type),
                                base::Bind(&FaviconHandler::OnDidDownloadFavicon,
                                           weak_ptr_factory_.GetWeakPtr()));
 
   // Download ids should be unique.
   DCHECK(download_requests_.find(download_id) == download_requests_.end());
   download_requests_[download_id] = DownloadRequest(image_url, icon_type);
 
   if (download_id == 0) {
     // If DownloadFavicon() did not start a download, it returns a download id
     // of 0. We still need to call OnDidDownloadFavicon() because the method is
     // responsible for initiating the data request for the next candidate.
     OnDidDownloadFavicon(download_id, 0, image_url, std::vector<SkBitmap>(),
                          std::vector<gfx::Size>());
   }
 }
 
 }  // namespace favicon