PreferencesManager

chrome/browser/prefs/preferences_manager.cc

Index: chrome/browser/prefs/preferences_manager.cc
diff --git a/chrome/browser/prefs/preferences_manager.cc b/chrome/browser/prefs/preferences_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..dc364973f5c43c7f0ee37b4432308c6b9ef1f508
--- /dev/null
+++ b/chrome/browser/prefs/preferences_manager.cc
@@ -0,0 +1,84 @@
+// 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 "chrome/browser/prefs/preferences_manager.h"
+
+#include "base/bind.h"
+#include "base/values.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_service.h"
+
+namespace chrome {
+
+PreferencesManager::PreferencesManager(Profile* profile)
+    : preferences_change_registrar_(new PrefChangeRegistrar),
+      setting_preferences_(false) {
+  DCHECK(profile);
+  service_ = profile->GetPrefs();
+  preferences_change_registrar_->Init(service_);
+}
+
+PreferencesManager::~PreferencesManager() {}
+
+void PreferencesManager::OnProfileDestroyed() {
+  preferences_change_registrar_.reset();
+}
+
+void PreferencesManager::AddObserver(
+    prefs::mojom::PreferencesObserverPtr client) {
+  client_ = std::move(client);
+}
+
+void PreferencesManager::SetPreferences(
+    const base::DictionaryValue& preferences) {
+  for (base::DictionaryValue::Iterator it(preferences); !it.IsAtEnd();
+       it.Advance()) {
+    if (!preferences_change_registrar_->IsObserved(it.key()))
+      continue;
+    const PrefService::Preference* pref = service_->FindPreference(it.key());
+    if (!pref)
+      continue;
+    if (it.value().Equals(pref->GetValue()))
+      continue;
+    // We ignore preference changes caused by us.
+    setting_preferences_ = true;

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

Move this out of the for loop, and use base::AutoReset<>? Maybe a DCHECK() that
|setting_preferences_| already isn't true when this function is called?

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

Done.

+    service_->Set(it.key(), it.value());
+    setting_preferences_ = false;
+  }
+}
+
+void PreferencesManager::Subscribe(
+    const std::vector<std::string>& preferences) {
+  base::DictionaryValue dictionary;
+  for (auto& it : preferences) {
+    const PrefService::Preference* pref = service_->FindPreference(it);
+    if (!pref)
+      continue;
+    // PreferenceManager lifetime is managed by PreferenceConnectionManager it
+    // will outlive |preferences_change_registrar_| which it owns.

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

This comment is slightly wrong: PCM does not actually manage the lifetime of
this PM, right?

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

Half true now with the wrapping mojo::StrongBindingPtr. Either the pipe dies,
cleaning up PM. Or chrome shuts down, and PCM clears the bindings.

Updated the comment, as either way PM outlives the PCR.

+    preferences_change_registrar_->Add(
+        it, base::Bind(&PreferencesManager::PreferenceChanged,
+                       base::Unretained(this)));
+    dictionary.Set(it, pref->GetValue()->CreateDeepCopy());
+  }
+
+  if (dictionary.empty())
+    return;
+  client_->OnPreferencesChanged(dictionary);
+}
+
+void PreferencesManager::PreferenceChanged(const std::string& preference_name) {
+  if (setting_preferences_)
+    return;
+  const PrefService::Preference* pref =
+      service_->FindPreference(preference_name);
+  if (!pref)
+    return;

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

Should |pref| ever be null?

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

Actually no. We don't subscribe if the pref doesn't exist.

+  base::DictionaryValue dictionary;
+  dictionary.Set(preference_name, pref->GetValue()->CreateDeepCopy());
+  client_->OnPreferencesChanged(dictionary);

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

If a lot of preferences are updating at the same time, it'd be nice to bundle up
all those updates in the same IPC, instead of a separate message for each pref
change. Maybe things like that don't happen [very often]?

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

It would be nice to reduce the number of IPCs. I don't currently have any
metrics on how often they are updated.

The Profile code has no batching, but we could do that here. With some post task
mechanism.

+}
+
+}  // namespace chrome