PreferencesManager

chrome/browser/prefs/preferences_connection_manager.cc

Index: chrome/browser/prefs/preferences_connection_manager.cc
diff --git a/chrome/browser/prefs/preferences_connection_manager.cc b/chrome/browser/prefs/preferences_connection_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e40176e743dba1c20efbb4da94e189e8b2651aac
--- /dev/null
+++ b/chrome/browser/prefs/preferences_connection_manager.cc
@@ -0,0 +1,71 @@
+// 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_connection_manager.h"
+
+#include "chrome/browser/prefs/preferences_manager.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
+#include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
+
+namespace {
+
+// Required singleton subclass to control dependencies between
+// PreferenceConnectionManager and the KeyedServiceShutdownNotifier.
+class ShutdownNotifierFactory
+    : public BrowserContextKeyedServiceShutdownNotifierFactory {
+ public:
+  static ShutdownNotifierFactory* GetInstance() {
+    return base::Singleton<ShutdownNotifierFactory>::get();
+  }
+
+ private:
+  friend struct base::DefaultSingletonTraits<ShutdownNotifierFactory>;
+
+  ShutdownNotifierFactory()
+      : BrowserContextKeyedServiceShutdownNotifierFactory(
+            "PreferencesConnectionManager") {
+    DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
+  }
+  ~ShutdownNotifierFactory() override {}
+
+  DISALLOW_COPY_AND_ASSIGN(ShutdownNotifierFactory);
+};
+
+}  // namespace
+
+namespace chrome {
+
+PreferencesConnectionManager::PreferencesConnectionManager()
+    : profile_shutdown_notification_(
+          ShutdownNotifierFactory::GetInstance()
+              ->Get(ProfileManager::GetActiveUserProfile())
+              ->Subscribe(
+                  base::Bind(&PreferencesConnectionManager::OnProfileDestroyed,
+                             base::Unretained(this)))) {}
+
+PreferencesConnectionManager::~PreferencesConnectionManager() {}
+
+void PreferencesConnectionManager::ProcessRequest(
+    prefs::mojom::PreferencesManagerRequest request) {
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  mojo::StrongBindingPtr<prefs::mojom::PreferencesManager> binding =
+      mojo::MakeStrongBinding(base::MakeUnique<PreferencesManager>(profile),
+                              std::move(request));
+  bindings_.push_back(binding);

[sky, 2016/12/06 23:37:37]

I'm surprised you don't need std::move here.

[jonross, 2016/12/07 15:46:39]

mojo::StrongBindingPtr is actually masking a
base::WeakPtr<mojo::StrongBinding<Interface>
so I was hitting WeakPtr's copy constructor. I've switched this over to the
explicit move.

+}
+
+void PreferencesConnectionManager::OnProfileDestroyed() {
+  for (auto& it : bindings_) {
+    // Check that the PreferencesManager was not cleared from a connection
+    // error.
+    if (it)
+      (static_cast<PreferencesManager*>(it->impl()))->OnProfileDestroyed();

[sky, 2016/12/06 23:37:37]

Is there a reason to keep the binding around? Won't doing so be error prone? By
that I mean if the profile is destroyed then won't something like SetPreference
fail?

[jonross, 2016/12/07 15:46:39]

I've switched this to just explicitly close the StrongBinding, which will clear
out the PreferenceManager. It's cleaner and will prevent any errors like you
describe, with SetPreferences arriving mid shutdown.

+  }
+
+  profile_shutdown_notification_.reset();
+}
+
+}  // namespace chrome