Move WasUnableToDownload logic to FaviconHandler

components/favicon/content/content_favicon_driver_unittest.cc

 // Copyright 2015 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/content/content_favicon_driver.h"
 
 #include <memory>
 #include <vector>
 
 #include "base/macros.h"
@@ skipping 52 matching lines @@
         ContentFaviconDriver::FromWebContents(web_contents());
     web_contents_tester()->NavigateAndCommit(page_url);
     static_cast<content::WebContentsObserver*>(favicon_driver)
         ->DidUpdateFaviconURL(candidates);
     base::RunLoop().RunUntilIdle();
   }
 
   testing::NiceMock<MockFaviconService> favicon_service_;
 };
 
-// Test that UnableToDownloadFavicon() is not called as a result of a favicon
-// download with 200 status.
-TEST_F(ContentFaviconDriverTest, ShouldNotCallUnableToDownloadFaviconFor200) {
-  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kIconURL)).Times(0);
+// Test that events are propagated until the FaviconHandler which should in turn
+// start a download.

[pkotwicz, 2017/03/02 21:20:26]

How about: Test that a download is initiated when there isn't a favicon in the
database for either the page URL or the icon URL.

[mastiz, 2017/03/03 14:21:13]

Done.

+TEST_F(ContentFaviconDriverTest, ShouldCauseImageDownload) {
   // Mimic a page load.
   TestFetchFaviconForPage(
       kPageURL,
       {content::FaviconURL(kIconURL, content::FaviconURL::FAVICON,
                            kEmptyIconSizes)});
-  // Completing the download should not cause a call to
-  // UnableToDownloadFavicon().
   EXPECT_TRUE(web_contents_tester()->TestDidDownloadImage(
       kIconURL, 200, kEmptyIcons, kEmptyIconSizes));
 }
 
-// Test that UnableToDownloadFavicon() is called as a result of a favicon
-// download with 404 status.
-TEST_F(ContentFaviconDriverTest, ShouldCallUnableToDownloadFaviconFor404) {
-  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kIconURL));
-  // Mimic a page load.
-  TestFetchFaviconForPage(
-      kPageURL,
-      {content::FaviconURL(kIconURL, content::FaviconURL::FAVICON,
-                           kEmptyIconSizes)});
-  // Mimic the completion of an image download.
-  EXPECT_TRUE(web_contents_tester()->TestDidDownloadImage(
-      kIconURL, 404, kEmptyIcons, kEmptyIconSizes));
-}
-
-// Test that UnableToDownloadFavicon() is not called as a result of a favicon
-// download with 503 status.
-TEST_F(ContentFaviconDriverTest, ShouldNotCallUnableToDownloadFaviconFor503) {
-  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kIconURL)).Times(0);
-  // Mimic a page load.
-  TestFetchFaviconForPage(
-      kPageURL,
-      {content::FaviconURL(kIconURL, content::FaviconURL::FAVICON,
-                           kEmptyIconSizes)});
-  // Completing the download should not cause a call to
-  // UnableToDownloadFavicon().
-  EXPECT_TRUE(web_contents_tester()->TestDidDownloadImage(
-      kIconURL, 503, kEmptyIcons, kEmptyIconSizes));
-}
-
 // Test that Favicon is not requested repeatedly during the same session if
 // the favicon is known to be unavailable (e.g. due to HTTP 404 status).
 TEST_F(ContentFaviconDriverTest, ShouldNotRequestRepeatedlyIfUnavailable) {
   ON_CALL(favicon_service_, WasUnableToDownloadFavicon(kIconURL))
       .WillByDefault(Return(true));
   // Mimic a page load.
   TestFetchFaviconForPage(
       kPageURL,
       {content::FaviconURL(kIconURL, content::FaviconURL::FAVICON,
                            kEmptyIconSizes)});
   // Verify that no download request is pending for the image.
   EXPECT_FALSE(web_contents_tester()->HasPendingDownloadImage(kIconURL));
 }
 
+TEST_F(ContentFaviconDriverTest, ShouldDownloadSecondIfFirstUnavailable) {
+  const GURL kOtherIconURL = GURL("http://www.google.com/other-favicon.ico");
+  ON_CALL(favicon_service_, WasUnableToDownloadFavicon(kIconURL))
+      .WillByDefault(Return(true));
+  // Mimic a page load.
+  TestFetchFaviconForPage(
+      kPageURL,
+      {content::FaviconURL(kIconURL, content::FaviconURL::FAVICON,
+                           kEmptyIconSizes),
+       content::FaviconURL(kOtherIconURL, content::FaviconURL::FAVICON,
+                           kEmptyIconSizes)});
+  // Verify a  download request is pending for the second image.
+  EXPECT_FALSE(web_contents_tester()->HasPendingDownloadImage(kIconURL));
+  EXPECT_TRUE(web_contents_tester()->HasPendingDownloadImage(kOtherIconURL));
+}
+
 // Test that ContentFaviconDriver ignores updated favicon URLs if there is no
 // last committed entry. This occurs when script is injected in about:blank.
 // See crbug.com/520759 for more details
 TEST_F(ContentFaviconDriverTest, FaviconUpdateNoLastCommittedEntry) {
   ASSERT_EQ(nullptr, web_contents()->GetController().GetLastCommittedEntry());
 
   std::vector<content::FaviconURL> favicon_urls;
   favicon_urls.push_back(content::FaviconURL(
       GURL("http://www.google.ca/favicon.ico"), content::FaviconURL::FAVICON,
       std::vector<gfx::Size>()));
   favicon::ContentFaviconDriver* driver =
       favicon::ContentFaviconDriver::FromWebContents(web_contents());
   static_cast<content::WebContentsObserver*>(driver)
       ->DidUpdateFaviconURL(favicon_urls);
 
   // Test that ContentFaviconDriver ignored the favicon url update.
   EXPECT_TRUE(driver->favicon_urls().empty());
 }
 
 }  // namespace
 }  // namespace favicon