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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 1295733002: Ignore duplicate FaviconHandler::OnUpdateFaviconURL() calls (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@page_changed_under_us
Patch Set: Created 5 years, 2 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/core/favicon_handler.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 698690397fed7df03b51f7383a3f05ad2fdf2847..ca89afaaf74ad0616a09a00876133a4a221f58cd 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -329,15 +329,14 @@ class TestFaviconHandler : public FaviconHandler {
return download_handler_.get();
}
- // Methods to access favicon internals.
- const std::vector<FaviconURL>& urls() {
- return image_urls_;
- }
-
FaviconURL* current_candidate() {
return FaviconHandler::current_candidate();
}
+ size_t current_candidate_index() const {
+ return current_candidate_index_;
+ }
+
const FaviconCandidate& best_favicon_candidate() {
return best_favicon_candidate_;
}
@@ -555,7 +554,7 @@ TEST_F(FaviconHandlerTest, GetFaviconFromHistory) {
helper.OnUpdateFaviconURL(page_url, urls);
// Verify FaviconHandler status
- EXPECT_EQ(1U, helper.urls().size());
+ EXPECT_EQ(1u, helper.image_urls().size());
ASSERT_TRUE(helper.current_candidate());
ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
@@ -597,7 +596,7 @@ TEST_F(FaviconHandlerTest, DownloadFavicon) {
helper.OnUpdateFaviconURL(page_url, urls);
// Verify FaviconHandler status
- EXPECT_EQ(1U, helper.urls().size());
+ EXPECT_EQ(1u, helper.image_urls().size());
ASSERT_TRUE(helper.current_candidate());
ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
@@ -666,7 +665,7 @@ TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) {
helper.OnUpdateFaviconURL(page_url, urls);
// Verify FaviconHandler status.
- EXPECT_EQ(1U, helper.urls().size());
+ EXPECT_EQ(1u, helper.image_urls().size());
ASSERT_TRUE(helper.current_candidate());
ASSERT_EQ(new_icon_url, helper.current_candidate()->icon_url);
ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
@@ -814,7 +813,7 @@ TEST_F(FaviconHandlerTest, UpdateFavicon) {
helper.OnUpdateFaviconURL(page_url, urls);
// Verify FaviconHandler status.
- EXPECT_EQ(1U, helper.urls().size());
+ EXPECT_EQ(1u, helper.image_urls().size());
ASSERT_TRUE(helper.current_candidate());
ASSERT_EQ(new_icon_url, helper.current_candidate()->icon_url);
ASSERT_EQ(favicon_base::FAVICON, helper.current_candidate()->icon_type);
@@ -883,7 +882,8 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
helper.OnUpdateFaviconURL(page_url, urls);
// Verify FaviconHandler status.
- EXPECT_EQ(2U, helper.urls().size());
+ EXPECT_EQ(2u, helper.image_urls().size());
+ EXPECT_EQ(0u, helper.current_candidate_index());
ASSERT_TRUE(helper.current_candidate());
ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
ASSERT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON,
@@ -913,7 +913,7 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
download_handler->InvokeCallback();
// Left 1 url.
- EXPECT_EQ(1U, helper.urls().size());
+ EXPECT_EQ(1u, helper.current_candidate_index());
ASSERT_TRUE(helper.current_candidate());
EXPECT_EQ(new_icon_url, helper.current_candidate()->icon_url);
EXPECT_EQ(favicon_base::TOUCH_ICON, helper.current_candidate()->icon_type);
@@ -994,8 +994,8 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
helper.OnUpdateFaviconURL(page_url, urls);
// Verify FaviconHandler status.
- EXPECT_EQ(2U, helper.urls().size());
- ASSERT_TRUE(helper.current_candidate());
+ EXPECT_EQ(2u, helper.image_urls().size());
+ ASSERT_EQ(0u, helper.current_candidate_index());
ASSERT_EQ(icon_url, helper.current_candidate()->icon_url);
ASSERT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON,
helper.current_candidate()->icon_type);
@@ -1027,7 +1027,8 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
latest_icon_url, favicon_base::TOUCH_ICON, std::vector<gfx::Size>()));
helper.OnUpdateFaviconURL(page_url, latest_urls);
- EXPECT_EQ(1U, helper.urls().size());
+ EXPECT_EQ(1u, helper.image_urls().size());
+ EXPECT_EQ(0u, helper.current_candidate_index());
EXPECT_EQ(latest_icon_url, helper.current_candidate()->icon_url);
EXPECT_EQ(favicon_base::TOUCH_ICON, helper.current_candidate()->icon_type);
@@ -1065,6 +1066,59 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
EXPECT_FALSE(download_handler->HasDownload());
}
+// Test that sending an icon URL update identical to the previous icon URL
+// update is a no-op.
+TEST_F(FaviconHandlerTest, UpdateSameIconURLs) {
+ const GURL page_url("http://www.google.com");
+ const GURL icon_url1("http://www.google.com/favicon1");
+ const GURL icon_url2("http://www.google.com/favicon2");
+ std::vector<FaviconURL> favicon_urls;
+ favicon_urls.push_back(FaviconURL(GURL("http://www.google.com/favicon1"),
+ favicon_base::FAVICON,
+ std::vector<gfx::Size>()));
+ favicon_urls.push_back(FaviconURL(GURL("http://www.google.com/favicon2"),
+ favicon_base::FAVICON,
+ std::vector<gfx::Size>()));
+
+ TestFaviconDriver driver;
+ TestFaviconHandler helper(page_url, &driver, FaviconHandler::FAVICON, false);
+
+ // Initiate a request for favicon data for |page_url|. History does not know
+ // about the page URL or the icon URLs.
+ helper.FetchFavicon(page_url);
+ helper.history_handler()->InvokeCallback();
+ helper.set_history_handler(nullptr);
+
+ // Got icon URLs.
+ helper.OnUpdateFaviconURL(page_url, favicon_urls);
+
+ // There should be an ongoing history request for |icon_url1|.
+ ASSERT_EQ(2u, helper.image_urls().size());
+ ASSERT_EQ(0u, helper.current_candidate_index());
+ HistoryRequestHandler* history_handler = helper.history_handler();
+ ASSERT_TRUE(history_handler);
+
+ // Calling OnUpdateFaviconURL() with the same icon URLs should have no effect.
+ helper.OnUpdateFaviconURL(page_url, favicon_urls);
+ EXPECT_EQ(history_handler, helper.history_handler());
+
+ // Complete history request for |icon_url1| and do download.
+ helper.history_handler()->InvokeCallback();
+ helper.set_history_handler(nullptr);
+ helper.download_handler()->SetImageSizes(std::vector<int>(1u, 10));
+ helper.download_handler()->InvokeCallback();
+ helper.download_handler()->Reset();
+
+ // There should now be an ongoing history request for |icon_url2|.
+ ASSERT_EQ(1u, helper.current_candidate_index());
+ history_handler = helper.history_handler();
+ ASSERT_TRUE(history_handler);
+
+ // Calling OnUpdateFaviconURL() with the same icon URLs should have no effect.
+ helper.OnUpdateFaviconURL(page_url, favicon_urls);
+ EXPECT_EQ(history_handler, helper.history_handler());
+}
+
// Test the favicon which is selected when the web page provides several
// favicons and none of the favicons are cached in history.
// The goal of this test is to be more of an integration test than
@@ -1107,7 +1161,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons) {
DownloadTillDoneIgnoringHistory(
&driver1, &handler1, kPageURL, urls1, kSizes1);
- EXPECT_EQ(0u, handler1.image_urls().size());
+ EXPECT_EQ(nullptr, handler1.current_candidate());
EXPECT_TRUE(driver1.GetActiveFaviconValidity());
EXPECT_FALSE(driver1.GetActiveFaviconImage().IsEmpty());
EXPECT_EQ(gfx::kFaviconSize, driver1.GetActiveFaviconImage().Width());
@@ -1210,7 +1264,7 @@ TEST_F(FaviconHandlerTest, MultipleFavicons404) {
DownloadTillDoneIgnoringHistory(
&driver, &handler, kPageURL, urls2, kSizes2);
- EXPECT_EQ(0u, handler.image_urls().size());
+ EXPECT_EQ(nullptr, handler.current_candidate());
EXPECT_TRUE(driver.GetActiveFaviconValidity());
EXPECT_FALSE(driver.GetActiveFaviconImage().IsEmpty());
int expected_index = 2u;
@@ -1262,7 +1316,7 @@ TEST_F(FaviconHandlerTest, MultipleFaviconsAll404) {
kFaviconURLs + arraysize(kFaviconURLs));
DownloadTillDoneIgnoringHistory(&driver, &handler, kPageURL, urls, kSizes);
- EXPECT_EQ(0u, handler.image_urls().size());
+ EXPECT_EQ(nullptr, handler.current_candidate());
EXPECT_FALSE(driver.GetActiveFaviconValidity());
EXPECT_TRUE(driver.GetActiveFaviconImage().IsEmpty());
}
@@ -1389,23 +1443,20 @@ TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
// Simulate the download failed, to check whether the icons were requested
// to download according their size.
struct ExpectedResult {
- // The size of image_urls_.
- size_t image_urls_size;
// The favicon's index in kSourceIconURLs.
size_t favicon_index;
// Width of largest bitmap.
int width;
} results[] = {
- {5, 0, 1024},
- {4, 2, 512},
- {3, 1, 15},
+ {0, 1024},
+ {2, 512},
+ {1, 15},
// The rest of bitmaps come in order.
- {2, 3, -1},
- {1, 4, -1},
+ {3, -1},
+ {4, -1},
};
for (int i = 0; i < 5; ++i) {
- ASSERT_EQ(results[i].image_urls_size, handler1.image_urls().size());
EXPECT_EQ(kSourceIconURLs[results[i].favicon_index].icon_url,
handler1.current_candidate()->icon_url);
if (results[i].width != -1) {
@@ -1450,7 +1501,7 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) {
kSourceIconURLs + arraysize(kSourceIconURLs));
UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
- ASSERT_EQ(2u, handler1.urls().size());
+ ASSERT_EQ(2u, handler1.image_urls().size());
// Index of largest favicon in kSourceIconURLs.
size_t i = 1;
@@ -1517,7 +1568,7 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
kSourceIconURLs + arraysize(kSourceIconURLs));
UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
- ASSERT_EQ(2u, handler1.urls().size());
+ ASSERT_EQ(2u, handler1.image_urls().size());
// Index of largest favicon in kSourceIconURLs.
size_t i = 1;
@@ -1577,7 +1628,7 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
std::vector<FaviconURL> urls1(kSourceIconURLs,
kSourceIconURLs + arraysize(kSourceIconURLs));
UpdateFaviconURL(&driver1, &handler1, kPageURL, urls1);
- ASSERT_EQ(3u, handler1.urls().size());
+ ASSERT_EQ(3u, handler1.image_urls().size());
// Simulate no favicon from history.
handler1.history_handler()->history_results_.clear();
« no previous file with comments | « components/favicon/core/favicon_handler.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698