assistant: refactor local voice interaction observers

Bug: 870515
Test: locally build and run
Change-Id: Ic2b5be8d7ff3bf694ddbb8cb29511b3bb5ad610e
Reviewed-on: https://chromium-review.googlesource.com/c/1318691
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606903}

ash/app_list/app_list_controller_impl.cc

@@ -42,8 +42,7 @@ namespace ash {
 
 AppListControllerImpl::AppListControllerImpl()
     : presenter_(std::make_unique<AppListPresenterDelegateImpl>(this)),
-      is_home_launcher_enabled_(app_list_features::IsHomeLauncherEnabled()),
-      voice_interaction_binding_(this) {
+      is_home_launcher_enabled_(app_list_features::IsHomeLauncherEnabled()) {
   model_.AddObserver(this);
 
   // Create only for non-mash. Mash uses window tree embed API to get a
@@ -76,9 +75,7 @@ AppListControllerImpl::AppListControllerImpl()
         std::make_unique<HomeLauncherGestureHandler>(this);
   }
 
-  mojom::VoiceInteractionObserverPtr ptr;
-  voice_interaction_binding_.Bind(mojo::MakeRequest(&ptr));
-  Shell::Get()->voice_interaction_controller()->AddObserver(std::move(ptr));
+  Shell::Get()->voice_interaction_controller()->AddLocalObserver(this);
 }
 
 AppListControllerImpl::~AppListControllerImpl() {
@@ -87,6 +84,7 @@ AppListControllerImpl::~AppListControllerImpl() {
   Shell::Get()->wallpaper_controller()->RemoveObserver(this);
   Shell::Get()->tablet_mode_controller()->RemoveObserver(this);
   Shell::Get()->session_controller()->RemoveObserver(this);
+  Shell::Get()->voice_interaction_controller()->RemoveLocalObserver(this);
   model_.RemoveObserver(this);
 }
 

ash/app_list/app_list_controller_impl.h

@@ -287,8 +287,6 @@ class ASH_EXPORT AppListControllerImpl
   // Whether we're currently in a window dragging process.
   bool in_window_dragging_ = false;
 
-  mojo::Binding<mojom::VoiceInteractionObserver> voice_interaction_binding_;
-
   DISALLOW_COPY_AND_ASSIGN(AppListControllerImpl);
 };
 

ash/assistant/assistant_cache_controller.cc

@@ -63,20 +63,15 @@ constexpr int kMaxNumOfConversationStarters = 3;
 
 AssistantCacheController::AssistantCacheController(
     AssistantController* assistant_controller)
-    : assistant_controller_(assistant_controller),
-      voice_interaction_binding_(this) {
+    : assistant_controller_(assistant_controller) {
   UpdateConversationStarters();
-
   assistant_controller_->AddObserver(this);
-
-  // Bind to observe changes to screen context preference.
-  mojom::VoiceInteractionObserverPtr ptr;
-  voice_interaction_binding_.Bind(mojo::MakeRequest(&ptr));
-  Shell::Get()->voice_interaction_controller()->AddObserver(std::move(ptr));
+  Shell::Get()->voice_interaction_controller()->AddLocalObserver(this);
 }
 
 AssistantCacheController::~AssistantCacheController() {
   assistant_controller_->RemoveObserver(this);
+  Shell::Get()->voice_interaction_controller()->RemoveLocalObserver(this);
 }
 
 void AssistantCacheController::AddModelObserver(

ash/assistant/assistant_cache_controller.h

@@ -11,7 +11,6 @@
 #include "ash/public/cpp/assistant/default_voice_interaction_observer.h"
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "base/macros.h"
-#include "mojo/public/cpp/bindings/binding.h"
 
 namespace ash {
 
@@ -49,8 +48,6 @@ class AssistantCacheController : public AssistantControllerObserver,
 
   AssistantController* const assistant_controller_;  // Owned by Shell.
 
-  mojo::Binding<mojom::VoiceInteractionObserver> voice_interaction_binding_;
-
   AssistantCacheModel model_;
 
   DISALLOW_COPY_AND_ASSIGN(AssistantCacheController);

ash/assistant/assistant_controller.cc

@@ -40,11 +40,8 @@ AssistantController::AssistantController()
       assistant_setup_controller_(
           std::make_unique<AssistantSetupController>(this)),
       assistant_ui_controller_(std::make_unique<AssistantUiController>(this)),
-      voice_interaction_binding_(this),
       weak_factory_(this) {
-  mojom::VoiceInteractionObserverPtr ptr;
-  voice_interaction_binding_.Bind(mojo::MakeRequest(&ptr));
-  Shell::Get()->voice_interaction_controller()->AddObserver(std::move(ptr));
+  Shell::Get()->voice_interaction_controller()->AddLocalObserver(this);
   chromeos::CrasAudioHandler::Get()->AddAudioObserver(this);
   AddObserver(this);
 
@@ -56,6 +53,7 @@ AssistantController::~AssistantController() {
 
   chromeos::CrasAudioHandler::Get()->RemoveAudioObserver(this);
   Shell::Get()->accessibility_controller()->RemoveObserver(this);
+  Shell::Get()->voice_interaction_controller()->RemoveLocalObserver(this);
   RemoveObserver(this);
 }
 

ash/assistant/assistant_controller.h

@@ -213,8 +213,6 @@ class ASH_EXPORT AssistantController
 
   std::unique_ptr<AssistantUiController> assistant_ui_controller_;
 
-  mojo::Binding<mojom::VoiceInteractionObserver> voice_interaction_binding_;
-
   base::WeakPtrFactory<AssistantController> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AssistantController);

ash/assistant/ui/main_stage/assistant_footer_view.cc

@@ -47,17 +47,14 @@ AssistantFooterView::AssistantFooterView(
               base::Unretained(this)),
           /*animation_ended_callback=*/base::BindRepeating(
               &AssistantFooterView::OnAnimationEnded,
-              base::Unretained(this)))),
-      voice_interaction_observer_binding_(this) {
+              base::Unretained(this)))) {
   InitLayout();
-
-  // Observe voice interaction for changes to consent state.
-  mojom::VoiceInteractionObserverPtr ptr;
-  voice_interaction_observer_binding_.Bind(mojo::MakeRequest(&ptr));
-  Shell::Get()->voice_interaction_controller()->AddObserver(std::move(ptr));
+  Shell::Get()->voice_interaction_controller()->AddLocalObserver(this);
 }
 
-AssistantFooterView::~AssistantFooterView() = default;
+AssistantFooterView::~AssistantFooterView() {
+  Shell::Get()->voice_interaction_controller()->RemoveLocalObserver(this);
+}
 
 const char* AssistantFooterView::GetClassName() const {
   return "AssistantFooterView";

ash/assistant/ui/main_stage/assistant_footer_view.h

@@ -11,7 +11,6 @@
 #include "ash/public/cpp/assistant/default_voice_interaction_observer.h"
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "base/macros.h"
-#include "mojo/public/cpp/bindings/binding_set.h"
 #include "ui/views/view.h"
 
 namespace ui {
@@ -51,9 +50,6 @@ class AssistantFooterView : public views::View,
 
   std::unique_ptr<ui::CallbackLayerAnimationObserver> animation_observer_;
 
-  mojo::Binding<mojom::VoiceInteractionObserver>
-      voice_interaction_observer_binding_;
-
   DISALLOW_COPY_AND_ASSIGN(AssistantFooterView);
 };
 

ash/shelf/app_list_button.cc

@@ -68,17 +68,14 @@ AppListButton::AppListButton(InkDropButtonListener* listener,
     : ShelfControlButton(),
       listener_(listener),
       shelf_view_(shelf_view),
-      shelf_(shelf),
-      voice_interaction_binding_(this) {
+      shelf_(shelf) {
   DCHECK(listener_);
   DCHECK(shelf_view_);
   DCHECK(shelf_);
   Shell::Get()->AddShellObserver(this);
   Shell::Get()->session_controller()->AddObserver(this);
 
-  mojom::VoiceInteractionObserverPtr ptr;
-  voice_interaction_binding_.Bind(mojo::MakeRequest(&ptr));
-  Shell::Get()->voice_interaction_controller()->AddObserver(std::move(ptr));
+  Shell::Get()->voice_interaction_controller()->AddLocalObserver(this);
   SetAccessibleName(
       l10n_util::GetStringUTF16(IDS_ASH_SHELF_APP_LIST_LAUNCHER_TITLE));
   set_notify_action(Button::NOTIFY_ON_PRESS);
@@ -96,6 +93,7 @@ AppListButton::AppListButton(InkDropButtonListener* listener,
 AppListButton::~AppListButton() {
   Shell::Get()->RemoveShellObserver(this);
   Shell::Get()->session_controller()->RemoveObserver(this);
+  Shell::Get()->voice_interaction_controller()->RemoveLocalObserver(this);
 }
 
 void AppListButton::OnAppListShown() {

ash/shelf/app_list_button.h

@@ -14,7 +14,6 @@
 #include "ash/shelf/shelf_control_button.h"
 #include "ash/shell_observer.h"
 #include "base/macros.h"
-#include "mojo/public/cpp/bindings/binding.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/views/controls/button/image_button.h"
 
@@ -98,8 +97,6 @@ class ASH_EXPORT AppListButton : public ShelfControlButton,
   std::unique_ptr<base::OneShotTimer> assistant_animation_hide_delay_timer_;
   base::TimeTicks voice_interaction_start_timestamp_;
 
-  mojo::Binding<mojom::VoiceInteractionObserver> voice_interaction_binding_;
-
   DISALLOW_COPY_AND_ASSIGN(AppListButton);
 };
 

ash/system/palette/tools/metalayer_mode.cc

@@ -36,18 +36,15 @@ const int kMaxStrokeGapWhenWritingMs = 1000;
 
 MetalayerMode::MetalayerMode(Delegate* delegate)
     : CommonPaletteTool(delegate),
-      voice_interaction_binding_(this),
       weak_factory_(this) {
   Shell::Get()->AddPreTargetHandler(this);
-
-  mojom::VoiceInteractionObserverPtr ptr;
-  voice_interaction_binding_.Bind(mojo::MakeRequest(&ptr));
-  Shell::Get()->voice_interaction_controller()->AddObserver(std::move(ptr));
+  Shell::Get()->voice_interaction_controller()->AddLocalObserver(this);
   Shell::Get()->highlighter_controller()->AddObserver(this);
 }
 
 MetalayerMode::~MetalayerMode() {
   Shell::Get()->highlighter_controller()->RemoveObserver(this);
+  Shell::Get()->voice_interaction_controller()->RemoveLocalObserver(this);
   Shell::Get()->RemovePreTargetHandler(this);
 }
 

ash/system/palette/tools/metalayer_mode.h

@@ -99,8 +99,6 @@ class ASH_EXPORT MetalayerMode : public CommonPaletteTool,
   // True when the mode is activated via the stylus barrel button.
   bool activated_via_button_ = false;
 
-  mojo::Binding<mojom::VoiceInteractionObserver> voice_interaction_binding_;
-
   base::WeakPtrFactory<MetalayerMode> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MetalayerMode);

ash/voice_interaction/voice_interaction_controller.cc

@@ -28,6 +28,8 @@ void VoiceInteractionController::NotifyStatusChanged(
   observers_.ForAllPtrs([state](auto* observer) {
     observer->OnVoiceInteractionStatusChanged(state);
   });
+  for (auto& observer : local_observers_)
+    observer.OnVoiceInteractionStatusChanged(state);
 }
 
 void VoiceInteractionController::NotifySettingsEnabled(bool enabled) {
@@ -35,6 +37,8 @@ void VoiceInteractionController::NotifySettingsEnabled(bool enabled) {
   observers_.ForAllPtrs([enabled](auto* observer) {
     observer->OnVoiceInteractionSettingsEnabled(enabled);
   });
+  for (auto& observer : local_observers_)
+    observer.OnVoiceInteractionSettingsEnabled(enabled);
 }
 
 void VoiceInteractionController::NotifyContextEnabled(bool enabled) {
@@ -42,6 +46,8 @@ void VoiceInteractionController::NotifyContextEnabled(bool enabled) {
   observers_.ForAllPtrs([enabled](auto* observer) {
     observer->OnVoiceInteractionContextEnabled(enabled);
   });
+  for (auto& observer : local_observers_)
+    observer.OnVoiceInteractionContextEnabled(enabled);
 }
 
 void VoiceInteractionController::NotifyHotwordEnabled(bool enabled) {
@@ -49,6 +55,8 @@ void VoiceInteractionController::NotifyHotwordEnabled(bool enabled) {
   observers_.ForAllPtrs([enabled](auto* observer) {
     observer->OnVoiceInteractionHotwordEnabled(enabled);
   });
+  for (auto& observer : local_observers_)
+    observer.OnVoiceInteractionHotwordEnabled(enabled);
 }
 
 void VoiceInteractionController::NotifySetupCompleted(bool completed) {
@@ -56,6 +64,8 @@ void VoiceInteractionController::NotifySetupCompleted(bool completed) {
   observers_.ForAllPtrs([completed](auto* observer) {
     observer->OnVoiceInteractionSetupCompleted(completed);
   });
+  for (auto& observer : local_observers_)
+    observer.OnVoiceInteractionSetupCompleted(completed);
 }
 
 void VoiceInteractionController::NotifyFeatureAllowed(
@@ -64,6 +74,8 @@ void VoiceInteractionController::NotifyFeatureAllowed(
   observers_.ForAllPtrs([state](auto* observer) {
     observer->OnAssistantFeatureAllowedChanged(state);
   });
+  for (auto& observer : local_observers_)
+    observer.OnAssistantFeatureAllowedChanged(state);
 }
 
 void VoiceInteractionController::NotifyNotificationEnabled(bool enabled) {
@@ -75,6 +87,8 @@ void VoiceInteractionController::NotifyLocaleChanged(
   locale_ = locale;
   observers_.ForAllPtrs(
       [locale](auto* observer) { observer->OnLocaleChanged(locale); });
+  for (auto& observer : local_observers_)
+    observer.OnLocaleChanged(locale);
 }
 
 void VoiceInteractionController::NotifyLaunchWithMicOpen(
@@ -82,9 +96,25 @@ void VoiceInteractionController::NotifyLaunchWithMicOpen(
   launch_with_mic_open_ = launch_with_mic_open;
 }
 
-
 void VoiceInteractionController::AddObserver(
     mojom::VoiceInteractionObserverPtr observer) {
+  InitObserver(observer.get());
+  observers_.AddPtr(std::move(observer));
+}
+
+void VoiceInteractionController::AddLocalObserver(
+    DefaultVoiceInteractionObserver* observer) {
+  InitObserver(observer);
+  local_observers_.AddObserver(observer);
+}
+
+void VoiceInteractionController::RemoveLocalObserver(
+    DefaultVoiceInteractionObserver* observer) {
+  local_observers_.RemoveObserver(observer);
+}
+
+void VoiceInteractionController::InitObserver(
+    mojom::VoiceInteractionObserver* observer) {
   if (voice_interaction_state_.has_value())
     observer->OnVoiceInteractionStatusChanged(voice_interaction_state_.value());
   if (settings_enabled_.has_value())
@@ -99,8 +129,6 @@ void VoiceInteractionController::AddObserver(
     observer->OnAssistantFeatureAllowedChanged(allowed_state_.value());
   if (locale_.has_value())
     observer->OnLocaleChanged(locale_.value());
-
-  observers_.AddPtr(std::move(observer));
 }
 
 void VoiceInteractionController::FlushForTesting() {

ash/voice_interaction/voice_interaction_controller.h

@@ -10,6 +10,7 @@
 
 #include "ash/ash_export.h"
 #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 "mojo/public/cpp/bindings/binding_set.h"
 #include "mojo/public/cpp/bindings/interface_ptr_set.h"
@@ -37,6 +38,11 @@ class ASH_EXPORT VoiceInteractionController
   void NotifyLaunchWithMicOpen(bool launch_with_mic_open) override;
   void AddObserver(mojom::VoiceInteractionObserverPtr observer) override;
 
+  // Adding local observers in the same process.
+  void AddLocalObserver(DefaultVoiceInteractionObserver* observer);
+  void RemoveLocalObserver(DefaultVoiceInteractionObserver* observer);
+  void InitObserver(mojom::VoiceInteractionObserver* observer);
+
   bool notification_enabled() const { return notification_enabled_; }
 
   bool launch_with_mic_open() const { return launch_with_mic_open_; }
@@ -54,6 +60,8 @@ class ASH_EXPORT VoiceInteractionController
 
   mojo::InterfacePtrSet<mojom::VoiceInteractionObserver> observers_;
 
+  base::ObserverList<DefaultVoiceInteractionObserver> local_observers_;
+
   DISALLOW_COPY_AND_ASSIGN(VoiceInteractionController);
 };