NQE: Add net log events when there is a significant change in metrics

net/nqe/event_creator.cc

Index: net/nqe/event_creator.cc
diff --git a/net/nqe/event_creator.cc b/net/nqe/event_creator.cc
index e55973ebed6748cb7390e2fb787ab8e181eaee92..730aedb159d19d921a50e1adfa0bb7908207fc13 100644
--- a/net/nqe/event_creator.cc
+++ b/net/nqe/event_creator.cc
@@ -38,16 +38,54 @@ std::unique_ptr<base::Value> EffectiveConnectionTypeChangedNetLogCallback(
 
 }  // namespace
 
-void AddEffectiveConnectionTypeChangedEventToNetLog(
-    const NetLogWithSource& net_log,
-    base::TimeDelta http_rtt,
-    base::TimeDelta transport_rtt,
-    int32_t downstream_throughput_kbps,
-    EffectiveConnectionType effective_connection_type) {
-  net_log.AddEvent(
+EventCreator::EventCreator(NetLogWithSource net_log)
+    : net_log_(net_log),
+      past_effective_connection_type_(EFFECTIVE_CONNECTION_TYPE_UNKNOWN) {}
+
+EventCreator::~EventCreator() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void EventCreator::MaybeAddEffectiveConnectionTypeChangedEventToNetLog(
+    EffectiveConnectionType effective_connection_type,
+    const NetworkQuality& network_quality) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // Check if any of the network quality metrics changed significantly.
+  bool effective_connection_type_changed =
+      past_effective_connection_type_ != effective_connection_type;
+  bool http_rtt_changed = (past_network_quality_.http_rtt() == InvalidRTT() &&

[RyanSturm, 2017/01/25 22:56:50]

nit: this pattern can be flattened a little bit:

instead of:

(a == invalid && b != invalid) || (a != invalid && b == invalid)

use:

(a == invalid) != (b == invalid)

[tbansal1, 2017/01/26 00:05:45]

Thanks, this cleaned up the code a lot. Done.

+                           network_quality.http_rtt() != InvalidRTT()) ||
+                          (past_network_quality_.http_rtt() != InvalidRTT() &&
+                           network_quality.http_rtt() == InvalidRTT());
+  bool transport_rtt_changed =
+      (past_network_quality_.transport_rtt() == InvalidRTT() &&
+       network_quality.transport_rtt() != InvalidRTT()) ||
+      (past_network_quality_.transport_rtt() != InvalidRTT() &&
+       network_quality.transport_rtt() == InvalidRTT());
+  bool kbps_changed =
+      (past_network_quality_.downstream_throughput_kbps() ==
+           INVALID_RTT_THROUGHPUT &&
+       network_quality.downstream_throughput_kbps() !=
+           INVALID_RTT_THROUGHPUT) ||
+      (past_network_quality_.downstream_throughput_kbps() !=
+           INVALID_RTT_THROUGHPUT &&
+       network_quality.downstream_throughput_kbps() == INVALID_RTT_THROUGHPUT);
+
+  if (!effective_connection_type_changed && !http_rtt_changed &&
+      !transport_rtt_changed && !kbps_changed) {
+    // Return since none of the metrics changed significantly.

[RyanSturm, 2017/01/25 22:56:50]

nit: s/significantly/meaningfully/ This might be me nit-picking with  too much
coffee in me, but increasing the RTT from 4G to slow2G is a significant change,
it's just not meaningful.

[tbansal1, 2017/01/26 00:05:45]

Done.

+    return;
+  }
+
+  past_effective_connection_type_ = effective_connection_type;
+  past_network_quality_ = network_quality;
+
+  net_log_.AddEvent(
       NetLogEventType::NETWORK_QUALITY_CHANGED,
-      base::Bind(&EffectiveConnectionTypeChangedNetLogCallback, http_rtt,
-                 transport_rtt, downstream_throughput_kbps,
+      base::Bind(&EffectiveConnectionTypeChangedNetLogCallback,
+                 network_quality.http_rtt(), network_quality.transport_rtt(),
+                 network_quality.downstream_throughput_kbps(),
                  effective_connection_type));
 }