Pref service: have Mash and Chrome connect to the right pref stores

services/preferences/pref_store_manager_impl.cc

 // Copyright 2017 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 "services/preferences/pref_store_manager_impl.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/memory/ref_counted.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "components/prefs/default_pref_store.h"
 #include "components/prefs/pref_value_store.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "services/preferences/persistent_pref_store_factory.h"
 #include "services/preferences/persistent_pref_store_impl.h"
 #include "services/preferences/public/cpp/pref_store_impl.h"
 #include "services/service_manager/public/cpp/interface_registry.h"
 
 namespace prefs {
 namespace {
 
 using ConnectCallback = mojom::PrefStoreConnector::ConnectCallback;
 using PrefStorePtrs =
-    std::unordered_map<PrefValueStore::PrefStoreType, mojom::PrefStorePtr>;
+    std::unordered_map<PrefValueStore::PrefStoreType, mojom::PrefStore*>;
 
 // Used to make sure all pref stores have been registered before replying to any
 // Connect calls.
 class ConnectionBarrier : public base::RefCounted<ConnectionBarrier> {
  public:
   static void Create(
       const PrefStorePtrs& pref_store_ptrs,
       mojom::PersistentPrefStoreConnectionPtr persistent_pref_store_connection,
       const std::vector<std::string>& observed_prefs,
       const ConnectCallback& callback);
@@ skipping 72 matching lines @@
 PrefStoreManagerImpl::PrefStoreManagerImpl(
     std::set<PrefValueStore::PrefStoreType> expected_pref_stores,
     scoped_refptr<base::SequencedWorkerPool> worker_pool)
     : expected_pref_stores_(std::move(expected_pref_stores)),
       init_binding_(this),
       defaults_(new DefaultPrefStore),
       defaults_wrapper_(base::MakeUnique<PrefStoreImpl>(
           defaults_,
           mojo::MakeRequest(&pref_store_ptrs_[PrefValueStore::DEFAULT_STORE]))),
       worker_pool_(std::move(worker_pool)) {
+  expected_pref_stores_.erase(PrefValueStore::USER_STORE);
   expected_pref_stores_.insert(PrefValueStore::DEFAULT_STORE);
 }
 
 PrefStoreManagerImpl::~PrefStoreManagerImpl() = default;
 
+struct PrefStoreManagerImpl::PendingConnect {
+  mojom::PrefRegistryPtr pref_registry;
+  // Pref stores the caller already is connected to (and hence we won't
+  // connect to these).
+  std::vector<PrefValueStore::PrefStoreType> already_connected_types;
+  ConnectCallback callback;
+};
+
 void PrefStoreManagerImpl::Register(PrefValueStore::PrefStoreType type,
                                     mojom::PrefStorePtr pref_store_ptr) {
   if (expected_pref_stores_.count(type) == 0) {
     LOG(WARNING) << "Not registering unexpected pref store: " << type;
     return;
   }
   DVLOG(1) << "Registering pref store: " << type;
   pref_store_ptr.set_connection_error_handler(
       base::Bind(&PrefStoreManagerImpl::OnPrefStoreDisconnect,
                  base::Unretained(this), type));
   const bool success =
       pref_store_ptrs_.insert(std::make_pair(type, std::move(pref_store_ptr)))
           .second;
   DCHECK(success) << "The same pref store registered twice: " << type;
   if (AllConnected()) {
     DVLOG(1) << "All pref stores registered.";
     ProcessPendingConnects();
   }
 }
 
-void PrefStoreManagerImpl::Connect(mojom::PrefRegistryPtr pref_registry,
-                                   const ConnectCallback& callback) {
+void PrefStoreManagerImpl::Connect(
+    mojom::PrefRegistryPtr pref_registry,
+    const std::vector<PrefValueStore::PrefStoreType>& already_connected_types,
+    const ConnectCallback& callback) {
   if (AllConnected()) {
-    ConnectImpl(std::move(pref_registry), callback);
+    ConnectImpl(std::move(pref_registry), already_connected_types, callback);
   } else {
     pending_connects_.push_back(
-        std::make_pair(callback, std::move(pref_registry)));
+        {std::move(pref_registry), already_connected_types, callback});
   }
 }
 
 void PrefStoreManagerImpl::Create(
     const service_manager::Identity& remote_identity,
     prefs::mojom::PrefStoreConnectorRequest request) {
   connector_bindings_.AddBinding(this, std::move(request));
 }
 
 void PrefStoreManagerImpl::Create(
@@ skipping 45 matching lines @@
   pref_store_ptrs_.erase(type);
 }
 
 bool PrefStoreManagerImpl::AllConnected() const {
   return pref_store_ptrs_.size() == expected_pref_stores_.size() &&
          persistent_pref_store_ && persistent_pref_store_->initialized();
 }
 
 void PrefStoreManagerImpl::ProcessPendingConnects() {
   for (auto& connect : pending_connects_)
-    ConnectImpl(std::move(connect.second), connect.first);
+    ConnectImpl(std::move(connect.pref_registry),
+                connect.already_connected_types, connect.callback);
   pending_connects_.clear();
 }
 
-void PrefStoreManagerImpl::ConnectImpl(mojom::PrefRegistryPtr pref_registry,
-                                       const ConnectCallback& callback) {
+void PrefStoreManagerImpl::ConnectImpl(
+    mojom::PrefRegistryPtr pref_registry,
+    const std::vector<PrefValueStore::PrefStoreType>& already_connected_types,
+    const ConnectCallback& callback) {
   std::vector<std::string> observed_prefs;
   for (auto& registration : pref_registry->registrations) {
     observed_prefs.push_back(registration.first);
     const auto& key = registration.first;
     auto& default_value = registration.second->default_value;
     const base::Value* old_default = nullptr;
     // TODO(sammc): Once non-owning registrations are supported, disallow
     // multiple owners instead of just checking for consistent defaults.
     if (defaults_->GetValue(key, &old_default))
       DCHECK(old_default->Equals(default_value.get()));
     else
       defaults_->SetDefaultValue(key, std::move(default_value));
   }
+
+  // Only connect to pref stores the client isn't already connected to.
+  PrefStorePtrs ptrs;
+  for (const auto& entry : pref_store_ptrs_) {
+    const bool already_connected =
+        std::find(already_connected_types.begin(),

[Bernhard Bauer, 2017/04/05 10:04:32]

Nit: base::ContainsValue() (from base/stl_util.h) does this, and looks slightly
more concise.

[tibell, 2017/04/06 01:05:52]

Done.

+                  already_connected_types.end(),
+                  entry.first) != already_connected_types.end();
+    if (!already_connected) {
+      ptrs.insert(std::make_pair(entry.first, entry.second.get()));
+    }
+  }
   ConnectionBarrier::Create(
-      pref_store_ptrs_,
+      ptrs,
       persistent_pref_store_->CreateConnection(
           PersistentPrefStoreImpl::ObservedPrefs(observed_prefs.begin(),
                                                  observed_prefs.end())),
       observed_prefs, callback);
 }
 
 void PrefStoreManagerImpl::OnPersistentPrefStoreReady() {
   DVLOG(1) << "PersistentPrefStore ready";
   if (AllConnected()) {
     ProcessPendingConnects();
   }
 }
 
 }  // namespace prefs