Batch pageload metrics pingbacks

components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.cc
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.cc
index 13942ad271dc5387facf02eb27634359926db3a2..4fd036c94f9f0c4713b7d7d5e133cbfa1772711d 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.cc
@@ -6,45 +6,40 @@
 
 #include "base/metrics/histogram.h"
 #include "base/optional.h"
 #include "base/rand_util.h"
 #include "base/time/time.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_data.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_page_load_timing.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_util.h"
 #include "components/data_reduction_proxy/proto/client_config.pb.h"
-#include "components/data_reduction_proxy/proto/pageload_metrics.pb.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 #include "url/gurl.h"
 
 namespace data_reduction_proxy {
 
 namespace {
 
 static const char kHistogramSucceeded[] =
     "DataReductionProxy.Pingback.Succeeded";
 static const char kHistogramAttempted[] =
     "DataReductionProxy.Pingback.Attempted";
 
-// Creates a RecordPageloadMetrics protobuf for this page load based on page
+// Adds the relevant information to |request| for this page load based on page
 // timing and data reduction proxy state.
-std::unique_ptr<RecordPageloadMetricsRequest>
-CreateRecordPageloadMetricsRequest(
-    const DataReductionProxyData& request_data,
-    const DataReductionProxyPageLoadTiming& timing) {
-  std::unique_ptr<RecordPageloadMetricsRequest> batched_request(
-      new RecordPageloadMetricsRequest());
-  PageloadMetrics* request = batched_request->add_pageloads();
+void AddDataToPageloadMetrics(const DataReductionProxyData& request_data,
+                              const DataReductionProxyPageLoadTiming& timing,
+                              PageloadMetrics* request) {
   request->set_session_key(request_data.session_key());
   // For the timing events, any of them could be zero. Fill the message as a
   // best effort.
   request->set_allocated_first_request_time(
       protobuf_parser::CreateTimestampFromTime(timing.navigation_start)
           .release());
   if (request_data.original_request_url().is_valid())
     request->set_first_request_url(request_data.original_request_url().spec());
   if (timing.first_contentful_paint) {
     request->set_allocated_time_to_first_contentful_paint(
@@ -63,21 +58,20 @@ CreateRecordPageloadMetricsRequest(
         protobuf_parser::CreateDurationFromTimeDelta(
             timing.response_start.value())
             .release());
   }
   if (timing.load_event_start) {
     request->set_allocated_page_load_time(
         protobuf_parser::CreateDurationFromTimeDelta(
             timing.load_event_start.value())
             .release());
   }
-  return batched_request;
 }
 
 // Adds |current_time| as the metrics sent time to |request_data|, and returns
 // the serialized request.
 std::string AddTimeAndSerializeRequest(
     RecordPageloadMetricsRequest* request_data,
     base::Time current_time) {
   request_data->set_allocated_metrics_sent_time(
       protobuf_parser::CreateTimestampFromTime(current_time).release());
   std::string serialized_request;
@@ -96,63 +90,57 @@ DataReductionProxyPingbackClient::DataReductionProxyPingbackClient(
 DataReductionProxyPingbackClient::~DataReductionProxyPingbackClient() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 void DataReductionProxyPingbackClient::OnURLFetchComplete(
     const net::URLFetcher* source) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(source == current_fetcher_.get());
   UMA_HISTOGRAM_BOOLEAN(kHistogramSucceeded, source->GetStatus().is_success());
   current_fetcher_.reset();
-  while (!current_fetcher_ && !data_to_send_.empty()) {
-    current_fetcher_ =
-        MaybeCreateFetcherForDataAndStart(data_to_send_.front().get());
-    data_to_send_.pop_front();
+  if (metrics_request_.pageloads_size() > 0) {
+    CreateFetcherForDataAndStart();
   }
 }
 
 void DataReductionProxyPingbackClient::SendPingback(
     const DataReductionProxyData& request_data,
     const DataReductionProxyPageLoadTiming& timing) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  std::unique_ptr<RecordPageloadMetricsRequest>
-      record_pageload_metrics_request =
-          CreateRecordPageloadMetricsRequest(request_data, timing);
-  if (current_fetcher_.get()) {
-    data_to_send_.push_back(std::move(record_pageload_metrics_request));
-  } else {
-    DCHECK(data_to_send_.empty());
-    current_fetcher_ = MaybeCreateFetcherForDataAndStart(
-        record_pageload_metrics_request.get());
-  }
-}
-
-std::unique_ptr<net::URLFetcher>
-DataReductionProxyPingbackClient::MaybeCreateFetcherForDataAndStart(
-    RecordPageloadMetricsRequest* request_data) {
   bool send_pingback = ShouldSendPingback();
   UMA_HISTOGRAM_BOOLEAN(kHistogramAttempted, send_pingback);
   if (!send_pingback)
-    return nullptr;
+    return;
+  PageloadMetrics* pageload_metrics = metrics_request_.add_pageloads();
+  AddDataToPageloadMetrics(request_data, timing, pageload_metrics);
+  if (current_fetcher_.get())
+    return;
+  DCHECK_EQ(1, metrics_request_.pageloads_size());
+  CreateFetcherForDataAndStart();
+}
+
+void DataReductionProxyPingbackClient::CreateFetcherForDataAndStart() {
+  DCHECK(!current_fetcher_);
+  DCHECK_GE(metrics_request_.pageloads_size(), 1);
   std::string serialized_request =
-      AddTimeAndSerializeRequest(request_data, CurrentTime());
-  std::unique_ptr<net::URLFetcher> fetcher(
-      net::URLFetcher::Create(pingback_url_, net::URLFetcher::POST, this));
-  fetcher->SetLoadFlags(net::LOAD_BYPASS_PROXY);
-  fetcher->SetUploadData("application/x-protobuf", serialized_request);
-  fetcher->SetRequestContext(url_request_context_);
+      AddTimeAndSerializeRequest(&metrics_request_, CurrentTime());
+  metrics_request_.Clear();
+  current_fetcher_ =
+      net::URLFetcher::Create(pingback_url_, net::URLFetcher::POST, this);
+  current_fetcher_->SetLoadFlags(net::LOAD_BYPASS_PROXY);
+  current_fetcher_->SetUploadData("application/x-protobuf", serialized_request);
+  current_fetcher_->SetRequestContext(url_request_context_);
   // Configure max retries to be at most kMaxRetries times for 5xx errors.
   static const int kMaxRetries = 5;
-  fetcher->SetMaxRetriesOn5xx(kMaxRetries);
-  fetcher->SetAutomaticallyRetryOnNetworkChanges(kMaxRetries);
-  fetcher->Start();
-  return fetcher;
+  current_fetcher_->SetMaxRetriesOn5xx(kMaxRetries);
+  current_fetcher_->SetAutomaticallyRetryOnNetworkChanges(kMaxRetries);
+  current_fetcher_->Start();
 }
 
 bool DataReductionProxyPingbackClient::ShouldSendPingback() const {
   return params::IsForcePingbackEnabledViaFlags() ||
          GenerateRandomFloat() < pingback_reporting_fraction_;
 }
 
 base::Time DataReductionProxyPingbackClient::CurrentTime() const {
   return base::Time::Now();
 }