NQE: Do not record correlation if metric is missing

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 608 matching lines @@
     return;
 
   // Use the system clock instead of |tick_clock_| to compare the current
   // timestamp with the |load_timing_info| timestamp since the latter is set by
   // the system clock, and may be different from |tick_clock_| in tests.
   const base::TimeTicks now = base::TimeTicks::Now();
   // Record UMA only for requests that started recently.
   if (now - last_main_frame_request_ > base::TimeDelta::FromSeconds(15))
     return;
 
+  if (last_connection_change_ >= last_main_frame_request_)
+    return;
+
   DCHECK_GE(now, load_timing_info.send_start);
 
   int32_t rtt = 0;
 
   if (UseTransportRTT()) {
-    rtt = estimated_quality_at_last_main_frame_.transport_rtt() !=
-                  nqe::internal::InvalidRTT()
-              ? FitInKBitsPerMetricBits(
-                    estimated_quality_at_last_main_frame_.transport_rtt()
-                        .InMilliseconds())
-              : 0;
+    if (estimated_quality_at_last_main_frame_.transport_rtt() ==
+        nqe::internal::InvalidRTT()) {
+      return;
+    }
+    rtt = FitInKBitsPerMetricBits(
+        estimated_quality_at_last_main_frame_.transport_rtt().InMilliseconds());
   } else {
-    rtt = estimated_quality_at_last_main_frame_.http_rtt() !=
-                  nqe::internal::InvalidRTT()
-              ? FitInKBitsPerMetricBits(
-                    estimated_quality_at_last_main_frame_.http_rtt()
-                        .InMilliseconds())
-              : 0;
+    if (estimated_quality_at_last_main_frame_.http_rtt() ==
+        nqe::internal::InvalidRTT()) {
+      return;
+    }
+    rtt = FitInKBitsPerMetricBits(
+        estimated_quality_at_last_main_frame_.http_rtt().InMilliseconds());
   }
 
-  const int32_t downstream_throughput =
-      estimated_quality_at_last_main_frame_.downstream_throughput_kbps() !=
-              nqe::internal::kInvalidThroughput
-          ? FitInKBitsPerMetricBits(estimated_quality_at_last_main_frame_
-                                        .downstream_throughput_kbps())
-          : 0;
+  if (estimated_quality_at_last_main_frame_.downstream_throughput_kbps() ==

[RyanSturm, 2017/02/02 23:06:16]

Move this if above the UseTrasportRTT block so that we don't need to run all the
extra code like FitInKBitsPerMetricsBits. Keep lines 657 and 658 below where
they are though.

[tbansal1, 2017/02/03 00:56:26]

Done.

+      nqe::internal::kInvalidThroughput) {
+    return;
+  }
+
+  const int32_t downstream_throughput = FitInKBitsPerMetricBits(
+      estimated_quality_at_last_main_frame_.downstream_throughput_kbps());
 
   const int32_t resource_load_time = FitInKBitsPerMetricBits(
       (now - load_timing_info.send_start).InMilliseconds());
 
   int64_t resource_size = (request.GetTotalReceivedBytes() * 8) / 1024;
   if (resource_size >= (1 << kBitsPerMetric)) {
     // Too large resource size (at least 128 Kb).
     return;
   }
 
@@ skipping 1058 matching lines @@
         NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE);
     downstream_throughput_kbps_observations_.AddObservation(
         throughput_observation);
     NotifyObserversOfThroughput(throughput_observation);
   }
 
   ComputeEffectiveConnectionType();
 }
 
 }  // namespace net