Record difference between the maximum and minimum wireless signal strength

net/nqe/network_quality_estimator.cc

 // Copyright 2015 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 "net/nqe/network_quality_estimator.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <utility>
@@ skipping 27 matching lines @@
 #include "net/nqe/throughput_analyzer.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_status.h"
 #include "url/gurl.h"
 
 #if defined(OS_ANDROID)
 #include "net/android/cellular_signal_strength.h"
 #include "net/android/network_library.h"
 #endif  // OS_ANDROID
 
+namespace net {

[RyanSturm, 2016/12/13 18:02:20]

Is this part of the change related? Can you add to the description that you are
cleaning up/refactoring?

[tbansal1, 2016/12/13 21:21:13]

Done.

+
 namespace {
 
 // Returns the histogram that should be used to record the given statistic.
 // |max_limit| is the maximum value that can be stored in the histogram.
-base::HistogramBase* GetHistogram(
-    const std::string& statistic_name,
-    net::NetworkChangeNotifier::ConnectionType type,
-    int32_t max_limit) {
+base::HistogramBase* GetHistogram(const std::string& statistic_name,
+                                  NetworkChangeNotifier::ConnectionType type,
+                                  int32_t max_limit) {
   const base::LinearHistogram::Sample kLowerLimit = 1;
   DCHECK_GT(max_limit, kLowerLimit);
   const size_t kBucketCount = 50;
 
   return base::Histogram::FactoryGet(
-      "NQE." + statistic_name +
-          net::nqe::internal::GetNameForConnectionType(type),
+      "NQE." + statistic_name + nqe::internal::GetNameForConnectionType(type),
       kLowerLimit, max_limit, kBucketCount,
       base::HistogramBase::kUmaTargetedHistogramFlag);
 }
 
-net::NetworkQualityObservationSource ProtocolSourceToObservationSource(
-    net::SocketPerformanceWatcherFactory::Protocol protocol) {
+NetworkQualityObservationSource ProtocolSourceToObservationSource(
+    SocketPerformanceWatcherFactory::Protocol protocol) {
   switch (protocol) {
-    case net::SocketPerformanceWatcherFactory::PROTOCOL_TCP:
-      return net::NETWORK_QUALITY_OBSERVATION_SOURCE_TCP;
-    case net::SocketPerformanceWatcherFactory::PROTOCOL_QUIC:
-      return net::NETWORK_QUALITY_OBSERVATION_SOURCE_QUIC;
+    case SocketPerformanceWatcherFactory::PROTOCOL_TCP:
+      return NETWORK_QUALITY_OBSERVATION_SOURCE_TCP;
+    case SocketPerformanceWatcherFactory::PROTOCOL_QUIC:
+      return NETWORK_QUALITY_OBSERVATION_SOURCE_QUIC;
   }
   NOTREACHED();
-  return net::NETWORK_QUALITY_OBSERVATION_SOURCE_TCP;
+  return NETWORK_QUALITY_OBSERVATION_SOURCE_TCP;
 }
 
 // Returns true if the scheme of the |request| is either HTTP or HTTPS.
-bool RequestSchemeIsHTTPOrHTTPS(const net::URLRequest& request) {
+bool RequestSchemeIsHTTPOrHTTPS(const URLRequest& request) {
   return request.url().is_valid() && request.url().SchemeIsHTTPOrHTTPS();
 }
 
 // Returns the suffix of the histogram that should be used for recording the
 // accuracy when the observed RTT is |observed_rtt|. The width of the intervals
 // are in exponentially increasing order.
 const char* GetHistogramSuffixObservedRTT(const base::TimeDelta& observed_rtt) {
   const float rtt_milliseconds = observed_rtt.InMillisecondsF();
   DCHECK_GE(rtt_milliseconds, 0);
 
@@ skipping 91 matching lines @@
   base::HistogramBase* histogram = base::Histogram::FactoryGet(
       histogram_name, 1, 1000 * 1000 /* 1 Gbps */, 50 /* Number of buckets */,
       base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->Add(std::abs(metric));
 }
 
 void RecordEffectiveConnectionTypeAccuracy(
     const char* prefix,
     int32_t metric,
     base::TimeDelta measuring_duration,
-    net::EffectiveConnectionType observed_effective_connection_type) {
-  const std::string histogram_name =
-      base::StringPrintf("%s.EstimatedObservedDiff.%s.%d.%s", prefix,
-                         metric >= 0 ? "Positive" : "Negative",
-                         static_cast<int32_t>(measuring_duration.InSeconds()),
-                         net::GetNameForEffectiveConnectionType(
-                             observed_effective_connection_type));
+    EffectiveConnectionType observed_effective_connection_type) {
+  const std::string histogram_name = base::StringPrintf(
+      "%s.EstimatedObservedDiff.%s.%d.%s", prefix,
+      metric >= 0 ? "Positive" : "Negative",
+      static_cast<int32_t>(measuring_duration.InSeconds()),
+      GetNameForEffectiveConnectionType(observed_effective_connection_type));
 
   base::HistogramBase* histogram = base::Histogram::FactoryGet(
-      histogram_name, 0, net::EFFECTIVE_CONNECTION_TYPE_LAST,
-      net::EFFECTIVE_CONNECTION_TYPE_LAST /* Number of buckets */,
+      histogram_name, 0, EFFECTIVE_CONNECTION_TYPE_LAST,
+      EFFECTIVE_CONNECTION_TYPE_LAST /* Number of buckets */,
       base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->Add(std::abs(metric));
 }
 
 }  // namespace
 
-namespace net {
-
 NetworkQualityEstimator::NetworkQualityEstimator(
     std::unique_ptr<ExternalEstimateProvider> external_estimates_provider,
     const std::map<std::string, std::string>& variation_params)
     : NetworkQualityEstimator(std::move(external_estimates_provider),
                               variation_params,
                               false,
                               false) {}
 
 NetworkQualityEstimator::NetworkQualityEstimator(
     std::unique_ptr<ExternalEstimateProvider> external_estimates_provider,
@@ skipping 18 matching lines @@
                                {"TransportRTTOrDownstreamThroughput",
                                 EffectiveConnectionTypeAlgorithm::
                                     TRANSPORT_RTT_OR_DOWNSTREAM_THROUGHOUT}}),
       use_localhost_requests_(use_local_host_requests_for_tests),
       use_small_responses_(use_smaller_responses_for_tests),
       add_default_platform_observations_(add_default_platform_observations),
       weight_multiplier_per_second_(
           nqe::internal::GetWeightMultiplierPerSecond(variation_params)),
       effective_connection_type_algorithm_(
           algorithm_name_to_enum_.find(
-              net::nqe::internal::GetEffectiveConnectionTypeAlgorithm(
+              nqe::internal::GetEffectiveConnectionTypeAlgorithm(
                   variation_params)) == algorithm_name_to_enum_.end()
               ? kDefaultEffectiveConnectionTypeAlgorithm
               : algorithm_name_to_enum_
-                    .find(
-                        net::nqe::internal::GetEffectiveConnectionTypeAlgorithm(
-                            variation_params))
+                    .find(nqe::internal::GetEffectiveConnectionTypeAlgorithm(
+                        variation_params))
                     ->second),
       tick_clock_(new base::DefaultTickClock()),
       last_connection_change_(tick_clock_->NowTicks()),
       current_network_id_(nqe::internal::NetworkID(
           NetworkChangeNotifier::ConnectionType::CONNECTION_UNKNOWN,
           std::string())),
       downstream_throughput_kbps_observations_(weight_multiplier_per_second_),
       rtt_observations_(weight_multiplier_per_second_),
       effective_connection_type_at_last_main_frame_(
           EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
@@ skipping 134 matching lines @@
 
   if (!RequestSchemeIsHTTPOrHTTPS(request) ||
       !RequestProvidesRTTObservation(request)) {
     return;
   }
 
   const base::TimeTicks now = tick_clock_->NowTicks();
 
   // Update |estimated_quality_at_last_main_frame_| if this is a main frame
   // request.
   if (request.load_flags() & LOAD_MAIN_FRAME_DEPRECATED) {

[RyanSturm, 2016/12/13 18:02:20]

In a later CL can you refactor this block into a method like
"RecomputeConnectionMetricsOnMainFrameRequest".

It would be more clear when looking at UpdateSignalStrength() that it is called
when a main frame request happens or a connection type changes without having to
dig into NotifyHeadersReceived.

WDYT?

[tbansal1, 2016/12/13 21:21:13]

Done.

     last_main_frame_request_ = now;
 
     ComputeEffectiveConnectionType();
     effective_connection_type_at_last_main_frame_ = effective_connection_type_;
     estimated_quality_at_last_main_frame_ = network_quality_;
 
     RecordMetricsOnMainFrameRequest();
     MaybeQueryExternalEstimateProvider();
 
     // Post the tasks which will run in the future and record the estimation
@@ skipping 346 matching lines @@
   peak_network_quality_ = nqe::internal::NetworkQuality();
   downstream_throughput_kbps_observations_.Clear();
   rtt_observations_.Clear();
 
 #if defined(OS_ANDROID)
   if (NetworkChangeNotifier::IsConnectionCellular(current_network_id_.type)) {
     UMA_HISTOGRAM_BOOLEAN(
         "NQE.CellularSignalStrengthAvailable",
         min_signal_strength_since_connection_change_ != INT32_MAX &&
             max_signal_strength_since_connection_change_ != INT32_MIN);
+
+    if (min_signal_strength_since_connection_change_ != INT32_MAX &&
+        max_signal_strength_since_connection_change_ != INT32_MIN) {
+      UMA_HISTOGRAM_COUNTS_100(
+          "NQE.CellularSignalStrengthDifference",
+          max_signal_strength_since_connection_change_ -
+              min_signal_strength_since_connection_change_);
+    }
   }
 #endif  // OS_ANDROID
+
   min_signal_strength_since_connection_change_ = INT32_MAX;
   max_signal_strength_since_connection_change_ = INT32_MIN;
   network_quality_ = nqe::internal::NetworkQuality();
   effective_connection_type_ = EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
   effective_connection_type_at_last_main_frame_ =
       EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
 
   // Update the local state as part of preparation for the new connection.
   current_network_id_ = GetCurrentNetworkID();
   RecordNetworkIDAvailability();
@@ skipping 804 matching lines @@
 void NetworkQualityEstimator::OnPrefsRead(
     const std::map<nqe::internal::NetworkID,
                    nqe::internal::CachedNetworkQuality> read_prefs) {
   DCHECK(thread_checker_.CalledOnValidThread());
   UMA_HISTOGRAM_COUNTS("NQE.Prefs.ReadSize", read_prefs.size());
   // TODO(tbansal): crbug.com/490870. Incorporate the network quality into the
   // current estimates.
 }
 
 }  // namespace net