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..5e2d8ad1e70c47c6c934d49db59482bf17ad33a3 100644
--- a/components/favicon/core/favicon_handler.cc
+++ b/components/favicon/core/favicon_handler.cc
@@ -661,6 +661,12 @@ 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 20:56:14]

You also need to add the entry in download_requests_. Otherwise
OnDidDownloadFavicon() will return early and not proceed to the next icon

[mastiz, 2017/03/06 09:11:41]

Done, thanks.

I managed to reproduce this case in tests and will be part of
https://codereview.chromium.org/2703603002/

+    DVLOG(1) << "Skip Failed FavIcon: " << image_url;
+    OnDidDownloadFavicon(0, 0, image_url, std::vector<SkBitmap>(),
+                         std::vector<gfx::Size>());
+    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.
@@ -673,6 +679,8 @@ void FaviconHandler::ScheduleDownload(const GURL& image_url,
   DCHECK(download_requests_.find(download_id) == download_requests_.end());
   download_requests_[download_id] = DownloadRequest(image_url, icon_type);
 
+  // TODO(mastiz): Remove the download_id == 0 handling because it's not used
+  // in production code, only tests.
   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