Move WasUnableToDownload logic to FaviconHandler

components/favicon/content/content_favicon_driver_unittest.cc

Index: components/favicon/content/content_favicon_driver_unittest.cc
diff --git a/components/favicon/content/content_favicon_driver_unittest.cc b/components/favicon/content/content_favicon_driver_unittest.cc
index d1ede99adfffb8888b2b18cecbb4c564245131ae..25a4b4a749eb76c719ce3f57ff0a45c1f83c2fbc 100644
--- a/components/favicon/content/content_favicon_driver_unittest.cc
+++ b/components/favicon/content/content_favicon_driver_unittest.cc
@@ -70,62 +70,46 @@ class ContentFaviconDriverTest : public content::RenderViewHostTestHarness {
   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);
+// 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)});
-  // Completing the download should not cause a call to
-  // UnableToDownloadFavicon().
-  EXPECT_TRUE(web_contents_tester()->TestDidDownloadImage(
-      kIconURL, 503, kEmptyIcons, kEmptyIconSizes));
+  // Verify that no download request is pending for the image.
+  EXPECT_FALSE(web_contents_tester()->HasPendingDownloadImage(kIconURL));
 }
 
-// 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) {
+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 that no download request is pending for the image.
+  // 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