Record UMA only when network quality is eligible for caching

net/nqe/network_quality_store.cc

 // Copyright 2016 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_store.h"
 
 #include "base/location.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "net/base/network_change_notifier.h"
 
@@ skipping 17 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 void NetworkQualityStore::Add(
     const nqe::internal::NetworkID& network_id,
     const nqe::internal::CachedNetworkQuality& cached_network_quality) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_LE(cached_network_qualities_.size(),
             static_cast<size_t>(kMaximumNetworkQualityCacheSize));
 
-  // If the network name is unavailable, caching should not be performed. If
-  // |disable_offline_check_| is set to true, cache the network quality even if
-  // the network is set to offline.
-  if (network_id.type != NetworkChangeNotifier::CONNECTION_ETHERNET &&
-      network_id.id.empty() &&
-      (network_id.type != NetworkChangeNotifier::CONNECTION_NONE ||
-       !disable_offline_check_)) {
+  if (!EligibleForCaching(network_id))
     return;
-  }
 
   // Remove the entry from the map, if it is already present.
   cached_network_qualities_.erase(network_id);
 
   if (cached_network_qualities_.size() == kMaximumNetworkQualityCacheSize) {
     // Remove the oldest entry.
     CachedNetworkQualities::iterator oldest_entry_iterator =
         cached_network_qualities_.begin();
 
     for (CachedNetworkQualities::iterator it =
@@ skipping 40 matching lines @@
       FROM_HERE, base::Bind(&NetworkQualityStore::NotifyCacheObserverIfPresent,
                             weak_ptr_factory_.GetWeakPtr(), observer));
 }
 
 void NetworkQualityStore::RemoveNetworkQualitiesCacheObserver(
     NetworkQualitiesCacheObserver* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   network_qualities_cache_observer_list_.RemoveObserver(observer);
 }
 
+bool NetworkQualityStore::EligibleForCaching(
+    const NetworkID& network_id) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // If the network name is unavailable, caching should not be performed. If

[RyanSturm, 2017/03/09 00:01:55]

This comment seems unnecessary. It describes the code below without providing
any more info. Just remove it.

[tbansal1, 2017/03/09 00:24:00]

Done.

+  // |disable_offline_check_| is set to true, cache the network quality even if
+  // the network is set to offline.
+  return network_id.type == NetworkChangeNotifier::CONNECTION_ETHERNET ||
+         !network_id.id.empty() ||
+         (network_id.type == NetworkChangeNotifier::CONNECTION_NONE &&
+          disable_offline_check_);
+}
+
 void NetworkQualityStore::DisableOfflineCheckForTesting(
     bool disable_offline_check) {
   DCHECK(thread_checker_.CalledOnValidThread());
   disable_offline_check_ = disable_offline_check;
 }
 
 void NetworkQualityStore::NotifyCacheObserverIfPresent(
     NetworkQualitiesCacheObserver* observer) const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!network_qualities_cache_observer_list_.HasObserver(observer))
     return;
   for (const auto it : cached_network_qualities_)
     observer->OnChangeInCachedNetworkQuality(it.first, it.second);
 }
 
 }  // namespace internal
 
 }  // namespace nqe
 
 }  // namespace net