Add Network Quality Estimator (NQE) pref manager

net/nqe/network_qualities_prefs_manager.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_qualities_prefs_manager.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/threading/thread_checker.h"
+#include "base/values.h"
+#include "net/nqe/cached_network_quality.h"
+#include "net/nqe/network_quality_estimator.h"
+#include "net/nqe/network_quality_store.h"
+
+namespace net {
+
+// CacheObserver lives on the network thread.
+class NetworkQualitiesPrefsManager::CacheObserver

[RyanSturm, 2016/09/12 20:26:55]

Is it cleaner to have NetworkQualitiesPrefsManager inherit from
NetworkQualitiesCacheObserver directly? It only uses one method, and there isn't
really a threading issue since NetworkQualitiesPrefsManager owns the lifetime
management as a unique_ptr.

[tbansal1, 2016/09/15 21:00:59]

Done.

+    : public nqe::internal::NetworkQualityStore::NetworkQualitiesCacheObserver {
+ public:
+  // CacheObserver receives notification when there is a change in the network
+  // quality. On receiving the notification, it calls |callback| on
+  // |callback_task_runner|.
+  CacheObserver(
+      OnChangeInCachedNetworkQualityCallback callback,
+      const scoped_refptr<base::SequencedTaskRunner>& callback_task_runner,
+      NetworkQualityEstimator* network_quality_estimator)
+      : callback_(callback),
+        callback_task_runner_(callback_task_runner),
+        network_quality_estimator_(network_quality_estimator) {
+    DCHECK(callback_task_runner_);
+    // Add |this| as a cache observer so that notification is received
+    // every time there is a change in the cached network quality.
+    network_quality_estimator_->AddNetworkQualitiesCacheObserver(this);
+  }
+  ~CacheObserver() override {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    network_quality_estimator_->RemoveNetworkQualitiesCacheObserver(this);
+  }
+
+  // nqe::internal::NetworkQualityStore::NetworkQualitiesCacheObserver
+  // implementation:
+  void OnChangeInCachedNetworkQuality(
+      const nqe::internal::NetworkID& network_id,
+      const nqe::internal::CachedNetworkQuality& cached_network_quality)
+      override {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+    // Run the callback on the provided task runner.
+    callback_task_runner_->PostTask(

[RyanSturm, 2016/09/12 20:26:55]

Why do you post this task directly to the prefs thread? It might be cleaner to
call the callback on the network thread and have NetworkQualitiesPrefsManager do
the PostTask. That way you don't have to pass a taskrunner in.

[tbansal1, 2016/09/15 21:00:58]

Removed CacheObserver.

+        FROM_HERE, base::Bind(callback_, network_id, cached_network_quality));
+  }
+
+ private:
+  // Called when a notification is received about change in the cached
+  // network quality. Should be called only on |callback_task_runner_|.
+  OnChangeInCachedNetworkQualityCallback callback_;
+
+  const scoped_refptr<base::SequencedTaskRunner> callback_task_runner_;
+
+  NetworkQualityEstimator* network_quality_estimator_;
+
+  base::ThreadChecker thread_checker_;
+  DISALLOW_COPY_AND_ASSIGN(CacheObserver);
+};
+
+NetworkQualitiesPrefsManager::NetworkQualitiesPrefsManager(
+    std::unique_ptr<PrefDelegate> pref_delegate)
+    : pref_delegate_(std::move(pref_delegate)),
+      pref_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
+  DCHECK(pref_delegate_);
+
+  pref_weak_ptr_factory_.reset(
+      new base::WeakPtrFactory<NetworkQualitiesPrefsManager>(this));
+  pref_weak_ptr_ = pref_weak_ptr_factory_->GetWeakPtr();
+}
+
+NetworkQualitiesPrefsManager::~NetworkQualitiesPrefsManager() {
+  DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
+  network_weak_ptr_factory_.reset();
+}
+
+void NetworkQualitiesPrefsManager::InitializeOnNetworkThread(
+    NetworkQualityEstimator* network_quality_estimator) {
+  DCHECK(!network_task_runner_);
+  DCHECK(network_quality_estimator);
+
+  network_task_runner_ = base::ThreadTaskRunnerHandle::Get();
+  network_weak_ptr_factory_.reset(

[Not at Google. Contact bengr, 2016/09/09 20:30:40]

|network_weak_ptr_factory_| unused?

[tbansal1, 2016/09/15 21:00:59]

Done.

+      new base::WeakPtrFactory<NetworkQualitiesPrefsManager>(this));
+
+  // Create the cache observer on the network thread, and provide the callback.
+  // |this| will be notified every time there is a change in the network
+  // quality.
+  cache_observer_.reset(new NetworkQualitiesPrefsManager::CacheObserver(
+      base::Bind(&NetworkQualitiesPrefsManager::
+                     OnChangeInCachedNetworkQualityOnPrefThread,
+                 pref_weak_ptr_),
+      pref_task_runner_, network_quality_estimator));

[RyanSturm, 2016/09/12 20:26:55]

Instead of passing in network_quality_estimator, can you just add the observer
from here?

[tbansal1, 2016/09/15 21:00:58]

Done.

+}
+
+void NetworkQualitiesPrefsManager::ShutdownOnPrefThread() {
+  DCHECK(pref_task_runner_->RunsTasksOnCurrentThread());
+  pref_weak_ptr_factory_.reset();
+  pref_delegate_.reset();
+}
+
+void NetworkQualitiesPrefsManager::OnChangeInCachedNetworkQualityOnPrefThread(
+    const nqe::internal::NetworkID& network_id,
+    const nqe::internal::CachedNetworkQuality& cached_network_quality) {
+  // The prefs can only be written on the pref thread.
+  DCHECK(pref_task_runner_->RunsTasksOnCurrentThread());
+
+  base::DictionaryValue dictionary_value;
+  dictionary_value.SetString(
+      network_id.ToString(),
+      GetNameForEffectiveConnectionType(
+          cached_network_quality.effective_connection_type()));
+
+  // Notify the pref delegate so that it updates the prefs on the disk.
+  pref_delegate_->SetDictionaryValue(dictionary_value);
+}
+
+}  // namespace net