PreferencesManager

chrome/browser/prefs/preferences_manager.h

Index: chrome/browser/prefs/preferences_manager.h
diff --git a/chrome/browser/prefs/preferences_manager.h b/chrome/browser/prefs/preferences_manager.h
new file mode 100644
index 0000000000000000000000000000000000000000..f7a80a52e3eb0c26aff3e866e69ff4ba303cf926
--- /dev/null
+++ b/chrome/browser/prefs/preferences_manager.h
@@ -0,0 +1,61 @@
+// 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 CHROME_BROWSER_PREFS_PREFERENCES_MANAGER_H_
+#define CHROME_BROWSER_PREFS_PREFERENCES_MANAGER_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "services/preferences/public/interfaces/preferences.mojom.h"
+
+namespace test {
+class PreferencesManagerTest;
+}
+
+class PrefChangeRegistrar;
+class PrefService;
+class Profile;
+
+namespace chrome {
+
+// Implementation of prefs::mojom::PreferencesManager that accepts a single
+// prefs::mojom::PreferencesObserver.
+//
+// After calling AddObserver PreferencesManager will begin observing changes to
+// the requested preferences, notifying the client of all changes.

[sadrul, 2016/11/29 17:25:49]

If we have a case where multiple clients are observing the same set of
preferences, then it may make sense to rearrange how this works. For example,
instead of having a PreferencesManager instance for each client, we could have a
single PreferencesManager instance that services all clients, and maintains a
{pref => [clients]} map to decide when to notify which clients. But for now,
this should be OK.

[jonross, 2016/11/30 01:01:31]

I went this this 1:1 model so that clients setting a pref wouldn't be notified
of that pref. While other clients listening for changes would.

We could come up with a model where the client needs to identify itself with
setting a pref, but it felt wrong to send the mojom ptr across on each call.

+class PreferencesManager : public prefs::mojom::PreferencesManager {
+ public:
+  explicit PreferencesManager(Profile* profile);
+  ~PreferencesManager() override;
+
+  // Must be called when the active PrefService is being destroyed.
+  void OnProfileDestroyed();
+
+ private:
+  friend class test::PreferencesManagerTest;
+
+  // mojom::PreferencesManager:
+  void AddObserver(prefs::mojom::PreferencesObserverPtr client) override;
+  void SetPreferences(const base::DictionaryValue& preferences) override;
+  void Subscribe(const std::vector<std::string>& preferences) override;
+
+  // PrefChangeRegistrar::NamedChangeCallback:
+  void PreferenceChanged(const std::string& preference_name);

[sadrul, 2016/11/29 17:25:49]

Non-override before override. (I seem to remember we used to have a style-guide
entry for this, but can't find it anymore ... so dunno)

[jonross, 2016/11/30 01:01:31]

Done.

+
+  // Tracks the desired preferences, and listens for updates.
+  std::unique_ptr<PrefChangeRegistrar> preferences_change_registrar_;
+  prefs::mojom::PreferencesObserverPtr client_;
+  PrefService* service_;
+
+  // Used to prevent notifying |client_| of changes caused by it calling
+  // SetPreferences.
+  bool setting_preferences_;
+
+  DISALLOW_COPY_AND_ASSIGN(PreferencesManager);
+};
+
+}  // namespace chrome

[sadrul, 2016/11/29 17:25:49]

Do we usually use chrome namespace?

[jonross, 2016/11/30 01:01:31]

So chrome/browser/prefs has one use of chrome, and one of chrome_prefs, then a
bunch with no namespace.

In general almost 400 examples of it being used. However I don't feel strongly,
can remove if desired.

[sadrul, 2016/12/06 18:34:29]

No strong preference either way. Whatever the code in the same place is doing is
fine.

+
+#endif  // CHROME_BROWSER_PREFS_PREFERENCES_MANAGER_H_