Start calculating new combined NetworkChangeNotifier signal

net/base/network_change_notifier.cc

 // Copyright (c) 2012 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/base/network_change_notifier.h"
 
 #include "base/metrics/histogram.h"
 #include "base/synchronization/lock.h"
 #include "build/build_config.h"
 #include "googleurl/src/gurl.h"
@@ skipping 17 matching lines @@
 // in ways that would require us to place locks around access to this object.
 // (The prohibition on global non-POD objects makes it tricky to do such a thing
 // anyway.)
 NetworkChangeNotifier* g_network_change_notifier = NULL;
 
 // Class factory singleton.
 NetworkChangeNotifierFactory* g_network_change_notifier_factory = NULL;
 
 class MockNetworkChangeNotifier : public NetworkChangeNotifier {
  public:
+  MockNetworkChangeNotifier() : NetworkChangeNotifier(0, 0, 0, 0) {}
   virtual ConnectionType GetCurrentConnectionType() const OVERRIDE {
     return CONNECTION_UNKNOWN;
   }
 };
 
 }  // namespace
 
 // The main observer class that records UMAs for network events.
 class HistogramWatcher
     : public NetworkChangeNotifier::ConnectionTypeObserver,
       public NetworkChangeNotifier::IPAddressObserver,
-      public NetworkChangeNotifier::DNSObserver {
+      public NetworkChangeNotifier::DNSObserver,
+      public NetworkChangeNotifier::NetworkChangeObserver {
  public:
   HistogramWatcher()
       : last_ip_address_change_(base::TimeTicks::Now()),
         last_connection_change_(base::TimeTicks::Now()),
         last_dns_change_(base::TimeTicks::Now()),
+        last_network_change_(base::TimeTicks::Now()),
         last_connection_type_(NetworkChangeNotifier::CONNECTION_UNKNOWN),
         offline_packets_received_(0) {}
 
   // Registers our three Observer implementations.  This is called from the
   // network thread so that our Observer implementations are also called
   // from the network thread.  This avoids multi-threaded race conditions
   // because the only other interface, |NotifyDataReceived| is also
   // only called from the network thread.
   void Init() {
     NetworkChangeNotifier::AddConnectionTypeObserver(this);
     NetworkChangeNotifier::AddIPAddressObserver(this);
     NetworkChangeNotifier::AddDNSObserver(this);
+    NetworkChangeNotifier::AddNetworkChangeObserver(this);
   }
 
   virtual ~HistogramWatcher() {}
 
   // NetworkChangeNotifier::IPAddressObserver implementation.
   virtual void OnIPAddressChanged() OVERRIDE {
     UMA_HISTOGRAM_MEDIUM_TIMES("NCN.IPAddressChange",
                                SinceLast(&last_ip_address_change_));
+    UMA_HISTOGRAM_MEDIUM_TIMES(
+        "NCN.ConnectionTypeChangeToIPAddressChange",
+        last_ip_address_change_ - last_connection_change_);
   }
 
   // NetworkChangeNotifier::ConnectionTypeObserver implementation.
   virtual void OnConnectionTypeChanged(
       NetworkChangeNotifier::ConnectionType type) OVERRIDE {
     if (type != NetworkChangeNotifier::CONNECTION_NONE) {
       UMA_HISTOGRAM_MEDIUM_TIMES("NCN.OnlineChange",
                                  SinceLast(&last_connection_change_));
 
       if (offline_packets_received_) {
         if ((last_connection_change_ - last_offline_packet_received_) <
             base::TimeDelta::FromSeconds(5)) {
           // We can compare this sum with the sum of NCN.OfflineDataRecv.
           UMA_HISTOGRAM_COUNTS_10000(
               "NCN.OfflineDataRecvAny5sBeforeOnline",
               offline_packets_received_);
         }
 
         UMA_HISTOGRAM_MEDIUM_TIMES("NCN.OfflineDataRecvUntilOnline",
                                    last_connection_change_ -
                                        last_offline_packet_received_);
       }
     } else {
       UMA_HISTOGRAM_MEDIUM_TIMES("NCN.OfflineChange",
                                  SinceLast(&last_connection_change_));
     }
+    UMA_HISTOGRAM_MEDIUM_TIMES(
+        "NCN.IPAddressChangeToConnectionTypeChange",
+        last_connection_change_ - last_ip_address_change_);
 
     offline_packets_received_ = 0;
     last_connection_type_ = type;
     polling_interval_ = base::TimeDelta::FromSeconds(1);
   }
 
   // NetworkChangeNotifier::DNSObserver implementation.
   virtual void OnDNSChanged() OVERRIDE {
     UMA_HISTOGRAM_MEDIUM_TIMES("NCN.DNSConfigChange",
                                SinceLast(&last_dns_change_));
   }
 
+  // NetworkChangeNotifier::NetworkChangeObserver implementation.
+  virtual void OnNetworkChanged(
+      NetworkChangeNotifier::ConnectionType type) OVERRIDE {
+    if (type != NetworkChangeNotifier::CONNECTION_NONE) {
+      UMA_HISTOGRAM_MEDIUM_TIMES("NCN.NetworkOnlineChange",
+                                 SinceLast(&last_network_change_));
+    } else {
+      UMA_HISTOGRAM_MEDIUM_TIMES("NCN.NetworkOfflineChange",
+                                 SinceLast(&last_network_change_));
+    }
+  }
+
   // Record histogram data whenever we receive a packet but think we're
   // offline.  Should only be called from the network thread.
   void NotifyDataReceived(const GURL& source) {
     if (last_connection_type_ != NetworkChangeNotifier::CONNECTION_NONE ||
         IsLocalhost(source.host()) ||
         !(source.SchemeIs("http") || source.SchemeIs("https"))) {
       return;
     }
 
     base::TimeTicks current_time = base::TimeTicks::Now();
@@ skipping 23 matching lines @@
   static base::TimeDelta SinceLast(base::TimeTicks *last_time) {
     base::TimeTicks current_time = base::TimeTicks::Now();
     base::TimeDelta delta = current_time - *last_time;
     *last_time = current_time;
     return delta;
   }
 
   base::TimeTicks last_ip_address_change_;
   base::TimeTicks last_connection_change_;
   base::TimeTicks last_dns_change_;
+  base::TimeTicks last_network_change_;
   base::TimeTicks last_offline_packet_received_;
   base::TimeTicks last_polled_connection_;
   // |polling_interval_| is initialized by |OnConnectionTypeChanged| on our
   // first transition to offline and on subsequent transitions.  Once offline,
   // |polling_interval_| doubles as offline data is received and we poll
   // with |NetworkChangeNotifier::GetConnectionType| to verify the connection
   // state.
   base::TimeDelta polling_interval_;
   // |last_connection_type_| is the last value passed to
   // |OnConnectionTypeChanged|.
@@ skipping 20 matching lines @@
   void SetDnsConfig(const DnsConfig& dns_config) {
     base::AutoLock lock(lock_);
     dns_config_ = dns_config;
   }
 
  private:
   mutable base::Lock lock_;
   DnsConfig dns_config_;
 };
 
+// Calculates NetworkChange signal from IPAddress and ConnectionType signals.
+class NetworkChangeNotifier::NetworkChangeCalculator
+    : public ConnectionTypeObserver,
+      public IPAddressObserver {
+ public:
+  // Delay arguments control how many milliseconds to delay notifications
+  // so as to combine duplicates. |ip_address_offline_delay_ms| controls
+  // delay after OnIPAddressChanged when transitioning from an offline state.
+  // |ip_address_online_delay_ms| controls delay after OnIPAddressChanged when
+  // transitioning from an online state. |connection_type_offline_delay_ms|
+  // controls delay after OnConnectionTypeChanged when transitioning from an
+  // offline state. |connection_type_online_delay_ms| controls delay after
+  // OnConnectionTypeChanged when transitioning from an online state.

[szym, 2012/11/25 07:37:37]

This comment needs to explain that the notifications are not just delayed but
also "debounced" or "de-duped", i.e., if two notifications arrive within XXX,
then only one will be sent out.

+  NetworkChangeCalculator(uint32 ip_address_offline_delay_ms,
+                          uint32 ip_address_online_delay_ms,
+                          uint32 connection_type_offline_delay_ms,
+                          uint32 connection_type_online_delay_ms)
+      : ip_address_offline_delay_(base::TimeDelta::FromMilliseconds(
+            ip_address_offline_delay_ms)),
+        ip_address_online_delay_(base::TimeDelta::FromMilliseconds(
+            ip_address_online_delay_ms)),
+        connection_type_offline_delay_(base::TimeDelta::FromMilliseconds(
+            connection_type_offline_delay_ms)),
+        connection_type_online_delay_(base::TimeDelta::FromMilliseconds(
+            connection_type_online_delay_ms)),
+        have_announced_(false),
+        last_announced_connection_type_(CONNECTION_NONE),
+        pending_connection_type_(CONNECTION_NONE) {}
+  void Init() {
+    AddConnectionTypeObserver(this);
+    AddIPAddressObserver(this);
+  }
+  virtual ~NetworkChangeCalculator() {
+    RemoveConnectionTypeObserver(this);
+    RemoveIPAddressObserver(this);
+  }
+
+  // NetworkChangeNotifier::IPAddressObserver implementation.

[szym, 2012/11/25 07:37:37]

nit: no need for "implementation"

[pauljensen, 2012/11/27 21:47:42]

I see a lot of people using "implementation": URLRequestJobFactoryImpl and
ProfileImpl and MessageLoopProxyImpl.
I was copying HistogramWatcher's style.

+  virtual void OnIPAddressChanged() OVERRIDE {
+    base::TimeDelta delay = last_announced_connection_type_ == CONNECTION_NONE
+        ? ip_address_offline_delay_ : ip_address_online_delay_;
+    timer_.Start(FROM_HERE, delay, this, &NetworkChangeCalculator::Notify);

[szym, 2012/11/25 07:37:37]

Perhaps a note (a reminder really) that this cancels the previous timer.

+  }
+
+  // NetworkChangeNotifier::ConnectionTypeObserver implementation.
+  virtual void OnConnectionTypeChanged(ConnectionType type) OVERRIDE {
+    pending_connection_type_ = type;
+    base::TimeDelta delay = last_announced_connection_type_ == CONNECTION_NONE
+        ? connection_type_offline_delay_ : connection_type_online_delay_;
+    timer_.Start(FROM_HERE, delay, this, &NetworkChangeCalculator::Notify);
+  }
+
+ private:
+  void Notify() {
+    // Don't bother signaling about dead connections.
+    if (have_announced_ && last_announced_connection_type_ == CONNECTION_NONE &&

[szym, 2012/11/25 07:37:37]

suggest parens for readability

+        pending_connection_type_ == CONNECTION_NONE)

[szym, 2012/11/25 07:37:37]

Could the same logic be accomplished by initializing
last_announced_connection_type_ to CONNECTION_UNKNOWN, or are the timeouts
essential even for the first announcement?

[pauljensen, 2012/11/27 21:47:42]

The timeouts are essential for all announcements; the first is no different. 
The timeouts for offline->online are always longer than the timeouts for
online->offline so I wanted to initialize last_announced_connect_type_ to an
offline state to essentially prepare for the worst deduping circumstances.

+      return;

[szym, 2012/11/25 07:37:37]

use braces when if condition is multiline

+    have_announced_ = true;
+    last_announced_connection_type_ = pending_connection_type_;
+    // Immediately before sending out an online signal, send out an offline
+    // signal to perform any destructive actions before constructive actions.
+    if (pending_connection_type_ != CONNECTION_NONE)
+      NetworkChangeNotifier::NotifyObserversOfNetworkChange(CONNECTION_NONE);
+    NetworkChangeNotifier::NotifyObserversOfNetworkChange(
+        pending_connection_type_);
+  }
+
+  const base::TimeDelta ip_address_offline_delay_;
+  const base::TimeDelta ip_address_online_delay_;
+  const base::TimeDelta connection_type_offline_delay_;
+  const base::TimeDelta connection_type_online_delay_;
+
+  // Indicates if NotifyObserversOfNetworkChange has been called yet.
+  bool have_announced_;
+  // Last value passed to NotifyObserversOfNetworkChange.
+  ConnectionType last_announced_connection_type_;
+  // Value to pass to NotifyObserversOfNetworkChange when Notify is called.
+  ConnectionType pending_connection_type_;
+  // Used to delay notifications so duplicates can be combined.
+  base::OneShotTimer<NetworkChangeCalculator> timer_;
+};
+
 NetworkChangeNotifier::~NetworkChangeNotifier() {
   DCHECK_EQ(this, g_network_change_notifier);
   g_network_change_notifier = NULL;
 }
 
 // static
 void NetworkChangeNotifier::SetFactory(
     NetworkChangeNotifierFactory* factory) {
   CHECK(!g_network_change_notifier_factory);
   g_network_change_notifier_factory = factory;
@@ skipping 133 matching lines @@
   }
 }
 
 void NetworkChangeNotifier::AddDNSObserver(DNSObserver* observer) {
   if (g_network_change_notifier) {
     g_network_change_notifier->resolver_state_observer_list_->AddObserver(
         observer);
   }
 }
 
+void NetworkChangeNotifier::AddNetworkChangeObserver(
+    NetworkChangeObserver* observer) {
+  if (g_network_change_notifier) {
+    g_network_change_notifier->network_change_observer_list_->AddObserver(
+        observer);
+  }
+}
+
 void NetworkChangeNotifier::RemoveIPAddressObserver(
     IPAddressObserver* observer) {
   if (g_network_change_notifier) {
     g_network_change_notifier->ip_address_observer_list_->RemoveObserver(
         observer);
   }
 }
 
 void NetworkChangeNotifier::RemoveConnectionTypeObserver(
     ConnectionTypeObserver* observer) {
   if (g_network_change_notifier) {
     g_network_change_notifier->connection_type_observer_list_->RemoveObserver(
         observer);
   }
 }
 
 void NetworkChangeNotifier::RemoveDNSObserver(DNSObserver* observer) {
   if (g_network_change_notifier) {
     g_network_change_notifier->resolver_state_observer_list_->RemoveObserver(
         observer);
   }
 }
 
-NetworkChangeNotifier::NetworkChangeNotifier()
+void NetworkChangeNotifier::RemoveNetworkChangeObserver(
+    NetworkChangeObserver* observer) {
+  if (g_network_change_notifier) {
+    g_network_change_notifier->network_change_observer_list_->RemoveObserver(
+        observer);
+  }
+}
+
+NetworkChangeNotifier::NetworkChangeNotifier(
+    uint32 ip_address_offline_delay_ms,
+    uint32 ip_address_online_delay_ms,
+    uint32 connection_type_offline_delay_ms,
+    uint32 connection_type_online_delay_ms)
     : ip_address_observer_list_(
         new ObserverListThreadSafe<IPAddressObserver>(
             ObserverListBase<IPAddressObserver>::NOTIFY_EXISTING_ONLY)),
       connection_type_observer_list_(
         new ObserverListThreadSafe<ConnectionTypeObserver>(
             ObserverListBase<ConnectionTypeObserver>::NOTIFY_EXISTING_ONLY)),
       resolver_state_observer_list_(
         new ObserverListThreadSafe<DNSObserver>(
             ObserverListBase<DNSObserver>::NOTIFY_EXISTING_ONLY)),
+      network_change_observer_list_(
+        new ObserverListThreadSafe<NetworkChangeObserver>(
+            ObserverListBase<NetworkChangeObserver>::NOTIFY_EXISTING_ONLY)),
       network_state_(new NetworkState()),
-      histogram_watcher_(new HistogramWatcher()) {
+      histogram_watcher_(new HistogramWatcher()),
+      network_change_calculator_(new NetworkChangeCalculator(
+          ip_address_offline_delay_ms,
+          ip_address_online_delay_ms,
+          connection_type_offline_delay_ms,
+          connection_type_online_delay_ms)) {
   DCHECK(!g_network_change_notifier);
   g_network_change_notifier = this;
+  network_change_calculator_->Init();
 }
 
 #if defined(OS_LINUX)
 const internal::AddressTrackerLinux*
 NetworkChangeNotifier::GetAddressTrackerInternal() const {
   return NULL;
 }
 #endif
 
 // static
@@ skipping 21 matching lines @@
 }
 
 void NetworkChangeNotifier::NotifyObserversOfConnectionTypeChange() {
   if (g_network_change_notifier) {
     g_network_change_notifier->connection_type_observer_list_->Notify(
         &ConnectionTypeObserver::OnConnectionTypeChanged,
         GetConnectionType());
   }
 }
 
+void NetworkChangeNotifier::NotifyObserversOfNetworkChange(
+    ConnectionType type) {
+  if (g_network_change_notifier) {
+    g_network_change_notifier->network_change_observer_list_->Notify(
+        &NetworkChangeObserver::OnNetworkChanged,
+        type);
+  }
+}
+
 NetworkChangeNotifier::DisableForTest::DisableForTest()
     : network_change_notifier_(g_network_change_notifier) {
   DCHECK(g_network_change_notifier);
   g_network_change_notifier = NULL;
 }
 
 NetworkChangeNotifier::DisableForTest::~DisableForTest() {
   DCHECK(!g_network_change_notifier);
   g_network_change_notifier = network_change_notifier_;
 }
 
 }  // namespace net