Remove mojo methods for ash<->browser communication for assistant

Two methods in AssistantClient interface is actually for the
communication between ash and the browser (and the comment also
said so). This CL replaces those two methods by C++ along with
removing the comment.

- OpenAssistantSettings(); this can be replaced as a single
  function.
- SetDeviceActions(); this comes up with the idea of having an
  interface for holding browser-owned objects which are used by Ash.
  This is the superclass of AssistantClient so it's named so.
  Since AssistantImageDownloder and AssistantSetup are owned by
  AssistantClient, this CL makes the new ash::AssistantSetup own
  them. With that, the static GetInstance()s are removed.

TBR=stevenjb@chromium.org

Bug: 958193
Test: trybot
Change-Id: I2d308e1d59777b952409f0c2c9bda1392d9b5154
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638179
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665659}

ash/BUILD.gn

@@ -48,6 +48,7 @@ component("ash") {
     "multi_user/multi_user_window_manager_impl.h",
     "public/cpp/arc_custom_tab.h",
     "public/cpp/ash_prefs.h",
+    "public/cpp/assistant/assistant_settings.h",
     "public/cpp/docked_magnifier_controller.h",
     "public/cpp/first_run_helper.h",
     "public/cpp/multi_user_window_manager.h",
@@ -179,6 +180,7 @@ component("ash") {
     "assistant/assistant_notification_controller.h",
     "assistant/assistant_screen_context_controller.cc",
     "assistant/assistant_screen_context_controller.h",
+    "assistant/assistant_settings.cc",
     "assistant/assistant_setup_controller.cc",
     "assistant/assistant_setup_controller.h",
     "assistant/assistant_ui_controller.cc",

ash/assistant/assistant_controller.cc

@@ -9,6 +9,7 @@
 
 #include "ash/accessibility/accessibility_controller.h"
 #include "ash/assistant/util/deep_link_util.h"
+#include "ash/public/cpp/android_intent_helper.h"
 #include "ash/public/cpp/ash_pref_names.h"
 #include "ash/public/cpp/new_window_delegate.h"
 #include "ash/session/session_controller_impl.h"
@@ -102,11 +103,6 @@ void AssistantController::SetAssistant(
     observer.OnAssistantReady();
 }
 
-void AssistantController::OpenAssistantSettings() {
-  // Launch Assistant settings via deeplink.
-  OpenUrl(assistant::util::CreateAssistantSettingsDeepLink());
-}
-
 void AssistantController::SendAssistantFeedback(
     bool assistant_debug_info_allowed,
     const std::string& feedback_description,
@@ -120,11 +116,6 @@ void AssistantController::SendAssistantFeedback(
   assistant_->SendAssistantFeedback(std::move(assistant_feedback));
 }
 
-void AssistantController::SetDeviceActions(
-    chromeos::assistant::mojom::DeviceActionsPtr device_actions) {
-  device_actions_ = std::move(device_actions);
-}
-
 void AssistantController::StartSpeakerIdEnrollmentFlow() {
   mojom::ConsentStatus consent_status =
       Shell::Get()->voice_interaction_controller()->consent_status().value_or(
@@ -239,8 +230,9 @@ void AssistantController::OnAccessibilityStatusChanged() {
 }
 
 void AssistantController::OpenUrl(const GURL& url, bool from_server) {
-  if (url.SchemeIs(kAndroidIntentScheme) && device_actions_) {
-    device_actions_->LaunchAndroidIntent(url.spec());
+  auto* android_helper = AndroidIntentHelper::GetInstance();
+  if (url.SchemeIs(kAndroidIntentScheme) && android_helper) {
+    android_helper->LaunchAndroidIntent(url.spec());
     return;
   }
 

ash/assistant/assistant_controller.h

@@ -79,13 +79,10 @@ class ASH_EXPORT AssistantController
   // TODO(updowndota): Refactor Set() calls to use a factory pattern.
   void SetAssistant(
       chromeos::assistant::mojom::AssistantPtr assistant) override;
-  void OpenAssistantSettings() override;
   void StartSpeakerIdEnrollmentFlow() override;
   void SendAssistantFeedback(bool assistant_debug_info_allowed,
                              const std::string& feedback_description,
                              const std::string& screenshot_png) override;
-  void SetDeviceActions(
-      chromeos::assistant::mojom::DeviceActionsPtr device_actions) override;
 
   // AssistantControllerObserver:
   void OnDeepLinkReceived(
@@ -168,8 +165,6 @@ class ASH_EXPORT AssistantController
 
   chromeos::assistant::mojom::AssistantPtr assistant_;
 
-  chromeos::assistant::mojom::DeviceActionsPtr device_actions_;
-
   // Assistant sub-controllers.
   AssistantAlarmTimerController assistant_alarm_timer_controller_;
   AssistantCacheController assistant_cache_controller_;

ash/assistant/assistant_settings.cc

+// 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 "ash/public/cpp/assistant/assistant_settings.h"
+
+#include "ash/assistant/assistant_controller.h"
+#include "ash/assistant/util/deep_link_util.h"
+#include "ash/shell.h"
+
+namespace ash {
+
+void OpenAssistantSettings() {
+  // Launch Assistant settings via deeplink.
+  Shell::Get()->assistant_controller()->OpenUrl(
+      assistant::util::CreateAssistantSettingsDeepLink());
+}
+
+}  // namespace ash

ash/public/cpp/BUILD.gn

@@ -9,6 +9,8 @@ component("cpp") {
   sources = [
     "accelerators.cc",
     "accelerators.h",
+    "android_intent_helper.cc",
+    "android_intent_helper.h",
     "app_list/app_list_client.h",
     "app_list/app_list_config.cc",
     "app_list/app_list_config.h",

ash/public/cpp/android_intent_helper.cc

+// 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 "ash/public/cpp/android_intent_helper.h"
+#include "base/logging.h"
+
+namespace ash {
+namespace {
+AndroidIntentHelper* g_android_intent_helper = nullptr;
+}
+
+// static
+AndroidIntentHelper* AndroidIntentHelper::GetInstance() {
+  return g_android_intent_helper;
+}
+
+AndroidIntentHelper::AndroidIntentHelper() {
+  DCHECK(!g_android_intent_helper);
+  g_android_intent_helper = this;
+}
+
+AndroidIntentHelper::~AndroidIntentHelper() {
+  DCHECK_EQ(g_android_intent_helper, this);
+  g_android_intent_helper = this;
+}
+
+}  // namespace ash

ash/public/cpp/android_intent_helper.h

+// 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_ANDROID_INTENT_HELPER_H_
+#define ASH_PUBLIC_CPP_ANDROID_INTENT_HELPER_H_
+
+#include <string>
+
+#include "ash/public/cpp/ash_public_export.h"
+#include "base/macros.h"
+
+namespace ash {
+
+// The class which allows to launch Android intent from Ash.
+class ASH_PUBLIC_EXPORT AndroidIntentHelper {
+ public:
+  static AndroidIntentHelper* GetInstance();
+
+  virtual void LaunchAndroidIntent(const std::string& intent) = 0;
+
+ protected:
+  AndroidIntentHelper();
+  virtual ~AndroidIntentHelper();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AndroidIntentHelper);
+};
+
+}  // namespace ash
+
+#endif  // ASH_PUBLIC_CPP_ANDROID_INTENT_HELPER_H_

ash/public/cpp/assistant/assistant_settings.h

+// 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_SETTINGS_H_
+#define ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_SETTINGS_H_
+
+#include "ash/ash_export.h"
+
+namespace ash {
+
+// Opens Google Assistant settings.
+ASH_EXPORT void OpenAssistantSettings();
+
+}  // namespace ash
+
+#endif  // ASH_PUBLIC_CPP_ASSISTANT_ASSISTANT_SETTINGS_H_

ash/public/interfaces/assistant_controller.mojom

@@ -10,15 +10,11 @@ import "mojo/public/mojom/base/time.mojom";
 
 // Interface to AssistantController which is owned by Shell in Ash. This is
 // typically used by the Assistant service to provide the controller with an
-// interface to itself, as well as by the Assistant client in chrome/browser to
-// provide interfaces to delegates which depend on browser.
+// interface to itself.
 interface AssistantController {
   // Provides a reference to the underlying |assistant| service.
   SetAssistant(chromeos.assistant.mojom.Assistant assistant);
 
-  // Opens Google Assistant settings.
-  OpenAssistantSettings();
-
   // Show speaker id enrollment flow.
   StartSpeakerIdEnrollmentFlow();
 
@@ -29,9 +25,6 @@ interface AssistantController {
     bool pii_allowed,
     string feedback_description,
     string screenshot_png);
-
-  // Provides a reference to DeviceActions service.
-  SetDeviceActions(chromeos.assistant.mojom.DeviceActions device_actions);
 };
 
 enum AssistantTimerState {

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

@@ -6,8 +6,6 @@
 
 #include <utility>
 
-#include "ash/public/interfaces/assistant_controller.mojom.h"
-#include "ash/public/interfaces/constants.mojom.h"
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.h"
 #include "chrome/browser/chromeos/assistant/assistant_util.h"
@@ -68,10 +66,6 @@ void AssistantClient::MaybeInit(Profile* profile) {
   chromeos::assistant::mojom::ClientPtr client_ptr;
   client_binding_.Bind(mojo::MakeRequest(&client_ptr));
 
-  ash::mojom::AssistantControllerPtr assistant_controller;
-  connector->BindInterface(ash::mojom::kServiceName, &assistant_controller);
-  assistant_controller->SetDeviceActions(device_actions_.AddBinding());
-
   assistant_connection_->Init(std::move(client_ptr),
                               device_actions_.AddBinding());
 

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

@@ -5,13 +5,15 @@
 #ifndef CHROME_BROWSER_UI_ASH_ASSISTANT_DEVICE_ACTIONS_H_
 #define CHROME_BROWSER_UI_ASH_ASSISTANT_DEVICE_ACTIONS_H_
 
+#include "ash/public/cpp/android_intent_helper.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "mojo/public/cpp/bindings/interface_ptr_set.h"
 
-class DeviceActions : public chromeos::assistant::mojom::DeviceActions,
+class DeviceActions : public ash::AndroidIntentHelper,
+                      public chromeos::assistant::mojom::DeviceActions,
                       public ArcAppListPrefs::Observer {
  public:
   DeviceActions();

chrome/browser/ui/webui/settings/chromeos/google_assistant_handler.cc

@@ -6,9 +6,8 @@
 
 #include <utility>
 
+#include "ash/public/cpp/assistant/assistant_settings.h"
 #include "ash/public/cpp/assistant/assistant_setup.h"
-#include "ash/public/interfaces/assistant_controller.mojom.h"
-#include "ash/public/interfaces/constants.mojom.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/values.h"
@@ -52,14 +51,8 @@ void GoogleAssistantHandler::RegisterMessages() {
 void GoogleAssistantHandler::HandleShowGoogleAssistantSettings(
     const base::ListValue* args) {
   CHECK_EQ(0U, args->GetSize());
-  if (chromeos::switches::IsAssistantEnabled()) {
-    // Opens Google Assistant settings.
-    service_manager::Connector* connector =
-        content::BrowserContext::GetConnectorFor(profile_);
-    ash::mojom::AssistantControllerPtr assistant_controller;
-    connector->BindInterface(ash::mojom::kServiceName, &assistant_controller);
-    assistant_controller->OpenAssistantSettings();
-  }
+  if (chromeos::switches::IsAssistantEnabled())
+    ash::OpenAssistantSettings();
 }
 
 void GoogleAssistantHandler::HandleRetrainVoiceModel(