Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(186)

Unified Diff: components/favicon/content/content_favicon_driver_unittest.cc

Issue 2728893004: Move WasUnableToDownload logic to FaviconHandler (Closed)
Patch Set: Reverted accidental changes. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/favicon/content/content_favicon_driver.cc ('k') | components/favicon/core/favicon_driver_impl.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 73769c5f2f222429a58d1a09df570d5584f7127b..5439aee3d760b81b3c1433a06942024dadb8166d 100644
--- a/components/favicon/content/content_favicon_driver_unittest.cc
+++ b/components/favicon/content/content_favicon_driver_unittest.cc
@@ -72,62 +72,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 a download is initiated when there isn't a favicon in the database
+// for either the page URL or the icon URL.
+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
« no previous file with comments | « components/favicon/content/content_favicon_driver.cc ('k') | components/favicon/core/favicon_driver_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698