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

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

Issue 2799273002: Add support to process favicons from Web Manifests (Closed)
Patch Set: Adopt icon URL. Created 3 years, 8 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 5e651a646febcaca5d9b28ba8b03e129f4fb5da8..e1ee239374536ee1f585f6c084528f93189cd631 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -51,6 +51,11 @@ MATCHER_P2(ImageSizeIs, width, height, "") {
return arg.Size() == gfx::Size(width, height);
}
+ACTION_P(PostDownloadReply, p0) {
+ base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+ base::Bind(arg1, p0));
+}
+
// Fill the given bmp with some test data.
SkBitmap CreateBitmapWithEdgeSize(int size) {
SkBitmap bmp;
@@ -202,6 +207,8 @@ class MockDelegate : public FaviconHandler::Delegate {
int(const GURL& url,
int max_image_size,
ImageDownloadCallback callback));
+ MOCK_METHOD2(DownloadManifest,
+ void(const GURL& url, ManifestDownloadCallback callback));
MOCK_METHOD0(IsOffTheRecord, bool());
MOCK_METHOD1(IsBookmarked, bool(const GURL& url));
MOCK_METHOD5(OnFaviconUpdated,
@@ -350,14 +357,15 @@ class FaviconHandlerTest : public testing::Test {
// Returns the handler in case tests want to exercise further steps.
std::unique_ptr<FaviconHandler> RunHandlerWithCandidates(
FaviconDriverObserver::NotificationIconType handler_type,
- const std::vector<favicon::FaviconURL>& candidates) {
+ const std::vector<favicon::FaviconURL>& candidates,
+ const base::Optional<GURL>& manifest_url = base::nullopt) {
auto handler = base::MakeUnique<FaviconHandler>(&favicon_service_,
&delegate_, handler_type);
handler->FetchFavicon(kPageURL);
// The first RunUntilIdle() causes the FaviconService lookups be faster than
- // OnUpdateFaviconURL(), which is the most likely scenario.
+ // OnUpdateCandidates(), which is the most likely scenario.
base::RunLoop().RunUntilIdle();
- handler->OnUpdateFaviconURL(kPageURL, candidates);
+ handler->OnUpdateCandidates(kPageURL, candidates, manifest_url);
base::RunLoop().RunUntilIdle();
return handler;
}
@@ -365,13 +373,14 @@ class FaviconHandlerTest : public testing::Test {
// Same as above, but for the simplest case where all types are FAVICON and
// no sizes are provided, using a FaviconHandler of type NON_TOUCH_16_DIP.
std::unique_ptr<FaviconHandler> RunHandlerWithSimpleFaviconCandidates(
- const std::vector<GURL>& urls) {
+ const std::vector<GURL>& urls,
+ const base::Optional<GURL>& manifest_url = base::nullopt) {
std::vector<favicon::FaviconURL> candidates;
for (const GURL& url : urls) {
candidates.emplace_back(url, FAVICON, kEmptySizes);
}
return RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
- candidates);
+ candidates, manifest_url);
}
base::MessageLoopForUI message_loop_;
@@ -409,7 +418,7 @@ TEST_F(FaviconHandlerTest, UpdateFaviconMappingsAndFetch) {
// - There is data in the database for neither the page URL nor the icon URL.
// AND
// - FaviconService::GetFaviconForPageURL() callback returns before
-// FaviconHandler::OnUpdateFaviconURL() is called.
+// FaviconHandler::OnUpdateCandidates() is called.
TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesSlower) {
EXPECT_CALL(favicon_service_, SetFavicons(kPageURL, kIconURL16x16, FAVICON,
ImageSizeIs(16, 16)));
@@ -420,10 +429,11 @@ TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesSlower) {
FaviconHandler handler(&favicon_service_, &delegate_,
FaviconDriverObserver::NON_TOUCH_16_DIP);
handler.FetchFavicon(kPageURL);
- // Causes FaviconService lookups be faster than OnUpdateFaviconURL().
+ // Causes FaviconService lookups be faster than OnUpdateCandidates().
base::RunLoop().RunUntilIdle();
- handler.OnUpdateFaviconURL(kPageURL,
- {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)});
+ handler.OnUpdateCandidates(kPageURL,
+ {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)},
+ base::nullopt);
base::RunLoop().RunUntilIdle();
EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL16x16));
@@ -435,7 +445,7 @@ TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesSlower) {
// - There is data in the database for neither the page URL nor the icon URL.
// AND
// - FaviconService::GetFaviconForPageURL() callback returns after
-// FaviconHandler::OnUpdateFaviconURL() is called.
+// FaviconHandler::OnUpdateCandidates() is called.
TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesFaster) {
EXPECT_CALL(favicon_service_, SetFavicons(kPageURL, kIconURL16x16, FAVICON,
ImageSizeIs(16, 16)));
@@ -447,8 +457,9 @@ TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesFaster) {
ASSERT_THAT(favicon_service_.fake()->db_requests(), ElementsAre(kPageURL));
// Feed in favicons without processing posted tasks (RunUntilIdle()).
- handler.OnUpdateFaviconURL(kPageURL,
- {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)});
+ handler.OnUpdateCandidates(kPageURL,
+ {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)},
+ base::nullopt);
base::RunLoop().RunUntilIdle();
EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL16x16));
@@ -609,7 +620,7 @@ TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) {
// Test that download data for icon URLs other than the current favicon
// candidate URLs is ignored. This test tests the scenario where a download is
-// in flight when FaviconHandler::OnUpdateFaviconURL() is called.
+// in flight when FaviconHandler::OnUpdateCandidates() is called.
// TODO(mastiz): Make this test deal with FaviconURLs of type
// favicon_base::FAVICON and add new ones like OnlyDownloadMatchingIconType and
// CallSetFaviconsWithCorrectIconType.
@@ -635,8 +646,8 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
EXPECT_CALL(favicon_service_, SetFavicons(_, kIconURL3, _, _));
EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL3, _, _));
- handler->OnUpdateFaviconURL(kPageURL,
- {FaviconURL(kIconURL3, FAVICON, kEmptySizes)});
+ handler->OnUpdateCandidates(
+ kPageURL, {FaviconURL(kIconURL3, FAVICON, kEmptySizes)}, base::nullopt);
// Finalizes download, which should be thrown away as the favicon URLs were
// updated.
@@ -670,9 +681,9 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsWhileProcessingShouldBeNoop) {
ASSERT_TRUE(VerifyAndClearExpectations());
ASSERT_TRUE(delegate_.fake_downloader().HasPendingManualCallback());
- // Calling OnUpdateFaviconURL() with the same icon URLs should have no effect,
+ // Calling OnUpdateCandidates() with the same icon URLs should have no effect,
// despite the ongoing download.
- handler->OnUpdateFaviconURL(kPageURL, favicon_urls);
+ handler->OnUpdateCandidates(kPageURL, favicon_urls, base::nullopt);
base::RunLoop().RunUntilIdle();
// Complete the download.
@@ -697,11 +708,11 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsAfterFinishedShouldBeNoop) {
ASSERT_TRUE(VerifyAndClearExpectations());
- // Calling OnUpdateFaviconURL() with identical data should be a no-op.
+ // Calling OnUpdateCandidates() with identical data should be a no-op.
EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, _, _, _)).Times(0);
EXPECT_CALL(favicon_service_, SetFavicons(_, _, _, _)).Times(0);
- handler->OnUpdateFaviconURL(kPageURL, favicon_urls);
+ handler->OnUpdateCandidates(kPageURL, favicon_urls, base::nullopt);
base::RunLoop().RunUntilIdle();
EXPECT_THAT(favicon_service_.fake()->db_requests(), IsEmpty());
EXPECT_THAT(delegate_.downloads(), IsEmpty());
@@ -743,8 +754,8 @@ TEST_F(FaviconHandlerTest,
// Simulate the page changing it's icon URL to just |kIconURL2| via
// Javascript.
EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL2, _, _));
- handler->OnUpdateFaviconURL(kPageURL,
- {FaviconURL(kIconURL2, FAVICON, kEmptySizes)});
+ handler->OnUpdateCandidates(
+ kPageURL, {FaviconURL(kIconURL2, FAVICON, kEmptySizes)}, base::nullopt);
base::RunLoop().RunUntilIdle();
}
@@ -1031,5 +1042,68 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) {
FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)});
}
+TEST_F(FaviconHandlerTest, GetFaviconFromManifestInHistory) {
+ const GURL kManifestURL("http://www.google.com/manifest.json");
+
+ favicon_service_.fake()->Store(kManifestURL, kIconURL16x16,
+ CreateRawBitmapResult(kIconURL16x16));
+
+ EXPECT_CALL(delegate_, DownloadManifest(_, _)).Times(0);
+
+ EXPECT_CALL(favicon_service_, UpdateFaviconMappingsAndFetch(
+ kPageURL, URLVector{kManifestURL}, FAVICON,
+ /*desired_size_in_dip=*/16, _, _));
+ EXPECT_CALL(delegate_, OnFaviconUpdated(
+ kPageURL, FaviconDriverObserver::NON_TOUCH_16_DIP,
+ kManifestURL, /*icon_url_changed=*/true, _));
+
+ RunHandlerWithSimpleFaviconCandidates(URLVector(), kManifestURL);
+ EXPECT_THAT(favicon_service_.fake()->db_requests(),
+ ElementsAre(kPageURL, kManifestURL));
+ EXPECT_THAT(delegate_.downloads(), IsEmpty());
+}
+
+TEST_F(FaviconHandlerTest, GetFaviconFromUnknownManifest) {
+ const GURL kManifestURL("http://www.google.com/manifest.json");
+ const std::vector<favicon::FaviconURL> kManifestIcons = {
+ FaviconURL(kIconURL16x16, FAVICON, kEmptySizes),
+ };
+
+ EXPECT_CALL(delegate_, DownloadManifest(kManifestURL, _))
+ .WillOnce(PostDownloadReply(kManifestIcons));
+ EXPECT_CALL(favicon_service_, SetFavicons(kPageURL, kManifestURL, FAVICON,
+ ImageSizeIs(16, 16)));
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kManifestURL, _, _));
+
+ RunHandlerWithSimpleFaviconCandidates(URLVector(), kManifestURL);
+ EXPECT_THAT(favicon_service_.fake()->db_requests(),
+ ElementsAre(kPageURL, kManifestURL));
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL16x16));
+}
+
+TEST_F(FaviconHandlerTest, GetFaviconFromExpiredManifestIcon) {
+ const GURL kManifestURL("http://www.google.com/manifest.json");
+ const std::vector<favicon::FaviconURL> kManifestIcons = {
+ FaviconURL(kIconURL64x64, FAVICON, kEmptySizes),
+ };
+
+ favicon_service_.fake()->Store(kPageURL, kManifestURL,
+ CreateRawBitmapResult(kManifestURL, FAVICON,
+ /*expired=*/true));
+
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kManifestURL, _, _));
+ EXPECT_CALL(delegate_, DownloadManifest(kManifestURL, _))
+ .WillOnce(PostDownloadReply(kManifestIcons));
+ EXPECT_CALL(favicon_service_, SetFavicons(kPageURL, kManifestURL, _, _));
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kIconURL64x64, _, _));
+
+ RunHandlerWithSimpleFaviconCandidates(URLVector(), kManifestURL);
+ EXPECT_THAT(favicon_service_.fake()->db_requests(),
+ ElementsAre(kPageURL, kManifestURL));
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL64x64));
+}
+
+// TODO(mastiz) / DONOTSUBMIT: Add missing tests.
+
} // namespace
} // namespace favicon

Powered by Google App Engine
This is Rietveld 408576698