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

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

Issue 2808063002: Add DownloadStatus metric to FaviconHandler (Closed)
Patch Set: 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 7ac69188630820a3e7be67807059e58274312047..f4a5a8dbf99b4f3ae5399762b5c0ac88cfc2b841 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -42,6 +42,7 @@ using testing::IsEmpty;
using testing::Return;
using testing::_;
+using DownloadOutcome = FaviconHandler::DownloadOutcome;
using IntVector = std::vector<int>;
using URLVector = std::vector<GURL>;
using BitmapVector = std::vector<SkBitmap>;
@@ -1042,7 +1043,7 @@ TEST_F(FaviconHandlerTest, TestRecordSingleFaviconDownloadAttempt) {
RunHandlerWithCandidates(
FaviconDriverObserver::NON_TOUCH_16_DIP,
- {FaviconURL(GURL("http://www.google.com/a"), FAVICON,
+ {FaviconURL(GURL(kIconURL64x64), FAVICON,
{gfx::Size(1024, 1024), gfx::Size(512, 512)})});
EXPECT_THAT(
@@ -1051,6 +1052,9 @@ TEST_F(FaviconHandlerTest, TestRecordSingleFaviconDownloadAttempt) {
EXPECT_THAT(
histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"),
IsEmpty());
+ EXPECT_THAT(histogram_tester.GetAllSamples("Favicons.DownloadOutcome"),
+ ElementsAre(base::Bucket(DownloadOutcome::SUCCEEDED,
+ /*expected_count=*/1)));
}
TEST_F(FaviconHandlerTest, TestRecordSingleLargeIconDownloadAttempts) {
@@ -1058,7 +1062,7 @@ TEST_F(FaviconHandlerTest, TestRecordSingleLargeIconDownloadAttempts) {
RunHandlerWithCandidates(
FaviconDriverObserver::NON_TOUCH_LARGEST,
- {FaviconURL(GURL("http://www.google.com/a"), FAVICON, kEmptySizes)});
+ {FaviconURL(GURL(kIconURL16x16), FAVICON, kEmptySizes)});
EXPECT_THAT(
histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"),
@@ -1067,7 +1071,7 @@ TEST_F(FaviconHandlerTest, TestRecordSingleLargeIconDownloadAttempts) {
// TOUCH_LARGEST fills the same bucket as NON_TOUCH_LARGEST.
RunHandlerWithCandidates(
FaviconDriverObserver::TOUCH_LARGEST,
- {FaviconURL(GURL("http://www.google.com/b"), TOUCH_ICON, kEmptySizes)});
+ {FaviconURL(GURL(kIconURL64x64), TOUCH_ICON, kEmptySizes)});
EXPECT_THAT(
histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"),
@@ -1075,6 +1079,9 @@ TEST_F(FaviconHandlerTest, TestRecordSingleLargeIconDownloadAttempts) {
EXPECT_THAT(
histogram_tester.GetAllSamples("Favicons.FaviconDownloadAttempts"),
IsEmpty());
+ EXPECT_THAT(histogram_tester.GetAllSamples("Favicons.DownloadOutcome"),
+ ElementsAre(base::Bucket(DownloadOutcome::SUCCEEDED,
+ /*expected_count=*/2)));
}
TEST_F(FaviconHandlerTest, TestRecordDownloadAttemptsFinishedByCache) {
@@ -1110,5 +1117,38 @@ TEST_F(FaviconHandlerTest, TestRecordDownloadAttemptsFinishedByCache) {
IsEmpty());
}
+TEST_F(FaviconHandlerTest, TestRecordSkippedDownloadAttemptAfterFailure) {
+ base::HistogramTester histogram_tester;
+ const GURL k404IconURL("http://www.google.com/404.png");
+
+ delegate_.fake_downloader().AddError(k404IconURL, 404);
+
+ // When calling an URL for the first time, it should log the failure.
+ EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(k404IconURL));
+
+ RunHandlerWithCandidates(
+ FaviconDriverObserver::NON_TOUCH_LARGEST,
+ {FaviconURL(GURL(k404IconURL), FAVICON, kEmptySizes)});
+
+ EXPECT_THAT(
+ histogram_tester.GetAllSamples("Favicons.DownloadOutcome"),
+ ElementsAre(base::Bucket(DownloadOutcome::FAILED, /*expected_count=*/1)));
+
+ // Now, the URL is known to be failing and should be skipped.
+
+ ON_CALL(favicon_service_, WasUnableToDownloadFavicon(k404IconURL))
+ .WillByDefault(Return(true));
+
+ RunHandlerWithCandidates(
+ FaviconDriverObserver::NON_TOUCH_LARGEST,
+ {FaviconURL(GURL(k404IconURL), FAVICON, kEmptySizes)});
+
+ EXPECT_THAT(
+ histogram_tester.GetAllSamples("Favicons.DownloadOutcome"),
+ ElementsAre(
+ base::Bucket(DownloadOutcome::FAILED, /*expected_count=*/1),
+ base::Bucket(DownloadOutcome::SKIPPED, /*expected_count=*/1)));
+}
+
} // namespace
} // namespace favicon

Powered by Google App Engine
This is Rietveld 408576698