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

Unified Diff: components/favicon/core/favicon_handler_unittest.cc

Issue 2739173002: Always select best favicon bitmap (Closed)
Patch Set: Rebased. 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
Index: components/favicon/core/favicon_handler_unittest.cc
diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc
index 5cbe691f441020857267ecfeac8505a508e94c74..62de65b6b54e29ca6d6cac9966e3eeeda922ea9a 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -786,9 +786,9 @@ TEST_F(FaviconHandlerMultipleFaviconsTest, ChooseLargestExactMatch) {
}
// Test that if there are several single resolution favicons to choose
-// from, the exact match is preferred even if it results in upsampling.
-TEST_F(FaviconHandlerMultipleFaviconsTest, ChooseExactMatchDespiteUpsampling) {
- EXPECT_EQ(16, DownloadTillDoneIgnoringHistory(IntVector{16, 24, 48, 256}));
+// from, the exact match is not preferred if it results in upsampling.
+TEST_F(FaviconHandlerMultipleFaviconsTest, ChooseBestDespiteSmallerExactMatch) {
+ EXPECT_EQ(48, DownloadTillDoneIgnoringHistory(IntVector{16, 24, 48, 256}));
}
// Test that favicons which need to be upsampled a little or downsampled
@@ -887,48 +887,6 @@ TEST_F(FaviconHandlerTest, FaviconInvalidURL) {
EXPECT_THAT(delegate_.downloads(), IsEmpty());
}
-TEST_F(FaviconHandlerTest, TestSortFavicon) {
- const std::vector<favicon::FaviconURL> kSourceIconURLs{
- FaviconURL(GURL("http://www.google.com/a"), FAVICON,
- {gfx::Size(1, 1), gfx::Size(17, 17)}),
- FaviconURL(GURL("http://www.google.com/b"), FAVICON,
- {gfx::Size(1024, 1024), gfx::Size(512, 512)}),
- FaviconURL(GURL("http://www.google.com/c"), FAVICON,
- {gfx::Size(16, 16), gfx::Size(14, 14)}),
- FaviconURL(GURL("http://www.google.com/d"), FAVICON, kEmptySizes),
- FaviconURL(GURL("http://www.google.com/e"), FAVICON, kEmptySizes)};
-
- std::unique_ptr<FaviconHandler> handler = RunHandlerWithCandidates(
- FaviconDriverObserver::NON_TOUCH_LARGEST, kSourceIconURLs);
-
- struct ExpectedResult {
- // The favicon's index in kSourceIconURLs.
- size_t favicon_index;
- // Width of largest bitmap.
- int width;
- } results[] = {
- // First is icon2, though its size larger than maximal.
- {1, 1024},
- // Second is icon1
- // The 17x17 is largest.
- {0, 17},
- // Third is icon3.
- // The 16x16 is largest.
- {2, 16},
- // The rest of bitmaps come in order, there is no "sizes" attribute.
- {3, -1},
- {4, -1},
- };
- const std::vector<FaviconURL>& icons = handler->image_urls();
- ASSERT_EQ(5u, icons.size());
- for (size_t i = 0; i < icons.size(); ++i) {
- EXPECT_EQ(kSourceIconURLs[results[i].favicon_index].icon_url,
- icons[i].icon_url);
- if (results[i].width != -1)
- EXPECT_EQ(results[i].width, icons[i].icon_sizes[0].width());
- }
-}
-
TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
// Names represent the bitmap sizes per icon.
const GURL kIconURL1024_512("http://www.google.com/a");
@@ -979,7 +937,11 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
}
TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
- const int kMaximalSize = FaviconHandler::GetMaximalIconSize(FAVICON);
+#if defined(OS_IOS)
+ const int kMaximalSize = 144;
+#else
+ const int kMaximalSize = 192;
+#endif
const GURL kIconURL1("http://www.google.com/b");
const GURL kIconURL2("http://www.google.com/c");
@@ -992,10 +954,10 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
delegate_.fake_downloader().AddWithOriginalSizes(
kIconURL2, IntVector{kMaximalSize}, IntVector{kOriginalSize2});
- // Verify the largest bitmap was selected although it was scaled down to
- // maximal size and smaller than |kIconURL1| now.
+ // Verify the best bitmap was selected although it was scaled down to maximal
+ // size and smaller than |kIconURL2| now.
EXPECT_CALL(delegate_,
- OnFaviconUpdated(_, _, kIconURL2, _,
+ OnFaviconUpdated(_, _, kIconURL1, _,
ImageSizeIs(kMaximalSize, kMaximalSize)));
RunHandlerWithCandidates(
@@ -1005,7 +967,7 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
FaviconURL(kIconURL2, FAVICON,
SizeVector{gfx::Size(kOriginalSize2, kOriginalSize2)})});
- EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL2));
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL1));
}
// Test that if several icons are downloaded because the icons are smaller than

Powered by Google App Engine
This is Rietveld 408576698