Add prefetch UMA metrics.

components/precache/core/precache_fetcher.cc

Index: components/precache/core/precache_fetcher.cc
diff --git a/components/precache/core/precache_fetcher.cc b/components/precache/core/precache_fetcher.cc
index 062cc75e6838405a529b36ae530cffa48f89935f..2fc20fcb7d13844b5b3bbbc13b0c7e92937e2828 100644
--- a/components/precache/core/precache_fetcher.cc
+++ b/components/precache/core/precache_fetcher.cc
@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/containers/hash_tables.h"
+#include "base/metrics/histogram_macros.h"
 #include "components/precache/core/precache_switches.h"
 #include "components/precache/core/proto/precache.pb.h"
 #include "net/base/escape.h"
@@ -27,6 +28,10 @@ namespace precache {
 
 namespace {
 
+// The 99%ile number of bytes we expect to receive during prefetch, and the max

[bengr, 2015/06/30 19:26:19]

99%ile -> 99th percentile

and the max -> which will be used as the maximum for the xxx histogram

[twifkak, 2015/06/30 20:23:51]

Done.

+// for the histogram.
+const int kMaxResponseBytes = 100 * 1024 * 1024;
+
 GURL GetConfigURL() {
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
@@ -108,19 +113,25 @@ class PrecacheFetcher::Fetcher : public net::URLFetcherDelegate {
   Fetcher(net::URLRequestContextGetter* request_context, const GURL& url,
           const base::Callback<void(const URLFetcher&)>& callback);
   ~Fetcher() override {}
+  void OnURLFetchDownloadProgress(const URLFetcher* source,
+                                  int64 current,
+                                  int64 total) override;
   void OnURLFetchComplete(const URLFetcher* source) override;
+  int response_bytes() { return response_bytes_; }
 
  private:
   const base::Callback<void(const URLFetcher&)> callback_;
   scoped_ptr<URLFetcher> url_fetcher_;
+  int response_bytes_;
 
   DISALLOW_COPY_AND_ASSIGN(Fetcher);
 };
 
 PrecacheFetcher::Fetcher::Fetcher(
-    net::URLRequestContextGetter* request_context, const GURL& url,
+    net::URLRequestContextGetter* request_context,
+    const GURL& url,
     const base::Callback<void(const URLFetcher&)>& callback)
-    : callback_(callback) {
+    : callback_(callback), response_bytes_(0) {
   url_fetcher_ = URLFetcher::Create(url, URLFetcher::GET, this);
   url_fetcher_->SetRequestContext(request_context);
   url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
@@ -128,6 +139,13 @@ PrecacheFetcher::Fetcher::Fetcher(
   url_fetcher_->Start();
 }
 
+void PrecacheFetcher::Fetcher::OnURLFetchDownloadProgress(
+    const URLFetcher* source,
+    int64 current,
+    int64 total) {
+  response_bytes_ = current;
+}
+
 void PrecacheFetcher::Fetcher::OnURLFetchComplete(const URLFetcher* source) {
   callback_.Run(*source);
 }
@@ -140,7 +158,9 @@ PrecacheFetcher::PrecacheFetcher(
     : starting_hosts_(starting_hosts),
       request_context_(request_context),
       manifest_url_prefix_(manifest_url_prefix),
-      precache_delegate_(precache_delegate) {
+      precache_delegate_(precache_delegate),
+      total_response_bytes_(0),
+      num_manifest_urls_to_fetch_(0) {
   DCHECK(request_context_.get());  // Request context must be non-NULL.
   DCHECK(precache_delegate_);  // Precache delegate must be non-NULL.
 
@@ -151,6 +171,22 @@ PrecacheFetcher::PrecacheFetcher(
 }
 
 PrecacheFetcher::~PrecacheFetcher() {
+  // Number of manifests for which we have downloaded all resources.
+  int manifests_completed =
+      num_manifest_urls_to_fetch_ - manifest_urls_to_fetch_.size();
+
+  // If there are resource URLs left to fetch, the last manifest is not yet
+  // completed.
+  if (!resource_urls_to_fetch_.empty())
+    --manifests_completed;
+
+  int percent_completed =
+      manifests_completed == 0 ? 0 : (static_cast<double>(manifests_completed) /
+                                      num_manifest_urls_to_fetch_ * 100);

[bengr, 2015/06/30 19:26:19]

Can num_manifest_urls_to_fetch_ be 0?

[twifkak, 2015/06/30 20:23:51]

Yes, but if num_manifests_urls_to_fetch_ is 0, so is manifests_completed. This
is tested by PrecacheFetcherTest.Cancel.

[bengr, 2015/07/06 17:31:08]

DCHECK(num_manifest_urls_to_fetch_ >= manifests_completed) ?

[twifkak, 2015/07/07 00:26:28]

OK, added a different DCHECK, but changed the conditional to make it clear
there's no SIGFPE.

+  UMA_HISTOGRAM_PERCENTAGE("Precache.Fetch.PercentCompleted",
+                           percent_completed);
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Precache.Fetch.ResponseBytes",
+                              total_response_bytes_, 1, kMaxResponseBytes, 50);
 }
 
 void PrecacheFetcher::Start() {
@@ -167,6 +203,8 @@ void PrecacheFetcher::Start() {
 }
 
 void PrecacheFetcher::StartNextFetch() {
+  total_response_bytes_ += fetcher_->response_bytes();
+
   if (!resource_urls_to_fetch_.empty()) {
     // Fetch the next resource URL.
     fetcher_.reset(
@@ -229,6 +267,7 @@ void PrecacheFetcher::OnConfigFetchComplete(const URLFetcher& source) {
 
   for (const std::string& manifest_url : unique_manifest_urls)
     manifest_urls_to_fetch_.push_back(GURL(manifest_url));
+  num_manifest_urls_to_fetch_ = manifest_urls_to_fetch_.size();
 
   StartNextFetch();
 }