Move WasUnableToDownload logic to FaviconHandler

components/favicon/core/favicon_handler.cc

Index: components/favicon/core/favicon_handler.cc
diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc
index adbb5df8e2a4b4c769f4cca819fc95f424a78fda..50802efd54fab1970d0de7776a74c164d41cecd4 100644
--- a/components/favicon/core/favicon_handler.cc
+++ b/components/favicon/core/favicon_handler.cc
@@ -326,6 +326,26 @@ void FaviconHandler::SetFavicon(const GURL& icon_url,
   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) {
@@ -442,7 +462,7 @@ void FaviconHandler::OnDidDownloadFavicon(
     return;
   }
 
-  bool request_next_icon = true;
+  bool exact_match_found = false;
   if (!bitmaps.empty()) {
     float score = 0.0f;
     gfx::ImageSkia image_skia;
@@ -470,28 +490,12 @@ void FaviconHandler::OnDidDownloadFavicon(
       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() {
@@ -661,6 +665,11 @@ void FaviconHandler::OnFaviconData(const std::vector<
 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.