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 4a997f382474f3ff6783867958eccd253e6ccf70..69824ad521953864f560bd996043569358dcaafb 100644 |
--- a/components/favicon/core/favicon_handler_unittest.cc |
+++ b/components/favicon/core/favicon_handler_unittest.cc |
@@ -104,14 +104,16 @@ class FakeImageDownloader { |
SizeVector original_bitmap_sizes; |
}; |
- FakeImageDownloader() : next_download_id_(1) {} |
+ // |downloads| must not be nullptr and must outlive this object. |
+ FakeImageDownloader(URLVector* downloads) |
+ : downloads_(downloads), next_download_id_(1) {} |
// Implementation of FaviconHalder::Delegate's DownloadImage(). If a given |
// URL is not known (i.e. not previously added via Add()), it produces 404s. |
int DownloadImage(const GURL& url, |
int max_image_size, |
FaviconHandler::Delegate::ImageDownloadCallback callback) { |
- downloads_.push_back(url); |
+ downloads_->push_back(url); |
const Response& response = responses_[url]; |
int download_id = next_download_id_++; |
@@ -169,17 +171,10 @@ class FakeImageDownloader { |
return true; |
} |
- // Returns pending and completed download URLs. |
- const URLVector& downloads() const { return downloads_; } |
- |
- void ClearDownloads() { downloads_.clear(); } |
- |
private: |
+ URLVector* downloads_; |
int next_download_id_; |
- // Pending and completed download URLs. |
- URLVector downloads_; |
- |
// URL to disable automatic callbacks for. |
GURL manual_callback_url_; |
@@ -192,19 +187,105 @@ class FakeImageDownloader { |
DISALLOW_COPY_AND_ASSIGN(FakeImageDownloader); |
}; |
+// Fake that implements the calls to FaviconHandler::Delegate's |
+// DownloadManifest(), delegated to this class through MockDelegate. |
+class FakeManifestDownloader { |
+ public: |
+ struct Response { |
+ int http_status_code = 404; |
+ std::vector<favicon::FaviconURL> favicon_urls; |
+ }; |
+ |
+ // |downloads| must not be nullptr and must outlive this object. |
+ FakeManifestDownloader(URLVector* downloads) : downloads_(downloads) {} |
+ |
+ // Implementation of FaviconHalder::Delegate's DownloadManifest(). If a given |
+ // URL is not known (i.e. not previously added via Add()), it produces 404s. |
+ void DownloadManifest( |
+ const GURL& url, |
+ FaviconHandler::Delegate::ManifestDownloadCallback callback) { |
+ downloads_->push_back(url); |
+ |
+ const Response& response = responses_[url]; |
+ base::Closure bound_callback = |
+ base::Bind(callback, response.http_status_code, response.favicon_urls); |
+ if (url == manual_callback_url_) |
+ manual_callback_ = bound_callback; |
+ else |
+ base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, bound_callback); |
+ } |
+ |
+ void Add(const GURL& manifest_url, |
+ const std::vector<favicon::FaviconURL>& favicon_urls) { |
+ Response response; |
+ response.http_status_code = 200; |
+ response.favicon_urls = favicon_urls; |
+ responses_[manifest_url] = response; |
+ } |
+ |
+ void AddError(const GURL& manifest_url, int http_status_code) { |
+ Response response; |
+ response.http_status_code = http_status_code; |
+ responses_[manifest_url] = response; |
+ } |
+ |
+ // Disables automatic callback for |url|. This is useful for emulating a |
+ // download taking a long time. The callback for DownloadManifest() will be |
+ // stored in |manual_callback_|. |
+ void SetRunCallbackManuallyForUrl(const GURL& url) { |
+ manual_callback_url_ = url; |
+ } |
+ |
+ // Returns whether an ongoing download exists for a url previously selected |
+ // via SetRunCallbackManuallyForUrl(). |
+ bool HasPendingManualCallback() { return !manual_callback_.is_null(); } |
+ |
+ // Triggers the response for a download previously selected for manual |
+ // triggering via SetRunCallbackManuallyForUrl(). |
+ bool RunCallbackManually() { |
+ if (!HasPendingManualCallback()) |
+ return false; |
+ manual_callback_.Run(); |
+ manual_callback_.Reset(); |
+ return true; |
+ } |
+ |
+ private: |
+ URLVector* downloads_; |
+ |
+ // URL to disable automatic callbacks for. |
+ GURL manual_callback_url_; |
+ |
+ // Callback for DownloadManifest() request for |manual_callback_url_|. |
+ base::Closure manual_callback_; |
+ |
+ // Registered responses. |
+ std::map<GURL, Response> responses_; |
+ |
+ DISALLOW_COPY_AND_ASSIGN(FakeManifestDownloader); |
+}; |
+ |
class MockDelegate : public FaviconHandler::Delegate { |
public: |
- MockDelegate() { |
+ MockDelegate() |
+ : fake_image_downloader_(&downloads_), |
+ fake_manifest_downloader_(&downloads_) { |
// Delegate image downloading to FakeImageDownloader. |
ON_CALL(*this, DownloadImage(_, _, _)) |
- .WillByDefault( |
- Invoke(&fake_downloader_, &FakeImageDownloader::DownloadImage)); |
+ .WillByDefault(Invoke(&fake_image_downloader_, |
+ &FakeImageDownloader::DownloadImage)); |
+ // Delegate manifest downloading to FakeManifestDownloader. |
+ ON_CALL(*this, DownloadManifest(_, _)) |
+ .WillByDefault(Invoke(&fake_manifest_downloader_, |
+ &FakeManifestDownloader::DownloadManifest)); |
} |
MOCK_METHOD3(DownloadImage, |
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, |
@@ -214,14 +295,24 @@ class MockDelegate : public FaviconHandler::Delegate { |
bool icon_url_changed, |
const gfx::Image& image)); |
- FakeImageDownloader& fake_downloader() { return fake_downloader_; } |
+ FakeImageDownloader& fake_image_downloader() { |
+ return fake_image_downloader_; |
+ } |
+ |
+ FakeManifestDownloader& fake_manifest_downloader() { |
+ return fake_manifest_downloader_; |
+ } |
- // Convenience getter for test readability. Returns pending and completed |
- // download URLs. |
- const URLVector& downloads() const { return fake_downloader_.downloads(); } |
+ // Returns pending and completed download URLs. |
+ const URLVector& downloads() const { return downloads_; } |
+ |
+ void ClearDownloads() { downloads_.clear(); } |
private: |
- FakeImageDownloader fake_downloader_; |
+ // Pending and completed download URLs. |
+ URLVector downloads_; |
+ FakeImageDownloader fake_image_downloader_; |
+ FakeManifestDownloader fake_manifest_downloader_; |
}; |
// FakeFaviconService mimics a FaviconService backend that allows setting up |
@@ -327,10 +418,10 @@ class FaviconHandlerTest : public testing::Test { |
FaviconHandlerTest() { |
// Register various known icon URLs. |
- delegate_.fake_downloader().Add(kIconURL10x10, IntVector{10}); |
- delegate_.fake_downloader().Add(kIconURL12x12, IntVector{12}); |
- delegate_.fake_downloader().Add(kIconURL16x16, IntVector{16}); |
- delegate_.fake_downloader().Add(kIconURL64x64, IntVector{64}); |
+ delegate_.fake_image_downloader().Add(kIconURL10x10, IntVector{10}); |
+ delegate_.fake_image_downloader().Add(kIconURL12x12, IntVector{12}); |
+ delegate_.fake_image_downloader().Add(kIconURL16x16, IntVector{16}); |
+ delegate_.fake_image_downloader().Add(kIconURL64x64, IntVector{64}); |
// The score computed by SelectFaviconFrames() is dependent on the supported |
// scale factors of the platform. It is used for determining the goodness of |
@@ -344,7 +435,7 @@ class FaviconHandlerTest : public testing::Test { |
bool VerifyAndClearExpectations() { |
base::RunLoop().RunUntilIdle(); |
favicon_service_.fake()->ClearDbRequests(); |
- delegate_.fake_downloader().ClearDownloads(); |
+ delegate_.ClearDownloads(); |
return testing::Mock::VerifyAndClearExpectations(&favicon_service_) && |
testing::Mock::VerifyAndClearExpectations(&delegate_); |
} |
@@ -353,14 +444,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; |
} |
@@ -368,13 +460,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_; |
@@ -419,7 +512,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))); |
@@ -430,10 +523,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)); |
@@ -445,7 +539,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))); |
@@ -457,8 +551,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)); |
@@ -589,7 +684,7 @@ TEST_F(FaviconHandlerTest, UpdateFavicon) { |
TEST_F(FaviconHandlerTest, Download2ndFaviconURLCandidate) { |
const GURL kIconURLReturning500("http://www.google.com/500.png"); |
- delegate_.fake_downloader().AddError(kIconURLReturning500, 500); |
+ delegate_.fake_image_downloader().AddError(kIconURLReturning500, 500); |
favicon_service_.fake()->Store( |
kPageURL, kIconURL64x64, |
@@ -616,7 +711,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. |
@@ -627,27 +722,27 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) { |
// Defer the download completion such that RunUntilIdle() doesn't complete |
// the download. |
- delegate_.fake_downloader().SetRunCallbackManuallyForUrl(kIconURL1); |
+ delegate_.fake_image_downloader().SetRunCallbackManuallyForUrl(kIconURL1); |
- delegate_.fake_downloader().Add(kIconURL1, IntVector{16}); |
- delegate_.fake_downloader().Add(kIconURL3, IntVector{64}); |
+ delegate_.fake_image_downloader().Add(kIconURL1, IntVector{16}); |
+ delegate_.fake_image_downloader().Add(kIconURL3, IntVector{64}); |
std::unique_ptr<FaviconHandler> handler = |
RunHandlerWithSimpleFaviconCandidates({kIconURL1, kIconURL2}); |
ASSERT_TRUE(VerifyAndClearExpectations()); |
- ASSERT_TRUE(delegate_.fake_downloader().HasPendingManualCallback()); |
+ ASSERT_TRUE(delegate_.fake_image_downloader().HasPendingManualCallback()); |
// Favicon update should invalidate the ongoing download. |
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. |
- EXPECT_TRUE(delegate_.fake_downloader().RunCallbackManually()); |
+ EXPECT_TRUE(delegate_.fake_image_downloader().RunCallbackManually()); |
base::RunLoop().RunUntilIdle(); |
EXPECT_THAT(favicon_service_.fake()->db_requests(), ElementsAre(kIconURL3)); |
@@ -666,8 +761,9 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsWhileProcessingShouldBeNoop) { |
// Defer the download completion such that RunUntilIdle() doesn't complete |
// the download. |
- delegate_.fake_downloader().SetRunCallbackManuallyForUrl(kSlowLoadingIconURL); |
- delegate_.fake_downloader().Add(kSlowLoadingIconURL, IntVector{16}); |
+ delegate_.fake_image_downloader().SetRunCallbackManuallyForUrl( |
+ kSlowLoadingIconURL); |
+ delegate_.fake_image_downloader().Add(kSlowLoadingIconURL, IntVector{16}); |
std::unique_ptr<FaviconHandler> handler = RunHandlerWithCandidates( |
FaviconDriverObserver::NON_TOUCH_16_DIP, favicon_urls); |
@@ -675,17 +771,17 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsWhileProcessingShouldBeNoop) { |
ASSERT_THAT(favicon_service_.fake()->db_requests(), |
ElementsAre(kPageURL, kIconURL64x64, kSlowLoadingIconURL)); |
ASSERT_TRUE(VerifyAndClearExpectations()); |
- ASSERT_TRUE(delegate_.fake_downloader().HasPendingManualCallback()); |
+ ASSERT_TRUE(delegate_.fake_image_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. |
EXPECT_CALL(favicon_service_, SetFavicons(_, _, _, _)); |
EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, _, _, _)); |
- EXPECT_TRUE(delegate_.fake_downloader().RunCallbackManually()); |
+ EXPECT_TRUE(delegate_.fake_image_downloader().RunCallbackManually()); |
base::RunLoop().RunUntilIdle(); |
EXPECT_THAT(delegate_.downloads(), IsEmpty()); |
} |
@@ -704,11 +800,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()); |
@@ -729,8 +825,8 @@ TEST_F(FaviconHandlerTest, |
"http://wwww.page_which_animates_favicon.com/frame2.png"); |
// |kIconURL1| is the better match. |
- delegate_.fake_downloader().Add(kIconURL1, IntVector{15}); |
- delegate_.fake_downloader().Add(kIconURL2, IntVector{10}); |
+ delegate_.fake_image_downloader().Add(kIconURL1, IntVector{15}); |
+ delegate_.fake_image_downloader().Add(kIconURL2, IntVector{10}); |
// Two FaviconDriver::OnFaviconUpdated() notifications should be sent for |
// |kIconURL1|, one before and one after the download. |
@@ -750,8 +846,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(); |
} |
@@ -788,7 +884,7 @@ class FaviconHandlerMultipleFaviconsTest : public FaviconHandlerTest { |
const GURL icon_url(base::StringPrintf( |
"https://www.google.com/generated/%dx%d", icon_size, icon_size)); |
// Set up 200 responses for all images, and the corresponding size. |
- delegate_.fake_downloader().Add(icon_url, IntVector{icon_size}); |
+ delegate_.fake_image_downloader().Add(icon_url, IntVector{icon_size}); |
// Create test candidates of type FAVICON and a fake URL. |
candidate_icons.emplace_back(icon_url, FAVICON, kEmptySizes); |
@@ -844,7 +940,7 @@ TEST_F(FaviconHandlerTest, Report404) { |
TEST_F(FaviconHandlerTest, NotReport503) { |
const GURL k503IconURL("http://www.google.com/503.png"); |
- delegate_.fake_downloader().AddError(k503IconURL, 503); |
+ delegate_.fake_image_downloader().AddError(k503IconURL, 503); |
EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).Times(0); |
@@ -977,8 +1073,8 @@ TEST_F(FaviconHandlerTest, TestSelectLargestFavicon) { |
const GURL kIconURL1("http://www.google.com/b"); |
const GURL kIconURL2("http://www.google.com/c"); |
- delegate_.fake_downloader().Add(kIconURL1, IntVector{15}); |
- delegate_.fake_downloader().Add(kIconURL2, IntVector{14, 16}); |
+ delegate_.fake_image_downloader().Add(kIconURL1, IntVector{15}); |
+ delegate_.fake_image_downloader().Add(kIconURL2, IntVector{14, 16}); |
// Verify NotifyFaviconAvailable(). |
EXPECT_CALL(delegate_, |
@@ -1003,9 +1099,9 @@ TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) { |
const int kOriginalSize1 = kMaximalSize + 1; |
const int kOriginalSize2 = kMaximalSize + 2; |
- delegate_.fake_downloader().AddWithOriginalSizes( |
+ delegate_.fake_image_downloader().AddWithOriginalSizes( |
kIconURL1, IntVector{kMaximalSize}, IntVector{kOriginalSize1}); |
- delegate_.fake_downloader().AddWithOriginalSizes( |
+ delegate_.fake_image_downloader().AddWithOriginalSizes( |
kIconURL2, IntVector{kMaximalSize}, IntVector{kOriginalSize2}); |
// Verify the best bitmap was selected (although smaller than |kIconURL2|) |
@@ -1165,7 +1261,7 @@ TEST_F(FaviconHandlerTest, TestRecordFailingDownloadAttempt) { |
base::HistogramTester histogram_tester; |
const GURL k404IconURL("http://www.google.com/404.png"); |
- delegate_.fake_downloader().AddError(k404IconURL, 404); |
+ delegate_.fake_image_downloader().AddError(k404IconURL, 404); |
EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(k404IconURL)); |
@@ -1192,5 +1288,349 @@ TEST_F(FaviconHandlerTest, TestRecordSkippedDownloadForKnownFailingUrl) { |
/*expected_count=*/1))); |
} |
pkotwicz
2017/05/01 04:56:53
Each test should be preceeded with a comment which
mastiz
2017/05/04 10:57:53
Done. No intent for sarcasm here but, based on my
|
+TEST_F(FaviconHandlerTest, GetFaviconFromManifestInHistory) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ |
+ favicon_service_.fake()->Store(kPageURL, kManifestURL, |
+ CreateRawBitmapResult(kManifestURL)); |
+ |
+ EXPECT_CALL(delegate_, DownloadManifest(_, _)).Times(0); |
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).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, _, _)) |
+ .Times(2); |
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL)); |
+ EXPECT_THAT(delegate_.downloads(), IsEmpty()); |
+} |
+ |
+TEST_F(FaviconHandlerTest, GetFaviconFromManifestInHistoryIfCandidatesFaster) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ |
+ favicon_service_.fake()->Store(kPageURL, kManifestURL, |
+ CreateRawBitmapResult(kManifestURL)); |
+ |
+ EXPECT_CALL(delegate_, DownloadManifest(_, _)).Times(0); |
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).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, _, _)) |
+ .Times(2); |
+ |
+ FaviconHandler handler(&favicon_service_, &delegate_, |
+ FaviconDriverObserver::NON_TOUCH_16_DIP); |
+ handler.FetchFavicon(kPageURL); |
+ // Feed in candidates without processing posted tasks (RunUntilIdle()). |
+ handler.OnUpdateCandidates(kPageURL, |
+ {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)}, |
+ kManifestURL); |
+ base::RunLoop().RunUntilIdle(); |
+ |
+ 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), |
+ }; |
+ |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons); |
+ |
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).Times(0); |
+ |
+ EXPECT_CALL(delegate_, DownloadManifest(kManifestURL, _)); |
+ EXPECT_CALL(favicon_service_, |
+ SetFavicons(kPageURL, kManifestURL, FAVICON, _)); |
pkotwicz
2017/05/01 04:56:53
kManifestURL is the only parameter we care about
mastiz
2017/05/04 10:57:53
Removed kPageURL. I think FAVICON is worth verifyi
pkotwicz
2017/05/04 17:28:25
Fair enough
|
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kManifestURL, _, _)); |
pkotwicz
2017/05/01 04:56:53
We don't care about the page URL
mastiz
2017/05/04 10:57:53
Done.
|
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL16x16)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, GetFaviconFromExpiredManifest) { |
+ 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)); |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons); |
+ |
+ // The third notification is unnecessary, but allows simplifying the code. |
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kManifestURL, _, _)) |
+ .Times(3); |
+ EXPECT_CALL(favicon_service_, SetFavicons(kPageURL, kManifestURL, _, _)); |
pkotwicz
2017/05/01 04:56:53
We don't care about the page URL for either OnFavi
mastiz
2017/05/04 10:57:53
Done.
|
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL64x64)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, GetFaviconFromExpiredManifestLinkedFromOtherPage) { |
+ const GURL kSomePreviousPageURL("https://www.google.com/previous"); |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ const std::vector<favicon::FaviconURL> kManifestIcons = { |
+ FaviconURL(kIconURL64x64, FAVICON, kEmptySizes), |
+ }; |
+ |
+ favicon_service_.fake()->Store(kSomePreviousPageURL, kManifestURL, |
+ CreateRawBitmapResult(kManifestURL, FAVICON, |
+ /*expired=*/true)); |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons); |
+ |
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kManifestURL, _, _)) |
+ .Times(2); |
+ EXPECT_CALL(favicon_service_, SetFavicons(kPageURL, kManifestURL, _, _)); |
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL64x64)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, GetFaviconFromUnknownManifestButKnownIcon) { |
+ const GURL kSomePreviousPageURL("https://www.google.com/previous"); |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ const std::vector<favicon::FaviconURL> kManifestIcons = { |
+ FaviconURL(kIconURL16x16, FAVICON, kEmptySizes), |
+ }; |
+ |
+ favicon_service_.fake()->Store(kSomePreviousPageURL, kIconURL16x16, |
+ CreateRawBitmapResult(kIconURL16x16)); |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons); |
+ |
+ EXPECT_CALL(favicon_service_, |
+ SetFavicons(kPageURL, kManifestURL, FAVICON, _)); |
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kManifestURL, _, _)); |
+ |
+ RunHandlerWithSimpleFaviconCandidates(URLVector(), kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL)); |
+ // In the current implementation, the icon is downloaded although it's in the |
+ // database, simply because it's not associated to the manifest's URL. |
pkotwicz
2017/05/01 04:56:53
This comment is confusing.
Maybe remove the "simp
mastiz
2017/05/04 10:57:53
Rephrased, since I believe the "because" part is p
|
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL16x16)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, UnknownManifestReturning404) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ |
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kManifestURL)); |
+ EXPECT_CALL(favicon_service_, |
+ SetFavicons(kPageURL, kIconURL12x12, FAVICON, _)); |
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL, kIconURL12x12)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL12x12)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, UnknownManifestReturning503) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ |
+ delegate_.fake_manifest_downloader().AddError(kManifestURL, 503); |
+ |
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).Times(0); |
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+} |
+ |
+TEST_F(FaviconHandlerTest, IgnoreManifestWithPrior404) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ |
+ ON_CALL(favicon_service_, WasUnableToDownloadFavicon(kManifestURL)) |
+ .WillByDefault(Return(true)); |
+ |
+ EXPECT_CALL(delegate_, DownloadManifest(_, _)).Times(0); |
pkotwicz
2017/05/01 04:56:53
Isn't this covered by the expectation on line 1469
mastiz
2017/05/04 10:57:53
Unless DownloadManifest were used with kIconURL12x
pkotwicz
2017/05/04 17:28:25
I would rather that you drop it
mastiz
2017/05/10 10:03:52
Done.
|
+ EXPECT_CALL(favicon_service_, |
+ SetFavicons(kPageURL, kIconURL12x12, FAVICON, _)); |
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kIconURL12x12)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL12x12)); |
+} |
+ |
+// Test that the regular favicon is selected when: |
+// - The page links to a Web Manifest. |
+// - The Web Manifest does not contain any icon URLs (it is not a 404). |
+// - The page has an icon URL provided via a <link rel="icon"> tag. |
+// - The database does not know about the page URL, manifest URL or icon URL. |
+TEST_F(FaviconHandlerTest, UnknownManifestWithoutIcons) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, |
+ std::vector<favicon::FaviconURL>()); |
+ |
+ // UnableToDownloadFavicon() is expected to prevent repeated downloads of the |
+ // same manifest (which is not otherwise cached, since it doesn't contain |
+ // icons). |
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kManifestURL)); |
+ EXPECT_CALL(favicon_service_, |
+ SetFavicons(kPageURL, kIconURL12x12, FAVICON, _)); |
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL, kIconURL12x12)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL12x12)); |
+} |
+ |
+// Test that the regular favicon is selected when: |
+// - The page links to a Web Manifest. |
+// - The Web Manifest does not contain any icon URLs (it is not a 404). |
+// - The page has an icon URL provided via a <link rel="icon"> tag. |
+// - The database does not know about the page URL. |
+// - The database does not know about the manifest URL. |
+// - The database knows about the icon URL. |
+TEST_F(FaviconHandlerTest, UnknownManifestWithoutIconsAndKnownRegularIcons) { |
+ const GURL kSomePreviousPageURL("https://www.google.com/previous"); |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, |
+ std::vector<favicon::FaviconURL>()); |
+ favicon_service_.fake()->Store(kSomePreviousPageURL, kIconURL12x12, |
+ CreateRawBitmapResult(kIconURL12x12)); |
+ |
+ EXPECT_CALL(favicon_service_, SetFavicons(_, _, _, _)).Times(0); |
+ |
+ // UnableToDownloadFavicon() is expected to prevent repeated downloads of the |
+ // same manifest (which is not otherwise cached, since it doesn't contain |
+ // icons). |
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kManifestURL)); |
+ EXPECT_CALL(favicon_service_, |
+ UpdateFaviconMappingsAndFetch(kPageURL, URLVector{kManifestURL}, |
+ _, _, _, _)); |
+ EXPECT_CALL(favicon_service_, |
+ UpdateFaviconMappingsAndFetch(kPageURL, URLVector{kIconURL12x12}, |
+ _, _, _, _)); |
+ EXPECT_CALL(delegate_, OnFaviconUpdated(kPageURL, _, kIconURL12x12, _, _)); |
+ |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL); |
+ EXPECT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL, kIconURL12x12)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, ManifestUpdateViaJavascript) { |
+ const GURL kManifestURL1("http://www.google.com/manifest1.json"); |
+ const GURL kManifestURL2("http://www.google.com/manifest2.json"); |
+ const std::vector<favicon::FaviconURL> kManifestIcons1 = { |
+ FaviconURL(kIconURL64x64, FAVICON, kEmptySizes), |
+ }; |
+ const std::vector<favicon::FaviconURL> kManifestIcons2 = { |
+ FaviconURL(kIconURL10x10, FAVICON, kEmptySizes), |
+ }; |
+ |
+ delegate_.fake_manifest_downloader().Add(kManifestURL1, kManifestIcons1); |
+ delegate_.fake_manifest_downloader().Add(kManifestURL2, kManifestIcons2); |
+ |
+ EXPECT_CALL(delegate_, DownloadManifest(kManifestURL1, _)); |
pkotwicz
2017/05/01 04:56:53
This is redundant with line 1551?
mastiz
2017/05/04 10:57:53
Ditto. I can either ignore the problem and drop th
pkotwicz
2017/05/04 17:28:25
I would vote for dropping the line and ignoring th
mastiz
2017/05/10 10:03:52
Done.
|
+ |
+ std::unique_ptr<FaviconHandler> handler = |
+ RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL1); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL1)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL1, kIconURL64x64)); |
+ ASSERT_TRUE(VerifyAndClearExpectations()); |
+ |
+ // Simulate the page changing it's manifest URL via Javascript. |
+ EXPECT_CALL(delegate_, DownloadManifest(kManifestURL2, _)); |
pkotwicz
2017/05/01 04:56:53
Isn't this redundant with line 1563?
mastiz
2017/05/04 10:57:53
Ditto.
|
+ EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL2, _, _)); |
+ handler->OnUpdateCandidates(kPageURL, |
+ {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)}, |
+ kManifestURL2); |
+ base::RunLoop().RunUntilIdle(); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kManifestURL2)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL2, kIconURL10x10)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, RemoveManifestViaJavascriptWhileHistoryQuery) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ const std::vector<favicon::FaviconURL> kManifestIcons = { |
+ FaviconURL(kIconURL64x64, FAVICON, kEmptySizes), |
+ }; |
+ |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons); |
+ |
+ EXPECT_CALL(delegate_, DownloadManifest(_, _)).Times(0); |
+ |
+ auto handler = base::MakeUnique<FaviconHandler>( |
+ &favicon_service_, &delegate_, FaviconDriverObserver::NON_TOUCH_16_DIP); |
+ handler->FetchFavicon(kPageURL); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), ElementsAre(kPageURL)); |
+ base::RunLoop().RunUntilIdle(); |
+ ASSERT_FALSE(handler->HasPendingTasksForTest()); |
+ handler->OnUpdateCandidates(kPageURL, |
+ {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)}, |
+ kManifestURL); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL)); |
+ // Database lookup for |kManifestURL1| is ongoing. |
+ ASSERT_TRUE(handler->HasPendingTasksForTest()); |
+ |
+ // Simulate the page changing it's manifest URL to empty via Javascript. |
+ EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL12x12, _, _)); |
+ handler->OnUpdateCandidates(kPageURL, |
+ {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)}, |
+ base::nullopt); |
+ base::RunLoop().RunUntilIdle(); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kManifestURL, kIconURL12x12)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL12x12)); |
+} |
+ |
+TEST_F(FaviconHandlerTest, AddManifestViaJavascriptWhileHistoryQuery) { |
+ const GURL kManifestURL("http://www.google.com/manifest.json"); |
+ const std::vector<favicon::FaviconURL> kManifestIcons = { |
+ FaviconURL(kIconURL64x64, FAVICON, kEmptySizes), |
+ }; |
+ |
+ delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons); |
+ |
+ auto handler = base::MakeUnique<FaviconHandler>( |
+ &favicon_service_, &delegate_, FaviconDriverObserver::NON_TOUCH_16_DIP); |
+ handler->FetchFavicon(kPageURL); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), ElementsAre(kPageURL)); |
+ base::RunLoop().RunUntilIdle(); |
+ ASSERT_FALSE(handler->HasPendingTasksForTest()); |
+ handler->OnUpdateCandidates(kPageURL, |
+ {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)}, |
+ base::nullopt); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kIconURL12x12)); |
+ // Database lookup for |kIconURL12x12| is ongoing. |
+ ASSERT_TRUE(handler->HasPendingTasksForTest()); |
+ |
+ // Simulate the page changing it's manifest URL to |kManifestURL| via |
+ // Javascript. |
+ EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL, _, _)); |
+ handler->OnUpdateCandidates(kPageURL, |
+ {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)}, |
+ kManifestURL); |
+ base::RunLoop().RunUntilIdle(); |
+ ASSERT_THAT(favicon_service_.fake()->db_requests(), |
+ ElementsAre(kPageURL, kIconURL12x12, kManifestURL)); |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL64x64)); |
+} |
+ |
} // namespace |
} // namespace favicon |