crostini: always launch terminal asynchronously

(cherry picked from commit 5e101e6ec50ba7657995ba4be80858ff377e8c81)

Bug: 1262890
Change-Id: Ica610358fc11746e0e9e38ab05794ba92fd87d82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258804
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Commit-Queue: Jason Lin <lxj@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#938688}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3278844
Cr-Commit-Position: refs/branch-heads/4664@{#1046}
Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}

diff --git a/chrome/browser/ash/crostini/crostini_terminal.cc b/chrome/browser/ash/crostini/crostini_terminal.cc
index fd542c7c..dba36edc 100644
--- a/chrome/browser/ash/crostini/crostini_terminal.cc
+++ b/chrome/browser/ash/crostini/crostini_terminal.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ash/crostini/crostini_terminal.h"
 
+#include "base/bind.h"
 #include "base/containers/fixed_flat_map.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/no_destructor.h"
@@ -14,7 +15,9 @@
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/ash/crostini/crostini_pref_names.h"
 #include "chrome/browser/ash/crostini/crostini_util.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/window_properties.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -24,6 +27,8 @@
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/grit/chrome_unscaled_resources.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "net/base/escape.h"
 #include "ui/base/base_window.h"
 #include "ui/base/window_open_disposition.h"
@@ -78,6 +83,29 @@
   return GURL(base::JoinString(pieces, "&args[]="));
 }
 
+void LaunchTerminalImpl(Profile* profile,
+                        const GURL& url,
+                        const apps::AppLaunchParams& params) {
+  // This function is called asynchronously, so we need to check whether
+  // `profile` is still valid first.
+  if (g_browser_process) {
+    auto* profile_manager = g_browser_process->profile_manager();
+    if (profile_manager && profile_manager->IsValidProfile(profile)) {
+      // This LaunchSystemWebAppImpl call is necessary. Terminal App uses its
+      // own CrostiniApps publisher for launching. Calling
+      // LaunchSystemWebAppAsync would ask AppService to launch the App, which
+      // routes the launch request to this function, resulting in a loop.
+      //
+      // System Web Apps managed by Web App publisher should call
+      // LaunchSystemWebAppAsync.
+      web_app::LaunchSystemWebAppImpl(profile, web_app::SystemAppType::TERMINAL,
+                                      url, params);
+      return;
+    }
+  }
+  LOG(WARNING) << "Profile becomes invalid. Abort launching terminal.";
+}
+
 }  // namespace
 
 void LaunchTerminal(Profile* profile,
@@ -101,15 +129,11 @@
   // force the VM to start.
   params->omit_from_session_restore = true;
 
-  // This LaunchSystemWebAppImpl call is necessary. Terminal App uses its own
-  // CrostiniApps publisher for launching. Calling LaunchSystemWebAppAsync
-  // would ask AppService to launch the App, which routes the launch request to
-  // this function, resulting in a loop.
-  //
-  // System Web Apps managed by Web App publisher should call
-  // LaunchSystemWebAppAsync.
-  web_app::LaunchSystemWebAppImpl(profile, web_app::SystemAppType::TERMINAL,
-                                  vsh_in_crosh_url, *params);
+  // Always launch asynchronously to avoid disturbing the caller. See
+  // https://crbug.com/1262890#c12 for more details.
+  content::GetUIThreadTaskRunner({})->PostTask(
+      FROM_HERE, base::BindOnce(LaunchTerminalImpl, profile, vsh_in_crosh_url,
+                                std::move(*params)));
 }
 
 void LaunchTerminalSettings(Profile* profile, int64_t display_id) {
@@ -123,17 +147,14 @@
   // Use an app pop window to host the settings page.
   params->disposition = WindowOpenDisposition::NEW_POPUP;
 
-  // This LaunchSystemWebAppImpl call is necessary. Terminal App uses its own
-  // CrostiniApps publisher for launching. Calling LaunchSystemWebAppAsync
-  // would ask AppService to launch the App, which routes the launch request to
-  // this function, resulting in a loop.
-  //
-  // System Web Apps managed by Web App publisher should call
-  // LaunchSystemWebAppAsync.
-  web_app::LaunchSystemWebAppImpl(
-      profile, web_app::SystemAppType::TERMINAL,
-      GURL(base::StrCat({chrome::kChromeUIUntrustedTerminalURL, path})),
-      *params);
+  // Always launch asynchronously to avoid disturbing the caller. See
+  // https://crbug.com/1262890#c12 for more details.
+  content::GetUIThreadTaskRunner({})->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          LaunchTerminalImpl, profile,
+          GURL(base::StrCat({chrome::kChromeUIUntrustedTerminalURL, path})),
+          std::move(*params)));
 }
 
 void RecordTerminalSettingsChangesUMAs(Profile* profile) {

diff --git a/chrome/browser/ui/ash/shelf/app_shortcut_shelf_item_controller_browsertest.cc b/chrome/browser/ui/ash/shelf/app_shortcut_shelf_item_controller_browsertest.cc
index 952ad9a..fb44b3e 100644
--- a/chrome/browser/ui/ash/shelf/app_shortcut_shelf_item_controller_browsertest.cc
+++ b/chrome/browser/ui/ash/shelf/app_shortcut_shelf_item_controller_browsertest.cc

@@ -13,6 +13,8 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chrome/browser/web_applications/system_web_apps/system_web_app_manager.h"
@@ -21,6 +23,33 @@
 #include "content/public/test/browser_test.h"
 #include "ui/events/event_constants.h"
 
+namespace {
+class Waiter : public BrowserListObserver {
+ public:
+  static Browser* WaitForNewBrowser() {
+    base::RunLoop loop;
+    Waiter waiter(loop.QuitClosure());
+    loop.Run();
+    return waiter.browser_;
+  }
+
+ private:
+  explicit Waiter(base::OnceClosure callback) : callback_{std::move(callback)} {
+    BrowserList::AddObserver(this);
+  }
+
+  ~Waiter() override { BrowserList::RemoveObserver(this); }
+
+  void OnBrowserAdded(Browser* browser) override {
+    browser_ = browser;
+    std::move(callback_).Run();
+  }
+
+  base::OnceClosure callback_;
+  Browser* browser_ = nullptr;
+};
+}  // namespace
+
 // Unit tests for the left click menu and interaction with the menu items. There
 // are integration tests in ./chrome_shelf_controller_browsertest.cc which
 // covers different cases in AppShortcutShelfItemController::ItemSelected().
@@ -47,7 +76,7 @@
 
   Browser* LaunchApp() {
     crostini::LaunchTerminal(browser()->profile());
-    return chrome::FindLastActive();
+    return Waiter::WaitForNewBrowser();
   }
 
   ash::ShelfItemDelegate* GetShelfItemDelegate() {

diff --git a/chrome/browser/ui/views/crostini/crostini_update_component_view_browsertest.cc b/chrome/browser/ui/views/crostini/crostini_update_component_view_browsertest.cc
index be1751d1..0d5c3e32 100644
--- a/chrome/browser/ui/views/crostini/crostini_update_component_view_browsertest.cc
+++ b/chrome/browser/ui/views/crostini/crostini_update_component_view_browsertest.cc

@@ -13,6 +13,8 @@
 #include "chrome/browser/ash/crostini/crostini_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/views/crostini/crostini_dialogue_browser_test_util.h"
 #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chrome/browser/web_applications/system_web_apps/system_web_app_manager.h"
@@ -25,6 +27,28 @@
 #include "content/public/test/browser_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+class Waiter : public BrowserListObserver {
+ public:
+  static void WaitForNewBrowser() {
+    base::RunLoop loop;
+    Waiter waiter(loop.QuitClosure());
+    loop.Run();
+  }
+
+ private:
+  explicit Waiter(base::OnceClosure callback) : callback_{std::move(callback)} {
+    BrowserList::AddObserver(this);
+  }
+
+  ~Waiter() override { BrowserList::RemoveObserver(this); }
+
+  void OnBrowserAdded(Browser*) override { std::move(callback_).Run(); }
+
+  base::OnceClosure callback_;
+};
+}  // namespace
+
 class CrostiniUpdateComponentViewBrowserTest
     : public CrostiniDialogBrowserTest {
  public:
@@ -132,6 +156,7 @@
   UnregisterTermina();
   crostini::LaunchCrostiniApp(browser()->profile(),
                               crostini::kCrostiniTerminalSystemAppId, 0);
+  Waiter::WaitForNewBrowser();
 
   // For Terminal System App, we must wait for browser to load.
   Browser* terminal_browser = web_app::FindSystemWebAppBrowser(