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..0d1877adc0bf1ebf7846a20ce8fd8aaf90573743 100644 |
--- a/components/favicon/core/favicon_handler_unittest.cc |
+++ b/components/favicon/core/favicon_handler_unittest.cc |
@@ -16,6 +16,7 @@ |
#include "base/message_loop/message_loop.h" |
#include "base/run_loop.h" |
#include "base/strings/stringprintf.h" |
+#include "base/test/histogram_tester.h" |
#include "components/favicon/core/favicon_driver.h" |
#include "components/favicon/core/test/mock_favicon_service.h" |
#include "testing/gmock/include/gmock/gmock.h" |
@@ -78,20 +79,21 @@ std::vector<unsigned char> FillBitmapWithEdgeSize(int size) { |
std::vector<FaviconRawBitmapResult> CreateRawBitmapResult( |
const GURL& icon_url, |
favicon_base::IconType icon_type = FAVICON, |
- bool expired = false) { |
+ bool expired = false, |
+ int edge_size = gfx::kFaviconSize) { |
scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes()); |
- data->data() = FillBitmapWithEdgeSize(gfx::kFaviconSize); |
+ data->data() = FillBitmapWithEdgeSize(edge_size); |
FaviconRawBitmapResult bitmap_result; |
bitmap_result.expired = expired; |
bitmap_result.bitmap_data = data; |
// Use a pixel size other than (0,0) as (0,0) has a special meaning. |
- bitmap_result.pixel_size = gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize); |
+ bitmap_result.pixel_size = gfx::Size(edge_size, edge_size); |
bitmap_result.icon_type = icon_type; |
bitmap_result.icon_url = icon_url; |
return {bitmap_result}; |
} |
-// Fake that implements the calls to FaviconHalder::Delegate's DownloadImage(), |
+// Fake that implements the calls to FaviconHandler::Delegate's DownloadImage(), |
// delegated to this class through MockDelegate. |
class FakeImageDownloader { |
public: |
@@ -382,6 +384,7 @@ class FaviconHandlerTest : public testing::Test { |
}; |
TEST_F(FaviconHandlerTest, GetFaviconFromHistory) { |
+ base::HistogramTester histogram_tester; |
const GURL kIconURL("http://www.google.com/favicon"); |
favicon_service_.fake()->Store(kPageURL, kIconURL, |
@@ -393,6 +396,12 @@ TEST_F(FaviconHandlerTest, GetFaviconFromHistory) { |
RunHandlerWithSimpleFaviconCandidates({kIconURL}); |
EXPECT_THAT(delegate_.downloads(), IsEmpty()); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"), |
+ IsEmpty()); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.FaviconDownloadAttempts"), |
+ IsEmpty()); |
} |
// Test that UpdateFaviconsAndFetch() is called with the appropriate parameters |
@@ -535,15 +544,12 @@ TEST_F(FaviconHandlerTest, UpdateAndDownloadFavicon) { |
// - The icon is redownloaded. |
TEST_F(FaviconHandlerTest, FaviconInHistoryInvalid) { |
// Set non empty but invalid data. |
- FaviconRawBitmapResult bitmap_result; |
- bitmap_result.expired = false; |
+ std::vector<FaviconRawBitmapResult> bitmap_result = |
+ CreateRawBitmapResult(kIconURL16x16); |
// Empty bitmap data is invalid. |
- bitmap_result.bitmap_data = new base::RefCountedBytes(); |
- bitmap_result.pixel_size = gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize); |
- bitmap_result.icon_type = FAVICON; |
- bitmap_result.icon_url = kIconURL16x16; |
+ bitmap_result[0].bitmap_data = new base::RefCountedBytes(); |
- favicon_service_.fake()->Store(kPageURL, kIconURL16x16, {bitmap_result}); |
+ favicon_service_.fake()->Store(kPageURL, kIconURL16x16, bitmap_result); |
// TODO(crbug.com/700811): It would be nice if we could check the image |
// being published to rule out invalid data. |
@@ -1031,5 +1037,110 @@ TEST_F(FaviconHandlerTest, TestKeepDownloadedLargestFavicon) { |
FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)}); |
} |
+TEST_F(FaviconHandlerTest, TestRecordMultipleDownloadAttempts) { |
+ base::HistogramTester histogram_tester; |
+ |
+ RunHandlerWithCandidates( |
+ FaviconDriverObserver::NON_TOUCH_LARGEST, |
+ {FaviconURL(GURL("http://www.google.com/a"), FAVICON, |
+ {gfx::Size(1024, 1024), gfx::Size(512, 512)}), |
+ FaviconURL(GURL("http://www.google.com/b"), FAVICON, |
+ {gfx::Size(15, 15), gfx::Size(14, 14)}), |
+ FaviconURL(GURL("http://www.google.com/c"), FAVICON, |
+ {gfx::Size(16, 16), gfx::Size(512, 512)})}); |
pkotwicz
2017/04/10 17:26:10
- Are multiple FaviconURLs helpful for this test?
fhorschig
2017/04/11 12:07:21
Yes, I want to log every single failed attempt. Co
|
+ |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"), |
+ ElementsAre(base::Bucket(/*sample=*/3, /*expected_count=*/1))); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.FaviconDownloadAttempts"), |
+ IsEmpty()); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.TouchIconDownloadAttempts"), |
+ IsEmpty()); |
+} |
+ |
+TEST_F(FaviconHandlerTest, TestRecordSingleFaviconDownloadAttempt) { |
+ base::HistogramTester histogram_tester; |
+ |
+ RunHandlerWithCandidates( |
+ FaviconDriverObserver::NON_TOUCH_16_DIP, |
+ {FaviconURL(GURL("http://www.google.com/a"), FAVICON, |
+ {gfx::Size(1024, 1024), gfx::Size(512, 512)})}); |
+ |
pkotwicz
2017/04/10 17:26:10
Are multiple sizes per FaviconURL helpful for this
fhorschig
2017/04/11 12:07:21
Gone.
|
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.FaviconDownloadAttempts"), |
+ ElementsAre(base::Bucket(/*sample=*/1, /*expected_count=*/1))); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"), |
+ IsEmpty()); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.TouchIconDownloadAttempts"), |
+ IsEmpty()); |
+} |
+ |
+TEST_F(FaviconHandlerTest, TestRecordSingleLargeIconDownloadAttempts) { |
+ base::HistogramTester histogram_tester; |
+ |
+ RunHandlerWithCandidates( |
pkotwicz
2017/04/10 17:26:10
Nit: Can you use RunHandlerWithSimpleCandidates()
fhorschig
2017/04/11 12:07:21
No, I want to set the NotificationIconType to NON_
|
+ FaviconDriverObserver::NON_TOUCH_LARGEST, |
+ {FaviconURL(GURL("http://www.google.com/a"), FAVICON, kEmptySizes)}); |
+ |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"), |
+ ElementsAre(base::Bucket(/*sample=*/1, /*expected_count=*/1))); |
+ |
+ // TOUCH_LARGEST fills the same bucket as NON_TOUCH_LARGEST. |
+ RunHandlerWithCandidates( |
+ FaviconDriverObserver::TOUCH_LARGEST, |
+ {FaviconURL(GURL("http://www.google.com/b"), TOUCH_ICON, kEmptySizes)}); |
+ |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"), |
+ ElementsAre(base::Bucket(/*sample=*/1, /*expected_count=*/1))); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.FaviconDownloadAttempts"), |
+ IsEmpty()); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.TouchIconDownloadAttempts"), |
+ ElementsAre(base::Bucket(/*sample=*/1, /*expected_count=*/1))); |
+} |
+ |
+TEST_F(FaviconHandlerTest, TestRecordDownloadAttemptsFinishedByCache) { |
+ // Names represent the bitmap sizes per icon. |
+ const GURL kIconURL1024_512("http://www.google.com/a"); |
+ const GURL kIconURL15_14("http://www.google.com/b"); |
+ const GURL kIconURL16_512("http://www.google.com/c"); |
pkotwicz
2017/04/10 17:26:10
Are multiple sizes per FaviconURL helpful for this
fhorschig
2017/04/11 12:07:21
Gone.
|
+ base::HistogramTester histogram_tester; |
+ favicon_service_.fake()->Store( |
+ GURL("http://so.de"), kIconURL16_512, |
+ {CreateRawBitmapResult(kIconURL16_512, FAVICON, /*expired=*/false, |
+ /*edge_size=*/16)[0], |
+ CreateRawBitmapResult(kIconURL16_512, FAVICON, /*expired=*/false, |
+ /*edge_size=*/512)[0]}); |
+ |
+ RunHandlerWithCandidates( |
+ FaviconDriverObserver::NON_TOUCH_LARGEST, |
+ {FaviconURL(kIconURL1024_512, FAVICON, |
+ {gfx::Size(1024, 1024), gfx::Size(512, 512)}), |
+ FaviconURL(kIconURL15_14, FAVICON, |
+ {gfx::Size(15, 15), gfx::Size(14, 14)}), |
+ FaviconURL(kIconURL16_512, FAVICON, |
+ {gfx::Size(16, 16), gfx::Size(512, 512)})}); |
+ |
+ // Should try only the first (receive 404) and get second icon from cache. |
+ EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL1024_512)); |
+ |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.LargeIconDownloadAttempts"), |
+ ElementsAre(base::Bucket(/*sample=*/1, /*expected_count=*/1))); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.FaviconDownloadAttempts"), |
+ IsEmpty()); |
+ EXPECT_THAT( |
+ histogram_tester.GetAllSamples("Favicons.TouchIconDownloadAttempts"), |
+ IsEmpty()); |
+} |
+ |
} // namespace |
} // namespace favicon |