Move AssistantStateProxy into Services

The AssistantStateProxy is only used by Assistant service now, move it
from Ash to Service and move pref related logics into the Proxy.

Bug: b/138679823
Test: Run existing tests
Change-Id: Ife1ded72321c5d0357870a57ffcd15d94b930655
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1774833Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Yue Li <updowndota@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691782}

ash/public/cpp/BUILD.gn

@@ -70,8 +70,6 @@ component("cpp") {
     "assistant/assistant_state.h",
     "assistant/assistant_state_base.cc",
     "assistant/assistant_state_base.h",
-    "assistant/assistant_state_proxy.cc",
-    "assistant/assistant_state_proxy.h",
     "assistant/proactive_suggestions.cc",
     "assistant/proactive_suggestions.h",
     "assistant/proactive_suggestions_client.cc",

ash/public/cpp/assistant/assistant_state.cc

@@ -38,10 +38,7 @@ void AssistantState::NotifyStatusChanged(mojom::VoiceInteractionState state) {
   if (voice_interaction_state_ == state)
     return;
 
-  voice_interaction_state_ = state;
-
-  for (auto& observer : observers_)
-    observer.OnAssistantStatusChanged(state);
+  UpdateAssistantStatus(state);
   remote_observers_.ForAllPtrs(
       [state](auto* observer) { observer->OnAssistantStatusChanged(state); });
 }
@@ -50,9 +47,7 @@ void AssistantState::NotifySettingsEnabled(bool enabled) {
   if (settings_enabled_.has_value() && settings_enabled_.value() == enabled)
     return;
 
-  settings_enabled_ = enabled;
-  for (auto& observer : observers_)
-    observer.OnAssistantSettingsEnabled(enabled);
+  UpdateSettingsEnabled(enabled);
   remote_observers_.ForAllPtrs([enabled](auto* observer) {
     observer->OnAssistantSettingsEnabled(enabled);
   });
@@ -62,9 +57,7 @@ void AssistantState::NotifyHotwordEnabled(bool enabled) {
   if (hotword_enabled_.has_value() && hotword_enabled_.value() == enabled)
     return;
 
-  hotword_enabled_ = enabled;
-  for (auto& observer : observers_)
-    observer.OnAssistantHotwordEnabled(enabled);
+  UpdateHotwordEnabled(enabled);
   remote_observers_.ForAllPtrs([enabled](auto* observer) {
     observer->OnAssistantHotwordEnabled(enabled);
   });
@@ -74,9 +67,7 @@ void AssistantState::NotifyFeatureAllowed(mojom::AssistantAllowedState state) {
   if (allowed_state_ == state)
     return;
 
-  allowed_state_ = state;
-  for (auto& observer : observers_)
-    observer.OnAssistantFeatureAllowedChanged(state);
+  UpdateFeatureAllowedState(state);
   remote_observers_.ForAllPtrs([state](auto* observer) {
     observer->OnAssistantFeatureAllowedChanged(state);
   });
@@ -86,9 +77,7 @@ void AssistantState::NotifyLocaleChanged(const std::string& locale) {
   if (locale_ == locale)
     return;
 
-  locale_ = locale;
-  for (auto& observer : observers_)
-    observer.OnLocaleChanged(locale);
+  UpdateLocale(locale);
   remote_observers_.ForAllPtrs(
       [locale](auto* observer) { observer->OnLocaleChanged(locale); });
 }
@@ -97,10 +86,7 @@ void AssistantState::NotifyArcPlayStoreEnabledChanged(bool enabled) {
   if (arc_play_store_enabled_ == enabled)
     return;
 
-  arc_play_store_enabled_ = enabled;
-
-  for (auto& observer : observers_)
-    observer.OnArcPlayStoreEnabledChanged(enabled);
+  UpdateArcPlayStoreEnabled(enabled);
   remote_observers_.ForAllPtrs([enabled](auto* observer) {
     observer->OnArcPlayStoreEnabledChanged(enabled);
   });
@@ -110,10 +96,7 @@ void AssistantState::NotifyLockedFullScreenStateChanged(bool enabled) {
   if (locked_full_screen_enabled_ == enabled)
     return;
 
-  locked_full_screen_enabled_ = enabled;
-
-  for (auto& observer : observers_)
-    observer.OnLockedFullScreenStateChanged(enabled);
+  UpdateLockedFullScreenState(enabled);
   remote_observers_.ForAllPtrs([enabled](auto* observer) {
     observer->OnLockedFullScreenStateChanged(enabled);
   });

ash/public/cpp/assistant/assistant_state_base.cc

@@ -181,4 +181,50 @@ void AssistantStateBase::UpdateNotificationEnabled() {
     observer.OnAssistantNotificationEnabled(notification_enabled_.value());
 }
 
+void AssistantStateBase::UpdateAssistantStatus(
+    mojom::VoiceInteractionState state) {
+  voice_interaction_state_ = state;
+  for (auto& observer : observers_)
+    observer.OnAssistantStatusChanged(voice_interaction_state_);
+}
+
+void AssistantStateBase::UpdateSettingsEnabled(bool enabled) {
+  settings_enabled_ = enabled;
+  for (auto& observer : observers_)
+    observer.OnAssistantSettingsEnabled(settings_enabled_.value());
+}
+
+void AssistantStateBase::UpdateHotwordEnabled(bool enabled) {
+  hotword_enabled_ = enabled;
+  for (auto& observer : observers_)
+    observer.OnAssistantHotwordEnabled(hotword_enabled_.value());
+}
+
+void AssistantStateBase::UpdateFeatureAllowedState(
+    mojom::AssistantAllowedState state) {
+  allowed_state_ = state;
+  for (auto& observer : observers_)
+    observer.OnAssistantFeatureAllowedChanged(allowed_state_.value());
+}
+
+void AssistantStateBase::UpdateLocale(const std::string& locale) {
+  locale_ = locale;
+  for (auto& observer : observers_)
+    observer.OnLocaleChanged(locale_.value());
+}
+
+void AssistantStateBase::UpdateArcPlayStoreEnabled(bool enabled) {
+  arc_play_store_enabled_ = enabled;
+  for (auto& observer : observers_)
+    observer.OnArcPlayStoreEnabledChanged(arc_play_store_enabled_.value());
+}
+
+void AssistantStateBase::UpdateLockedFullScreenState(bool enabled) {
+  locked_full_screen_enabled_ = enabled;
+  for (auto& observer : observers_) {
+    observer.OnLockedFullScreenStateChanged(
+        locked_full_screen_enabled_.value());
+  }
+}
+
 }  // namespace ash

ash/public/cpp/assistant/assistant_state_base.h

@@ -119,6 +119,15 @@ class ASH_PUBLIC_EXPORT AssistantStateBase {
   void UpdateLaunchWithMicOpen();
   void UpdateNotificationEnabled();
 
+  // Called when new values of the listened states are received.
+  void UpdateAssistantStatus(mojom::VoiceInteractionState state);
+  void UpdateSettingsEnabled(bool enabled);
+  void UpdateHotwordEnabled(bool enabled);
+  void UpdateFeatureAllowedState(mojom::AssistantAllowedState state);
+  void UpdateLocale(const std::string& locale);
+  void UpdateArcPlayStoreEnabled(bool enabled);
+  void UpdateLockedFullScreenState(bool enabled);
+
   mojom::VoiceInteractionState voice_interaction_state_ =
       mojom::VoiceInteractionState::NOT_READY;
 

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

@@ -99,7 +99,7 @@ void AssistantClient::MaybeInit(Profile* profile) {
 
   if (chromeos::assistant::features::IsProactiveSuggestionsEnabled()) {
     proactive_suggestions_client_ =
-        std::make_unique<ProactiveSuggestionsClientImpl>(this, profile_);
+        std::make_unique<ProactiveSuggestionsClientImpl>(profile_);
   }
 
   for (auto& receiver : pending_assistant_receivers_)

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

@@ -6,25 +6,16 @@
 
 #include "ash/public/cpp/assistant/proactive_suggestions.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/ash/assistant/assistant_client.h"
 #include "chrome/browser/ui/ash/assistant/proactive_suggestions_loader.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
-#include "services/service_manager/public/cpp/connector.h"
 
 ProactiveSuggestionsClientImpl::ProactiveSuggestionsClientImpl(
-    AssistantClient* client,
     Profile* profile)
     : profile_(profile) {
-  // Initialize the Assistant state proxy.
-  mojo::PendingRemote<ash::mojom::AssistantStateController> controller;
-  client->RequestAssistantStateController(
-      controller.InitWithNewPipeAndPassReceiver());
-  assistant_state_.Init(std::move(controller));
-
   // We observe Assistant state to detect enabling/disabling of Assistant in
   // settings as well as enabling/disabling of screen context.
-  assistant_state_.AddObserver(this);
+  ash::AssistantState::Get()->AddObserver(this);
 
   // We observe the singleton BrowserList to receive events pertaining to the
   // currently active browser.
@@ -41,7 +32,7 @@ ProactiveSuggestionsClientImpl::~ProactiveSuggestionsClientImpl() {
     active_browser_->tab_strip_model()->RemoveObserver(this);
 
   BrowserList::RemoveObserver(this);
-  assistant_state_.RemoveObserver(this);
+  ash::AssistantState::Get()->RemoveObserver(this);
 }
 
 void ProactiveSuggestionsClientImpl::SetDelegate(Delegate* delegate) {
@@ -172,8 +163,8 @@ void ProactiveSuggestionsClientImpl::UpdateActiveState() {
   // We never observe browsers that are off the record and we never observe
   // browsers when the user has disabled either Assistant or screen context.
   if (active_browser_->profile()->IsOffTheRecord() ||
-      !assistant_state_.settings_enabled().value_or(false) ||
-      !assistant_state_.context_enabled().value_or(false)) {
+      !ash::AssistantState::Get()->settings_enabled().value_or(false) ||
+      !ash::AssistantState::Get()->context_enabled().value_or(false)) {
     tab_strip_model->RemoveObserver(this);
     SetActiveContents(nullptr);
     return;

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

@@ -7,7 +7,7 @@
 
 #include <memory>
 
-#include "ash/public/cpp/assistant/assistant_state_proxy.h"
+#include "ash/public/cpp/assistant/assistant_state.h"
 #include "ash/public/cpp/assistant/proactive_suggestions_client.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
@@ -15,7 +15,6 @@
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "content/public/browser/web_contents_observer.h"
 
-class AssistantClient;
 class ProactiveSuggestionsLoader;
 class Profile;
 
@@ -28,7 +27,7 @@ class ProactiveSuggestionsClientImpl : public ash::ProactiveSuggestionsClient,
                                        public content::WebContentsObserver,
                                        public ash::AssistantStateObserver {
  public:
-  ProactiveSuggestionsClientImpl(AssistantClient* client, Profile* profile);
+  explicit ProactiveSuggestionsClientImpl(Profile* profile);
   ~ProactiveSuggestionsClientImpl() override;
 
   // ash::ProactiveSuggestionsClient:
@@ -63,7 +62,6 @@ class ProactiveSuggestionsClientImpl : public ash::ProactiveSuggestionsClient,
   void UpdateActiveState();
 
   Profile* const profile_;
-  ash::AssistantStateProxy assistant_state_;
 
   Delegate* delegate_ = nullptr;
 

chromeos/services/assistant/BUILD.gn

@@ -20,6 +20,8 @@ component("lib") {
   sources = [
     "assistant_manager_service.h",
     "assistant_settings_manager.h",
+    "assistant_state_proxy.cc",
+    "assistant_state_proxy.h",
     "constants.cc",
     "constants.h",
     "fake_assistant_manager_service_impl.cc",

ash/public/cpp/assistant/assistant_state_proxy.cc → chromeos/services/assistant/assistant_state_proxy.cc

-// Copyright 2018 The Chromium Authors. All rights reserved.
+// 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/assistant_state_proxy.h"
+
 #include <algorithm>
 #include <utility>
 
-#include "ash/public/cpp/assistant/assistant_state_proxy.h"
-#include "ash/public/mojom/constants.mojom.h"
 #include "services/service_manager/public/cpp/connector.h"
 
-namespace ash {
+namespace chromeos {
+namespace assistant {
 
 AssistantStateProxy::AssistantStateProxy()
-    : assistant_state_observer_binding_(this) {}
+    : assistant_state_observer_binding_(this),
+      pref_connection_delegate_(std::make_unique<PrefConnectionDelegate>()) {}
 
-AssistantStateProxy::~AssistantStateProxy() = default;
+AssistantStateProxy::~AssistantStateProxy() {
+  // Reset pref change registar.
+  RegisterPrefChanges(nullptr);
+}
 
-void AssistantStateProxy::Init(
-    mojo::PendingRemote<mojom::AssistantStateController>
-        assistant_state_controller) {
-  assistant_state_controller_.Bind(std::move(assistant_state_controller));
+void AssistantStateProxy::Init(mojom::ClientProxy* client) {
+  // Bind to AssistantStateController.
+  mojo::PendingRemote<ash::mojom::AssistantStateController> remote_controller;
+  client->RequestAssistantStateController(
+      remote_controller.InitWithNewPipeAndPassReceiver());
+  assistant_state_controller_.Bind(std::move(remote_controller));
 
-  mojom::AssistantStateObserverPtr ptr;
+  ash::mojom::AssistantStateObserverPtr ptr;
   assistant_state_observer_binding_.Bind(mojo::MakeRequest(&ptr));
   assistant_state_controller_->AddMojomObserver(std::move(ptr));
+
+  // 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);
 }
 
 void AssistantStateProxy::OnAssistantStatusChanged(
     ash::mojom::VoiceInteractionState state) {
-  voice_interaction_state_ = state;
-  for (auto& observer : observers_)
-    observer.OnAssistantStatusChanged(voice_interaction_state_);
+  UpdateAssistantStatus(state);
 }
 
 void AssistantStateProxy::OnAssistantSettingsEnabled(bool enabled) {
-  settings_enabled_ = enabled;
-  for (auto& observer : observers_)
-    observer.OnAssistantSettingsEnabled(settings_enabled_.value());
+  UpdateSettingsEnabled(enabled);
 }
 
 void AssistantStateProxy::OnAssistantHotwordEnabled(bool enabled) {
-  hotword_enabled_ = enabled;
-  for (auto& observer : observers_)
-    observer.OnAssistantHotwordEnabled(hotword_enabled_.value());
+  UpdateHotwordEnabled(enabled);
 }
 
 void AssistantStateProxy::OnAssistantFeatureAllowedChanged(
     ash::mojom::AssistantAllowedState state) {
-  allowed_state_ = state;
-  for (auto& observer : observers_)
-    observer.OnAssistantFeatureAllowedChanged(allowed_state_.value());
+  UpdateFeatureAllowedState(state);
 }
 
 void AssistantStateProxy::OnLocaleChanged(const std::string& locale) {
-  locale_ = locale;
-  for (auto& observer : observers_)
-    observer.OnLocaleChanged(locale_.value());
+  UpdateLocale(locale);
 }
 
 void AssistantStateProxy::OnArcPlayStoreEnabledChanged(bool enabled) {
-  arc_play_store_enabled_ = enabled;
-  for (auto& observer : observers_)
-    observer.OnArcPlayStoreEnabledChanged(arc_play_store_enabled_.value());
+  UpdateArcPlayStoreEnabled(enabled);
 }
 
 void AssistantStateProxy::OnLockedFullScreenStateChanged(bool enabled) {
-  locked_full_screen_enabled_ = enabled;
-  for (auto& observer : observers_) {
-    observer.OnLockedFullScreenStateChanged(
-        locked_full_screen_enabled_.value());
-  }
+  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 ash
+}  // namespace assistant
+}  // namespace chromeos

ash/public/cpp/assistant/assistant_state_proxy.h → chromeos/services/assistant/assistant_state_proxy.h

-// Copyright 2018 The Chromium Authors. All rights reserved.
+// 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 ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_STATE_PROXY_H_
-#define ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_STATE_PROXY_H_
+#ifndef CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_STATE_PROXY_H_
+#define CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_STATE_PROXY_H_
 
 #include <string>
 #include <vector>
@@ -14,43 +14,57 @@
 #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/binding.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 
-namespace ash {
+namespace chromeos {
+namespace assistant {
 
 // Provides a convenient client to access various Assistant states. The state
 // 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.
-class ASH_PUBLIC_EXPORT AssistantStateProxy
-    : public AssistantStateBase,
-      public mojom::AssistantStateObserver {
+// Also connect to pref service to register for preference changes.
+class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantStateProxy
+    : public ash::AssistantStateBase,
+      public ash::mojom::AssistantStateObserver {
  public:
   AssistantStateProxy();
   ~AssistantStateProxy() override;
 
-  void Init(mojo::PendingRemote<mojom::AssistantStateController>
-                assistant_state_controller);
+  void Init(mojom::ClientProxy* client);
+
+  void SetPrefConnectionDelegateForTesting(
+      std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);
 
  private:
-  // mojom::AssistantStateObserver:
-  void OnAssistantStatusChanged(mojom::VoiceInteractionState state) override;
+  // AssistantStateObserver:
+  void OnAssistantStatusChanged(
+      ash::mojom::VoiceInteractionState state) override;
   void OnAssistantSettingsEnabled(bool enabled) override;
   void OnAssistantHotwordEnabled(bool enabled) override;
   void OnAssistantFeatureAllowedChanged(
-      mojom::AssistantAllowedState state) override;
+      ash::mojom::AssistantAllowedState state) override;
   void OnLocaleChanged(const std::string& locale) override;
   void OnArcPlayStoreEnabledChanged(bool enabled) override;
   void OnLockedFullScreenStateChanged(bool enabled) override;
 
-  mojom::AssistantStateControllerPtr assistant_state_controller_;
-  mojo::Binding<mojom::AssistantStateObserver>
+  void OnPrefServiceConnected(std::unique_ptr<::PrefService> pref_service);
+
+  ash::mojom::AssistantStateControllerPtr assistant_state_controller_;
+  mojo::Binding<ash::mojom::AssistantStateObserver>
       assistant_state_observer_binding_;
 
+  std::unique_ptr<PrefService> pref_service_;
+
+  std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate_;
+
   DISALLOW_COPY_AND_ASSIGN(AssistantStateProxy);
 };
 
-}  // namespace ash
+}  // namespace assistant
+}  // namespace chromeos
 
-#endif  // ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_STATE_PROXY_H_
+#endif  // CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_STATE_PROXY_H_

chromeos/services/assistant/service.cc

@@ -27,7 +27,6 @@
 #include "chromeos/services/assistant/assistant_settings_manager.h"
 #include "chromeos/services/assistant/fake_assistant_manager_service_impl.h"
 #include "chromeos/services/assistant/fake_assistant_settings_manager_impl.h"
-#include "chromeos/services/assistant/pref_connection_delegate.h"
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
 #include "chromeos/services/assistant/public/features.h"
 #include "components/prefs/pref_service.h"
@@ -72,8 +71,7 @@ Service::Service(mojo::PendingReceiver<mojom::AssistantService> receiver,
     : receiver_(this, std::move(receiver)),
       token_refresh_timer_(std::make_unique<base::OneShotTimer>()),
       main_task_runner_(base::SequencedTaskRunnerHandle::Get()),
-      url_loader_factory_info_(std::move(url_loader_factory_info)),
-      pref_connection_delegate_(std::make_unique<PrefConnectionDelegate>()) {
+      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.
   chromeos::PowerManagerClient* power_manager_client =
@@ -134,9 +132,8 @@ void Service::SetTimerForTesting(std::unique_ptr<base::OneShotTimer> timer) {
   token_refresh_timer_ = std::move(timer);
 }
 
-void Service::SetPrefConnectionDelegateForTesting(
-    std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate) {
-  pref_connection_delegate_ = std::move(pref_connection_delegate);
+AssistantStateProxy* Service::GetAssistantStateProxyForTesting() {
+  return &assistant_state_;
 }
 
 void Service::Init(mojom::ClientPtr client,
@@ -147,22 +144,9 @@ void Service::Init(mojom::ClientPtr client,
   client_ = std::move(client);
   device_actions_ = std::move(device_actions);
 
-  // TODO(b/138679823): Moving the logics into AssistantStateProxy.
-  mojo::PendingRemote<ash::mojom::AssistantStateController> remote_controller;
-  client_->RequestAssistantStateController(
-      remote_controller.InitWithNewPipeAndPassReceiver());
-  assistant_state_.Init(std::move(remote_controller));
+  assistant_state_.Init(client_.get());
   assistant_state_.AddObserver(this);
 
-  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(&Service::OnPrefServiceConnected, base::Unretained(this)));
-
   DCHECK(!assistant_manager_service_);
 
   // Don't fetch token for test.
@@ -269,9 +253,6 @@ void Service::OnLockedFullScreenStateChanged(bool enabled) {
 void Service::UpdateAssistantManagerState() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (!pref_service_)
-    return;
-
   if (!assistant_state_.hotword_enabled().has_value() ||
       !assistant_state_.settings_enabled().has_value() ||
       !assistant_state_.locale().has_value() ||
@@ -326,18 +307,6 @@ void Service::UpdateAssistantManagerState() {
   }
 }
 
-void Service::OnPrefServiceConnected(
-    std::unique_ptr<::PrefService> pref_service) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  // TODO(b/110211045): Add testing support for Assistant prefs.
-  if (!pref_service)
-    return;
-
-  pref_service_ = std::move(pref_service);
-  assistant_state_.RegisterPrefChanges(pref_service_.get());
-}
-
 identity::mojom::IdentityAccessor* Service::GetIdentityAccessor() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!identity_accessor_)

chromeos/services/assistant/service.h

@@ -8,7 +8,6 @@
 #include <memory>
 #include <string>
 
-#include "ash/public/cpp/assistant/assistant_state_proxy.h"
 #include "ash/public/cpp/session/session_activation_observer.h"
 #include "ash/public/mojom/assistant_controller.mojom.h"
 #include "base/callback.h"
@@ -21,7 +20,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/time/time.h"
 #include "chromeos/dbus/power/power_manager_client.h"
-#include "chromeos/services/assistant/pref_connection_delegate.h"
+#include "chromeos/services/assistant/assistant_state_proxy.h"
 #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"
@@ -32,7 +31,6 @@
 #include "services/identity/public/mojom/identity_accessor.mojom.h"
 
 class GoogleServiceAuthError;
-class PrefService;
 
 namespace base {
 class OneShotTimer;
@@ -51,6 +49,7 @@ namespace assistant {
 
 class AssistantManagerService;
 class AssistantSettingsManager;
+class PrefConnectionDelegate;
 
 // |AssistantManagerService|'s state won't update if it's currently in the
 // process of starting up. This is the delay before we will try to update
@@ -118,6 +117,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
   void SetPrefConnectionDelegateForTesting(
       std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);
 
+  AssistantStateProxy* GetAssistantStateProxyForTesting();
+
  private:
   friend class AssistantServiceTest;
 
@@ -147,8 +148,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
 
   void UpdateAssistantManagerState();
 
-  void OnPrefServiceConnected(std::unique_ptr<::PrefService> pref_service);
-
   identity::mojom::IdentityAccessor* GetIdentityAccessor();
 
   void GetPrimaryAccountInfoCallback(
@@ -206,23 +205,17 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
 
   mojom::AssistantControllerPtr assistant_controller_;
 
-  // NOTE: |pref_service_| is used by |assistant_state_| and must be declared
-  // before so it will be destructed after.
-  std::unique_ptr<PrefService> pref_service_;
-
   ash::mojom::AssistantAlarmTimerControllerPtr
       assistant_alarm_timer_controller_;
   ash::mojom::AssistantNotificationControllerPtr
       assistant_notification_controller_;
   ash::mojom::AssistantScreenContextControllerPtr
       assistant_screen_context_controller_;
-  ash::AssistantStateProxy assistant_state_;
+  AssistantStateProxy assistant_state_;
 
   // non-null until |assistant_manager_service_| is created.
   std::unique_ptr<network::SharedURLLoaderFactoryInfo> url_loader_factory_info_;
 
-  std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate_;
-
   base::CancelableOnceClosure update_assistant_manager_callback_;
 
   SEQUENCE_CHECKER(sequence_checker_);

chromeos/services/assistant/service_unittest.cc

@@ -22,6 +22,7 @@
 #include "base/timer/timer.h"
 #include "chromeos/audio/cras_audio_handler.h"
 #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/fake_client.h"
 #include "chromeos/services/assistant/pref_connection_delegate.h"
@@ -215,8 +216,8 @@ class AssistantServiceTest : public testing::Test {
     service_ =
         std::make_unique<Service>(remote_service_.BindNewPipeAndPassReceiver(),
                                   shared_url_loader_factory_->Clone());
-    service_->SetPrefConnectionDelegateForTesting(
-        std::move(fake_pref_connection));
+    service_->GetAssistantStateProxyForTesting()
+        ->SetPrefConnectionDelegateForTesting(std::move(fake_pref_connection));
     service_->is_test_ = true;
 
     mock_task_runner_ = base::MakeRefCounted<base::TestMockTimeTaskRunner>(