Migrate the Chrome OS assistant service off the mojo pref service

The assistant service runs in the browser process on the UI thread.
It can directly use the Profile's PrefService* to access prefs.
This simplifies the code and is a step toward deleting the mojo
pref service entirely.

The only non-trivial change is that the AssistantServiceConnection
object needs to destroy itself earlier in Profile teardown.
AssistantStateBase has a PrefChangeRegistrar that needs to be
destroyed before the Profile's PrefService is destroyed.

I manually tested on linux-chromeos under ASAN and on device.

TBR=battre@chromium.org

Bug: 977637
Test: chromeos_unittests
Change-Id: I6dec70a346b611c0825746cd1c39321d0dce466c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919790
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Yue Li <updowndota@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716399}

ash/app_list/views/assistant/assistant_test_api_impl.cc

@@ -13,6 +13,7 @@
 #include "ash/public/cpp/tablet_mode.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
+#include "components/prefs/pref_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/events/test/event_generator.h"

ash/ash_prefs.cc

@@ -48,7 +48,10 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry, bool for_test) {
   ShelfController::RegisterProfilePrefs(registry);
   TouchDevicesController::RegisterProfilePrefs(registry);
   tray::VPNListView::RegisterProfilePrefs(registry);
-  chromeos::assistant::prefs::RegisterProfilePrefsForeign(registry, for_test);
+
+  // ash_unittests relies on assistant prefs.
+  if (for_test)
+    chromeos::assistant::prefs::RegisterProfilePrefs(registry);
 }
 
 }  // namespace

ash/public/cpp/assistant/assistant_state_base.h

@@ -9,6 +9,8 @@
 
 #include "ash/public/mojom/assistant_state_controller.mojom.h"
 #include "base/macros.h"
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
 #include "base/optional.h"
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
 

chrome/browser/prefs/browser_prefs.cc

@@ -942,7 +942,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry,
   certificate_manager::CertificatesHandler::RegisterProfilePrefs(registry);
   chromeos::AccountManager::RegisterPrefs(registry);
   chromeos::ApkWebAppService::RegisterProfilePrefs(registry);
-  chromeos::assistant::prefs::RegisterProfilePrefsForBrowser(registry);
+  chromeos::assistant::prefs::RegisterProfilePrefs(registry);
   chromeos::bluetooth::DebugLogsManager::RegisterPrefs(registry);
   chromeos::CupsPrintersManager::RegisterProfilePrefs(registry);
   chromeos::device_sync::DeviceSyncImpl::RegisterProfilePrefs(registry);

chrome/browser/ui/ash/assistant/assistant_service_connection.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/ash/assistant/assistant_service_connection.h"
 
+#include "chrome/browser/profiles/profile.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace {
@@ -14,9 +15,15 @@ constexpr const char kConnectionKey[] = "assistant_service_connection";
 
 AssistantServiceConnection::AssistantServiceConnection(Profile* profile)
     : service_(remote_.BindNewPipeAndPassReceiver(),
-               profile->GetURLLoaderFactory()->Clone()) {}
+               profile->GetURLLoaderFactory()->Clone(),
+               profile->GetPrefs()),
+      profile_(profile) {
+  profile_->AddObserver(this);
+}
 
-AssistantServiceConnection::~AssistantServiceConnection() = default;
+AssistantServiceConnection::~AssistantServiceConnection() {
+  profile_->RemoveObserver(this);
+}
 
 // static
 AssistantServiceConnection* AssistantServiceConnection::GetForProfile(
@@ -30,3 +37,9 @@ AssistantServiceConnection* AssistantServiceConnection::GetForProfile(
   }
   return connection;
 }
+
+void AssistantServiceConnection::OnProfileWillBeDestroyed(Profile* profile) {
+  // Clean up connection before the profile's PrefService is destroyed.
+  profile->RemoveUserData(kConnectionKey);
+  // |this| is deleted.
+}

chrome/browser/ui/ash/assistant/assistant_service_connection.h

@@ -7,13 +7,16 @@
 
 #include "base/macros.h"
 #include "base/supports_user_data.h"
-#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_observer.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "chromeos/services/assistant/service.h"
 
+class Profile;
+
 // AssistantServiceConnection exposes a Mojo interface connection to a Profile's
 // own instance of the in-process Assistant service.
-class AssistantServiceConnection : public base::SupportsUserData::Data {
+class AssistantServiceConnection : public base::SupportsUserData::Data,
+                                   public ProfileObserver {
  public:
   explicit AssistantServiceConnection(Profile* profile);
   ~AssistantServiceConnection() override;
@@ -25,8 +28,12 @@ class AssistantServiceConnection : public base::SupportsUserData::Data {
   }
 
  private:
+  // ProfileObserver:
+  void OnProfileWillBeDestroyed(Profile* profile) override;
+
   mojo::Remote<chromeos::assistant::mojom::AssistantService> remote_;
   chromeos::assistant::Service service_;
+  Profile* const profile_;
 
   DISALLOW_COPY_AND_ASSIGN(AssistantServiceConnection);
 };

chrome/browser/ui/ash/assistant/assistant_state_client.cc

@@ -6,11 +6,13 @@
 
 #include "ash/public/cpp/assistant/assistant_state.h"
 #include "ash/public/mojom/assistant_state_controller.mojom.h"
+#include "base/bind.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/assistant/assistant_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "components/language/core/browser/pref_names.h"
 #include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_service.h"
 
 AssistantStateClient::AssistantStateClient() {
   arc::ArcSessionManager::Get()->AddObserver(this);

chromeos/services/assistant/BUILD.gn

@@ -27,8 +27,6 @@ component("lib") {
     "fake_assistant_manager_service_impl.h",
     "fake_assistant_settings_manager_impl.cc",
     "fake_assistant_settings_manager_impl.h",
-    "pref_connection_delegate.cc",
-    "pref_connection_delegate.h",
     "service.cc",
     "service.h",
     "service_context.h",
@@ -50,8 +48,6 @@ component("lib") {
     "//services/device/public/mojom",
     "//services/identity/public/mojom",
     "//services/network/public/cpp:cpp",
-    "//services/preferences/public/cpp",
-    "//services/preferences/public/mojom",
     "//ui/accessibility:ax_assistant",
   ]
 

chromeos/services/assistant/DEPS

@@ -12,7 +12,6 @@ include_rules = [
   "+services/identity/public",
   "+services/media_session/public",
   "+services/network/public",
-  "+services/preferences/public",
   "+ui/accessibility/ax_assistant_structure.h",
   "+ui/accessibility/mojom",
   "+ui/base",

chromeos/services/assistant/assistant_state_proxy.cc

@@ -15,15 +15,15 @@
 namespace chromeos {
 namespace assistant {
 
-AssistantStateProxy::AssistantStateProxy()
-    : pref_connection_delegate_(std::make_unique<PrefConnectionDelegate>()) {}
+AssistantStateProxy::AssistantStateProxy() = default;
 
 AssistantStateProxy::~AssistantStateProxy() {
   // Reset pref change registar.
   RegisterPrefChanges(nullptr);
 }
 
-void AssistantStateProxy::Init(mojom::ClientProxy* client) {
+void AssistantStateProxy::Init(mojom::ClientProxy* client,
+                               PrefService* profile_prefs) {
   // Bind to AssistantStateController.
   mojo::PendingRemote<ash::mojom::AssistantStateController> remote_controller;
   client->RequestAssistantStateController(
@@ -35,21 +35,8 @@ void AssistantStateProxy::Init(mojom::ClientProxy* client) {
       observer.InitWithNewPipeAndPassReceiver());
   assistant_state_controller_->AddMojomObserver(std::move(observer));
 
-  // Connect to pref service.
-  auto pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
-  prefs::RegisterProfilePrefsForeign(pref_registry.get());
-  mojo::PendingRemote<::prefs::mojom::PrefStoreConnector> remote_connector;
-  client->RequestPrefStoreConnector(
-      remote_connector.InitWithNewPipeAndPassReceiver());
-  pref_connection_delegate_->ConnectToPrefService(
-      std::move(remote_connector), std::move(pref_registry),
-      base::Bind(&AssistantStateProxy::OnPrefServiceConnected,
-                 base::Unretained(this)));
-}
-
-void AssistantStateProxy::SetPrefConnectionDelegateForTesting(
-    std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate) {
-  pref_connection_delegate_ = std::move(pref_connection_delegate);
+  pref_service_ = profile_prefs;
+  RegisterPrefChanges(pref_service_);
 }
 
 void AssistantStateProxy::OnAssistantStatusChanged(
@@ -74,15 +61,5 @@ void AssistantStateProxy::OnLockedFullScreenStateChanged(bool enabled) {
   UpdateLockedFullScreenState(enabled);
 }
 
-void AssistantStateProxy::OnPrefServiceConnected(
-    std::unique_ptr<::PrefService> pref_service) {
-  // TODO(b/110211045): Add testing support for Assistant prefs.
-  if (!pref_service)
-    return;
-
-  pref_service_ = std::move(pref_service);
-  RegisterPrefChanges(pref_service_.get());
-}
-
 }  // namespace assistant
 }  // namespace chromeos

chromeos/services/assistant/assistant_state_proxy.h

@@ -6,18 +6,16 @@
 #define CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_STATE_PROXY_H_
 
 #include <string>
-#include <vector>
 
 #include "ash/public/cpp/assistant/assistant_state_base.h"
 #include "ash/public/mojom/assistant_state_controller.mojom.h"
-#include "base/callback.h"
 #include "base/macros.h"
-#include "base/observer_list.h"
-#include "chromeos/services/assistant/pref_connection_delegate.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 
+class PrefService;
+
 namespace chromeos {
 namespace assistant {
 
@@ -25,7 +23,6 @@ namespace assistant {
 // information can be accessed through direct accessors which returns
 // |base::Optional<>| or observers. When adding an observer, all change events
 // will fire if this client already have data.
-// Also connect to pref service to register for preference changes.
 class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantStateProxy
     : public ash::AssistantStateBase,
       public ash::mojom::AssistantStateObserver {
@@ -33,10 +30,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantStateProxy
   AssistantStateProxy();
   ~AssistantStateProxy() override;
 
-  void Init(mojom::ClientProxy* client);
-
-  void SetPrefConnectionDelegateForTesting(
-      std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);
+  void Init(mojom::ClientProxy* client, PrefService* profile_prefs);
 
  private:
   // AssistantStateObserver:
@@ -47,16 +41,12 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantStateProxy
   void OnArcPlayStoreEnabledChanged(bool enabled) override;
   void OnLockedFullScreenStateChanged(bool enabled) override;
 
-  void OnPrefServiceConnected(std::unique_ptr<::PrefService> pref_service);
-
   mojo::Remote<ash::mojom::AssistantStateController>
       assistant_state_controller_;
   mojo::Receiver<ash::mojom::AssistantStateObserver>
       assistant_state_observer_receiver_{this};
 
-  std::unique_ptr<PrefService> pref_service_;
-
-  std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate_;
+  PrefService* pref_service_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(AssistantStateProxy);
 };

chromeos/services/assistant/pref_connection_delegate.cc

-// Copyright 2019 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 "chromeos/services/assistant/pref_connection_delegate.h"
-#include "base/token.h"
-#include "services/preferences/public/cpp/pref_service_factory.h"
-
-#include <utility>
-
-namespace chromeos {
-namespace assistant {
-
-void PrefConnectionDelegate::ConnectToPrefService(
-    mojo::PendingRemote<prefs::mojom::PrefStoreConnector> connector,
-    scoped_refptr<PrefRegistrySimple> pref_registry,
-    prefs::ConnectCallback callback) {
-  ::prefs::ConnectToPrefService(std::move(connector), std::move(pref_registry),
-                                base::Token::CreateRandom(), callback);
-}
-
-}  // namespace assistant
-}  // namespace chromeos

chromeos/services/assistant/pref_connection_delegate.h

-// Copyright 2019 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 CHROMEOS_SERVICES_ASSISTANT_PREF_CONNECTION_DELEGATE_H_
-#define CHROMEOS_SERVICES_ASSISTANT_PREF_CONNECTION_DELEGATE_H_
-
-#include "base/macros.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "mojo/public/cpp/bindings/pending_remote.h"
-#include "services/preferences/public/cpp/pref_service_factory.h"
-#include "services/preferences/public/mojom/preferences.mojom.h"
-
-namespace chromeos {
-namespace assistant {
-
-// Helper interface class to allow overriding pref services in tests.
-class COMPONENT_EXPORT(ASSISTANT_SERVICE) PrefConnectionDelegate {
- public:
-  PrefConnectionDelegate() = default;
-  virtual ~PrefConnectionDelegate() = default;
-
-  virtual void ConnectToPrefService(
-      mojo::PendingRemote<::prefs::mojom::PrefStoreConnector> connector,
-      scoped_refptr<PrefRegistrySimple> pref_registry,
-      ::prefs::ConnectCallback callback);
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(PrefConnectionDelegate);
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_PREF_CONNECTION_DELEGATE_H_

chromeos/services/assistant/public/cpp/assistant_prefs.cc

@@ -4,8 +4,6 @@
 
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
 
-#include <string>
-
 #include "components/prefs/pref_registry_simple.h"
 
 namespace chromeos {
@@ -50,39 +48,16 @@ const char kAssistantLaunchWithMicOpen[] =
 const char kAssistantNotificationEnabled[] =
     "settings.voice_interaction.notification.enabled";
 
-void RegisterProfilePrefsForBrowser(PrefRegistrySimple* registry) {
+void RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(kAssistantConsentStatus,
-                                ConsentStatus::kUnknown, PrefRegistry::PUBLIC);
-  registry->RegisterBooleanPref(kAssistantContextEnabled, false,
-                                PrefRegistry::PUBLIC);
-  registry->RegisterBooleanPref(kAssistantDisabledByPolicy, false,
-                                PrefRegistry::PUBLIC);
-  registry->RegisterBooleanPref(kAssistantEnabled, false, PrefRegistry::PUBLIC);
-  registry->RegisterBooleanPref(kAssistantHotwordAlwaysOn, false,
-                                PrefRegistry::PUBLIC);
-  registry->RegisterBooleanPref(kAssistantHotwordEnabled, false,
-                                PrefRegistry::PUBLIC);
-  registry->RegisterBooleanPref(kAssistantLaunchWithMicOpen, false,
-                                PrefRegistry::PUBLIC);
-  registry->RegisterBooleanPref(kAssistantNotificationEnabled, true,
-                                PrefRegistry::PUBLIC);
-}
-
-void RegisterProfilePrefsForeign(PrefRegistrySimple* registry, bool for_test) {
-  if (for_test) {
-    // In tests there are no remote pref service. Register the prefs as own if
-    // necessary.
-    RegisterProfilePrefsForBrowser(registry);
-    return;
-  }
-  registry->RegisterForeignPref(kAssistantConsentStatus);
-  registry->RegisterForeignPref(kAssistantContextEnabled);
-  registry->RegisterForeignPref(kAssistantDisabledByPolicy);
-  registry->RegisterForeignPref(kAssistantEnabled);
-  registry->RegisterForeignPref(kAssistantHotwordAlwaysOn);
-  registry->RegisterForeignPref(kAssistantHotwordEnabled);
-  registry->RegisterForeignPref(kAssistantLaunchWithMicOpen);
-  registry->RegisterForeignPref(kAssistantNotificationEnabled);
+                                ConsentStatus::kUnknown);
+  registry->RegisterBooleanPref(kAssistantContextEnabled, false);
+  registry->RegisterBooleanPref(kAssistantDisabledByPolicy, false);
+  registry->RegisterBooleanPref(kAssistantEnabled, false);
+  registry->RegisterBooleanPref(kAssistantHotwordAlwaysOn, false);
+  registry->RegisterBooleanPref(kAssistantHotwordEnabled, false);
+  registry->RegisterBooleanPref(kAssistantLaunchWithMicOpen, false);
+  registry->RegisterBooleanPref(kAssistantNotificationEnabled, true);
 }
 
 }  // namespace prefs

chromeos/services/assistant/public/cpp/assistant_prefs.h

@@ -5,8 +5,6 @@
 #ifndef CHROMEOS_SERVICES_ASSISTANT_PUBLIC_CPP_ASSISTANT_PREFS_H_
 #define CHROMEOS_SERVICES_ASSISTANT_PUBLIC_CPP_ASSISTANT_PREFS_H_
 
-#include "components/prefs/pref_service.h"
-
 class PrefRegistrySimple;
 
 namespace chromeos {
@@ -40,11 +38,7 @@ extern const char kAssistantLaunchWithMicOpen[];
 extern const char kAssistantNotificationEnabled[];
 
 // Registers Assistant specific profile preferences for browser prefs.
-void RegisterProfilePrefsForBrowser(PrefRegistrySimple* registry);
-
-// Registers Assistant specific profile preferences from foreign pref services.
-void RegisterProfilePrefsForeign(PrefRegistrySimple* registry,
-                                 bool for_test = false);
+void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
 }  // namespace prefs
 }  // namespace assistant

chromeos/services/assistant/service.cc

@@ -31,12 +31,10 @@
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
 #include "chromeos/services/assistant/public/features.h"
 #include "chromeos/services/assistant/service_context.h"
-#include "components/prefs/pref_service.h"
 #include "components/user_manager/known_user.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "services/identity/public/cpp/scope_set.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
-#include "services/preferences/public/mojom/preferences.mojom.h"
 
 #if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
 #include "chromeos/assistant/internal/internal_constants.h"
@@ -138,14 +136,17 @@ class Service::Context : public ServiceContext {
 
 Service::Service(mojo::PendingReceiver<mojom::AssistantService> receiver,
                  std::unique_ptr<network::SharedURLLoaderFactoryInfo>
-                     url_loader_factory_info)
+                     url_loader_factory_info,
+                 PrefService* profile_prefs)
     : receiver_(this, std::move(receiver)),
       token_refresh_timer_(std::make_unique<base::OneShotTimer>()),
       main_task_runner_(base::SequencedTaskRunnerHandle::Get()),
       context_(std::make_unique<Context>(this)),
-      url_loader_factory_info_(std::move(url_loader_factory_info)) {
-  // TODO(xiaohuic): in MASH we will need to setup the dbus client if assistant
-  // service runs in its own process.
+      url_loader_factory_info_(std::move(url_loader_factory_info)),
+      profile_prefs_(profile_prefs) {
+  DCHECK(profile_prefs_);
+  // TODO(xiaohuic): We will need to setup the power manager dbus client if
+  // assistant service runs in its own process.
   chromeos::PowerManagerClient* power_manager_client =
       context_->power_manager_client();
   power_manager_observer_.Add(power_manager_client);
@@ -177,10 +178,6 @@ void Service::SetTimerForTesting(std::unique_ptr<base::OneShotTimer> timer) {
   token_refresh_timer_ = std::move(timer);
 }
 
-AssistantStateProxy* Service::GetAssistantStateProxyForTesting() {
-  return &assistant_state_;
-}
-
 void Service::Init(mojo::PendingRemote<mojom::Client> client,
                    mojo::PendingRemote<mojom::DeviceActions> device_actions,
                    bool is_test) {
@@ -189,7 +186,7 @@ void Service::Init(mojo::PendingRemote<mojom::Client> client,
   client_.Bind(std::move(client));
   device_actions_.Bind(std::move(device_actions));
 
-  assistant_state_.Init(client_.get());
+  assistant_state_.Init(client_.get(), profile_prefs_);
   assistant_state_.AddObserver(this);
 
   DCHECK(!assistant_manager_service_);

chromeos/services/assistant/service.h

@@ -25,7 +25,6 @@
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "chromeos/services/assistant/public/mojom/settings.mojom.h"
 #include "components/account_id/account_id.h"
-#include "components/prefs/pref_registry_simple.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -34,6 +33,7 @@
 #include "services/identity/public/mojom/identity_accessor.mojom.h"
 
 class GoogleServiceAuthError;
+class PrefService;
 
 namespace base {
 class OneShotTimer;
@@ -51,7 +51,6 @@ namespace chromeos {
 namespace assistant {
 
 class AssistantSettingsManager;
-class PrefConnectionDelegate;
 class ServiceContext;
 
 // |AssistantManagerService|'s state won't update if it's currently in the
@@ -69,7 +68,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
  public:
   Service(mojo::PendingReceiver<mojom::AssistantService> receiver,
           std::unique_ptr<network::SharedURLLoaderFactoryInfo>
-              url_loader_factory_info);
+              url_loader_factory_info,
+          PrefService* profile_prefs);
   ~Service() override;
 
   // Allows tests to override the AssistantSettingsManager bound by the service.
@@ -81,11 +81,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
 
   void SetTimerForTesting(std::unique_ptr<base::OneShotTimer> timer);
 
-  void SetPrefConnectionDelegateForTesting(
-      std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);
-
-  AssistantStateProxy* GetAssistantStateProxyForTesting();
-
  private:
   friend class AssistantServiceTest;
 
@@ -208,6 +203,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
   // non-null until |assistant_manager_service_| is created.
   std::unique_ptr<network::SharedURLLoaderFactoryInfo> url_loader_factory_info_;
 
+  // User profile preferences.
+  PrefService* const profile_prefs_;
+
   base::CancelableOnceClosure update_assistant_manager_callback_;
 
   SEQUENCE_CHECKER(sequence_checker_);

chromeos/services/assistant/service_unittest.cc

@@ -9,7 +9,6 @@
 #include <vector>
 
 #include "ash/public/mojom/assistant_state_controller.mojom.h"
-#include "base/bind.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
@@ -23,7 +22,6 @@
 #include "chromeos/dbus/power/fake_power_manager_client.h"
 #include "chromeos/services/assistant/assistant_state_proxy.h"
 #include "chromeos/services/assistant/fake_assistant_manager_service_impl.h"
-#include "chromeos/services/assistant/pref_connection_delegate.h"
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
 #include "chromeos/services/assistant/test_support/fake_client.h"
 #include "chromeos/services/assistant/test_support/fully_initialized_assistant_state.h"
@@ -43,30 +41,6 @@ constexpr base::TimeDelta kDefaultTokenExpirationDelay =
     base::TimeDelta::FromMilliseconds(1000);
 }  // namespace
 
-class FakePrefConnectionDelegate : public PrefConnectionDelegate {
- public:
-  FakePrefConnectionDelegate()
-      : pref_service_(std::make_unique<TestingPrefServiceSimple>()) {
-    prefs::RegisterProfilePrefsForBrowser(pref_service_->registry());
-  }
-  ~FakePrefConnectionDelegate() override = default;
-
-  // PrefConnectionDelegate overrides:
-  void ConnectToPrefService(
-      mojo::PendingRemote<::prefs::mojom::PrefStoreConnector> connector,
-      scoped_refptr<PrefRegistrySimple> pref_registry,
-      ::prefs::ConnectCallback callback) override {
-    callback.Run(std::move(pref_service_));
-  }
-
-  TestingPrefServiceSimple* pref_service() { return pref_service_.get(); }
-
- private:
-  std::unique_ptr<TestingPrefServiceSimple> pref_service_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakePrefConnectionDelegate);
-};
-
 class FakeIdentityAccessor : identity::mojom::IdentityAccessor {
  public:
   FakeIdentityAccessor()
@@ -203,18 +177,13 @@ class AssistantServiceTest : public testing::Test {
         base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
             &url_loader_factory_);
 
-    auto fake_pref_connection = std::make_unique<FakePrefConnectionDelegate>();
-    pref_service_ = fake_pref_connection->pref_service();
-
-    pref_service()->SetBoolean(prefs::kAssistantEnabled, true);
-    pref_service()->SetBoolean(prefs::kAssistantHotwordEnabled, true);
+    prefs::RegisterProfilePrefs(pref_service_.registry());
+    pref_service_.SetBoolean(prefs::kAssistantEnabled, true);
+    pref_service_.SetBoolean(prefs::kAssistantHotwordEnabled, true);
 
-    fake_pref_connection_ = fake_pref_connection.get();
-    service_ =
-        std::make_unique<Service>(remote_service_.BindNewPipeAndPassReceiver(),
-                                  shared_url_loader_factory_->Clone());
-    service_->GetAssistantStateProxyForTesting()
-        ->SetPrefConnectionDelegateForTesting(std::move(fake_pref_connection));
+    service_ = std::make_unique<Service>(
+        remote_service_.BindNewPipeAndPassReceiver(),
+        shared_url_loader_factory_->Clone(), &pref_service_);
     service_->is_test_ = true;
 
     mock_task_runner_ = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
@@ -252,7 +221,7 @@ class AssistantServiceTest : public testing::Test {
 
   ash::AssistantState* assistant_state() { return &assistant_state_; }
 
-  PrefService* pref_service() { return pref_service_; }
+  PrefService* pref_service() { return &pref_service_; }
 
   FakeAssistantClient* client() { return &fake_assistant_client_; }
 
@@ -272,8 +241,7 @@ class AssistantServiceTest : public testing::Test {
   FakeAssistantClient fake_assistant_client_{&assistant_state_};
   FakeDeviceActions fake_device_actions_;
 
-  FakePrefConnectionDelegate* fake_pref_connection_;
-  PrefService* pref_service_;
+  TestingPrefServiceSimple pref_service_;
 
   network::TestURLLoaderFactory url_loader_factory_;
   scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;