[Offline Pages] Adds UMA for effective network connection on api calls.

components/offline_pages/background/request_coordinator.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/background/request_coordinator.h"
 
 #include <limits>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 102 matching lines @@
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         "OfflinePages.Background.RequestSuccess.StartedAttemptCount",
         request.started_attempt_count(), 1, 10, 11);
   } else {
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         "OfflinePages.Background.RequestFailure.StartedAttemptCount",
         request.started_attempt_count(), 1, 10, 11);
   }
 }
 
+// Record the network quality at request creation time per namespace.
+void RecordSavePageLaterNetworkQuality(
+    const ClientId& client_id,
+    const net::EffectiveConnectionType effective_connection) {
+  // The histogram below is an expansion of the UMA_HISTOGRAM_ENUMERATION
+  // macro adapted to allow for a dynamically suffixed histogram name.
+  // Note: The factory creates and owns the histogram.
+  base::HistogramBase* histogram = base::LinearHistogram::FactoryGet(
+      AddHistogramSuffix(
+          client_id,
+          "OfflinePages.Background.EffectiveConnectionType.SavePageLater"),
+      1, net::EFFECTIVE_CONNECTION_TYPE_LAST - 1,
+      net::EFFECTIVE_CONNECTION_TYPE_LAST,
+      base::HistogramBase::kUmaTargetedHistogramFlag);
+  histogram->Add(effective_connection);
+}
+
 // This should use the same algorithm as we use for OfflinePageItem, so the IDs
 // are similar.
 int64_t GenerateOfflineId() {
   return base::RandGenerator(std::numeric_limits<int64_t>::max()) + 1;
 }
 
 // In case we start processing from SavePageLater, we need a callback, but there
 // is nothing for it to do.
 void EmptySchedulerCallback(bool started) {}
 
@@ skipping 50 matching lines @@
 
   // If the download manager is not done with the request, put it on the
   // disabled list.
   if (availability == RequestAvailability::DISABLED_FOR_OFFLINER)
     disabled_requests_.insert(id);
 
   // Put the request on the request queue.
   queue_->AddRequest(request,
                      base::Bind(&RequestCoordinator::AddRequestResultCallback,
                                 weak_ptr_factory_.GetWeakPtr()));
+
+  // Record the network quality when this request is made.
+  if (network_quality_estimator_) {
+    RecordSavePageLaterNetworkQuality(
+        client_id, network_quality_estimator_->GetEffectiveConnectionType());
+  }
+
   return id;
 }
 void RequestCoordinator::GetAllRequests(const GetRequestsCallback& callback) {
   // Get all matching requests from the request queue, send them to our
   // callback.  We bind the namespace and callback to the front of the callback
   // param set.
   queue_->GetRequests(base::Bind(&RequestCoordinator::GetQueuedRequestsCallback,
                                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
@@ skipping 112 matching lines @@
 
 void RequestCoordinator::RemoveRequests(
     const std::vector<int64_t>& request_ids,
     const RemoveRequestsCallback& callback) {
   bool canceled = CancelActiveRequestIfItMatches(request_ids);
   queue_->RemoveRequests(
       request_ids,
       base::Bind(&RequestCoordinator::HandleRemovedRequestsAndCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback,
                  BackgroundSavePageResult::REMOVED));
+
+  // Record the network quality when this request is made.
+  // TODO(dougarnett): Should we update api to pass namespace so we
+  // can record per namespace?
+  if (network_quality_estimator_) {

[Pete Williamson, 2016/11/01 19:58:47]

These three added sections are very similar - can you refactor them to use a
common function?

[dewittj, 2016/11/01 21:52:41]

I suspect there won't be much to gain from that, since you can no longer use the
UMA macros..

Though an interesting one would be to add a histogram value for "no quality
estimator"

+    UMA_HISTOGRAM_ENUMERATION(
+        "OfflinePages.Background.EffectiveConnectionType.RemoveRequests",
+        network_quality_estimator_->GetEffectiveConnectionType(),
+        net::EFFECTIVE_CONNECTION_TYPE_LAST);
+  }
+
   if (canceled)
     TryNextRequest();
 }
 
 void RequestCoordinator::PauseRequests(
     const std::vector<int64_t>& request_ids) {
   bool canceled = CancelActiveRequestIfItMatches(request_ids);
   queue_->ChangeRequestsState(
       request_ids, SavePageRequest::RequestState::PAUSED,
       base::Bind(&RequestCoordinator::UpdateMultipleRequestsCallback,
                  weak_ptr_factory_.GetWeakPtr()));
 
+  // Record the network quality when this request is made.
+  if (network_quality_estimator_) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "OfflinePages.Background.EffectiveConnectionType.PauseRequests",
+        network_quality_estimator_->GetEffectiveConnectionType(),
+        net::EFFECTIVE_CONNECTION_TYPE_LAST);
+  }
+
   if (canceled)
     TryNextRequest();
 }
 
 void RequestCoordinator::ResumeRequests(
     const std::vector<int64_t>& request_ids) {
   queue_->ChangeRequestsState(
       request_ids, SavePageRequest::RequestState::AVAILABLE,
       base::Bind(&RequestCoordinator::UpdateMultipleRequestsCallback,
                  weak_ptr_factory_.GetWeakPtr()));
+
+  // Record the network quality when this request is made.
+  if (network_quality_estimator_) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "OfflinePages.Background.EffectiveConnectionType.ResumeRequests",
+        network_quality_estimator_->GetEffectiveConnectionType(),
+        net::EFFECTIVE_CONNECTION_TYPE_LAST);
+  }
+
   // Schedule a task, in case there is not one scheduled.
   ScheduleAsNeeded();
 }
 
 net::NetworkChangeNotifier::ConnectionType
 RequestCoordinator::GetConnectionType() {
   // If we have a connection type set for test, use that.
   if (use_test_connection_type_)
     return test_connection_type_;
 
@@ skipping 457 matching lines @@
 
 ClientPolicyController* RequestCoordinator::GetPolicyController() {
   return policy_controller_.get();
 }
 
 void RequestCoordinator::Shutdown() {
   network_quality_estimator_ = nullptr;
 }
 
 }  // namespace offline_pages