Add Network Quality Estimator (NQE) pref manager

net/nqe/network_qualities_prefs_manager.h

Index: net/nqe/network_qualities_prefs_manager.h
diff --git a/net/nqe/network_qualities_prefs_manager.h b/net/nqe/network_qualities_prefs_manager.h
new file mode 100644
index 0000000000000000000000000000000000000000..161abd23dc53270f03f69547d4666f5775c2d0fe
--- /dev/null
+++ b/net/nqe/network_qualities_prefs_manager.h
@@ -0,0 +1,113 @@
+// 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.
+
+#ifndef NET_NQE_NETWORK_QUALITIES_PREFS_MANAGER_H_
+#define NET_NQE_NETWORK_QUALITIES_PREFS_MANAGER_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "net/base/net_export.h"
+#include "net/nqe/effective_connection_type.h"
+#include "net/nqe/network_id.h"
+
+namespace base {
+class DictionaryValue;
+class SequencedTaskRunner;
+}
+
+namespace net {
+namespace nqe {

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

What's the rationale behind nqe being a namespace for some NQE classes and not
others?

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

Anything that can be called outside from //net should be in net namespace. Other
than that, I try to keep things in net::nqe::internal namespace.

[RyanSturm, 2016/09/16 16:23:41]

Acknowledged.

+namespace internal {
+class CachedNetworkQuality;
+}
+}
+class NetworkQualityEstimator;
+
+typedef base::Callback<void(
+    const nqe::internal::NetworkID& network_id,
+    const nqe::internal::CachedNetworkQuality& cached_network_quality)>
+    OnChangeInCachedNetworkQualityCallback;
+
+// The manager for creating and updating network quality prefs.
+//
+// This class interacts with both the pref thread, where the pref is
+// read or written, and the network thread, which owns it, and it
+// persists the network quality changes.
+//
+// It must be constructed on the pref thread, to set up |pref_task_runner_| and
+// the prefs listeners.
+//
+// ShutdownOnPrefThread must be called from pref thread before destruction.
+class NET_EXPORT NetworkQualitiesPrefsManager {

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

Does this class need to be prefs specific? Could this be a generic
NetworkQualitiesCacheObserver on a different thread? 

This way if something else needs to listen to these events on the UI thread, the
you can reuse this functionality.

Similar to UINetworkEstimatorService using IONetworkQualityObserver to propagate
events to UI thread. Instead of hardcoding threads, as UINetworkEstimatorService
does, the task runner could be passed in.

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

I have rewritten the class comment to make it clearer.

+ public:
+  // Provides an interface that must be implemented by the embedder.
+  class NET_EXPORT PrefDelegate {
+   public:
+    // Sets the persistent pref to the given value.
+    virtual void SetDictionaryValue(const base::DictionaryValue& value) = 0;
+  };
+
+  // Create an instance of the NetworkQualitiesPrefsManager. Ownership of
+  // |pref_delegate| is taken by this class. Must be constructed on the Pref
+  // thread, and then moved to network thread.
+  explicit NetworkQualitiesPrefsManager(
+      std::unique_ptr<PrefDelegate> pref_delegate);
+  virtual ~NetworkQualitiesPrefsManager();
+
+  // Initialize on Network thread.
+  void InitializeOnNetworkThread(
+      NetworkQualityEstimator* network_quality_estimator);
+
+  // Prepare for shutdown. Must be called on the Pref thread before destruction.
+  void ShutdownOnPrefThread();
+
+ private:
+  // -----------
+  // Pref thread
+  // -----------
+
+  // Called on pref thread when there is a change in the cached network quality.
+  void OnChangeInCachedNetworkQualityOnPrefThread(
+      const nqe::internal::NetworkID& network_id,
+      const nqe::internal::CachedNetworkQuality& cached_network_quality);
+
+  // Responsible for writing the persistent prefs to the disk.
+  std::unique_ptr<PrefDelegate> pref_delegate_;
+
+  scoped_refptr<base::SequencedTaskRunner> pref_task_runner_;
+
+  // Should be accessed only on the pref thread.
+  base::WeakPtr<NetworkQualitiesPrefsManager> pref_weak_ptr_;
+
+  // --------------
+  // Network thread
+  // --------------
+
+  // Responsible for receiving notifications about changes in the network
+  // quality from the network quality estimator.
+  class CacheObserver;
+  std::unique_ptr<CacheObserver> cache_observer_;
+
+  scoped_refptr<base::SequencedTaskRunner> network_task_runner_;
+
+  // --------------
+  // Common
+  // --------------
+
+  // Used to get |weak_ptr_| to self on the pref thread.
+  std::unique_ptr<base::WeakPtrFactory<NetworkQualitiesPrefsManager>>

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

Do these weak factories need to be std::unique_ptr? I think weak_ptr binds to
the thread when the first pointer is dereferenced, not when the factory is
created. Please remove the network one from this CL.

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

Done.

+      pref_weak_ptr_factory_;
+
+  // Used to get |weak_ptr_| to self on the network thread.
+  std::unique_ptr<base::WeakPtrFactory<NetworkQualitiesPrefsManager>>
+      network_weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkQualitiesPrefsManager);
+};
+
+}  // namespace net
+
+#endif  // NET_NQE_NETWORK_QUALITIES_PREFS_MANAGER_H_