From cde8b3c324f7eeecb51dbbd8a966dad3ae0de2d9 Mon Sep 17 00:00:00 2001 From: "pauljensen@chromium.org" Date: Mon, 13 Aug 2012 19:20:52 +0000 Subject: Add histograms to NetworkChangeNotifier to identify ways to improve interface and narrow down offline false-positives. BUG=124069 Review URL: https://chromiumcodereview.appspot.com/10837123 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151325 0039d316-1c4b-4281-b951-d872f2087c98 --- net/base/network_change_notifier.cc | 121 ++++++++++++++++++++++++++++++++++++ net/base/network_change_notifier.h | 15 +++++ 2 files changed, 136 insertions(+) (limited to 'net/base') diff --git a/net/base/network_change_notifier.cc b/net/base/network_change_notifier.cc index 8c5f446..fa0ad77 100644 --- a/net/base/network_change_notifier.cc +++ b/net/base/network_change_notifier.cc @@ -4,7 +4,10 @@ #include "net/base/network_change_notifier.h" #include "net/base/network_change_notifier_factory.h" +#include "base/metrics/histogram.h" #include "build/build_config.h" +#include "googleurl/src/gurl.h" +#include "net/base/net_util.h" #if defined(OS_WIN) #include "net/base/network_change_notifier_win.h" #elif defined(OS_LINUX) && !defined(OS_CHROMEOS) @@ -35,6 +38,109 @@ class MockNetworkChangeNotifier : public NetworkChangeNotifier { } // namespace +// The main observer class that records UMAs for network events. +class HistogramWatcher + : public NetworkChangeNotifier::ConnectionTypeObserver, + public NetworkChangeNotifier::IPAddressObserver, + public NetworkChangeNotifier::DNSObserver { + public: + HistogramWatcher() + : last_ip_address_change_(base::TimeTicks::Now()), + last_connection_change_(base::TimeTicks::Now()), + last_dns_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 InitHistogramWatcher() { + NetworkChangeNotifier::AddConnectionTypeObserver(this); + NetworkChangeNotifier::AddIPAddressObserver(this); + NetworkChangeNotifier::AddDNSObserver(this); + } + + virtual ~HistogramWatcher() {} + + // NetworkChangeNotifier::IPAddressObserver implementation. + virtual void OnIPAddressChanged() OVERRIDE { + UMA_HISTOGRAM_MEDIUM_TIMES("NCN.IPAddressChange", + SinceLast(&last_ip_address_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_)); + } + + offline_packets_received_ = 0; + last_connection_type_ = type; + } + + // NetworkChangeNotifier::DNSObserver implementation. + virtual void OnDNSChanged(unsigned detail) OVERRIDE { + if (detail == NetworkChangeNotifier::CHANGE_DNS_SETTINGS) { + UMA_HISTOGRAM_MEDIUM_TIMES("NCN.DNSConfigChange", + SinceLast(&last_dns_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(); + UMA_HISTOGRAM_MEDIUM_TIMES("NCN.OfflineDataRecv", + current_time - last_connection_change_); + offline_packets_received_++; + last_offline_packet_received_ = current_time; + } + + private: + 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_offline_packet_received_; + NetworkChangeNotifier::ConnectionType last_connection_type_; + int32 offline_packets_received_; + + DISALLOW_COPY_AND_ASSIGN(HistogramWatcher); +}; + NetworkChangeNotifier::~NetworkChangeNotifier() { DCHECK_EQ(this, g_network_change_notifier); g_network_change_notifier = NULL; @@ -84,6 +190,20 @@ NetworkChangeNotifier::GetConnectionType() { CONNECTION_UNKNOWN; } +// static +void NetworkChangeNotifier::NotifyDataReceived(const GURL& source) { + if (!g_network_change_notifier) + return; + g_network_change_notifier->histogram_watcher_->NotifyDataReceived(source); +} + +// static +void NetworkChangeNotifier::InitHistogramWatcher() { + if (!g_network_change_notifier) + return; + g_network_change_notifier->histogram_watcher_->InitHistogramWatcher(); +} + #if defined(OS_LINUX) // static const internal::AddressTrackerLinux* @@ -162,6 +282,7 @@ NetworkChangeNotifier::NetworkChangeNotifier() watching_dns_(false) { DCHECK(!g_network_change_notifier); g_network_change_notifier = this; + histogram_watcher_.reset(new HistogramWatcher()); } #if defined(OS_LINUX) diff --git a/net/base/network_change_notifier.h b/net/base/network_change_notifier.h index 03bcbf3..10c5064 100644 --- a/net/base/network_change_notifier.h +++ b/net/base/network_change_notifier.h @@ -10,8 +10,11 @@ #include "base/synchronization/lock.h" #include "net/base/net_export.h" +class GURL; + namespace net { +class HistogramWatcher; class NetworkChangeNotifierFactory; namespace internal { @@ -177,6 +180,15 @@ class NET_EXPORT NetworkChangeNotifier { NotifyObserversOfIPAddressChange(); } + // Let the NetworkChangeNotifier know we received some data. + // This is used strictly for producing histogram data about the accuracy of + // the NetworkChangenotifier's online detection. + static void NotifyDataReceived(const GURL& source); + + // Register the Observer callbacks for producing histogram data. This + // should be called from the network thread to avoid race conditions. + static void InitHistogramWatcher(); + protected: friend class internal::DnsConfigWatcher; @@ -230,6 +242,9 @@ class NET_EXPORT NetworkChangeNotifier { base::Lock watching_dns_lock_; bool watching_dns_; + // A little-piggy-back observer that simply logs UMA histogram data. + scoped_ptr histogram_watcher_; + DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifier); }; -- cgit v1.1