assistant: refactor voice interaction controller

* Introduce assistant_state_proxy as a convenient mojom proxy client for
  voice_interaction_controller.
* Introduce assistant_state_base as POD that can be shared between
  assistant_state_proxy and voice_interaction_controller to store state
* Change assistant state accessers to base::Optional to reflect the async
  nature of reading these values.
* Remove async Is___Enabled(Callback). The observer now will initialize
  state after attach.
* Fixes assistant_manager_service_impl start up sequence to avoid potential
  multiple restarts with async assistant pref queries
* Fixes a potential race when restarting assistant_manager

Bug: b/112281490
Test: locally build and run
Change-Id: I7c57adef0919583283f2fb1312f6d6e191bf266a
Reviewed-on: https://chromium-review.googlesource.com/c/1303333
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604678}

ash/accelerators/accelerator_controller.cc

@@ -660,7 +660,9 @@ void HandleToggleVoiceInteraction(const ui::Accelerator& accelerator) {
         base::UserMetricsAction("VoiceInteraction.Started.Assistant"));
   }
 
-  switch (Shell::Get()->voice_interaction_controller()->allowed_state()) {
+  switch (
+      Shell::Get()->voice_interaction_controller()->allowed_state().value_or(
+          mojom::AssistantAllowedState::ALLOWED)) {
     case mojom::AssistantAllowedState::DISALLOWED_BY_NONPRIMARY_USER:
       // Show a toast if the active user is not primary.
       ShowToast(kVoiceInteractionErrorToastId,

ash/app_list/app_list_controller_impl.cc

@@ -875,7 +875,7 @@ void AppListControllerImpl::UpdateAssistantVisibility() {
 
   auto* controller = Shell::Get()->voice_interaction_controller();
   GetSearchModel()->search_box()->SetShowAssistantButton(
-      controller->settings_enabled() &&
+      controller->settings_enabled().value_or(false) &&
       controller->allowed_state() == mojom::AssistantAllowedState::ALLOWED);
 }
 

ash/assistant/assistant_cache_controller.cc

@@ -4,6 +4,7 @@
 
 #include "ash/assistant/assistant_cache_controller.h"
 
+#include <utility>
 #include <vector>
 
 #include "ash/assistant/assistant_controller.h"
@@ -116,7 +117,8 @@ void AssistantCacheController::UpdateConversationStarters() {
   if (!base::FeatureList::IsEnabled(kConversationStartersFeature))
     return;
 
-  using namespace chromeos::assistant::mojom;
+  using chromeos::assistant::mojom::AssistantSuggestion;
+  using chromeos::assistant::mojom::AssistantSuggestionPtr;
 
   std::vector<AssistantSuggestionPtr> conversation_starters;
 
@@ -134,7 +136,8 @@ void AssistantCacheController::UpdateConversationStarters() {
 
   // If enabled, always show the "What's on my screen?" conversation starter.
   if (kWhatsOnMyScreenChipEnabled.Get() &&
-      Shell::Get()->voice_interaction_controller()->context_enabled()) {
+      Shell::Get()->voice_interaction_controller()->context_enabled().value_or(
+          false)) {
     AddConversationStarter(IDS_ASH_ASSISTANT_CHIP_WHATS_ON_MY_SCREEN,
                            assistant::util::CreateWhatsOnMyScreenDeepLink());
   }

ash/assistant/assistant_ui_controller.cc

@@ -279,8 +279,12 @@ void AssistantUiController::OnUiVisibilityChanged(
 }
 
 void AssistantUiController::ShowUi(AssistantSource source) {
-  if (!Shell::Get()->voice_interaction_controller()->settings_enabled())
+  if (!Shell::Get()
+           ->voice_interaction_controller()
+           ->settings_enabled()
+           .value_or(false)) {
     return;
+  }
 
   // TODO(dmblack): Show a more helpful message to the user.
   if (Shell::Get()->voice_interaction_controller()->voice_interaction_state() ==

ash/assistant/ui/main_stage/assistant_footer_view.cc

@@ -76,7 +76,8 @@ void AssistantFooterView::InitLayout() {
 
   // Initial view state is based on user consent state.
   const bool setup_completed =
-      Shell::Get()->voice_interaction_controller()->setup_completed();
+      Shell::Get()->voice_interaction_controller()->setup_completed().value_or(
+          false);
 
   // Suggestion container.
   suggestion_container_ = new SuggestionContainerView(assistant_controller_);
@@ -155,7 +156,8 @@ void AssistantFooterView::OnAnimationStarted(
 bool AssistantFooterView::OnAnimationEnded(
     const ui::CallbackLayerAnimationObserver& observer) {
   const bool setup_completed =
-      Shell::Get()->voice_interaction_controller()->setup_completed();
+      Shell::Get()->voice_interaction_controller()->setup_completed().value_or(
+          false);
 
   // Only the view relevant to our consent state should process events.
   suggestion_container_->set_can_process_events_within_subtree(setup_completed);

ash/public/cpp/BUILD.gn

@@ -47,6 +47,10 @@ component("cpp") {
     "ash_typography.cc",
     "ash_typography.h",
     "ash_view_ids.h",
+    "assistant/assistant_state_base.cc",
+    "assistant/assistant_state_base.h",
+    "assistant/assistant_state_proxy.cc",
+    "assistant/assistant_state_proxy.h",
     "assistant/default_voice_interaction_observer.h",
     "caption_buttons/caption_button_model.h",
     "caption_buttons/caption_button_types.h",
@@ -127,6 +131,7 @@ component("cpp") {
     "//ash/public/cpp/vector_icons",
     "//chromeos:power_manager_proto",
     "//components/prefs",
+    "//services/service_manager/public/cpp",
     "//services/ws/public/mojom",
     "//skia/public/interfaces",
     "//ui/aura",

ash/public/cpp/assistant/assistant_state_base.cc

+// Copyright 2018 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 "ash/public/cpp/assistant/assistant_state_base.h"
+
+namespace ash {
+
+AssistantStateBase::AssistantStateBase() = default;
+
+AssistantStateBase::~AssistantStateBase() = default;
+
+}  // namespace ash

ash/public/cpp/assistant/assistant_state_base.h

+// Copyright 2018 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_BASE_H_
+#define ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_STATE_BASE_H_
+
+#include <string>
+
+#include "ash/public/interfaces/voice_interaction_controller.mojom.h"
+#include "base/macros.h"
+#include "base/optional.h"
+
+namespace ash {
+
+// Plain data class that holds Assistant related prefs and states. This is
+// shared by both the controller that controlls these values and client proxy
+// that caches these values locally. Please do not use this object directly,
+// most likely you want to use |AssistantStateProxy|.
+class ASH_PUBLIC_EXPORT AssistantStateBase {
+ public:
+  AssistantStateBase();
+  virtual ~AssistantStateBase();
+
+  const base::Optional<mojom::VoiceInteractionState>& voice_interaction_state()
+      const {
+    return voice_interaction_state_;
+  }
+
+  const base::Optional<bool>& settings_enabled() const {
+    return settings_enabled_;
+  }
+
+  const base::Optional<bool>& setup_completed() const {
+    return setup_completed_;
+  }
+
+  const base::Optional<bool>& context_enabled() const {
+    return context_enabled_;
+  }
+
+  const base::Optional<bool>& hotword_enabled() const {
+    return hotword_enabled_;
+  }
+
+  const base::Optional<mojom::AssistantAllowedState>& allowed_state() const {
+    return allowed_state_;
+  }
+
+  const base::Optional<std::string>& locale() const { return locale_; }
+
+ protected:
+  base::Optional<mojom::VoiceInteractionState> voice_interaction_state_;
+
+  // Whether voice interaction is enabled in system settings.
+  base::Optional<bool> settings_enabled_;
+
+  // Whether voice interaction setup flow has completed.
+  base::Optional<bool> setup_completed_;
+
+  // Whether screen context is enabled.
+  base::Optional<bool> context_enabled_;
+
+  // Whether hotword listening is enabled.
+  base::Optional<bool> hotword_enabled_;
+
+  // Whether voice interaction feature is allowed or disallowed for what reason.
+  base::Optional<mojom::AssistantAllowedState> allowed_state_;
+
+  base::Optional<std::string> locale_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AssistantStateBase);
+};
+
+}  // namespace ash
+
+#endif  // ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_STATE_BASE_H_

ash/public/cpp/assistant/assistant_state_proxy.cc

+// Copyright 2018 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 <algorithm>
+#include <utility>
+
+#include "ash/public/cpp/assistant/assistant_state_proxy.h"
+#include "ash/public/interfaces/constants.mojom.h"
+#include "services/service_manager/public/cpp/connector.h"
+
+namespace ash {
+
+AssistantStateProxy::AssistantStateProxy()
+    : voice_interaction_observer_binding_(this) {}
+
+AssistantStateProxy::~AssistantStateProxy() = default;
+
+void AssistantStateProxy::Init(service_manager::Connector* connector) {
+  connector->BindInterface(ash::mojom::kServiceName,
+                           &voice_interaction_controller_);
+
+  ash::mojom::VoiceInteractionObserverPtr ptr;
+  voice_interaction_observer_binding_.Bind(mojo::MakeRequest(&ptr));
+  voice_interaction_controller_->AddObserver(std::move(ptr));
+}
+
+void AssistantStateProxy::AddObserver(
+    DefaultVoiceInteractionObserver* observer) {
+  if (voice_interaction_state_.has_value())
+    observer->OnVoiceInteractionStatusChanged(voice_interaction_state_.value());
+  if (settings_enabled_.has_value())
+    observer->OnVoiceInteractionSettingsEnabled(settings_enabled_.value());
+  if (context_enabled_.has_value())
+    observer->OnVoiceInteractionContextEnabled(context_enabled_.value());
+  if (hotword_enabled_.has_value())
+    observer->OnVoiceInteractionHotwordEnabled(hotword_enabled_.value());
+  if (setup_completed_.has_value())
+    observer->OnVoiceInteractionSetupCompleted(setup_completed_.value());
+  if (allowed_state_.has_value())
+    observer->OnAssistantFeatureAllowedChanged(allowed_state_.value());
+  if (locale_.has_value())
+    observer->OnLocaleChanged(locale_.value());
+
+  observers_.AddObserver(observer);
+}
+
+void AssistantStateProxy::RemoveObserver(
+    DefaultVoiceInteractionObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void AssistantStateProxy::OnVoiceInteractionStatusChanged(
+    ash::mojom::VoiceInteractionState state) {
+  voice_interaction_state_ = state;
+  for (auto& observer : observers_)
+    observer.OnVoiceInteractionStatusChanged(voice_interaction_state_.value());
+}
+
+void AssistantStateProxy::OnVoiceInteractionSettingsEnabled(bool enabled) {
+  settings_enabled_ = enabled;
+  for (auto& observer : observers_)
+    observer.OnVoiceInteractionSettingsEnabled(settings_enabled_.value());
+}
+
+void AssistantStateProxy::OnVoiceInteractionContextEnabled(bool enabled) {
+  context_enabled_ = enabled;
+  for (auto& observer : observers_)
+    observer.OnVoiceInteractionContextEnabled(context_enabled_.value());
+}
+
+void AssistantStateProxy::OnVoiceInteractionHotwordEnabled(bool enabled) {
+  hotword_enabled_ = enabled;
+  for (auto& observer : observers_)
+    observer.OnVoiceInteractionHotwordEnabled(hotword_enabled_.value());
+}
+
+void AssistantStateProxy::OnVoiceInteractionSetupCompleted(bool completed) {
+  setup_completed_ = completed;
+  for (auto& observer : observers_)
+    observer.OnVoiceInteractionSetupCompleted(setup_completed_.value());
+}
+
+void AssistantStateProxy::OnAssistantFeatureAllowedChanged(
+    ash::mojom::AssistantAllowedState state) {
+  allowed_state_ = state;
+  for (auto& observer : observers_)
+    observer.OnAssistantFeatureAllowedChanged(allowed_state_.value());
+}
+
+void AssistantStateProxy::OnLocaleChanged(const std::string& locale) {
+  locale_ = locale;
+  for (auto& observer : observers_)
+    observer.OnLocaleChanged(locale_.value());
+}
+
+}  // namespace ash

ash/public/cpp/assistant/assistant_state_proxy.h

+// Copyright 2018 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_
+
+#include <string>
+#include <vector>
+
+#include "ash/public/cpp/assistant/assistant_state_base.h"
+#include "ash/public/cpp/assistant/default_voice_interaction_observer.h"
+#include "ash/public/interfaces/voice_interaction_controller.mojom.h"
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/observer_list.h"
+#include "mojo/public/cpp/bindings/binding.h"
+
+namespace service_manager {
+class Connector;
+}  // namespace service_manager
+
+namespace ash {
+
+// Provides a convenient client to access various Assistant states. The state
+// infomration 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::VoiceInteractionObserver {
+ public:
+  AssistantStateProxy();
+  ~AssistantStateProxy() override;
+
+  void Init(service_manager::Connector* connector);
+  void AddObserver(DefaultVoiceInteractionObserver* observer);
+  void RemoveObserver(DefaultVoiceInteractionObserver* observer);
+
+ private:
+  // mojom::VoiceInteractionObserver:
+  void OnVoiceInteractionStatusChanged(
+      ash::mojom::VoiceInteractionState state) override;
+  void OnVoiceInteractionSettingsEnabled(bool enabled) override;
+  void OnVoiceInteractionContextEnabled(bool enabled) override;
+  void OnVoiceInteractionHotwordEnabled(bool enabled) override;
+  void OnVoiceInteractionSetupCompleted(bool completed) override;
+  void OnAssistantFeatureAllowedChanged(
+      ash::mojom::AssistantAllowedState state) override;
+  void OnLocaleChanged(const std::string& locale) override;
+
+  base::ObserverList<DefaultVoiceInteractionObserver> observers_;
+
+  ash::mojom::VoiceInteractionControllerPtr voice_interaction_controller_;
+  mojo::Binding<ash::mojom::VoiceInteractionObserver>
+      voice_interaction_observer_binding_;
+
+  DISALLOW_COPY_AND_ASSIGN(AssistantStateProxy);
+};
+
+}  // namespace ash
+
+#endif  // ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_STATE_PROXY_H_

ash/public/cpp/assistant/default_voice_interaction_observer.h

@@ -9,6 +9,7 @@
 
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "base/macros.h"
+#include "base/observer_list_types.h"
 
 namespace ash {
 
@@ -16,7 +17,8 @@ namespace ash {
 // ash::mojom::VoiceInteractionObserver interface. Child class only need to
 // override the methods they are actually interested in.
 class ASH_PUBLIC_EXPORT DefaultVoiceInteractionObserver
-    : public mojom::VoiceInteractionObserver {
+    : public mojom::VoiceInteractionObserver,
+      public base::CheckedObserver {
  public:
   ~DefaultVoiceInteractionObserver() override = default;
 

ash/public/interfaces/voice_interaction_controller.mojom

@@ -109,19 +109,6 @@ interface VoiceInteractionController {
   // Called when the launch with mic open state is changed.
   NotifyLaunchWithMicOpen(bool launch_with_mic_open);
 
-  // Return if the voice interaction setting is enabled/disabled.
-  IsSettingEnabled() => (bool enabled);
-
-  // Return the voice interaction setup complete status.
-  IsSetupCompleted() => (bool completed);
-
-  // Return if the user has granted permission to access screen "context", the
-  // text and graphics content that is currently on screen.
-  IsContextEnabled() => (bool enabled);
-
-  // Return the voice interaction hotword listening status.
-  IsHotwordEnabled() => (bool enabled);
-
   // Add an observer.
   AddObserver(VoiceInteractionObserver observer);
 };

ash/shelf/app_list_button.cc

@@ -272,9 +272,11 @@ void AppListButton::PaintButtonContents(gfx::Canvas* canvas) {
     fg_flags.setColor(kShelfIconColor);
 
     if (UseVoiceInteractionStyle()) {
-      mojom::VoiceInteractionState state = Shell::Get()
+      mojom::VoiceInteractionState state =
+          Shell::Get()
               ->voice_interaction_controller()
-                                               ->voice_interaction_state();
+              ->voice_interaction_state()
+              .value_or(mojom::VoiceInteractionState::STOPPED);
       // active: 100% alpha, inactive: 54% alpha
       fg_flags.setAlpha(state == mojom::VoiceInteractionState::RUNNING
                             ? kVoiceInteractionRunningAlpha
@@ -393,12 +395,16 @@ void AppListButton::StartVoiceInteractionAnimation() {
   // voice interaction setup flow has completed.
   ShelfAlignment alignment = shelf_->alignment();
   mojom::VoiceInteractionState state =
-      Shell::Get()->voice_interaction_controller()->voice_interaction_state();
+      Shell::Get()
+          ->voice_interaction_controller()
+          ->voice_interaction_state()
+          .value_or(mojom::VoiceInteractionState::STOPPED);
   bool show_icon =
       (alignment == SHELF_ALIGNMENT_BOTTOM ||
        alignment == SHELF_ALIGNMENT_BOTTOM_LOCKED) &&
       state == mojom::VoiceInteractionState::STOPPED &&
-      Shell::Get()->voice_interaction_controller()->setup_completed() &&
+      Shell::Get()->voice_interaction_controller()->setup_completed().value_or(
+          false) &&
       chromeos::switches::IsVoiceInteractionEnabled();
   assistant_overlay_->StartAnimation(show_icon);
 }
@@ -406,8 +412,8 @@ void AppListButton::StartVoiceInteractionAnimation() {
 bool AppListButton::UseVoiceInteractionStyle() {
   VoiceInteractionController* controller =
       Shell::Get()->voice_interaction_controller();
-  bool settings_enabled = controller->settings_enabled();
-  bool setup_completed = controller->setup_completed();
+  bool settings_enabled = controller->settings_enabled().value_or(false);
+  bool setup_completed = controller->setup_completed().value_or(false);
   bool is_feature_allowed =
       controller->allowed_state() == mojom::AssistantAllowedState::ALLOWED;
   if (assistant_overlay_ && is_feature_allowed &&

ash/shelf/app_list_button_unittest.cc

@@ -156,6 +156,8 @@ TEST_F(VoiceInteractionAppListButtonTest,
 
   // Enable voice interaction in system settings.
   Shell::Get()->voice_interaction_controller()->NotifySettingsEnabled(true);
+  Shell::Get()->voice_interaction_controller()->NotifyFeatureAllowed(
+      mojom::AssistantAllowedState::ALLOWED);
 
   ui::GestureEvent long_press =
       CreateGestureEvent(ui::GestureEventDetails(ui::ET_GESTURE_LONG_PRESS));
@@ -198,6 +200,8 @@ TEST_F(VoiceInteractionAppListButtonTest,
   // interaction in settings.
   Shell::Get()->voice_interaction_controller()->NotifySettingsEnabled(false);
   Shell::Get()->voice_interaction_controller()->NotifySetupCompleted(true);
+  Shell::Get()->voice_interaction_controller()->NotifyFeatureAllowed(
+      mojom::AssistantAllowedState::ALLOWED);
 
   ui::GestureEvent long_press =
       CreateGestureEvent(ui::GestureEventDetails(ui::ET_GESTURE_LONG_PRESS));
@@ -218,6 +222,8 @@ TEST_F(VoiceInteractionAppListButtonTest,
 
   // Disable voice interaction in system settings.
   Shell::Get()->voice_interaction_controller()->NotifySettingsEnabled(false);
+  Shell::Get()->voice_interaction_controller()->NotifyFeatureAllowed(
+      mojom::AssistantAllowedState::ALLOWED);
 
   ui::GestureEvent long_press =
       CreateGestureEvent(ui::GestureEventDetails(ui::ET_GESTURE_LONG_PRESS));

ash/voice_interaction/voice_interaction_controller.cc

@@ -82,32 +82,24 @@ void VoiceInteractionController::NotifyLaunchWithMicOpen(
   launch_with_mic_open_ = launch_with_mic_open;
 }
 
-void VoiceInteractionController::IsSettingEnabled(
-    IsSettingEnabledCallback callback) {
-  std::move(callback).Run(settings_enabled_);
-}
-
-void VoiceInteractionController::IsSetupCompleted(
-    IsSetupCompletedCallback callback) {
-  std::move(callback).Run(setup_completed_);
-}
-
-void VoiceInteractionController::IsContextEnabled(
-    IsContextEnabledCallback callback) {
-  std::move(callback).Run(context_enabled_);
-}
-
-void VoiceInteractionController::IsHotwordEnabled(
-    IsHotwordEnabledCallback callback) {
-  std::move(callback).Run(hotword_enabled_);
-}
 
 void VoiceInteractionController::AddObserver(
     mojom::VoiceInteractionObserverPtr observer) {
-  // Locale needs to be notified when adding observer as this property is
-  // changed at profile initialization and some observers (e.g. assistant) may
-  // be constructed at later timing.
-  observer->OnLocaleChanged(locale_);
+  if (voice_interaction_state_.has_value())
+    observer->OnVoiceInteractionStatusChanged(voice_interaction_state_.value());
+  if (settings_enabled_.has_value())
+    observer->OnVoiceInteractionSettingsEnabled(settings_enabled_.value());
+  if (context_enabled_.has_value())
+    observer->OnVoiceInteractionContextEnabled(context_enabled_.value());
+  if (hotword_enabled_.has_value())
+    observer->OnVoiceInteractionHotwordEnabled(hotword_enabled_.value());
+  if (setup_completed_.has_value())
+    observer->OnVoiceInteractionSetupCompleted(setup_completed_.value());
+  if (allowed_state_.has_value())
+    observer->OnAssistantFeatureAllowedChanged(allowed_state_.value());
+  if (locale_.has_value())
+    observer->OnLocaleChanged(locale_.value());
+
   observers_.AddPtr(std::move(observer));
 }
 

ash/voice_interaction/voice_interaction_controller.h

@@ -6,8 +6,10 @@
 #define ASH_VOICE_INTERACTION_VOICE_INTERACTION_CONTROLLER_H_
 
 #include <memory>
+#include <string>
 
 #include "ash/ash_export.h"
+#include "ash/public/cpp/assistant/assistant_state_base.h"
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "mojo/public/cpp/bindings/interface_ptr_set.h"
@@ -15,7 +17,8 @@
 namespace ash {
 
 class ASH_EXPORT VoiceInteractionController
-    : public mojom::VoiceInteractionController {
+    : public mojom::VoiceInteractionController,
+      public AssistantStateBase {
  public:
   VoiceInteractionController();
   ~VoiceInteractionController() override;
@@ -32,24 +35,8 @@ class ASH_EXPORT VoiceInteractionController
   void NotifyNotificationEnabled(bool enabled) override;
   void NotifyLocaleChanged(const std::string& locale) override;
   void NotifyLaunchWithMicOpen(bool launch_with_mic_open) override;
-  void IsSettingEnabled(IsSettingEnabledCallback callback) override;
-  void IsSetupCompleted(IsSetupCompletedCallback callback) override;
-  void IsContextEnabled(IsContextEnabledCallback callback) override;
-  void IsHotwordEnabled(IsHotwordEnabledCallback callback) override;
   void AddObserver(mojom::VoiceInteractionObserverPtr observer) override;
 
-  mojom::VoiceInteractionState voice_interaction_state() const {
-    return voice_interaction_state_;
-  }
-
-  bool settings_enabled() const { return settings_enabled_; }
-
-  bool setup_completed() const { return setup_completed_; }
-
-  bool context_enabled() const { return context_enabled_; }
-
-  mojom::AssistantAllowedState allowed_state() const { return allowed_state_; }
-
   bool notification_enabled() const { return notification_enabled_; }
 
   bool launch_with_mic_open() const { return launch_with_mic_open_; }
@@ -57,32 +44,9 @@ class ASH_EXPORT VoiceInteractionController
   void FlushForTesting();
 
  private:
-  // Voice interaction state. The initial value should be set to STOPPED to make
-  // sure the app list button burst animation could be correctly shown.
-  mojom::VoiceInteractionState voice_interaction_state_ =
-      mojom::VoiceInteractionState::STOPPED;
-
-  // Whether voice interaction is enabled in system settings.
-  bool settings_enabled_ = false;
-
-  // Whether voice interaction setup flow has completed.
-  bool setup_completed_ = false;
-
-  // Whether screen context is enabled.
-  bool context_enabled_ = false;
-
-  // Whether hotword listening is enabled.
-  bool hotword_enabled_ = false;
-
   // Whether notification is enabled.
   bool notification_enabled_ = false;
 
-  // Whether voice interaction feature is allowed or disallowed for what reason.
-  mojom::AssistantAllowedState allowed_state_ =
-      mojom::AssistantAllowedState::ALLOWED;
-
-  std::string locale_;
-
   // Whether the Assistant should launch with mic open;
   bool launch_with_mic_open_ = false;
 

chrome/browser/chromeos/arc/voice_interaction/fake_voice_interaction_controller.cc

@@ -58,24 +58,4 @@ void FakeVoiceInteractionController::NotifyLaunchWithMicOpen(
   launch_with_mic_open_ = launch_with_mic_open;
 }
 
-void FakeVoiceInteractionController::IsSettingEnabled(
-    IsSettingEnabledCallback callback) {
-  std::move(callback).Run(voice_interaction_settings_enabled_);
-}
-
-void FakeVoiceInteractionController::IsSetupCompleted(
-    IsSetupCompletedCallback callback) {
-  std::move(callback).Run(voice_interaction_setup_completed_);
-}
-
-void FakeVoiceInteractionController::IsContextEnabled(
-    IsContextEnabledCallback callback) {
-  std::move(callback).Run(voice_interaction_context_enabled_);
-}
-
-void FakeVoiceInteractionController::IsHotwordEnabled(
-    IsHotwordEnabledCallback callback) {
-  std::move(callback).Run(voice_interaction_hotword_enabled_);
-}
-
 }  // namespace arc

chrome/browser/chromeos/arc/voice_interaction/fake_voice_interaction_controller.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_CHROMEOS_ARC_VOICE_INTERACTION_FAKE_VOICE_INTERACTION_CONTROLLER_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_VOICE_INTERACTION_FAKE_VOICE_INTERACTION_CONTROLLER_H_
 
+#include <string>
+
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -28,10 +30,6 @@ class FakeVoiceInteractionController
   void NotifyNotificationEnabled(bool enabled) override;
   void NotifyLocaleChanged(const std::string& locale) override;
   void NotifyLaunchWithMicOpen(bool launch_with_mic_open) override;
-  void IsSettingEnabled(IsSettingEnabledCallback callback) override;
-  void IsSetupCompleted(IsSetupCompletedCallback callback) override;
-  void IsContextEnabled(IsContextEnabledCallback callback) override;
-  void IsHotwordEnabled(IsHotwordEnabledCallback callback) override;
   void AddObserver(ash::mojom::VoiceInteractionObserverPtr observer) override {}
 
   ash::mojom::VoiceInteractionState voice_interaction_state() const {

chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.h"
 
+#include <string>
 #include <utility>
 
 #include "ash/public/cpp/ash_pref_names.h"
@@ -104,8 +105,6 @@ void VoiceInteractionControllerClient::NotifyContextEnabled() {
 void VoiceInteractionControllerClient::NotifyHotwordEnabled() {
   DCHECK(profile_);
   PrefService* prefs = profile_->GetPrefs();
-  // Make sure voice interaction is enabled.
-  DCHECK(prefs->GetBoolean(prefs::kVoiceInteractionEnabled));
   bool enabled = prefs->GetBoolean(prefs::kVoiceInteractionHotwordEnabled);
   voice_interaction_controller_->NotifyHotwordEnabled(enabled);
 }
@@ -232,7 +231,6 @@ void VoiceInteractionControllerClient::SetProfile(Profile* profile) {
   NotifyLocaleChanged();
   NotifyNotificationEnabled();
   NotifyLaunchWithMicOpen();
-  if (prefs->GetBoolean(prefs::kVoiceInteractionEnabled))
   NotifyHotwordEnabled();
 }
 

chromeos/services/assistant/assistant_manager_service.h

@@ -31,9 +31,11 @@ class AssistantManagerService : public mojom::Assistant {
 
   ~AssistantManagerService() override = default;
 
-  // Start the assistant in the background with |token|. When the service is
-  // fully started |callback| will be called on the thread where ctor was run.
+  // Start the assistant in the background with |access_token|. When the service
+  // is fully started |callback| will be called on the thread where ctor was
+  // run.
   virtual void Start(const std::string& access_token,
+                     bool enable_hotword,
                      base::OnceClosure callback) = 0;
 
   // Stop the assistant.

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -56,7 +56,6 @@ class AssistantManagerServiceImpl
       public AssistantEventObserver,
       public assistant_client::ConversationStateListener,
       public assistant_client::AssistantManagerDelegate,
-      public ash::DefaultVoiceInteractionObserver,
       public assistant_client::DeviceStateListener {
  public:
   // |service| owns this class and must outlive this class.
@@ -64,13 +63,13 @@ class AssistantManagerServiceImpl
       service_manager::Connector* connector,
       device::mojom::BatteryMonitorPtr battery_monitor,
       Service* service,
-      bool enable_hotword,
       network::NetworkConnectionTracker* network_connection_tracker);
 
   ~AssistantManagerServiceImpl() override;
 
   // assistant::AssistantManagerService overrides
   void Start(const std::string& access_token,
+             bool enable_hotword,
              base::OnceClosure callback) override;
   void Stop() override;
   State GetState() const override;
@@ -141,27 +140,24 @@ class AssistantManagerServiceImpl
   // Last search source will be cleared after it is retrieved.
   std::string GetLastSearchSource() override;
 
-  // ash::mojom::VoiceInteractionObserver:
-  void OnVoiceInteractionSettingsEnabled(bool enabled) override;
-  void OnVoiceInteractionContextEnabled(bool enabled) override;
-  void OnVoiceInteractionHotwordEnabled(bool enabled) override;
-  void OnLocaleChanged(const std::string& locale) override;
-
   // assistant_client::DeviceStateListener overrides:
   void OnStartFinished() override;
   void OnTimerSoundingStarted() override;
   void OnTimerSoundingFinished() override;
 
  private:
-  void StartAssistantInternal(const std::string& access_token,
-                              const std::string& arc_version);
-  void PostInitAssistant(base::OnceClosure post_init_callback);
-
-  std::string BuildUserAgent(const std::string& arc_version) const;
+  std::unique_ptr<assistant_client::AssistantManager> StartAssistantInternal(
+      const std::string& access_token,
+      bool enable_hotword,
+      const std::string& arc_version,
+      const std::string& locale,
+      bool spoken_feedback_enabled);
+  void PostInitAssistant(
+      base::OnceClosure post_init_callback,
+      std::unique_ptr<assistant_client::AssistantManager>* assistant_manager);
 
   // Update device id, type and locale
   void UpdateDeviceSettings();
-  void UpdateInternalOptions();
 
   void HandleGetSettingsResponse(
       base::RepeatingCallback<void(const std::string&)> callback,
@@ -210,11 +206,10 @@ class AssistantManagerServiceImpl
       std::unique_ptr<ui::AssistantTree> assistant_tree,
       const std::vector<uint8_t>& assistant_screenshot);
 
-  void FillServerExperimentIds(std::vector<std::string>& server_experiment_ids);
+  void FillServerExperimentIds(std::vector<std::string>* server_experiment_ids);
 
   State state_ = State::STOPPED;
   std::unique_ptr<PlatformApiImpl> platform_api_;
-  bool enable_hotword_;
   std::unique_ptr<action::CrosActionModule> action_module_;
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
   std::unique_ptr<assistant_client::AssistantManager> assistant_manager_;
@@ -227,20 +222,12 @@ class AssistantManagerServiceImpl
       interaction_subscribers_;
   mojo::InterfacePtrSet<mojom::AssistantNotificationSubscriber>
       notification_subscribers_;
-  ash::mojom::VoiceInteractionControllerPtr voice_interaction_controller_;
-  mojo::Binding<ash::mojom::VoiceInteractionObserver>
-      voice_interaction_observer_binding_;
   ash::mojom::AshMessageCenterControllerPtr ash_message_center_controller_;
   mojom::SpeakerIdEnrollmentClientPtr speaker_id_enrollment_client_;
 
   Service* service_;  // unowned.
 
-  base::Optional<std::string> arc_version_;
-
-  bool assistant_enabled_ = false;
-  bool context_enabled_ = false;
   bool spoken_feedback_enabled_ = false;
-  std::string locale_;
 
   ax::mojom::AssistantExtraPtr assistant_extra_;
   std::unique_ptr<ui::AssistantTree> assistant_tree_;

chromeos/services/assistant/fake_assistant_manager_service_impl.cc

@@ -4,6 +4,8 @@
 
 #include "chromeos/services/assistant/fake_assistant_manager_service_impl.h"
 
+#include <utility>
+
 namespace chromeos {
 namespace assistant {
 
@@ -12,6 +14,7 @@ FakeAssistantManagerServiceImpl::FakeAssistantManagerServiceImpl() = default;
 FakeAssistantManagerServiceImpl::~FakeAssistantManagerServiceImpl() = default;
 
 void FakeAssistantManagerServiceImpl::Start(const std::string& access_token,
+                                            bool enable_hotword,
                                             base::OnceClosure callback) {
   state_ = State::RUNNING;
 

chromeos/services/assistant/fake_assistant_manager_service_impl.h

@@ -25,6 +25,7 @@ class FakeAssistantManagerServiceImpl : public AssistantManagerService {
 
   // assistant::AssistantManagerService overrides
   void Start(const std::string& access_token,
+             bool enable_hotword,
              base::OnceClosure callback) override;
   void Stop() override;
   void SetAccessToken(const std::string& access_token) override;

chromeos/services/assistant/platform/audio_input_provider_impl.cc

@@ -21,7 +21,6 @@ namespace assistant {
 
 namespace {
 
-// This format should match //c/b/c/assistant/platform_audio_input_host.cc.
 constexpr assistant_client::BufferFormat kFormat{
     16000 /* sample_rate */, assistant_client::INTERLEAVED_S32, 1 /* channels */
 };
@@ -52,12 +51,10 @@ int AudioInputBufferImpl::GetFrameCount() const {
 }
 
 AudioInputImpl::AudioInputImpl(
-    std::unique_ptr<service_manager::Connector> connector,
-    bool default_on)
+    std::unique_ptr<service_manager::Connector> connector)
     : source_(audio::CreateInputDevice(
           std::move(connector),
           media::AudioDeviceDescription::kDefaultDeviceId)),
-      default_on_(default_on),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       weak_factory_(this) {
   DETACH_FROM_SEQUENCE(observer_sequence_checker_);
@@ -199,9 +196,8 @@ void AudioInputImpl::StopRecording() {
 }
 
 AudioInputProviderImpl::AudioInputProviderImpl(
-    service_manager::Connector* connector,
-    bool default_on)
-    : audio_input_(connector->Clone(), default_on) {}
+    service_manager::Connector* connector)
+    : audio_input_(connector->Clone()) {}
 
 AudioInputProviderImpl::~AudioInputProviderImpl() = default;
 

chromeos/services/assistant/platform/audio_input_provider_impl.h

@@ -50,8 +50,8 @@ class AudioInputBufferImpl : public assistant_client::AudioBuffer {
 class AudioInputImpl : public assistant_client::AudioInput,
                        public media::AudioCapturerSource::CaptureCallback {
  public:
-  AudioInputImpl(std::unique_ptr<service_manager::Connector> connector,
-                 bool default_on);
+  explicit AudioInputImpl(
+      std::unique_ptr<service_manager::Connector> connector);
   ~AudioInputImpl() override;
 
   // media::AudioCapturerSource::CaptureCallback overrides:
@@ -83,7 +83,7 @@ class AudioInputImpl : public assistant_client::AudioInput,
   scoped_refptr<media::AudioCapturerSource> source_;
 
   // Should audio input always recording actively.
-  bool default_on_;
+  bool default_on_ = false;
 
   // Guards observers_;
   base::Lock lock_;
@@ -108,8 +108,7 @@ class AudioInputImpl : public assistant_client::AudioInput,
 
 class AudioInputProviderImpl : public assistant_client::AudioInputProvider {
  public:
-  explicit AudioInputProviderImpl(service_manager::Connector* connector,
-                                  bool default_on);
+  explicit AudioInputProviderImpl(service_manager::Connector* connector);
   ~AudioInputProviderImpl() override;
 
   // assistant_client::AudioInputProvider overrides:

chromeos/services/assistant/platform_api_impl.cc

@@ -74,10 +74,9 @@ void PlatformApiImpl::DummyAuthProvider::Reset() {}
 PlatformApiImpl::PlatformApiImpl(
     service_manager::Connector* connector,
     device::mojom::BatteryMonitorPtr battery_monitor,
-    bool enable_hotword,
     scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
     network::NetworkConnectionTracker* network_connection_tracker)
-    : audio_input_provider_(connector, enable_hotword),
+    : audio_input_provider_(connector),
       audio_output_provider_(connector, background_task_runner),
       network_provider_(network_connection_tracker),
       system_provider_(std::move(battery_monitor)) {}

chromeos/services/assistant/platform_api_impl.h

@@ -33,7 +33,6 @@ class PlatformApiImpl : public assistant_client::PlatformApi {
   PlatformApiImpl(
       service_manager::Connector* connector,
       device::mojom::BatteryMonitorPtr battery_monitor,
-      bool enable_hotword,
       scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
       network::NetworkConnectionTracker* network_connection_tracker);
   ~PlatformApiImpl() override;

chromeos/services/assistant/service.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "ash/public/cpp/assistant/assistant_state_proxy.h"
 #include "ash/public/cpp/assistant/default_voice_interaction_observer.h"
 #include "ash/public/interfaces/assistant_controller.mojom.h"
 #include "ash/public/interfaces/session_controller.mojom.h"
@@ -59,6 +60,7 @@ class Service : public service_manager::Service,
   ash::mojom::AssistantController* assistant_controller() {
     return assistant_controller_.get();
   }
+  ash::AssistantStateBase* assistant_state() { return &assistant_state_; }
 
   void RequestAccessToken();
 
@@ -83,10 +85,6 @@ class Service : public service_manager::Service,
   // chromeos::PowerManagerClient::Observer overrides:
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
 
-  // mojom::AssistantPlatform overrides:
-  void Init(mojom::ClientPtr client,
-            mojom::DeviceActionsPtr device_actions) override;
-
   // ash::mojom::SessionActivationObserver overrides:
   void OnSessionActivated(bool activated) override;
   void OnLockStateChanged(bool locked) override;
@@ -94,10 +92,16 @@ class Service : public service_manager::Service,
   // ash::mojom::VoiceInteractionObserver:
   void OnVoiceInteractionSettingsEnabled(bool enabled) override;
   void OnVoiceInteractionHotwordEnabled(bool enabled) override;
+  void OnLocaleChanged(const std::string& locale) override;
 
+  void UpdateAssistantManagerState();
   void BindAssistantSettingsManager(
       mojom::AssistantSettingsManagerRequest request);
 
+  // mojom::AssistantPlatform overrides:
+  void Init(mojom::ClientPtr client,
+            mojom::DeviceActionsPtr device_actions) override;
+
   identity::mojom::IdentityManager* GetIdentityManager();
 
   void GetPrimaryAccountInfoCallback(
@@ -107,16 +111,17 @@ class Service : public service_manager::Service,
   void GetAccessTokenCallback(const base::Optional<std::string>& token,
                               base::Time expiration_time,
                               const GoogleServiceAuthError& error);
+  void RetryRefreshToken();
 
-  void AddAshSessionObserver();
+  void CreateAssistantManagerService();
 
-  void UpdateListeningState();
+  void FinalizeAssistantManagerService();
 
-  void CreateAssistantManagerService(bool enable_hotword);
+  void StopAssistantManagerService();
 
-  void FinalizeAssistantManagerService();
+  void AddAshSessionObserver();
 
-  void RetryRefreshToken();
+  void UpdateListeningState();
 
   service_manager::BinderRegistry registry_;
 
@@ -143,15 +148,11 @@ class Service : public service_manager::Service,
   bool session_active_ = false;
   // Whether the lock screen is on.
   bool locked_ = false;
-  // Whether the assistant has been enabled in settings.
-  bool settings_enabled_ = false;
-  // Whether the hotword has been enabled.
-  bool hotword_enabled_ = false;
+
+  base::Optional<std::string> access_token_;
 
   ash::mojom::AssistantControllerPtr assistant_controller_;
-  ash::mojom::VoiceInteractionControllerPtr voice_interaction_controller_;
-  mojo::Binding<ash::mojom::VoiceInteractionObserver>
-      voice_interaction_observer_binding_;
+  ash::AssistantStateProxy assistant_state_;
 
   network::NetworkConnectionTracker* network_connection_tracker_;