Crostini and Plugin VM launch args can be files or strings

In order to send a URL as a param, we must be allow plain string args
which will be passed through unchanged as well as files which will be
shared and remapped.

Bug: b/168506505
Change-Id: Ica2ca4c7b79047d1811961c19e79f9db0d3cfcda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423744
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Jason Lin <lxj@google.com>
Cr-Commit-Position: refs/heads/master@{#809648}

chrome/browser/chromeos/crostini/crostini_util.cc

@@ -130,7 +130,7 @@ void OnSharePathForLaunchApplication(
     const std::string& app_id,
     guest_os::GuestOsRegistryService::Registration registration,
     int64_t display_id,
-    const std::vector<std::string>& files,
+    const std::vector<std::string>& args,
     crostini::CrostiniSuccessCallback callback,
     bool success,
     const std::string& failure_reason) {
@@ -143,7 +143,7 @@ void OnSharePathForLaunchApplication(
                                            registration.ContainerName());
   if (app_id == kCrostiniTerminalSystemAppId) {
     // Use first file as 'cwd'.
-    std::string cwd = !files.empty() ? files[0] : "";
+    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");
@@ -152,8 +152,7 @@ void OnSharePathForLaunchApplication(
                                  crostini::CrostiniResult::SUCCESS, true, "");
   }
   crostini::CrostiniManager::GetForProfile(profile)->LaunchContainerApplication(
-      container_id, registration.DesktopFileId(), files,
-      registration.IsScaled(),
+      container_id, registration.DesktopFileId(), args, registration.IsScaled(),
       base::BindOnce(OnApplicationLaunched, app_id, std::move(callback),
                      crostini::CrostiniResult::UNKNOWN_ERROR));
 }
@@ -163,7 +162,7 @@ void LaunchApplication(
     const std::string& app_id,
     guest_os::GuestOsRegistryService::Registration registration,
     int64_t display_id,
-    const std::vector<storage::FileSystemURL>& files,
+    const std::vector<LaunchArg>& args,
     crostini::CrostiniSuccessCallback callback) {
   ChromeLauncherController* chrome_launcher_controller =
       ChromeLauncherController::instance();
@@ -178,8 +177,14 @@ void LaunchApplication(
   // Share any paths not in crostini.  The user will see the spinner while this
   // is happening.
   std::vector<base::FilePath> paths_to_share;
-  std::vector<std::string> files_to_launch;
-  for (const storage::FileSystemURL& url : files) {
+  std::vector<std::string> launch_args;
+  launch_args.reserve(args.size());
+  for (const auto& arg : args) {
+    if (absl::holds_alternative<std::string>(arg)) {
+      launch_args.push_back(absl::get<std::string>(arg));
+      continue;
+    }
+    const storage::FileSystemURL& url = absl::get<storage::FileSystemURL>(arg);
     base::FilePath path;
     if (!file_manager::util::ConvertFileSystemURLToPathInsideCrostini(
             profile, url, &path)) {
@@ -191,19 +196,19 @@ void LaunchApplication(
         file_manager::util::GetCrostiniMountPointName(profile)) {
       paths_to_share.push_back(url.path());
     }
-    files_to_launch.push_back(path.value());
+    launch_args.push_back(path.value());
   }
 
   if (paths_to_share.empty()) {
     OnSharePathForLaunchApplication(profile, app_id, std::move(registration),
-                                    display_id, std::move(files_to_launch),
+                                    display_id, std::move(launch_args),
                                     std::move(callback), true, "");
   } else {
     guest_os::GuestOsSharePath::GetForProfile(profile)->SharePaths(
         registration.VmName(), std::move(paths_to_share), /*persist=*/false,
         base::BindOnce(OnSharePathForLaunchApplication, profile, app_id,
                        std::move(registration), display_id,
-                       std::move(files_to_launch), std::move(callback)));
+                       std::move(launch_args), std::move(callback)));
   }
 }
 
@@ -297,7 +302,7 @@ void LaunchCrostiniAppImpl(
     const std::string& app_id,
     guest_os::GuestOsRegistryService::Registration registration,
     int64_t display_id,
-    const std::vector<storage::FileSystemURL>& files,
+    const std::vector<LaunchArg>& args,
     CrostiniSuccessCallback callback) {
   auto* crostini_manager = crostini::CrostiniManager::GetForProfile(profile);
   auto* registry_service =
@@ -311,13 +316,16 @@ void LaunchCrostiniAppImpl(
     // and share the path before launching terminal.
     bool requires_share = false;
     base::FilePath cwd;
-    if (!files.empty()) {
-      if (files[0].mount_filesystem_id() !=
+    if (!args.empty() &&
+        absl::holds_alternative<storage::FileSystemURL>(args[0])) {
+      const storage::FileSystemURL& url =
+          absl::get<storage::FileSystemURL>(args[0]);
+      if (url.mount_filesystem_id() !=
           file_manager::util::GetCrostiniMountPointName(profile)) {
         requires_share = true;
       } else {
-        file_manager::util::ConvertFileSystemURLToPathInsideCrostini(
-            profile, files[0], &cwd);
+        file_manager::util::ConvertFileSystemURLToPathInsideCrostini(profile,
+                                                                     url, &cwd);
       }
     }
 
@@ -343,8 +351,7 @@ void LaunchCrostiniAppImpl(
       base::BindOnce(
           [](Profile* profile, const std::string& app_id,
              guest_os::GuestOsRegistryService::Registration registration,
-             int64_t display_id,
-             const std::vector<storage::FileSystemURL> files,
+             int64_t display_id, const std::vector<LaunchArg> args,
              crostini::CrostiniSuccessCallback callback,
              crostini::CrostiniResult result) {
             if (result != crostini::CrostiniResult::SUCCESS) {
@@ -361,9 +368,9 @@ void LaunchCrostiniAppImpl(
             }
 
             LaunchApplication(profile, app_id, std::move(registration),
-                              display_id, files, std::move(callback));
+                              display_id, args, std::move(callback));
           },
-          profile, app_id, std::move(registration), display_id, files,
+          profile, app_id, std::move(registration), display_id, args,
           std::move(callback)));
 
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
@@ -374,7 +381,7 @@ void LaunchCrostiniAppImpl(
 void LaunchCrostiniApp(Profile* profile,
                        const std::string& app_id,
                        int64_t display_id,
-                       const std::vector<storage::FileSystemURL>& files,
+                       const std::vector<LaunchArg>& args,
                        CrostiniSuccessCallback callback) {
   // Policies can change under us, and crostini may now be forbidden.
   if (!CrostiniFeatures::Get()->IsUIAllowed(profile)) {
@@ -404,7 +411,7 @@ void LaunchCrostiniApp(Profile* profile,
     // Prompt for user-restart.
     return ShowCrostiniRecoveryView(
         profile, crostini::CrostiniUISurface::kAppList, app_id, display_id,
-        files, std::move(callback));
+        args, std::move(callback));
   }
 
   if (crostini_manager->GetCrostiniDialogStatus(DialogType::UPGRADER)) {
@@ -416,7 +423,7 @@ void LaunchCrostiniApp(Profile* profile,
     return;
   }
   LaunchCrostiniAppImpl(profile, app_id, std::move(*registration), display_id,
-                        files, std::move(callback));
+                        args, std::move(callback));
 }
 
 std::string CryptohomeIdForProfile(Profile* profile) {

chrome/browser/chromeos/crostini/crostini_util.h

@@ -16,6 +16,7 @@
 #include "base/values.h"
 #include "chrome/browser/chromeos/crostini/crostini_simple_types.h"
 #include "storage/browser/file_system/file_system_url.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 
 namespace base {
 class FilePath;
@@ -89,13 +90,15 @@ bool ShouldConfigureDefaultContainer(Profile* profile);
 bool MaybeShowCrostiniDialogBeforeLaunch(Profile* profile,
                                          CrostiniResult result);
 
+using LaunchArg = absl::variant<storage::FileSystemURL, std::string>;
+
 // Launch a Crostini App with a given set of files, given as absolute paths in
 // the container. For apps which can only be launched with a single file,
 // launch multiple instances.
 void LaunchCrostiniApp(Profile* profile,
                        const std::string& app_id,
                        int64_t display_id,
-                       const std::vector<storage::FileSystemURL>& files = {},
+                       const std::vector<LaunchArg>& args = {},
                        CrostiniSuccessCallback callback = base::DoNothing());
 
 // Retrieves cryptohome_id from profile.
@@ -172,7 +175,7 @@ void ShowCrostiniRecoveryView(Profile* profile,
                               CrostiniUISurface ui_surface,
                               const std::string& app_id,
                               int64_t display_id,
-                              const std::vector<storage::FileSystemURL>& files,
+                              const std::vector<LaunchArg>& args,
                               CrostiniSuccessCallback callback);
 
 // Add a newly created LXD container to the kCrostiniContainers pref

chrome/browser/chromeos/file_manager/guest_os_file_tasks.cc

@@ -282,13 +282,19 @@ void ExecuteGuestOsTask(
     return;
   }
 
+  using LaunchArg = absl::variant<storage::FileSystemURL, std::string>;
+  std::vector<LaunchArg> args;
+  args.reserve(file_system_urls.size());
+  for (const auto& url : file_system_urls) {
+    args.emplace_back(url);
+  }
   guest_os::GuestOsRegistryService::VmType vm_type = registration->VmType();
   switch (vm_type) {
     case guest_os::GuestOsRegistryService::VmType::
         ApplicationList_VmType_TERMINA:
       DCHECK(crostini::CrostiniFeatures::Get()->IsUIAllowed(profile));
       crostini::LaunchCrostiniApp(
-          profile, task.app_id, display::kInvalidDisplayId, file_system_urls,
+          profile, task.app_id, display::kInvalidDisplayId, args,
           base::BindOnce(
               [](FileTaskFinishedCallback done, bool success,
                  const std::string& failure_reason) {
@@ -310,7 +316,7 @@ void ExecuteGuestOsTask(
         ApplicationList_VmType_PLUGIN_VM:
       DCHECK(plugin_vm::PluginVmFeatures::Get()->IsEnabled(profile));
       plugin_vm::LaunchPluginVmApp(
-          profile, task.app_id, file_system_urls,
+          profile, task.app_id, args,
           base::BindOnce(
               [](FileTaskFinishedCallback done,
                  plugin_vm::LaunchPluginVmAppResult result,

chrome/browser/chromeos/plugin_vm/plugin_vm_files.cc

@@ -147,7 +147,7 @@ void EnsureDefaultSharedDirExists(
 
 void LaunchPluginVmApp(Profile* profile,
                        std::string app_id,
-                       const std::vector<storage::FileSystemURL>& files,
+                       const std::vector<LaunchArg>& args,
                        LaunchPluginVmAppCallback callback) {
   if (!plugin_vm::PluginVmFeatures::Get()->IsEnabled(profile)) {
     return std::move(callback).Run(LaunchPluginVmAppResult::FAILED,
@@ -164,31 +164,35 @@ void LaunchPluginVmApp(Profile* profile,
   // Forward slashes are converted to backslash during path conversion.
   base::FilePath vm_mount("//ChromeOS");
 
-  std::vector<std::string> file_paths;
-  file_paths.reserve(files.size());
-  for (const auto& file : files) {
+  std::vector<std::string> launch_args;
+  launch_args.reserve(args.size());
+  for (const auto& arg : args) {
+    if (absl::holds_alternative<std::string>(arg)) {
+      launch_args.push_back(absl::get<std::string>(arg));
+      continue;
+    }
+    const storage::FileSystemURL& url = absl::get<storage::FileSystemURL>(arg);
     base::FilePath file_path;
     // Validate paths are already shared, and convert file paths.
-    if (!share_path->IsPathShared(kPluginVmName, file.path()) ||
+    if (!share_path->IsPathShared(kPluginVmName, url.path()) ||
         !file_manager::util::ConvertFileSystemURLToPathInsideVM(
-            profile, file, vm_mount, &file_path)) {
+            profile, url, vm_mount, &file_path)) {
       return std::move(callback).Run(
           file_manager::util::GetMyFilesFolderForProfile(profile).IsParent(
-              file.path())
+              url.path())
               ? LaunchPluginVmAppResult::FAILED_DIRECTORY_NOT_SHARED
               : LaunchPluginVmAppResult::FAILED_FILE_ON_EXTERNAL_DRIVE,
-          "Only files in shared dirs are supported. Got: " +
-              file.DebugString());
+          "Only files in shared dirs are supported. Got: " + url.DebugString());
     }
     // Convert slashes: '/' => '\'.
     std::string result;
     base::ReplaceChars(file_path.value(), "/", "\\", &result);
-    file_paths.push_back(std::move(result));
+    launch_args.push_back(std::move(result));
   }
 
   manager->LaunchPluginVm(
       base::BindOnce(&LaunchPluginVmAppImpl, profile, std::move(app_id),
-                     std::move(file_paths), std::move(callback)));
+                     std::move(launch_args), std::move(callback)));
 }
 
 }  // namespace plugin_vm

chrome/browser/chromeos/plugin_vm/plugin_vm_files.h

@@ -8,6 +8,7 @@
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "storage/browser/file_system/file_system_url.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 
 class Profile;
 
@@ -27,6 +28,8 @@ enum class LaunchPluginVmAppResult {
   FAILED_FILE_ON_EXTERNAL_DRIVE,
 };
 
+using LaunchArg = absl::variant<storage::FileSystemURL, std::string>;
+
 using LaunchPluginVmAppCallback =
     base::OnceCallback<void(LaunchPluginVmAppResult result,
                             const std::string& failure_reason)>;
@@ -35,7 +38,7 @@ using LaunchPluginVmAppCallback =
 // the VM. Will start Plugin VM if it is not already running.
 void LaunchPluginVmApp(Profile* profile,
                        std::string app_id,
-                       const std::vector<storage::FileSystemURL>& files,
+                       const std::vector<LaunchArg>& files,
                        LaunchPluginVmAppCallback callback);
 
 }  // namespace plugin_vm

chrome/browser/chromeos/plugin_vm/plugin_vm_files_unittest.cc

@@ -122,8 +122,7 @@ TEST_F(PluginVmFilesTest, LaunchPluginVmApp) {
   LaunchPluginVmApp(&profile_,
                     crostini::CrostiniTestHelper::GenerateAppId(app_id, vm_name,
                                                                 container_name),
-                    std::vector<storage::FileSystemURL>{},
-                    app_launched_callback.Get());
+                    {}, app_launched_callback.Get());
   ASSERT_FALSE(launch_plugin_vm_callback.is_null());
 
   // Add app to app_list.

chrome/browser/ui/views/crostini/crostini_recovery_view.cc

@@ -36,27 +36,26 @@ void crostini::ShowCrostiniRecoveryView(
     crostini::CrostiniUISurface ui_surface,
     const std::string& app_id,
     int64_t display_id,
-    const std::vector<storage::FileSystemURL>& files,
+    const std::vector<crostini::LaunchArg>& args,
     crostini::CrostiniSuccessCallback callback) {
-  CrostiniRecoveryView::Show(profile, app_id, display_id, files,
+  CrostiniRecoveryView::Show(profile, app_id, display_id, args,
                              std::move(callback));
   base::UmaHistogramEnumeration(kCrostiniRecoverySourceHistogram, ui_surface,
                                 crostini::CrostiniUISurface::kCount);
 }
 
-void CrostiniRecoveryView::Show(
-    Profile* profile,
-    const std::string& app_id,
-    int64_t display_id,
-    const std::vector<storage::FileSystemURL>& files,
-    crostini::CrostiniSuccessCallback callback) {
+void CrostiniRecoveryView::Show(Profile* profile,
+                                const std::string& app_id,
+                                int64_t display_id,
+                                const std::vector<crostini::LaunchArg>& args,
+                                crostini::CrostiniSuccessCallback callback) {
   DCHECK(crostini::CrostiniFeatures::Get()->IsUIAllowed(profile));
   // Any new apps launched during recovery are immediately cancelled.
   if (g_crostini_recovery_view) {
     std::move(callback).Run(false, "recovery in progress");
   } else {
     g_crostini_recovery_view = new CrostiniRecoveryView(
-        profile, app_id, display_id, files, std::move(callback));
+        profile, app_id, display_id, args, std::move(callback));
     CreateDialogWidget(g_crostini_recovery_view, nullptr, nullptr);
   }
   // Always call Show to bring the dialog to the front of the screen.
@@ -88,7 +87,7 @@ void CrostiniRecoveryView::OnStopVm(crostini::CrostiniResult result) {
   }
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(&crostini::LaunchCrostiniApp, profile_, app_id_,
-                                display_id_, files_, std::move(callback_)));
+                                display_id_, args_, std::move(callback_)));
   GetWidget()->CloseWithReason(
       views::Widget::ClosedReason::kAcceptButtonClicked);
 }
@@ -110,12 +109,12 @@ CrostiniRecoveryView::CrostiniRecoveryView(
     Profile* profile,
     const std::string& app_id,
     int64_t display_id,
-    const std::vector<storage::FileSystemURL>& files,
+    const std::vector<crostini::LaunchArg>& args,
     crostini::CrostiniSuccessCallback callback)
     : profile_(profile),
       app_id_(app_id),
       display_id_(display_id),
-      files_(files),
+      args_(args),
       callback_(std::move(callback)),
       weak_ptr_factory_(this) {
   SetButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);

chrome/browser/ui/views/crostini/crostini_recovery_view.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_VIEWS_CROSTINI_CROSTINI_RECOVERY_VIEW_H_
 
 #include "chrome/browser/chromeos/crostini/crostini_simple_types.h"
+#include "chrome/browser/chromeos/crostini/crostini_util.h"
 #include "storage/browser/file_system/file_system_url.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 
@@ -22,7 +23,7 @@ class CrostiniRecoveryView : public views::BubbleDialogDelegateView {
   static void Show(Profile* profile,
                    const std::string& app_id,
                    int64_t display_id,
-                   const std::vector<storage::FileSystemURL>& files,
+                   const std::vector<crostini::LaunchArg>& args,
                    crostini::CrostiniSuccessCallback callback);
 
   // views::DialogDelegateView:
@@ -36,7 +37,7 @@ class CrostiniRecoveryView : public views::BubbleDialogDelegateView {
   CrostiniRecoveryView(Profile* profile,
                        const std::string& app_id,
                        int64_t display_id,
-                       const std::vector<storage::FileSystemURL>& files,
+                       const std::vector<crostini::LaunchArg>& args,
                        crostini::CrostiniSuccessCallback callback);
   ~CrostiniRecoveryView() override;
 
@@ -45,7 +46,7 @@ class CrostiniRecoveryView : public views::BubbleDialogDelegateView {
   Profile* profile_;  // Not owned.
   std::string app_id_;
   int64_t display_id_;
-  const std::vector<storage::FileSystemURL> files_;
+  const std::vector<crostini::LaunchArg> args_;
   crostini::CrostiniSuccessCallback callback_;
 
   base::WeakPtrFactory<CrostiniRecoveryView> weak_ptr_factory_;