crostini: deprecate LaunchSystemWebApp Browser* return value

This is a preparation for implementing a new LaunchSystemWebApp API,
where we use AppService to launch apps and don't return the Browser
object hosting the app (because AppService doesn't return it to us).

This change is safe for terminal, because LaunchSystemWebApp won't fail
if it is called with appropriate parameters after OnAppsSynchronized.

The metrics for CrostiniResult::UNKNOWN_ERROR contains other error
origins, and don't reflect whether LaunchSystemWebApp fails.

Bug: 1154540
Change-Id: I517202293140a7907f8af061ecbffec3dac623e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576861Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836524}

chrome/browser/chromeos/crostini/crostini_terminal.cc

@@ -79,34 +79,34 @@ GURL GenerateVshInCroshUrl(Profile* profile,
 
 }  // namespace
 
-Browser* LaunchTerminal(Profile* profile,
-                        int64_t display_id,
-                        const ContainerId& container_id,
-                        const std::string& cwd,
-                        const std::vector<std::string>& terminal_args) {
+void LaunchTerminal(Profile* profile,
+                    int64_t display_id,
+                    const ContainerId& container_id,
+                    const std::string& cwd,
+                    const std::vector<std::string>& terminal_args) {
   GURL vsh_in_crosh_url =
       GenerateVshInCroshUrl(profile, container_id, cwd, terminal_args);
   auto params = web_app::CreateSystemWebAppLaunchParams(
       profile, web_app::SystemAppType::TERMINAL, display_id);
   if (!params.has_value()) {
     LOG(WARNING) << "Empty launch params for terminal";
-    return nullptr;
+    return;
   }
-  return web_app::LaunchSystemWebApp(profile, web_app::SystemAppType::TERMINAL,
-                                     vsh_in_crosh_url, std::move(*params));
+  web_app::LaunchSystemWebApp(profile, web_app::SystemAppType::TERMINAL,
+                              vsh_in_crosh_url, std::move(*params));
 }
 
-Browser* LaunchTerminalSettings(Profile* profile, int64_t display_id) {
+void LaunchTerminalSettings(Profile* profile, int64_t display_id) {
   auto params = web_app::CreateSystemWebAppLaunchParams(
       profile, web_app::SystemAppType::TERMINAL, display_id);
   if (!params.has_value()) {
     LOG(WARNING) << "Empty launch params for terminal";
-    return nullptr;
+    return;
   }
   std::string path = "html/terminal_settings.html";
   // Use an app pop window to host the settings page.
   params->disposition = WindowOpenDisposition::NEW_POPUP;
-  return web_app::LaunchSystemWebApp(
+  web_app::LaunchSystemWebApp(
       profile, web_app::SystemAppType::TERMINAL,
       GURL(base::StrCat({chrome::kChromeUIUntrustedTerminalURL, path})),
       std::move(*params));

chrome/browser/chromeos/crostini/crostini_terminal.h

@@ -13,7 +13,6 @@
 #include "ui/display/types/display_constants.h"
 #include "ui/gfx/geometry/point.h"
 
-class Browser;
 class Profile;
 
 namespace crostini {
@@ -99,17 +98,15 @@ enum class TerminalSetting {
 };
 
 // Launches the terminal tabbed app.
-Browser* LaunchTerminal(
-    Profile* profile,
-    int64_t display_id = display::kInvalidDisplayId,
-    const ContainerId& container_id = ContainerId::GetDefault(),
-    const std::string& cwd = "",
-    const std::vector<std::string>& terminal_args = {});
+void LaunchTerminal(Profile* profile,
+                    int64_t display_id = display::kInvalidDisplayId,
+                    const ContainerId& container_id = ContainerId::GetDefault(),
+                    const std::string& cwd = "",
+                    const std::vector<std::string>& terminal_args = {});
 
 // Launches the terminal settings popup window.
-Browser* LaunchTerminalSettings(
-    Profile* profile,
-    int64_t display_id = display::kInvalidDisplayId);
+void LaunchTerminalSettings(Profile* profile,
+                            int64_t display_id = display::kInvalidDisplayId);
 
 // Record which terminal settings have been changed by users.
 void RecordTerminalSettingsChangesUMAs(Profile* profile);

chrome/browser/chromeos/crostini/crostini_util.cc

@@ -146,10 +146,7 @@ void OnSharePathForLaunchApplication(
   if (app_id == kCrostiniTerminalSystemAppId) {
     // Use first file as 'cwd'.
     std::string cwd = !args.empty() ? args[0] : "";
-    if (!LaunchTerminal(profile, display_id, container_id, cwd)) {
-      return OnLaunchFailed(app_id, std::move(callback),
-                            "failed to launch terminal");
-    }
+    LaunchTerminal(profile, display_id, container_id, cwd);
     return OnApplicationLaunched(app_id, std::move(callback),
                                  crostini::CrostiniResult::SUCCESS, true, "");
   }
@@ -338,10 +335,7 @@ void LaunchCrostiniAppImpl(
 
     if (!requires_share) {
       RecordAppLaunchHistogram(CrostiniAppLaunchAppType::kTerminal);
-      if (!LaunchTerminal(profile, display_id, container_id, cwd.value())) {
-        RecordAppLaunchResultHistogram(crostini::CrostiniResult::UNKNOWN_ERROR);
-        return std::move(callback).Run(false, "failed to launch terminal");
-      }
+      LaunchTerminal(profile, display_id, container_id, cwd.value());
       RecordAppLaunchResultHistogram(crostini::CrostiniResult::SUCCESS);
       return std::move(callback).Run(true, "");
     }