Allow VM terminal apps to run

Apps such as vim.desktop set Terminal=true and expect to run inside a terminal. We can launch these apps in the Terminal System App with the args provided. Bug: 853560 Change-Id: Id0c3352f3bb569a588841b7233a81de7460a0ce3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4567780 Reviewed-by: Jason Lin <lxj@google.com> Commit-Queue: Joel Hockey <joelhockey@chromium.org> Cr-Commit-Position: refs/heads/main@{#1150409}
chromium · May 30, 2023 · 2aed0b6 · 2aed0b6
1 parent 57b220f
commit 2aed0b6
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 16 deletions.
diff --git a/chrome/browser/ash/crostini/crostini_util.cc b/chrome/browser/ash/crostini/crostini_util.cc
@@ -30,6 +30,7 @@
 #include "chrome/browser/ash/guest_os/guest_os_registry_service_factory.h"
 #include "chrome/browser/ash/guest_os/guest_os_session_tracker.h"
 #include "chrome/browser/ash/guest_os/guest_os_share_path.h"
+#include "chrome/browser/ash/guest_os/guest_os_terminal.h"
 #include "chrome/browser/ash/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/shelf/app_service/app_service_app_window_crostini_tracker.h"
@@ -104,33 +105,52 @@ void OnLaunchFailed(const std::string& app_id,
 
 void OnSharePathForLaunchApplication(
     Profile* profile,
-    const std::string& desktop_file_id,
+    const std::string& app_id,
     guest_os::GuestOsRegistryService::Registration registration,
+    const guest_os::GuestId& container_id,
     int64_t display_id,
     const std::vector<std::string>& args,
     crostini::CrostiniSuccessCallback callback,
     bool success,
     const std::string& failure_reason) {
   if (!success) {
-    return OnLaunchFailed(desktop_file_id, std::move(callback),
-                          "Failed to share paths to launch " + desktop_file_id +
-                              ": " + failure_reason,
-                          CrostiniResult::SHARE_PATHS_FAILED);
+    return OnLaunchFailed(
+        app_id, std::move(callback),
+        "Failed to share paths to launch " + app_id + ": " + failure_reason,
+        CrostiniResult::SHARE_PATHS_FAILED);
+  }
+
+  if (registration.Terminal()) {
+    // TODO(crbug.com/853560): This could be improved by using garcon
+    // DesktopFile::GenerateArgvWithFiles().
+    std::vector<std::string> terminal_args = {
+        registration.ExecutableFileName()};
+    terminal_args.insert(terminal_args.end(), args.begin(), args.end());
+    guest_os::LaunchTerminal(profile, display_id, container_id,
+                             /*cwd=*/std::string(), terminal_args);
+    OnApplicationLaunched(app_id, std::move(callback),
+                          crostini::CrostiniResult::SUCCESS, true,
+                          std::string());
+    // The spinner for the app may start, but since terminal will launch rather
+    // that the GUI app with `app_id`, we must close the spinner.
+    if (auto* chrome_controller = ChromeShelfController::instance()) {
+      chrome_controller->GetShelfSpinnerController()->CloseSpinner(app_id);
+    }
+    return;
   }
-  const guest_os::GuestId guest_id(registration.VmType(), registration.VmName(),
-                                   registration.ContainerName());
+
   guest_os::launcher::LaunchApplication(
-      profile, guest_id, registration.DesktopFileId(), args,
+      profile, container_id, registration.DesktopFileId(), args,
       registration.IsScaled(),
-      base::BindOnce(OnApplicationLaunched, desktop_file_id,
-                     std::move(callback),
+      base::BindOnce(OnApplicationLaunched, app_id, std::move(callback),
                      crostini::CrostiniResult::UNKNOWN_ERROR));
 }
 
 void LaunchApplication(
     Profile* profile,
     const std::string& app_id,
     guest_os::GuestOsRegistryService::Registration registration,
+    const guest_os::GuestId& container_id,
     int64_t display_id,
     const std::vector<LaunchArg>& args,
     crostini::CrostiniSuccessCallback callback) {
@@ -189,7 +209,7 @@ void LaunchApplication(
       vm_name, vm_info->info.seneschal_server_handle(),
       std::move(paths_to_share),
       base::BindOnce(OnSharePathForLaunchApplication, profile, app_id,
-                     std::move(registration), display_id,
+                     std::move(registration), container_id, display_id,
                      std::move(launch_args), std::move(callback)));
 }
 
@@ -245,7 +265,7 @@ void LaunchCrostiniAppImpl(
     Profile* profile,
     const std::string& app_id,
     guest_os::GuestOsRegistryService::Registration registration,
-    const guest_os::GuestId container_id,
+    const guest_os::GuestId& container_id,
     int64_t display_id,
     const std::vector<LaunchArg>& args,
     CrostiniSuccessCallback callback) {
@@ -263,7 +283,8 @@ void LaunchCrostiniAppImpl(
       base::BindOnce(
           [](Profile* profile, const std::string& app_id,
              guest_os::GuestOsRegistryService::Registration registration,
-             int64_t display_id, const std::vector<LaunchArg> args,
+             const guest_os::GuestId& container_id, int64_t display_id,
+             const std::vector<LaunchArg> args,
              crostini::CrostiniSuccessCallback callback,
              crostini::CrostiniResult result) {
             if (result != crostini::CrostiniResult::SUCCESS) {
@@ -276,10 +297,11 @@ void LaunchCrostiniAppImpl(
             }
 
             LaunchApplication(profile, app_id, std::move(registration),
-                              display_id, args, std::move(callback));
+                              container_id, display_id, args,
+                              std::move(callback));
           },
-          profile, app_id, std::move(registration), display_id, args,
-          std::move(callback)));
+          profile, app_id, std::move(registration), container_id, display_id,
+          args, std::move(callback)));
 
   base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
       FROM_HERE, base::BindOnce(&AddSpinner, restart_id, app_id, profile),

diff --git a/chrome/browser/ash/guest_os/guest_os_pref_names.cc b/chrome/browser/ash/guest_os/guest_os_pref_names.cc
@@ -35,6 +35,7 @@ const char kAppExecKey[] = "exec";
 const char kAppExecutableFileNameKey[] = "executable_file_name";
 const char kAppNameKey[] = "name";
 const char kAppNoDisplayKey[] = "no_display";
+const char kAppTerminalKey[] = "terminal";
 const char kAppScaledKey[] = "scaled";
 const char kAppPackageIdKey[] = "package_id";
 const char kAppStartupWMClassKey[] = "startup_wm_class";

diff --git a/chrome/browser/ash/guest_os/guest_os_pref_names.h b/chrome/browser/ash/guest_os/guest_os_pref_names.h
@@ -31,6 +31,7 @@ extern const char kAppExecKey[];
 extern const char kAppExecutableFileNameKey[];
 extern const char kAppNameKey[];
 extern const char kAppNoDisplayKey[];
+extern const char kAppTerminalKey[];
 extern const char kAppScaledKey[];
 extern const char kAppPackageIdKey[];
 extern const char kAppStartupWMClassKey[];

diff --git a/chrome/browser/ash/guest_os/guest_os_registry_service.cc b/chrome/browser/ash/guest_os/guest_os_registry_service.cc
@@ -191,6 +191,8 @@ void PopulatePrefRegistrationFromApp(base::Value::Dict& pref_registration,
                         LocaleStringsProtoToDictionary(app.keywords()));
   pref_registration.Set(guest_os::prefs::kAppNoDisplayKey,
                         base::Value(app.no_display()));
+  pref_registration.Set(guest_os::prefs::kAppTerminalKey,
+                        base::Value(app.terminal()));
   pref_registration.Set(guest_os::prefs::kAppStartupWMClassKey,
                         base::Value(app.startup_wm_class()));
   pref_registration.Set(guest_os::prefs::kAppStartupNotifyKey,
@@ -291,6 +293,7 @@ static std::string ToString(const vm_tools::apps::App& app) {
          ", comment: " + ToString(app.comment()) +
          ", mime_types: " + ToString(app.mime_types()) +
          ", no_display: " + ToString(app.no_display()) +
+         ", terminal: " + ToString(app.terminal()) +
          ", startup_wm_class: " + ToString(app.startup_wm_class()) +
          ", startup_notify: " + ToString(app.startup_notify()) +
          ", keywords: " + ToString(app.keywords()) +
@@ -405,6 +408,9 @@ bool GuestOsRegistryService::Registration::NoDisplay() const {
   return GetBool(guest_os::prefs::kAppNoDisplayKey);
 }
 
+bool GuestOsRegistryService::Registration::Terminal() const {
+  return GetBool(guest_os::prefs::kAppTerminalKey);
+}
 std::string GuestOsRegistryService::Registration::PackageId() const {
   return GetString(guest_os::prefs::kAppPackageIdKey);
 }

diff --git a/chrome/browser/ash/guest_os/guest_os_registry_service.h b/chrome/browser/ash/guest_os/guest_os_registry_service.h
@@ -105,6 +105,7 @@ class GuestOsRegistryService : public KeyedService {
     std::set<std::string> MimeTypes() const;
     std::set<std::string> Keywords() const;
     bool NoDisplay() const;
+    bool Terminal() const;
 
     std::string PackageId() const;
 

diff --git a/chrome/browser/extensions/api/terminal/terminal_private_api.cc b/chrome/browser/extensions/api/terminal/terminal_private_api.cc
@@ -344,6 +344,14 @@ TerminalPrivateOpenTerminalProcessFunction::OpenProcess(
     if (!container_features.empty())
       cmdline.AppendSwitchASCII(kSwitchContainerFeatures, container_features);
 
+    // Append trailing passthrough args if any.  E.g. `-- vim file.txt`
+    auto passthrough_args = params_args.GetArgs();
+    if (!passthrough_args.empty()) {
+      cmdline.AppendArg("--");
+      for (const auto& arg : passthrough_args) {
+        cmdline.AppendArg(arg);
+      }
+    }
     VLOG(1) << "Starting " << *guest_id_
             << ", cmdline=" << cmdline.GetCommandLineString();