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

Unified Diff: components/precache/core/precache_fetcher_unittest.cc

Issue 2711473006: Add Precache.Fetch.MinWeight UMA. (Closed)
Patch Set: Track min_weight_fetched in unfinished_work, and add tests. Created 3 years, 10 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/precache/core/precache_fetcher_unittest.cc
diff --git a/components/precache/core/precache_fetcher_unittest.cc b/components/precache/core/precache_fetcher_unittest.cc
index f42c6c39ba1a525dba24316bbbf010103281b0c9..0549e215bcc778f3d6e824892d7c896194e0c173 100644
--- a/components/precache/core/precache_fetcher_unittest.cc
+++ b/components/precache/core/precache_fetcher_unittest.cc
@@ -1091,7 +1091,9 @@ TEST_F(PrecacheFetcherTest, MaxBytesTotal) {
SetDefaultFlags();
std::unique_ptr<PrecacheUnfinishedWork> unfinished_work(
new PrecacheUnfinishedWork());
- unfinished_work->add_top_host()->set_hostname("good-manifest.com");
+ auto* top_host = unfinished_work->add_top_host();
+ top_host->set_hostname("good-manifest.com");
+ top_host->set_visits(1);
unfinished_work->set_start_time(base::Time::UnixEpoch().ToInternalValue());
// Should be greater than kMaxParallelFetches, so that we can observe
@@ -1110,6 +1112,7 @@ TEST_F(PrecacheFetcherTest, MaxBytesTotal) {
PrecacheConfigurationSettings config;
config.set_max_bytes_total(kMaxBytesTotal);
+ config.set_global_ranking(true);
factory_.SetFakeResponse(GURL(kConfigURL), config.SerializeAsString(),
net::HTTP_OK, net::URLRequestStatus::SUCCESS);
@@ -1117,7 +1120,9 @@ TEST_F(PrecacheFetcherTest, MaxBytesTotal) {
PrecacheManifest good_manifest;
for (size_t i = 0; i < kNumResources; ++i) {
const std::string url = "http://good-manifest.com/" + std::to_string(i);
- good_manifest.add_resource()->set_url(url);
+ auto* resource = good_manifest.add_resource();
+ resource->set_url(url);
+ resource->set_weight_ratio(static_cast<double>(i) / kNumResources);
factory_.SetFakeResponse(GURL(url), std::string(kBytesPerResource, '.'),
net::HTTP_OK, net::URLRequestStatus::SUCCESS);
}
@@ -1139,14 +1144,17 @@ TEST_F(PrecacheFetcherTest, MaxBytesTotal) {
}
// Fetcher should request config, manifest, and all but 3 resources.
- // TODO(twifkak): I expected all but 2 resources; this result is surprising.
- // Figure it out and explain it here.
- EXPECT_EQ(kNumResources - 1, url_callback_.requested_urls().size());
+ EXPECT_EQ(kNumResources - 2, url_callback_.requested_urls().size());
jamartin 2017/02/24 21:23:29 If it is config (1), manifest (1) and all but 3 re
twifkak 2017/02/25 01:13:39 Good point. Added a comment, and clarified the mat
EXPECT_TRUE(precache_delegate_.was_on_done_called());
histogram.ExpectTotalCount("Precache.Fetch.PercentCompleted", 1);
histogram.ExpectTotalCount("Precache.Fetch.TimeToComplete", 1);
+
+ const double expected_min_weight =
+ good_manifest.resource(kNumResources - 3).weight_ratio();
jamartin 2017/02/24 21:23:29 good_manifest.resource.weight_ratio is the weight
twifkak 2017/02/25 01:13:39 Per line 1096, top_host->visits() == 1, up until l
+ histogram.ExpectBucketCount("Precache.Fetch.MinWeight",
+ 1000.0 * expected_min_weight, 1);
}
// Tests the parallel fetch behaviour when more precache resource and manifest
@@ -1618,6 +1626,9 @@ TEST_F(PrecacheFetcherTest, GloballyRankResourcesAfterPauseResume) {
// Continuing with the precache should fetch all resources, as the previous
// run was cancelled before any finished. They should be fetched in global
// ranking order.
+
+ base::HistogramTester histogram;
+
{
PrecacheFetcher precache_fetcher(
request_context_.get(), GURL(), std::string(),
@@ -1629,6 +1640,9 @@ TEST_F(PrecacheFetcherTest, GloballyRankResourcesAfterPauseResume) {
}
EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls());
EXPECT_TRUE(precache_delegate_.was_on_done_called());
+
+ histogram.ExpectBucketCount("Precache.Fetch.MinWeight",
+ 1000.0 * resources.back().second, 1);
}
TEST_F(PrecacheFetcherTest, MaxTotalResources) {
@@ -1674,6 +1688,8 @@ TEST_F(PrecacheFetcherTest, MaxTotalResources) {
manifest.SerializeAsString(), net::HTTP_OK,
net::URLRequestStatus::SUCCESS);
+ base::HistogramTester histogram;
+
{
PrecacheFetcher precache_fetcher(
request_context_.get(), GURL(), std::string(),
@@ -1685,6 +1701,11 @@ TEST_F(PrecacheFetcherTest, MaxTotalResources) {
EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls());
EXPECT_TRUE(precache_delegate_.was_on_done_called());
+
+ const float expected_min_weight =
+ manifest.resource(config.total_resources_count() - 1).weight_ratio();
+ histogram.ExpectUniqueSample("Precache.Fetch.MinWeight",
+ 1000.0 * expected_min_weight, 1);
}
TEST_F(PrecacheFetcherTest, MinWeight) {

Powered by Google App Engine
This is Rietveld 408576698