Moves open URL logic to AssistantController.

Logic was previously in AssistantInteractionController.

Bug: b:80542452
Change-Id: I3e92d2ed8cbf1afa3577a7180eef5260a592f8d6
Reviewed-on: https://chromium-review.googlesource.com/1112656
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570291}

ash/assistant/assistant_controller.cc

@@ -6,6 +6,7 @@
 
 #include "ash/assistant/assistant_interaction_controller.h"
 #include "ash/assistant/assistant_ui_controller.h"
+#include "ash/new_window_controller.h"
 #include "ash/session/session_controller.h"
 #include "ash/shell.h"
 #include "base/bind.h"
@@ -17,7 +18,7 @@ namespace ash {
 
 AssistantController::AssistantController()
     : assistant_interaction_controller_(
-          std::make_unique<AssistantInteractionController>()),
+          std::make_unique<AssistantInteractionController>(this)),
       assistant_ui_controller_(std::make_unique<AssistantUiController>(this)) {
   // Note that the sub-controllers have a circular dependency.
   // TODO(dmblack): Remove this circular dependency.
@@ -107,10 +108,9 @@ void AssistantController::ManageWebContents(
   params->account_id = user_session->user_info->account_id;
 
   // Specify that we will handle top level browser requests.
-  // TODO(dmblack): Make AssistantController the OpenUrl delegate.
   ash::mojom::ManagedWebContentsOpenUrlDelegatePtr ptr;
-  web_contents_open_url_delegate_bindings_.AddBinding(
-      assistant_interaction_controller_.get(), mojo::MakeRequest(&ptr));
+  web_contents_open_url_delegate_bindings_.AddBinding(this,
+                                                      mojo::MakeRequest(&ptr));
   params->open_url_delegate_ptr_info = ptr.PassInterface();
 
   web_contents_manager_->ManageWebContents(id_token, std::move(params),
@@ -151,4 +151,15 @@ void AssistantController::OnHighlighterSelectionRecognized(
   NOTIMPLEMENTED();
 }
 
+void AssistantController::OnOpenUrlFromTab(const GURL& url) {
+  OpenUrl(url);
+}
+
+void AssistantController::OpenUrl(const GURL& url) {
+  Shell::Get()->new_window_controller()->NewTabWithUrl(url);
+
+  // We dismiss Assistant UI when opening a new browser tab.
+  assistant_ui_controller_->HideUi(AssistantSource::kUnspecified);
+}
+
 }  // namespace ash

ash/assistant/assistant_controller.h

@@ -29,7 +29,8 @@ class AssistantInteractionController;
 class AssistantUiController;
 
 class AssistantController : public mojom::AssistantController,
-                            public HighlighterController::Observer {
+                            public HighlighterController::Observer,
+                            public mojom::ManagedWebContentsOpenUrlDelegate {
  public:
   AssistantController();
   ~AssistantController() override;
@@ -75,6 +76,13 @@ class AssistantController : public mojom::AssistantController,
   // HighlighterController::Observer:
   void OnHighlighterSelectionRecognized(const gfx::Rect& rect) override;
 
+  // mojom::ManagedWebContentsOpenUrlDelegate:
+  void OnOpenUrlFromTab(const GURL& url) override;
+
+  // Opens the specified |url| in a new browser tab.
+  // TODO(dmblack): Support opening specific URLs in the Assistant container.
+  void OpenUrl(const GURL& url);
+
   AssistantInteractionController* interaction_controller() {
     DCHECK(assistant_interaction_controller_);
     return assistant_interaction_controller_.get();

ash/assistant/assistant_interaction_controller.cc

@@ -9,14 +9,15 @@
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
 #include "ash/assistant/model/assistant_query.h"
 #include "ash/assistant/model/assistant_ui_element.h"
-#include "ash/new_window_controller.h"
 #include "ash/shell.h"
 #include "base/strings/utf_string_conversions.h"
 
 namespace ash {
 
-AssistantInteractionController::AssistantInteractionController()
-    : assistant_event_subscriber_binding_(this) {
+AssistantInteractionController::AssistantInteractionController(
+    AssistantController* assistant_controller)
+    : assistant_controller_(assistant_controller),
+      assistant_event_subscriber_binding_(this) {
   AddModelObserver(this);
   Shell::Get()->highlighter_controller()->AddObserver(this);
 }
@@ -144,7 +145,7 @@ void AssistantInteractionController::OnSuggestionChipPressed(int id) {
   // If the suggestion contains a non-empty action url, we will handle the
   // suggestion chip pressed event by launching the action url in the browser.
   if (!suggestion->action_url.is_empty()) {
-    OpenUrl(suggestion->action_url);
+    assistant_controller_->OpenUrl(suggestion->action_url);
     return;
   }
 
@@ -208,12 +209,7 @@ void AssistantInteractionController::OnOpenUrlResponse(const GURL& url) {
       InteractionState::kActive) {
     return;
   }
-
-  OpenUrl(url);
-}
-
-void AssistantInteractionController::OnOpenUrlFromTab(const GURL& url) {
-  OpenUrl(url);
+  assistant_controller_->OpenUrl(url);
 }
 
 void AssistantInteractionController::OnDialogPlateButtonPressed(
@@ -272,13 +268,4 @@ void AssistantInteractionController::StopActiveInteraction() {
   assistant_->StopActiveInteraction();
 }
 
-// TODO(dmblack): Move OpenUrl logic into AssistantController. The interaction
-// sub-controller shouldn't need to talk to the UI controller directly.
-void AssistantInteractionController::OpenUrl(const GURL& url) {
-  Shell::Get()->new_window_controller()->NewTabWithUrl(url);
-
-  if (assistant_ui_controller_)
-    assistant_ui_controller_->HideUi(AssistantSource::kUnspecified);
-}
-
 }  // namespace ash

ash/assistant/assistant_interaction_controller.h

@@ -14,13 +14,13 @@
 #include "ash/assistant/model/assistant_ui_model_observer.h"
 #include "ash/assistant/ui/dialog_plate/dialog_plate.h"
 #include "ash/highlighter/highlighter_controller.h"
-#include "ash/public/interfaces/web_contents_manager.mojom.h"
 #include "base/macros.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
 namespace ash {
 
+class AssistantController;
 class AssistantInteractionModelObserver;
 class AssistantUiController;
 
@@ -29,8 +29,7 @@ class AssistantInteractionController
       public AssistantInteractionModelObserver,
       public AssistantUiModelObserver,
       public HighlighterController::Observer,
-      public DialogPlateObserver,
-      public mojom::ManagedWebContentsOpenUrlDelegate {
+      public DialogPlateObserver {
  public:
   using AssistantSuggestion = chromeos::assistant::mojom::AssistantSuggestion;
   using AssistantSuggestionPtr =
@@ -38,7 +37,8 @@ class AssistantInteractionController
   using AssistantInteractionResolution =
       chromeos::assistant::mojom::AssistantInteractionResolution;
 
-  AssistantInteractionController();
+  explicit AssistantInteractionController(
+      AssistantController* assistant_controller);
   ~AssistantInteractionController() override;
 
   // Provides a pointer to the |assistant| owned by AssistantController.
@@ -87,9 +87,6 @@ class AssistantInteractionController
   void OnSpeechRecognitionFinalResult(const std::string& final_result) override;
   void OnSpeechLevelUpdated(float speech_level) override;
 
-  // mojom::ManagedWebContentsOpenUrlDelegate:
-  void OnOpenUrlFromTab(const GURL& url) override;
-
   // DialogPlateObserver:
   void OnDialogPlateButtonPressed(DialogPlateButtonId id) override;
   void OnDialogPlateContentsCommitted(const std::string& text) override;
@@ -101,6 +98,8 @@ class AssistantInteractionController
 
   void OpenUrl(const GURL& url);
 
+  AssistantController* const assistant_controller_;  // Owned by Shell.
+
   // Owned by AssistantController.
   chromeos::assistant::mojom::Assistant* assistant_ = nullptr;