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

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

Issue 2781093002: Add attempt count metric to FaviconHandler (Closed)
Patch Set: Created 3 years, 9 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 5cbe691f441020857267ecfeac8505a508e94c74..26e8c32dbf96d7eca2c7779ca4242e0d2d9b4c76 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -8,6 +8,7 @@
#include <memory>
#include <set>
+#include <string>
#include <vector>
#include "base/macros.h"
@@ -15,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"
@@ -33,6 +35,7 @@ using favicon_base::FaviconRawBitmapResult;
using favicon_base::TOUCH_ICON;
using favicon_base::TOUCH_PRECOMPOSED_ICON;
using testing::Assign;
+using testing::Contains;
using testing::ElementsAre;
using testing::InSequence;
using testing::Invoke;
@@ -90,7 +93,7 @@ std::vector<FaviconRawBitmapResult> CreateRawBitmapResult(
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:
@@ -321,7 +324,7 @@ class FaviconHandlerTest : public testing::Test {
const GURL kIconURL16x16 = GURL("http://www.google.com/favicon16x16");
const GURL kIconURL64x64 = GURL("http://www.google.com/favicon64x64");
- FaviconHandlerTest() {
+ FaviconHandlerTest() : download_histogram_(new base::HistogramTester()) {
// Register various known icon URLs.
delegate_.fake_downloader().Add(kIconURL10x10, IntVector{10});
delegate_.fake_downloader().Add(kIconURL12x12, IntVector{12});
@@ -337,10 +340,22 @@ class FaviconHandlerTest : public testing::Test {
new ui::test::ScopedSetSupportedScaleFactors({ui::SCALE_FACTOR_100P}));
}
+ void TearDown() override {
+ // Check that if any downloads occurred, the attempt count was recorded.
+ if (!download_histogram_name_.empty() && delegate_.downloads().size() > 0) {
+ download_histogram_->ExpectBucketCount(
+ download_histogram_name_,
+ /*sample=*/delegate_.downloads().size(),
+ /*expected_count=*/1);
+ }
+ }
+
bool VerifyAndClearExpectations() {
base::RunLoop().RunUntilIdle();
favicon_service_.fake()->ClearDbRequests();
delegate_.fake_downloader().ClearDownloads();
+ // Drop download histogram without verification.
+ download_histogram_ = base::MakeUnique<base::HistogramTester>();
return testing::Mock::VerifyAndClearExpectations(&favicon_service_) &&
testing::Mock::VerifyAndClearExpectations(&delegate_);
}
@@ -350,6 +365,10 @@ class FaviconHandlerTest : public testing::Test {
std::unique_ptr<FaviconHandler> RunHandlerWithCandidates(
FaviconDriverObserver::NotificationIconType handler_type,
const std::vector<favicon::FaviconURL>& candidates) {
+ download_histogram_name_ =
+ handler_type == FaviconDriverObserver::TOUCH_LARGEST
+ ? "Favicons.LargeIconDownloadAttempts"
+ : "Favicons.DownloadAttempts";
auto handler = base::MakeUnique<FaviconHandler>(&favicon_service_,
&delegate_, handler_type);
handler->FetchFavicon(kPageURL);
@@ -373,9 +392,11 @@ class FaviconHandlerTest : public testing::Test {
candidates);
}
+ std::string download_histogram_name_;
base::MessageLoopForUI message_loop_;
std::unique_ptr<ui::test::ScopedSetSupportedScaleFactors>
scoped_set_supported_scale_factors_;
+ std::unique_ptr<base::HistogramTester> download_histogram_;
testing::NiceMock<MockFaviconServiceWithFake> favicon_service_;
testing::NiceMock<MockDelegate> delegate_;
};
@@ -607,6 +628,7 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
std::unique_ptr<FaviconHandler> handler =
RunHandlerWithSimpleFaviconCandidates({kIconURL1, kIconURL2});
+ download_histogram_->ExpectTotalCount("Favicons.DownloadAttempts", 0);
ASSERT_TRUE(VerifyAndClearExpectations());
ASSERT_TRUE(delegate_.fake_downloader().HasPendingManualCallback());
@@ -646,6 +668,7 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsWhileProcessingShouldBeNoop) {
ASSERT_THAT(favicon_service_.fake()->db_requests(),
ElementsAre(kPageURL, kIconURL64x64, kSlowLoadingIconURL));
+ download_histogram_->ExpectTotalCount("Favicons.DownloadAttempts", 0);
ASSERT_TRUE(VerifyAndClearExpectations());
ASSERT_TRUE(delegate_.fake_downloader().HasPendingManualCallback());
@@ -674,6 +697,10 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsAfterFinishedShouldBeNoop) {
std::unique_ptr<FaviconHandler> handler = RunHandlerWithCandidates(
FaviconDriverObserver::NON_TOUCH_16_DIP, favicon_urls);
+ download_histogram_->ExpectBucketCount(
+ "Favicons.DownloadAttempts",
+ /*sample=*/delegate_.downloads().size(),
+ /*expected_count=*/1);
ASSERT_TRUE(VerifyAndClearExpectations());
// Calling OnUpdateFaviconURL() with identical data should be a no-op.

Powered by Google App Engine
This is Rietveld 408576698