Following OpenUrl for opening Android apps

Currently opening android app does not clear the Assistant UI state,
which is not ideal. Following the OpenUrl path through ash controllers
instead of directly call DeviceActions could solve the issue.

Bug: b/130315570
Test: Manual Test
Change-Id: Id456aa1b821d7e024ebcf9edc3b8e39ff548662f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809068
Commit-Queue: Yue Li <updowndota@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699531}

ash/assistant/assistant_controller.cc

@@ -233,14 +233,14 @@ void AssistantController::OnAccessibilityStatusChanged() {
 void AssistantController::OpenUrl(const GURL& url,
                                   bool in_background,
                                   bool from_server) {
-  auto* android_helper = AndroidIntentHelper::GetInstance();
-  if (url.SchemeIs(kAndroidIntentScheme) && android_helper) {
-    android_helper->LaunchAndroidIntent(url.spec());
+  if (assistant::util::IsDeepLinkUrl(url)) {
+    NotifyDeepLinkReceived(url);
     return;
   }
 
-  if (assistant::util::IsDeepLinkUrl(url)) {
-    NotifyDeepLinkReceived(url);
+  auto* android_helper = AndroidIntentHelper::GetInstance();
+  if (url.SchemeIs(kAndroidIntentScheme) && !android_helper) {
+    NOTREACHED();
     return;
   }
 
@@ -248,14 +248,18 @@ void AssistantController::OpenUrl(const GURL& url,
   // open the specified |url| in a new browser tab.
   NotifyOpeningUrl(url, in_background, from_server);
 
-  // The new tab should be opened with a user activation since the user
-  // interacted with the Assistant to open the url. |in_background| describes
-  // the relationship between |url| and Assistant UI, not the browser. As such,
-  // the browser will always be instructed to open |url| in a new browser tab
-  // and Assistant UI state will be updated downstream to respect
-  // |in_background|.
-  NewWindowDelegate::GetInstance()->NewTabWithUrl(
-      url, /*from_user_interaction=*/true);
+  if (url.SchemeIs(kAndroidIntentScheme)) {
+    android_helper->LaunchAndroidIntent(url.spec());
+  } else {
+    // The new tab should be opened with a user activation since the user
+    // interacted with the Assistant to open the url. |in_background| describes
+    // the relationship between |url| and Assistant UI, not the browser. As
+    // such, the browser will always be instructed to open |url| in a new
+    // browser tab and Assistant UI state will be updated downstream to respect
+    // |in_background|.
+    NewWindowDelegate::GetInstance()->NewTabWithUrl(
+        url, /*from_user_interaction=*/true);
+  }
   NotifyUrlOpened(url, from_server);
 }
 

ash/assistant/assistant_interaction_controller.cc

@@ -19,6 +19,7 @@
 #include "ash/assistant/util/assistant_util.h"
 #include "ash/assistant/util/deep_link_util.h"
 #include "ash/assistant/util/histogram_util.h"
+#include "ash/public/cpp/android_intent_helper.h"
 #include "ash/public/cpp/app_list/app_list_features.h"
 #include "ash/public/cpp/ash_pref_names.h"
 #include "ash/public/cpp/assistant/assistant_setup.h"
@@ -29,6 +30,7 @@
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "base/bind.h"
 #include "base/optional.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chromeos/services/assistant/public/features.h"
 #include "components/prefs/pref_service.h"
@@ -41,6 +43,8 @@ namespace ash {
 namespace {
 
 constexpr int kWarmerWelcomesMaxTimesTriggered = 3;
+constexpr char kAndroidIntentScheme[] = "intent://";
+constexpr char kAndroidIntentPrefix[] = "#Intent";
 
 // Helpers ---------------------------------------------------------------------
 
@@ -657,6 +661,40 @@ void AssistantInteractionController::OnOpenUrlResponse(const GURL& url,
   assistant_controller_->OpenUrl(url, in_background, /*from_server=*/true);
 }
 
+void AssistantInteractionController::OnOpenAppResponse(
+    chromeos::assistant::mojom::AndroidAppInfoPtr app_info,
+    OnOpenAppResponseCallback callback) {
+  if (model_.interaction_state() != InteractionState::kActive)
+    return;
+
+  auto* android_helper = AndroidIntentHelper::GetInstance();
+  if (!android_helper) {
+    std::move(callback).Run(false);
+    return;
+  }
+
+  auto intent = android_helper->GetAndroidAppLaunchIntent(std::move(app_info));
+  if (!intent.has_value()) {
+    std::move(callback).Run(false);
+    return;
+  }
+
+  // Common Android intent might starts with intent scheme "intent://" or
+  // Android app scheme "android-app://". But it might also only contains
+  // reference starts with "#Intent".
+  // However, GURL requires the URL spec to be non-empty, which invalidate the
+  // intent starts with "#Intent". For this case, we adding the Android intent
+  // scheme to the intent to validate it for GURL constructor.
+  auto intent_str = intent.value();
+  if (base::StartsWith(intent_str, kAndroidIntentPrefix,
+                       base::CompareCase::SENSITIVE)) {
+    intent_str = kAndroidIntentScheme + intent_str;
+  }
+  assistant_controller_->OpenUrl(GURL(intent_str), /*in_background=*/false,
+                                 /*from_server=*/true);
+  std::move(callback).Run(true);
+}
+
 void AssistantInteractionController::OnDialogPlateButtonPressed(
     AssistantButtonId id) {
   if (id == AssistantButtonId::kKeyboardInputToggle) {

ash/assistant/assistant_interaction_controller.h

@@ -98,6 +98,8 @@ class AssistantInteractionController
       std::vector<AssistantSuggestionPtr> response) override;
   void OnTextResponse(const std::string& response) override;
   void OnOpenUrlResponse(const GURL& url, bool in_background) override;
+  void OnOpenAppResponse(chromeos::assistant::mojom::AndroidAppInfoPtr app_info,
+                         OnOpenAppResponseCallback callback) override;
   void OnSpeechRecognitionStarted() override;
   void OnSpeechRecognitionIntermediateResult(
       const std::string& high_confidence_text,

ash/public/cpp/android_intent_helper.h

@@ -9,6 +9,7 @@
 
 #include "ash/public/cpp/ash_public_export.h"
 #include "base/macros.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 
 namespace ash {
 
@@ -17,8 +18,14 @@ class ASH_PUBLIC_EXPORT AndroidIntentHelper {
  public:
   static AndroidIntentHelper* GetInstance();
 
+  // Launch the given Android |intent|.
   virtual void LaunchAndroidIntent(const std::string& intent) = 0;
 
+  // Get the intent that can be used to launch an Android activity specified by
+  // the |app_info|.
+  virtual base::Optional<std::string> GetAndroidAppLaunchIntent(
+      chromeos::assistant::mojom::AndroidAppInfoPtr app_info) = 0;
+
  protected:
   AndroidIntentHelper();
   virtual ~AndroidIntentHelper();

chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.h

@@ -539,6 +539,8 @@ class AutotestPrivateSendAssistantTextQueryFunction
   void OnSuggestionsResponse(
       std::vector<AssistantSuggestionPtr> response) override {}
   void OnOpenUrlResponse(const GURL& url, bool in_background) override {}
+  void OnOpenAppResponse(chromeos::assistant::mojom::AndroidAppInfoPtr app_info,
+                         OnOpenAppResponseCallback callback) override {}
   void OnSpeechRecognitionStarted() override {}
   void OnSpeechRecognitionIntermediateResult(
       const std::string& high_confidence_text,

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

@@ -241,6 +241,16 @@ void DeviceActions::AddAppListEventSubscriber(
     scoped_prefs_observer_.Add(prefs);
 }
 
+base::Optional<std::string> DeviceActions::GetAndroidAppLaunchIntent(
+    chromeos::assistant::mojom::AndroidAppInfoPtr app_info) {
+  app_info->status = GetAndroidAppStatus(app_info->package_name);
+
+  if (app_info->status != AppStatus::AVAILABLE)
+    return base::nullopt;
+
+  return GetLaunchIntent(std::move(app_info));
+}
+
 void DeviceActions::OnPackageListInitialRefreshed() {
   NotifyAndroidAppListRefreshed(app_list_subscribers_);
 }

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

@@ -38,6 +38,10 @@ class DeviceActions : public ash::AndroidIntentHelper,
       chromeos::assistant::mojom::AppListEventSubscriberPtr subscriber)
       override;
 
+  // ash::AndroidIntentHelper overrides:
+  base::Optional<std::string> GetAndroidAppLaunchIntent(
+      chromeos::assistant::mojom::AndroidAppInfoPtr app_info) override;
+
  private:
   // ArcAppListPrefs::Observer overrides.
   void OnPackageListInitialRefreshed() override;

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -665,10 +665,13 @@ void AssistantManagerServiceImpl::OnOpenAndroidApp(
                      interaction);
   mojom::AndroidAppInfoPtr app_info_ptr = mojom::AndroidAppInfo::New();
   app_info_ptr->package_name = app_info.package_name;
-  device_actions()->OpenAndroidApp(
-      std::move(app_info_ptr),
-      base::BindOnce(&AssistantManagerServiceImpl::HandleOpenAndroidAppResponse,
-                     weak_factory_.GetWeakPtr(), interaction));
+  for (auto& it : interaction_subscribers_) {
+    it->OnOpenAppResponse(
+        mojo::Clone(app_info_ptr),
+        base::BindOnce(
+            &AssistantManagerServiceImpl::HandleOpenAndroidAppResponse,
+            weak_factory_.GetWeakPtr(), interaction));
+  }
 }
 
 void AssistantManagerServiceImpl::OnVerifyAndroidApp(
@@ -704,11 +707,13 @@ void AssistantManagerServiceImpl::OnOpenMediaAndroidIntentOnMainThread(
       app_info_ptr->intent = url;
     }
   }
-  device_actions()->OpenAndroidApp(
-      std::move(app_info_ptr),
-      base::BindOnce(
-          &AssistantManagerServiceImpl::HandleLaunchMediaIntentResponse,
-          weak_factory_.GetWeakPtr()));
+  for (auto& it : interaction_subscribers_) {
+    it->OnOpenAppResponse(
+        mojo::Clone(app_info_ptr),
+        base::BindOnce(
+            &AssistantManagerServiceImpl::HandleLaunchMediaIntentResponse,
+            weak_factory_.GetWeakPtr()));
+  }
 }
 
 void AssistantManagerServiceImpl::OnPlayMedia(

chromeos/services/assistant/public/mojom/assistant.mojom

@@ -174,6 +174,9 @@ interface AssistantInteractionSubscriber {
   // being in background of Assistant UI, not in background of browser.
   OnOpenUrlResponse(url.mojom.Url url, bool in_background);
 
+  // Assistant got open Android app response from server.
+  OnOpenAppResponse(AndroidAppInfo app_info) => (bool app_opened);
+
   // Assistant speech recognition has started.
   OnSpeechRecognitionStarted();