assistant: deprecate mojom::AssistantController

* merge with ash::AssistantController
* finish a pending TODO and moved one method to
  ash::AssistantInteractionController

Bug: b:154268518
Change-Id: I746ecdbd1ee9c79c9f2d2c69a0fb019816c59118
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2166585
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/master@{#763018}

ash/assistant/assistant_controller_impl.cc

@@ -48,12 +48,6 @@ void AssistantControllerImpl::RegisterProfilePrefs(
   registry->RegisterIntegerPref(prefs::kAssistantNumWarmerWelcomeTriggered, 0);
 }
 
-void AssistantControllerImpl::BindReceiver(
-    mojo::PendingReceiver<chromeos::assistant::mojom::AssistantController>
-        receiver) {
-  assistant_controller_receivers_.Add(this, std::move(receiver));
-}
-
 void AssistantControllerImpl::BindReceiver(
     mojo::PendingReceiver<mojom::AssistantVolumeControl> receiver) {
   assistant_volume_control_receiver_.Bind(std::move(receiver));
@@ -93,14 +87,6 @@ void AssistantControllerImpl::SendAssistantFeedback(
   assistant_->SendAssistantFeedback(std::move(assistant_feedback));
 }
 
-void AssistantControllerImpl::StartTextInteraction(
-    const std::string& query,
-    bool allow_tts,
-    chromeos::assistant::mojom::AssistantQuerySource source) {
-  assistant_interaction_controller_.StartTextInteraction(query, allow_tts,
-                                                         source);
-}
-
 void AssistantControllerImpl::StartSpeakerIdEnrollmentFlow() {
   if (assistant_state_controller_.consent_status().value_or(
           chromeos::assistant::prefs::ConsentStatus::kUnknown) ==
@@ -316,12 +302,6 @@ void AssistantControllerImpl::OnLockedFullScreenStateChanged(bool enabled) {
         chromeos::assistant::mojom::AssistantExitPoint::kUnspecified);
 }
 
-void AssistantControllerImpl::BindController(
-    mojo::PendingReceiver<chromeos::assistant::mojom::AssistantController>
-        receiver) {
-  BindReceiver(std::move(receiver));
-}
-
 void AssistantControllerImpl::BindAlarmTimerController(
     mojo::PendingReceiver<mojom::AssistantAlarmTimerController> receiver) {
   Shell::Get()->assistant_controller()->alarm_timer_controller()->BindReceiver(

ash/assistant/assistant_controller_impl.h

@@ -47,7 +47,6 @@ namespace ash {
 
 class ASH_EXPORT AssistantControllerImpl
     : public AssistantController,
-      public chromeos::assistant::mojom::AssistantController,
       public AssistantControllerObserver,
       public AssistantStateObserver,
       public mojom::AssistantVolumeControl,
@@ -60,9 +59,6 @@ class ASH_EXPORT AssistantControllerImpl
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
-  void BindReceiver(
-      mojo::PendingReceiver<chromeos::assistant::mojom::AssistantController>
-          receiver);
   void BindReceiver(
       mojo::PendingReceiver<mojom::AssistantVolumeControl> receiver);
 
@@ -77,19 +73,13 @@ class ASH_EXPORT AssistantControllerImpl
   void RemoveObserver(AssistantControllerObserver* observer) override;
   void OpenUrl(const GURL& url, bool in_background, bool from_server) override;
   base::WeakPtr<ash::AssistantController> GetWeakPtr() override;
-
-  // chromeos::assistant::mojom::AssistantController:
   // TODO(updowndota): Refactor Set() calls to use a factory pattern.
   void SetAssistant(mojo::PendingRemote<chromeos::assistant::mojom::Assistant>
                         assistant) override;
   void StartSpeakerIdEnrollmentFlow() override;
   void SendAssistantFeedback(bool assistant_debug_info_allowed,
                              const std::string& feedback_description,
                              const std::string& screenshot_png) override;
-  void StartTextInteraction(
-      const std::string& query,
-      bool allow_tts,
-      chromeos::assistant::mojom::AssistantQuerySource source) override;
 
   // AssistantControllerObserver:
   void OnDeepLinkReceived(
@@ -149,9 +139,6 @@ class ASH_EXPORT AssistantControllerImpl
   void OnLockedFullScreenStateChanged(bool enabled) override;
 
   // AssistantInterfaceBinder implementation:
-  void BindController(
-      mojo::PendingReceiver<chromeos::assistant::mojom::AssistantController>
-          receiver) override;
   void BindAlarmTimerController(
       mojo::PendingReceiver<mojom::AssistantAlarmTimerController> receiver)
       override;
@@ -170,9 +157,6 @@ class ASH_EXPORT AssistantControllerImpl
   // register as observers during their construction.
   base::ObserverList<AssistantControllerObserver> observers_;
 
-  mojo::ReceiverSet<chromeos::assistant::mojom::AssistantController>
-      assistant_controller_receivers_;
-
   mojo::Receiver<mojom::AssistantVolumeControl>
       assistant_volume_control_receiver_{this};
   mojo::RemoteSet<mojom::VolumeObserver> volume_observers_;

ash/assistant/assistant_interaction_controller_impl.cc

@@ -149,6 +149,20 @@ void AssistantInteractionControllerImpl::RemoveModelObserver(
   model_.RemoveObserver(observer);
 }
 
+void AssistantInteractionControllerImpl::StartTextInteraction(
+    const std::string& text,
+    bool allow_tts,
+    AssistantQuerySource query_source) {
+  DCHECK(assistant_);
+
+  StopActiveInteraction(false);
+
+  model_.SetPendingQuery(
+      std::make_unique<AssistantTextQuery>(text, query_source));
+
+  assistant_->StartTextInteraction(text, query_source, allow_tts);
+}
+
 void AssistantInteractionControllerImpl::OnAssistantControllerConstructed() {
   AssistantUiController::Get()->AddModelObserver(this);
   assistant_controller_->view_delegate()->AddObserver(this);
@@ -998,20 +1012,6 @@ void AssistantInteractionControllerImpl::StartScreenContextInteraction(
           screen_context_request_factory_.GetWeakPtr()));
 }
 
-void AssistantInteractionControllerImpl::StartTextInteraction(
-    const std::string& text,
-    bool allow_tts,
-    AssistantQuerySource query_source) {
-  DCHECK(assistant_);
-
-  StopActiveInteraction(false);
-
-  model_.SetPendingQuery(
-      std::make_unique<AssistantTextQuery>(text, query_source));
-
-  assistant_->StartTextInteraction(text, query_source, allow_tts);
-}
-
 void AssistantInteractionControllerImpl::StartVoiceInteraction() {
   StopActiveInteraction(false);
 

ash/assistant/assistant_interaction_controller_impl.h

@@ -70,6 +70,9 @@ class AssistantInteractionControllerImpl
   const AssistantInteractionModel* GetModel() const override;
   void AddModelObserver(AssistantInteractionModelObserver*) override;
   void RemoveModelObserver(AssistantInteractionModelObserver*) override;
+  void StartTextInteraction(const std::string& text,
+                            bool allow_tts,
+                            AssistantQuerySource query_source) override;
 
   // AssistantControllerObserver:
   void OnAssistantControllerConstructed() override;
@@ -126,10 +129,6 @@ class AssistantInteractionControllerImpl
   void OnTabletModeStarted() override;
   void OnTabletModeEnded() override;
 
-  void StartTextInteraction(const std::string& text,
-                            bool allow_tts,
-                            AssistantQuerySource query_source);
-
  private:
   void OnTabletModeChanged();
 

ash/public/cpp/assistant/assistant_interface_binder.h

@@ -21,9 +21,6 @@ class ASH_PUBLIC_EXPORT AssistantInterfaceBinder {
   static AssistantInterfaceBinder* GetInstance();
   static void SetInstance(AssistantInterfaceBinder* binder);
 
-  virtual void BindController(
-      mojo::PendingReceiver<chromeos::assistant::mojom::AssistantController>
-          receiver) = 0;
   virtual void BindAlarmTimerController(
       mojo::PendingReceiver<mojom::AssistantAlarmTimerController> receiver) = 0;
   virtual void BindNotificationController(

ash/public/cpp/assistant/controller/assistant_controller.h

@@ -5,8 +5,12 @@
 #ifndef ASH_PUBLIC_CPP_ASSISTANT_CONTROLLER_ASSISTANT_CONTROLLER_H_
 #define ASH_PUBLIC_CPP_ASSISTANT_CONTROLLER_ASSISTANT_CONTROLLER_H_
 
+#include <string>
+
 #include "ash/public/cpp/ash_public_export.h"
 #include "base/memory/weak_ptr.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 
 class GURL;
 
@@ -33,6 +37,21 @@ class ASH_PUBLIC_EXPORT AssistantController {
   // Returns a weak pointer to this instance.
   virtual base::WeakPtr<AssistantController> GetWeakPtr() = 0;
 
+  // Provides a reference to the underlying |assistant| service.
+  virtual void SetAssistant(
+      mojo::PendingRemote<chromeos::assistant::mojom::Assistant> assistant) = 0;
+
+  // Methods below may only be called after |SetAssistant| is called.
+  // Show speaker id enrollment flow.
+  virtual void StartSpeakerIdEnrollmentFlow() = 0;
+
+  // Send Assistant feedback to Assistant server. If |pii_allowed| is
+  // true then the user gives permission to attach Assistant debug info.
+  // |feedback_description| is user's feedback input.
+  virtual void SendAssistantFeedback(bool pii_allowed,
+                                     const std::string& feedback_description,
+                                     const std::string& screenshot_png) = 0;
+
  protected:
   AssistantController();
   virtual ~AssistantController();

ash/public/cpp/assistant/controller/assistant_interaction_controller.h

@@ -5,7 +5,10 @@
 #ifndef ASH_PUBLIC_CPP_ASSISTANT_CONTROLLER_ASSISTANT_INTERACTION_CONTROLLER_H_
 #define ASH_PUBLIC_CPP_ASSISTANT_CONTROLLER_ASSISTANT_INTERACTION_CONTROLLER_H_
 
+#include <string>
+
 #include "ash/public/cpp/ash_public_export.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom-forward.h"
 
 namespace ash {
 
@@ -25,6 +28,12 @@ class ASH_PUBLIC_EXPORT AssistantInteractionController {
   virtual void AddModelObserver(AssistantInteractionModelObserver*) = 0;
   virtual void RemoveModelObserver(AssistantInteractionModelObserver*) = 0;
 
+  // Start Assistant text interaction.
+  virtual void StartTextInteraction(
+      const std::string& query,
+      bool allow_tts,
+      chromeos::assistant::mojom::AssistantQuerySource source) = 0;
+
  protected:
   AssistantInteractionController();
   virtual ~AssistantInteractionController();

ash/public/cpp/assistant/test_support/mock_assistant_controller.h

@@ -5,6 +5,8 @@
 #ifndef ASH_PUBLIC_CPP_ASSISTANT_TEST_SUPPORT_MOCK_ASSISTANT_CONTROLLER_H_
 #define ASH_PUBLIC_CPP_ASSISTANT_TEST_SUPPORT_MOCK_ASSISTANT_CONTROLLER_H_
 
+#include <string>
+
 #include "ash/public/cpp/assistant/controller/assistant_controller.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "url/gurl.h"
@@ -28,6 +30,21 @@ class MockAssistantController : public AssistantController {
               (override));
 
   MOCK_METHOD(base::WeakPtr<AssistantController>, GetWeakPtr, (), (override));
+
+  MOCK_METHOD(
+      void,
+      SetAssistant,
+      (mojo::PendingRemote<chromeos::assistant::mojom::Assistant> assistant),
+      (override));
+
+  MOCK_METHOD(void, StartSpeakerIdEnrollmentFlow, (), (override));
+
+  MOCK_METHOD(void,
+              SendAssistantFeedback,
+              (bool pii_allowed,
+               const std::string& feedback_description,
+               const std::string& screenshot_png),
+              (override));
 };
 
 }  // namespace ash

ash/quick_answers/quick_answers_ui_controller.cc

@@ -4,7 +4,7 @@
 
 #include "ash/quick_answers/quick_answers_ui_controller.h"
 
-#include "ash/public/cpp/assistant/assistant_interface_binder.h"
+#include "ash/public/cpp/assistant/controller/assistant_interaction_controller.h"
 #include "ash/quick_answers/quick_answers_controller_impl.h"
 #include "ash/quick_answers/ui/quick_answers_view.h"
 #include "ash/quick_answers/ui/user_consent_view.h"
@@ -44,11 +44,7 @@ void QuickAnswersUiController::CreateQuickAnswersView(
 
 void QuickAnswersUiController::OnQuickAnswersViewPressed() {
   CloseQuickAnswersView();
-  mojo::Remote<chromeos::assistant::mojom::AssistantController>
-      assistant_controller;
-  ash::AssistantInterfaceBinder::GetInstance()->BindController(
-      assistant_controller.BindNewPipeAndPassReceiver());
-  assistant_controller->StartTextInteraction(
+  ash::AssistantInteractionController::Get()->StartTextInteraction(
       query_, /*allow_tts=*/false,
       chromeos::assistant::mojom::AssistantQuerySource::kQuickAnswers);
   controller_->OnQuickAnswerClick();

chrome/browser/renderer_context_menu/quick_answers_menu_observer.cc

@@ -6,7 +6,7 @@
 
 #include <utility>
 
-#include "ash/public/cpp/assistant/assistant_interface_binder.h"
+#include "ash/public/cpp/assistant/controller/assistant_interaction_controller.h"
 #include "ash/public/cpp/quick_answers_controller.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -234,11 +234,7 @@ void QuickAnswersMenuObserver::SetQuickAnswerClientForTesting(
 }
 
 void QuickAnswersMenuObserver::SendAssistantQuery(const std::string& query) {
-  mojo::Remote<chromeos::assistant::mojom::AssistantController>
-      assistant_controller;
-  ash::AssistantInterfaceBinder::GetInstance()->BindController(
-      assistant_controller.BindNewPipeAndPassReceiver());
-  assistant_controller->StartTextInteraction(
+  ash::AssistantInteractionController::Get()->StartTextInteraction(
       query, /*allow_tts=*/false,
       chromeos::assistant::mojom::AssistantQuerySource::kQuickAnswers);
 }

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

@@ -136,13 +136,6 @@ void AssistantClientImpl::OnAssistantStatusChanged(
   ash::AssistantState::Get()->NotifyStatusChanged(new_state);
 }
 
-void AssistantClientImpl::RequestAssistantController(
-    mojo::PendingReceiver<chromeos::assistant::mojom::AssistantController>
-        receiver) {
-  ash::AssistantInterfaceBinder::GetInstance()->BindController(
-      std::move(receiver));
-}
-
 void AssistantClientImpl::RequestAssistantAlarmTimerController(
     mojo::PendingReceiver<ash::mojom::AssistantAlarmTimerController> receiver) {
   ash::AssistantInterfaceBinder::GetInstance()->BindAlarmTimerController(

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

@@ -58,9 +58,6 @@ class AssistantClientImpl : public ash::AssistantClient,
 
   // chromeos::assistant::AssisantClient overrides:
   void OnAssistantStatusChanged(ash::mojom::AssistantState new_state) override;
-  void RequestAssistantController(
-      mojo::PendingReceiver<chromeos::assistant::mojom::AssistantController>
-          receiver) override;
   void RequestAssistantAlarmTimerController(
       mojo::PendingReceiver<ash::mojom::AssistantAlarmTimerController> receiver)
       override;

chromeos/services/assistant/BUILD.gn

@@ -134,6 +134,7 @@ source_set("tests") {
   testonly = true
   deps = [
     ":lib",
+    "//ash/public/cpp/assistant/test_support",
     "//ash/public/mojom",
     "//base",
     "//base/test:test_support",

chromeos/services/assistant/assistant_settings_manager_impl.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "ash/public/cpp/assistant/assistant_state_base.h"
+#include "ash/public/cpp/assistant/controller/assistant_controller.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
@@ -267,8 +268,7 @@ ash::AssistantStateBase* AssistantSettingsManagerImpl::assistant_state() {
   return context_->assistant_state();
 }
 
-mojom::AssistantController*
-AssistantSettingsManagerImpl::assistant_controller() {
+ash::AssistantController* AssistantSettingsManagerImpl::assistant_controller() {
   return context_->assistant_controller();
 }
 

chromeos/services/assistant/assistant_settings_manager_impl.h

@@ -16,6 +16,7 @@
 #include "mojo/public/cpp/bindings/remote.h"
 
 namespace ash {
+class AssistantController;
 class AssistantStateBase;
 }  // namespace ash
 
@@ -24,14 +25,6 @@ struct SpeakerIdEnrollmentStatus;
 struct SpeakerIdEnrollmentUpdate;
 }  // namespace assistant_client
 
-namespace chromeos {
-namespace assistant {
-namespace mojom {
-class AssistantController;
-}  // namespace mojom
-}  // namespace assistant
-}  // namespace chromeos
-
 namespace chromeos {
 namespace assistant {
 
@@ -77,7 +70,7 @@ class AssistantSettingsManagerImpl : public AssistantSettingsManager {
                                   const std::string& settings);
 
   ash::AssistantStateBase* assistant_state();
-  mojom::AssistantController* assistant_controller();
+  ash::AssistantController* assistant_controller();
   scoped_refptr<base::SequencedTaskRunner> main_task_runner();
 
   ServiceContext* const context_;

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

@@ -28,10 +28,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) AssistantClient {
   virtual void OnAssistantStatusChanged(
       ash::mojom::AssistantState new_state) = 0;
 
-  // Requests an AssistantController implementation from Ash, via the browser.
-  virtual void RequestAssistantController(
-      mojo::PendingReceiver<mojom::AssistantController> receiver) = 0;
-
   // Requests Ash's AssistantAlarmTimerController interface from the browser.
   virtual void RequestAssistantAlarmTimerController(
       mojo::PendingReceiver<ash::mojom::AssistantAlarmTimerController>