Skip to content

Commit

Permalink
Add UMA Precache.Freshness.Prefetch and include validated resources.
Browse files Browse the repository at this point in the history 
 - Precache.Freshness.Prefetch measures the freshness (in seconds) of
   all the prefetched resources in seconds since the moment they were
   downloaded.
 - Expose HttpResponseInfo through the call chain of
   UpdatePrecacheMetricsAndState

Precache.Freshness.Prefetch will be useful as a comparison to
Precache.TimeSinceLastPrecache.

BUG=619211

Review-Url: https://codereview.chromium.org/2146023003
Cr-Commit-Position: refs/heads/master@{#407974}
  • Loading branch information
jamartin authored and Commit bot committed Jul 26, 2016
1 parent a254bd8 commit 8e1bdb6
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 81 deletions.
10 changes: 7 additions & 3 deletions chrome/browser/precache/precache_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@
#include "net/url_request/url_request.h"
#include "url/gurl.h"

namespace net {
class HttpResponseInfo;
}

namespace {

void UpdatePrecacheMetricsAndStateOnUIThread(const GURL& url,
const GURL& referrer,
base::TimeDelta latency,
const base::Time& fetch_time,
const net::HttpResponseInfo& info,
int64_t size,
bool was_cached,
bool is_user_traffic,
void* profile_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
Expand All @@ -38,7 +42,7 @@ void UpdatePrecacheMetricsAndStateOnUIThread(const GURL& url,
return;

precache_manager->UpdatePrecacheMetricsAndState(
url, referrer, latency, fetch_time, size, was_cached, is_user_traffic);
url, referrer, latency, fetch_time, info, size, is_user_traffic);
}

} // namespace
Expand All @@ -63,7 +67,7 @@ void UpdatePrecacheMetricsAndState(const net::URLRequest* request,
base::Bind(
&UpdatePrecacheMetricsAndStateOnUIThread, request->url(),
GURL(request->referrer()), latency, base::Time::Now(),
received_content_length, request->was_cached(),
request->response_info(), received_content_length,
data_use_measurement::DataUseMeasurement::IsUserInitiatedRequest(
request),
profile_id));
Expand Down
20 changes: 9 additions & 11 deletions components/precache/content/precache_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,12 @@ void PrecacheManager::UpdatePrecacheMetricsAndState(
const GURL& referrer,
const base::TimeDelta& latency,
const base::Time& fetch_time,
const net::HttpResponseInfo& info,
int64_t size,
bool was_cached,
bool is_user_traffic) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

RecordStatsForFetch(url, referrer, latency, fetch_time, size, was_cached);
RecordStatsForFetch(url, referrer, latency, fetch_time, info, size);
if (is_user_traffic && IsPrecaching())
CancelPrecaching();
}
Expand All @@ -238,8 +238,8 @@ void PrecacheManager::RecordStatsForFetch(const GURL& url,
const GURL& referrer,
const base::TimeDelta& latency,
const base::Time& fetch_time,
int64_t size,
bool was_cached) {
const net::HttpResponseInfo& info,
int64_t size) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

if (size == 0 || url.is_empty() || !url.SchemeIsHTTPOrHTTPS()) {
Expand All @@ -251,17 +251,16 @@ void PrecacheManager::RecordStatsForFetch(const GURL& url,
return;

history_service_->HostRankIfAvailable(
referrer,
base::Bind(&PrecacheManager::RecordStatsForFetchInternal, AsWeakPtr(),
url, latency, fetch_time, size, was_cached));
referrer, base::Bind(&PrecacheManager::RecordStatsForFetchInternal,
AsWeakPtr(), url, latency, fetch_time, info, size));
}

void PrecacheManager::RecordStatsForFetchInternal(
const GURL& url,
const base::TimeDelta& latency,
const base::Time& fetch_time,
const net::HttpResponseInfo& info,
int64_t size,
bool was_cached,
int host_rank) {
if (is_precaching_) {
// Assume that precache is responsible for all requests made while
Expand All @@ -273,7 +272,7 @@ void PrecacheManager::RecordStatsForFetchInternal(
BrowserThread::DB, FROM_HERE,
base::Bind(&PrecacheDatabase::RecordURLPrefetch,
base::Unretained(precache_database_.get()), url, latency,
fetch_time, size, was_cached));
fetch_time, info, size));
} else {
bool is_connection_cellular =
net::NetworkChangeNotifier::IsConnectionCellular(
Expand All @@ -283,8 +282,7 @@ void PrecacheManager::RecordStatsForFetchInternal(
BrowserThread::DB, FROM_HERE,
base::Bind(&PrecacheDatabase::RecordURLNonPrefetch,
base::Unretained(precache_database_.get()), url, latency,
fetch_time, size, was_cached, host_rank,
is_connection_cellular));
fetch_time, info, size, host_rank, is_connection_cellular));
}
}

Expand Down
12 changes: 8 additions & 4 deletions components/precache/content/precache_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ namespace history {
class HistoryService;
}

namespace net {
class HttpResponseInfo;
}

namespace sync_driver {
class SyncService;
}
Expand Down Expand Up @@ -108,8 +112,8 @@ class PrecacheManager : public KeyedService,
const GURL& referrer,
const base::TimeDelta& latency,
const base::Time& fetch_time,
const net::HttpResponseInfo& info,
int64_t size,
bool was_cached,
bool is_user_traffic);

private:
Expand Down Expand Up @@ -159,16 +163,16 @@ class PrecacheManager : public KeyedService,
const GURL& referrer,
const base::TimeDelta& latency,
const base::Time& fetch_time,
int64_t size,
bool was_cached);
const net::HttpResponseInfo& info,
int64_t size);

// Update precache-related metrics in response to a URL being fetched. Called
// by RecordStatsForFetch() by way of an asynchronous HistoryService callback.
void RecordStatsForFetchInternal(const GURL& url,
const base::TimeDelta& latency,
const base::Time& fetch_time,
const net::HttpResponseInfo& info,
int64_t size,
bool was_cached,
int host_rank);

// The browser context that owns this PrecacheManager.
Expand Down
69 changes: 40 additions & 29 deletions components/precache/content/precache_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/test/test_browser_context.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_response_info.h"
#include "net/http/http_status_code.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_status.h"
Expand All @@ -46,6 +48,7 @@ namespace {
using ::testing::_;
using ::testing::ContainerEq;
using ::testing::ElementsAre;
using ::testing::UnorderedElementsAre;
using ::testing::Invoke;
using ::testing::IsEmpty;
using ::testing::Pair;
Expand Down Expand Up @@ -189,6 +192,8 @@ class PrecacheManagerTest : public testing::Test {
&browser_context_, nullptr /* sync_service */,
&history_service_, db_path, std::move(precache_database)));
base::RunLoop().RunUntilIdle();

info_.headers = new net::HttpResponseHeaders("");
}

// Must be declared first so that it is destroyed last.
Expand All @@ -202,6 +207,7 @@ class PrecacheManagerTest : public testing::Test {
TestPrecacheCompletionCallback precache_callback_;
testing::NiceMock<MockHistoryService> history_service_;
base::HistogramTester histograms_;
net::HttpResponseInfo info_;
};

TEST_F(PrecacheManagerTest, StartAndFinishPrecaching) {
Expand Down Expand Up @@ -354,25 +360,25 @@ TEST_F(PrecacheManagerTest, StartAndCancelPrecachingAfterURLsReceived) {
TEST_F(PrecacheManagerTest, RecordStatsForFetchWithSizeZero) {
// Fetches with size 0 should be ignored.
precache_manager_->RecordStatsForFetch(GURL("http://url.com"), GURL(),
base::TimeDelta(), base::Time(), 0,
false);
base::TimeDelta(), base::Time(), info_,
0);
base::RunLoop().RunUntilIdle();
EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."), IsEmpty());
}

TEST_F(PrecacheManagerTest, RecordStatsForFetchWithNonHTTP) {
// Fetches for URLs with schemes other than HTTP or HTTPS should be ignored.
precache_manager_->RecordStatsForFetch(GURL("ftp://ftp.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
base::TimeDelta(), base::Time(), info_,
1000);
base::RunLoop().RunUntilIdle();
EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."), IsEmpty());
}

TEST_F(PrecacheManagerTest, RecordStatsForFetchWithEmptyURL) {
// Fetches for empty URLs should be ignored.
precache_manager_->RecordStatsForFetch(GURL(), GURL(), base::TimeDelta(),
base::Time(), 1000, false);
base::Time(), info_, 1000);
base::RunLoop().RunUntilIdle();
EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."), IsEmpty());
}
Expand All @@ -385,26 +391,28 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchDuringPrecaching) {

EXPECT_TRUE(precache_manager_->IsPrecaching());
precache_manager_->RecordStatsForFetch(GURL("http://url.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
base::TimeDelta(), base::Time(), info_,
1000);
base::RunLoop().RunUntilIdle();
precache_manager_->CancelPrecaching();

// For PrecacheFetcher and RecordURLPrecached.
base::RunLoop().RunUntilIdle();
EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."),
ElementsAre(Pair("Precache.DownloadedPrecacheMotivated", 1),
Pair("Precache.Fetch.PercentCompleted", 1),
Pair("Precache.Fetch.ResponseBytes.Network", 1),
Pair("Precache.Fetch.ResponseBytes.Total", 1),
Pair("Precache.Fetch.TimeToComplete", 1),
Pair("Precache.Latency.Prefetch", 1)));
EXPECT_THAT(
histograms_.GetTotalCountsForPrefix("Precache."),
UnorderedElementsAre(Pair("Precache.DownloadedPrecacheMotivated", 1),
Pair("Precache.Fetch.PercentCompleted", 1),
Pair("Precache.Fetch.ResponseBytes.Network", 1),
Pair("Precache.Fetch.ResponseBytes.Total", 1),
Pair("Precache.Fetch.TimeToComplete", 1),
Pair("Precache.Latency.Prefetch", 1),
Pair("Precache.Freshness.Prefetch", 1)));
}

TEST_F(PrecacheManagerTest, RecordStatsForFetchHTTP) {
precache_manager_->RecordStatsForFetch(GURL("http://http-url.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
base::TimeDelta(), base::Time(), info_,
1000);
base::RunLoop().RunUntilIdle();

EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."),
Expand All @@ -415,8 +423,8 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchHTTP) {

TEST_F(PrecacheManagerTest, RecordStatsForFetchHTTPS) {
precache_manager_->RecordStatsForFetch(GURL("https://https-url.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
base::TimeDelta(), base::Time(), info_,
1000);
base::RunLoop().RunUntilIdle();

EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."),
Expand All @@ -434,7 +442,7 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchInTopHosts) {
}));
precache_manager_->RecordStatsForFetch(
GURL("http://http-url.com"), GURL("http://referrer.com"),
base::TimeDelta(), base::Time(), 1000, false);
base::TimeDelta(), base::Time(), info_, 1000);
base::RunLoop().RunUntilIdle();

EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."),
Expand All @@ -459,19 +467,20 @@ TEST_F(PrecacheManagerTest, DeleteExpiredPrecacheHistory) {
// Precache a bunch of URLs, with different fetch times.
precache_manager_->RecordStatsForFetch(
GURL("http://old-fetch.com"), GURL(), base::TimeDelta(),
kCurrentTime - base::TimeDelta::FromDays(61), 1000, false);
kCurrentTime - base::TimeDelta::FromDays(61), info_, 1000);
precache_manager_->RecordStatsForFetch(
GURL("http://recent-fetch.com"), GURL(), base::TimeDelta(),
kCurrentTime - base::TimeDelta::FromDays(59), 1000, false);
kCurrentTime - base::TimeDelta::FromDays(59), info_, 1000);
precache_manager_->RecordStatsForFetch(
GURL("http://yesterday-fetch.com"), GURL(), base::TimeDelta(),
kCurrentTime - base::TimeDelta::FromDays(1), 1000, false);
kCurrentTime - base::TimeDelta::FromDays(1), info_, 1000);
expected_histogram_count_map["Precache.DownloadedPrecacheMotivated"] += 3;
expected_histogram_count_map["Precache.Fetch.PercentCompleted"]++;
expected_histogram_count_map["Precache.Fetch.ResponseBytes.Network"]++;
expected_histogram_count_map["Precache.Fetch.ResponseBytes.Total"]++;
expected_histogram_count_map["Precache.Fetch.TimeToComplete"]++;
expected_histogram_count_map["Precache.Latency.Prefetch"] += 3;
expected_histogram_count_map["Precache.Freshness.Prefetch"] += 3;
base::RunLoop().RunUntilIdle();

precache_manager_->CancelPrecaching();
Expand Down Expand Up @@ -504,9 +513,10 @@ TEST_F(PrecacheManagerTest, DeleteExpiredPrecacheHistory) {
// A fetch for the same URL as the expired precache was served from the cache,
// but it isn't reported as saved bytes because it had expired in the precache
// history.
info_.was_cached = true; // From now on all fetches are cached.
precache_manager_->RecordStatsForFetch(GURL("http://old-fetch.com"), GURL(),
base::TimeDelta(), kCurrentTime, 1000,
true);
base::TimeDelta(), kCurrentTime, info_,
1000);
expected_histogram_count_map["Precache.Fetch.TimeToComplete"]++;
expected_histogram_count_map["Precache.Latency.NonPrefetch"]++;
expected_histogram_count_map["Precache.Latency.NonPrefetch.NonTopHosts"]++;
Expand All @@ -518,16 +528,17 @@ TEST_F(PrecacheManagerTest, DeleteExpiredPrecacheHistory) {

// The other precaches should not have expired, so the following fetches from
// the cache should count as saved bytes.
precache_manager_->RecordStatsForFetch(GURL("http://recent-fetch.com"), GURL(),
base::TimeDelta(), kCurrentTime, 1000,
true);
precache_manager_->RecordStatsForFetch(GURL("http://recent-fetch.com"),
GURL(), base::TimeDelta(),
kCurrentTime, info_, 1000);
precache_manager_->RecordStatsForFetch(GURL("http://yesterday-fetch.com"),
GURL(), base::TimeDelta(), kCurrentTime,
1000, true);
GURL(), base::TimeDelta(),
kCurrentTime, info_, 1000);
expected_histogram_count_map["Precache.Latency.NonPrefetch"] += 2;
expected_histogram_count_map["Precache.Latency.NonPrefetch.NonTopHosts"] += 2;
expected_histogram_count_map["Precache.Saved"] += 2;
expected_histogram_count_map["Precache.TimeSinceLastPrecache"] += 2;
expected_histogram_count_map["Precache.Saved.Freshness"] = 2;

base::RunLoop().RunUntilIdle();
EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."),
Expand Down
Loading

0 comments on commit 8e1bdb6

Please sign in to comment.