Wire NQE Prefs to Profile

chrome/browser/net/nqe/ui_network_quality_estimator_service.cc

Index: chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
diff --git a/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc b/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
index 87c4a6afb39338ae235989d703ccf75c5d0bd012..d9993e64772f55e15962da7dcf90a81fcef207c4 100644
--- a/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
+++ b/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
@@ -4,10 +4,93 @@
 
 #include "chrome/browser/net/nqe/ui_network_quality_estimator_service.h"
 
+#include <map>
+#include <string>
+
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/io_thread.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_registry.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
+#include "net/nqe/network_qualities_prefs_manager.h"
+
+namespace {
+
+// Name of the network quality estimator field trial.
+const char kNetworkQualityEstimatorFieldTrialName[] = "NetworkQualityEstimator";
+
+// Returns the variation value for |parameter_name|. If the value is
+// unavailable, |default_value| is returned.
+std::string GetStringValueForVariationParamWithDefaultValue(
+    const std::string& parameter_name,
+    const std::string& default_value) {
+  std::map<std::string, std::string> network_quality_estimator_params;
+  variations::GetVariationParams(kNetworkQualityEstimatorFieldTrialName,
+                                 &network_quality_estimator_params);
+
+  const auto it = network_quality_estimator_params.find(parameter_name);
+  return it == network_quality_estimator_params.end() ? default_value
+                                                      : it->second;
+}
+
+// Returns true if persistent caching has been enabled in the field trial.
+bool persistent_caching_enabled() {
+  return GetStringValueForVariationParamWithDefaultValue(
+             "persistent_caching_enabled", "false") == "true";
+}
+
+// PrefDelegateImpl writes the provided dictionary value to the network quality
+// estimator prefs on the disk.
+class PrefDelegateImpl
+    : public net::NetworkQualitiesPrefsManager::PrefDelegate {
+ public:
+  explicit PrefDelegateImpl(PrefService* pref_service)
+      : pref_service_(pref_service), path_(prefs::kNetworkQualities) {
+    DCHECK(pref_service_);
+  }
+  ~PrefDelegateImpl() {}
+
+  void SetDictionaryValue(const base::DictionaryValue& value) override {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!persistent_caching_enabled())
+      return;
+
+    pref_service_->Set(path_, value);

[RyanSturm, 2016/10/05 20:59:42]

Won't this overwrite the previous value? Is this only supposed to store one
cachedNetworkId at a time?

pref_service_->GetDictionary(path_)->MergeDictionary(&value);

[tbansal1, 2016/10/11 20:49:26]

Done.
I was planning to do multiple values in the next CL, but now this CL does it.

+    UMA_HISTOGRAM_COUNTS_1000("NQE.Prefs.WriteCount", 1);
+  }
+
+ private:
+  PrefService* pref_service_;
+
+  // |path_| is the location of the network quality estimator prefs.
+  const std::string path_;
+
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrefDelegateImpl);
+};
+
+// Initializes |pref_manager| on the IO thread.
+void SetNQEOnIOThread(net::NetworkQualitiesPrefsManager* prefs_manager,
+                      IOThread* io_thread) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // Avoid null pointer referencing during browser shutdown.
+  if (!io_thread->globals()->network_quality_estimator)
+    return;
+
+  prefs_manager->InitializeOnNetworkThread(
+      io_thread->globals()->network_quality_estimator.get());
+}
+
+}  // namespace
 
 // A class that sets itself as an observer of the EffectiveconnectionType for
 // the browser IO thread. It reports any change in EffectiveConnectionType back
@@ -16,7 +99,7 @@
 class UINetworkQualityEstimatorService::IONetworkQualityObserver
     : public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver {
  public:
-  IONetworkQualityObserver(
+  explicit IONetworkQualityObserver(
       base::WeakPtr<UINetworkQualityEstimatorService> service)
       : service_(service) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -64,21 +147,30 @@ class UINetworkQualityEstimatorService::IONetworkQualityObserver
   DISALLOW_COPY_AND_ASSIGN(IONetworkQualityObserver);
 };
 
-UINetworkQualityEstimatorService::UINetworkQualityEstimatorService()
+UINetworkQualityEstimatorService::UINetworkQualityEstimatorService(
+    Profile* profile)
     : type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
       io_observer_(nullptr),
+      prefs_manager_(nullptr),
       weak_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
   // If this is running in a context without an IOThread, don't try to create
   // the IO object.
   if (!g_browser_process->io_thread())
     return;
   io_observer_ = new IONetworkQualityObserver(weak_factory_.GetWeakPtr());
+  prefs_manager_ = new net::NetworkQualitiesPrefsManager(
+      base::WrapUnique(new PrefDelegateImpl(profile->GetPrefs())));
+
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&IONetworkQualityObserver::InitializeOnIOThread,
                  base::Unretained(io_observer_),
                  g_browser_process->io_thread()));
+  content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
+                                   base::Bind(&SetNQEOnIOThread, prefs_manager_,
+                                              g_browser_process->io_thread()));
 }
 
 UINetworkQualityEstimatorService::~UINetworkQualityEstimatorService() {
@@ -88,8 +180,15 @@ UINetworkQualityEstimatorService::~UINetworkQualityEstimatorService() {
 void UINetworkQualityEstimatorService::Shutdown() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   weak_factory_.InvalidateWeakPtrs();
-  DCHECK(content::BrowserThread::DeleteSoon(content::BrowserThread::IO,
-                                            FROM_HERE, io_observer_));
+  if (io_observer_) {
+    DCHECK(content::BrowserThread::DeleteSoon(content::BrowserThread::IO,

[RyanSturm, 2016/10/05 20:59:42]

When this got moved to Shutdown, we should have invalidated the pointers. Please
set io_observer_ and prefs_manager_ to nullptr.

[tbansal1, 2016/10/11 20:49:26]

Done.

+                                              FROM_HERE, io_observer_));
+  }
+  if (prefs_manager_) {
+    prefs_manager_->ShutdownOnPrefThread();
+    DCHECK(content::BrowserThread::DeleteSoon(content::BrowserThread::IO,
+                                              FROM_HERE, prefs_manager_));
+  }
 }
 
 void UINetworkQualityEstimatorService::EffectiveConnectionTypeChanged(
@@ -109,3 +208,10 @@ UINetworkQualityEstimatorService::GetEffectiveConnectionType() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return type_;
 }
+
+// static
+void UINetworkQualityEstimatorService::RegisterProfilePrefs(
+    PrefRegistrySimple* registry) {
+  registry->RegisterDictionaryPref(prefs::kNetworkQualities,
+                                   PrefRegistry::LOSSY_PREF);
+}