bruschetta: Implement LaunchAppWithIntent.

This lets Files App "Open With" bruschetta apps.

- Factor out intent filter code into guest_os_apps.
- Move intent filter tests from Crostini to GuestOS code.

Bug: b:265601951
Test: Manually on a DUT: Files App -> "Open With" -> a Bruschetta app.
Change-Id: Iec51868c95067365b425f5a73d552a32e5f89d55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4302311
Commit-Queue: Emil Mikulic <easy@google.com>
Reviewed-by: David Munro <davidmunro@google.com>
Reviewed-by: Josh Simmons <simmonsjosh@google.com>
Reviewed-by: Fergus Dall <sidereal@google.com>
Cr-Commit-Position: refs/heads/main@{#1118503}

chrome/browser/apps/app_service/publishers/bruschetta_apps.cc

@@ -8,19 +8,27 @@
 #include <utility>
 #include <vector>
 
+#include "base/functional/callback_helpers.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/ash/bruschetta/bruschetta_features.h"
 #include "chrome/browser/ash/bruschetta/bruschetta_launcher.h"
 #include "chrome/browser/ash/bruschetta/bruschetta_service.h"
 #include "chrome/browser/ash/bruschetta/bruschetta_util.h"
 #include "chrome/browser/ash/crostini/crostini_manager.h"
+#include "chrome/browser/ash/crostini/crostini_util.h"
+#include "chrome/browser/ash/file_manager/fileapi_util.h"
+#include "chrome/browser/ash/file_manager/path_util.h"
 #include "chrome/browser/ash/guest_os/guest_os_registry_service.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/profiles/profile.h"
 #include "chrome/browser/ui/ash/shelf/chrome_shelf_controller.h"
 #include "chrome/browser/ui/ash/shelf/shelf_spinner_controller.h"
 #include "chrome/browser/ui/ash/shelf/shelf_spinner_item_controller.h"
 #include "components/services/app_service/public/cpp/app_types.h"
+#include "components/services/app_service/public/cpp/intent.h"
+#include "storage/browser/file_system/file_system_context.h"
 
 namespace apps {
 
@@ -33,39 +41,110 @@ void AddSpinner(const std::string& app_id) {
   }
 }
 
+// It's safe to remove a non-existent spinner.
 void RemoveSpinner(const std::string& app_id) {
   if (auto* chrome_controller = ChromeShelfController::instance()) {
     chrome_controller->GetShelfSpinnerController()->CloseSpinner(app_id);
   }
 }
 
-void LaunchApplication(
+void OnLaunchFailed(const std::string& app_id,
+                    LaunchCallback callback,
+                    const std::string& reason) {
+  LOG(ERROR) << "Failed to launch Bruschetta app " << app_id << ": " << reason;
+  RemoveSpinner(app_id);
+  std::move(callback).Run(ConvertBoolToLaunchResult(false));
+}
+
+void OnSharePathForLaunchApplication(
     Profile* profile,
     const std::string& app_id,
     guest_os::GuestOsRegistryService::Registration registration,
-    int64_t display_id) {
-  // TODO(b/265601951): Handle window permissions. Crostini uses
-  // AppServiceAppWindowCrostiniTracker::OnAppLaunchRequested for this.
-  // TODO(b/245412929): Share paths to files.
-  const guest_os::GuestId container_id(registration.VmType(),
-                                       registration.VmName(),
-                                       registration.ContainerName());
-
-  std::vector<std::string> files;
+    const guest_os::GuestId container_id,
+    const std::vector<std::string>& args,
+    LaunchCallback callback,
+    bool success,
+    const std::string& failure_reason) {
+  if (!success) {
+    OnLaunchFailed(app_id, std::move(callback),
+                   "Failed to share paths with Bruschetta: " + failure_reason);
+    return;
+  }
   // TODO(b/265601951): Factor this out of CrostiniManager.
   crostini::CrostiniManager::GetForProfile(profile)->LaunchContainerApplication(
-      container_id, registration.DesktopFileId(), files,
-      registration.IsScaled(),
+      container_id, registration.DesktopFileId(), args, registration.IsScaled(),
       base::BindOnce(
-          [](const std::string& app_id, bool success,
+          [](const std::string& app_id, LaunchCallback callback, bool success,
              const std::string& failure_reason) {
             if (!success) {
-              LOG(ERROR) << "Failed to launch Bruschetta app " << app_id << ": "
-                         << failure_reason;
+              OnLaunchFailed(app_id, std::move(callback), failure_reason);
+              return;
             }
             RemoveSpinner(app_id);
+            std::move(callback).Run(ConvertBoolToLaunchResult(success));
           },
-          app_id));
+          app_id, std::move(callback)));
+}
+
+void LaunchApplication(
+    Profile* profile,
+    const std::string& app_id,
+    guest_os::GuestOsRegistryService::Registration registration,
+    int64_t display_id,
+    const std::vector<crostini::LaunchArg> args,
+    LaunchCallback callback) {
+  // TODO(b/265601951): Handle window permissions. Crostini uses
+  // AppServiceAppWindowCrostiniTracker::OnAppLaunchRequested for this.
+
+  const guest_os::GuestId container_id(registration.VmType(),
+                                       registration.VmName(),
+                                       registration.ContainerName());
+
+  // TODO(b/265601951): Consider factoring SharePath code against
+  // crostini_util.cc LaunchApplication.
+  auto* share_path = guest_os::GuestOsSharePath::GetForProfile(profile);
+  const std::string& vm_name = registration.VmName();
+  auto vm_info =
+      guest_os::GuestOsSessionTracker::GetForProfile(profile)->GetVmInfo(
+          vm_name);
+  if (!vm_info) {
+    OnLaunchFailed(app_id, std::move(callback),
+                   "Bruschetta VM not running: " + vm_name);
+    return;
+  }
+
+  // Share any paths not in Bruschetta.
+  std::vector<base::FilePath> paths_to_share;
+  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::ConvertFileSystemURLToPathInsideVM(
+            profile, url, bruschetta::BruschettaChromeOSBaseDirectory(),
+            /*map_crostini_home=*/false, &path)) {
+      OnLaunchFailed(app_id, std::move(callback),
+                     "Cannot share file with Bruschetta: " + url.DebugString());
+      return;
+    }
+    if (url.mount_filesystem_id() !=
+            file_manager::util::GetGuestOsMountPointName(profile,
+                                                         container_id) &&
+        !share_path->IsPathShared(vm_name, url.path())) {
+      paths_to_share.push_back(url.path());
+    }
+    launch_args.push_back(path.value());
+  }
+
+  share_path->SharePaths(
+      vm_name, vm_info->seneschal_server_handle(), std::move(paths_to_share),
+      base::BindOnce(OnSharePathForLaunchApplication, profile, app_id,
+                     std::move(registration), std::move(container_id),
+                     std::move(launch_args), std::move(callback)));
 }
 
 }  // namespace
@@ -101,17 +180,44 @@ void BruschettaApps::Launch(const std::string& app_id,
                             int32_t event_flags,
                             LaunchSource launch_source,
                             WindowInfoPtr window_info) {
+  LaunchAppWithIntent(app_id, event_flags, /*intent=*/nullptr, launch_source,
+                      std::move(window_info), /*callback=*/base::DoNothing());
+}
+
+void BruschettaApps::LaunchAppWithIntent(const std::string& app_id,
+                                         int32_t event_flags,
+                                         IntentPtr intent,
+                                         LaunchSource launch_source,
+                                         WindowInfoPtr window_info,
+                                         LaunchCallback callback) {
+  // TODO(b/265601951): Consider factoring args code against
+  // CrostiniApps::LaunchAppWithIntent.
+
+  // Retrieve URLs from the files in the intent.
+  std::vector<crostini::LaunchArg> args;
+  if (intent && intent->files.size() > 0) {
+    args.reserve(intent->files.size());
+    storage::FileSystemContext* file_system_context =
+        file_manager::util::GetFileManagerFileSystemContext(profile());
+    for (auto& file : intent->files) {
+      args.emplace_back(
+          file_system_context->CrackURLInFirstPartyContext(file->url));
+    }
+  }
+
   const int64_t display_id =
       window_info ? window_info->display_id : display::kInvalidDisplayId;
+
   absl::optional<guest_os::GuestOsRegistryService::Registration> registration =
       registry()->GetRegistration(app_id);
   if (!registration) {
     // TODO(b/247638226): RecordAppLaunchHistogram(kUnknown) to collect usage
     // stats for failed launches.
-    LOG(ERROR) << "BruschettaApps::Launch called with an unknown app_id: "
-               << app_id;
+    OnLaunchFailed(app_id, std::move(callback),
+                   "Unknown Bruschetta app_id: " + app_id);
     return;
   }
+
   // TODO(b/247638226): RecordAppLaunchHistogram(kRegisteredApp) to collect
   // usage stats for successful launches.
 
@@ -124,24 +230,28 @@ void BruschettaApps::Launch(const std::string& app_id,
       bruschetta::BruschettaService::GetForProfile(profile())->GetLauncher(
           vm_name);
   if (!launcher) {
-    LOG(ERROR) << "Unknown Bruschetta VM name: " << vm_name;
+    OnLaunchFailed(app_id, std::move(callback),
+                   "Unknown Bruschetta VM name: " + vm_name);
     return;
   }
   AddSpinner(app_id);
   launcher->EnsureRunning(base::BindOnce(
       [](Profile* profile, const std::string& app_id,
          guest_os::GuestOsRegistryService::Registration registration,
-         int64_t display_id, const std::string& vm_name,
+         int64_t display_id, const std::vector<crostini::LaunchArg> args,
+         const std::string& vm_name, LaunchCallback callback,
          bruschetta::BruschettaResult result) {
         if (result != bruschetta::BruschettaResult::kSuccess) {
-          LOG(ERROR) << "Failed to start Bruschetta VM " << vm_name << ": "
-                     << bruschetta::BruschettaResultString(result);
-          RemoveSpinner(app_id);
+          OnLaunchFailed(app_id, std::move(callback),
+                         "Failed to start Bruschetta VM " + vm_name + ": " +
+                             bruschetta::BruschettaResultString(result));
           return;
         }
-        LaunchApplication(profile, app_id, std::move(registration), display_id);
+        LaunchApplication(profile, app_id, std::move(registration), display_id,
+                          std::move(args), std::move(callback));
       },
-      profile(), app_id, std::move(registration.value()), display_id, vm_name));
+      profile(), app_id, std::move(registration.value()), display_id,
+      std::move(args), vm_name, std::move(callback)));
 }
 
 void BruschettaApps::LaunchAppWithParams(AppLaunchParams&& params,
@@ -153,7 +263,6 @@ void BruschettaApps::CreateAppOverrides(
     const guest_os::GuestOsRegistryService::Registration& registration,
     App* app) {
   // TODO(b/247638042): Implement IsUninstallable and use it here.
-  // TODO(b/245412929): Implement intent filter and use it here.
 }
 
 }  // namespace apps

chrome/browser/apps/app_service/publishers/bruschetta_apps.h

@@ -44,6 +44,12 @@ class BruschettaApps : public GuestOSApps {
               int32_t event_flags,
               LaunchSource launch_source,
               WindowInfoPtr window_info) override;
+  void LaunchAppWithIntent(const std::string& app_id,
+                           int32_t event_flags,
+                           IntentPtr intent,
+                           LaunchSource launch_source,
+                           WindowInfoPtr window_info,
+                           LaunchCallback callback) override;
   void LaunchAppWithParams(AppLaunchParams&& params,
                            LaunchCallback callback) override;
 

chrome/browser/apps/app_service/publishers/bruschetta_apps_unittest.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/ash/bruschetta/fake_bruschetta_launcher.h"
 #include "chrome/browser/ash/guest_os/dbus_test_helper.h"
 #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/profiles/profile.h"
 #include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
 #include "chrome/test/base/testing_profile.h"
@@ -48,6 +49,9 @@ class BruschettaAppsTest : public testing::Test,
         guest_os::GuestOsRegistryServiceFactory::GetForProfile(profile_.get());
     scoped_feature_list_.InitAndEnableFeature(ash::features::kBruschetta);
     bruschetta::BruschettaServiceFactory::EnableForTesting(profile_.get());
+    const guest_os::GuestId id(bruschetta::kBruschettaVmName, "test_container");
+    guest_os::GuestOsSessionTracker::GetForProfile(profile_.get())
+        ->AddGuestForTesting(id);
   }
 
   void TearDown() override { profile_.reset(); }

chrome/browser/apps/app_service/publishers/crostini_apps.cc

@@ -7,26 +7,20 @@
 #include <utility>
 #include <vector>
 
-#include "ash/constants/ash_features.h"
 #include "ash/public/cpp/app_menu_constants.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
-#include "chrome/browser/apps/app_service/intent_util.h"
 #include "chrome/browser/apps/app_service/launch_utils.h"
 #include "chrome/browser/apps/app_service/menu_util.h"
 #include "chrome/browser/ash/crostini/crostini_features.h"
 #include "chrome/browser/ash/crostini/crostini_package_service.h"
 #include "chrome/browser/ash/crostini/crostini_util.h"
 #include "chrome/browser/ash/file_manager/fileapi_util.h"
-#include "chrome/browser/ash/guest_os/guest_os_mime_types_service.h"
-#include "chrome/browser/ash/guest_os/guest_os_mime_types_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/grit/chrome_unscaled_resources.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/services/app_service/public/cpp/app_types.h"
 #include "components/services/app_service/public/cpp/intent.h"
-#include "components/services/app_service/public/cpp/intent_filter.h"
-#include "components/services/app_service/public/cpp/intent_util.h"
 #include "storage/browser/file_system/file_system_context.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
@@ -39,10 +33,6 @@
 
 namespace {
 
-const char kTextPlainMimeType[] = "text/plain";
-const char kTextTypeMimeType[] = "text/";
-const char kTextWildcardMimeType[] = "text/*";
-
 bool ShouldShowDisplayDensityMenuItem(const std::string& app_id,
                                       apps::MenuType menu_type,
                                       int64_t display_id) {
@@ -60,52 +50,6 @@ bool ShouldShowDisplayDensityMenuItem(const std::string& app_id,
   return d.device_scale_factor() != 1.0;
 }
 
-// Create a file intent filter with mime type conditions for App Service.
-apps::IntentFilters CreateIntentFilterForCrostini(
-    const guest_os::GuestOsMimeTypesService* mime_types_service,
-    const guest_os::GuestOsRegistryService::Registration& registration) {
-  const std::set<std::string> mime_types_set = registration.MimeTypes();
-  if (mime_types_set.empty()) {
-    return {};
-  }
-
-  // When a file has a mime type that Files App can't recognise but Crostini can
-  // (e.g. a proprietary file type), we should look at the file extensions that
-  // the app can support. We find these extension types by checking what
-  // extensions correspond to the app's supported mime types.
-  std::vector<std::string> extension_types;
-  if (ash::features::ShouldGuestOsFileTasksUseAppService()) {
-    extension_types = mime_types_service->GetExtensionTypesFromMimeTypes(
-        mime_types_set, registration.VmName(), registration.ContainerName());
-  }
-  std::vector<std::string> mime_types(mime_types_set.begin(),
-                                      mime_types_set.end());
-
-  // If we see that the app supports the text/plain mime-type, then the app
-  // supports all files with type text/*, as per xdg spec.
-  // https://specifications.freedesktop.org/shared-mime-info-spec/shared-mime-info-spec-latest.html.
-  // In this case, remove all mime types that begin with "text/" and replace
-  // them with a single "text/*" mime type.
-  if (base::Contains(mime_types, kTextPlainMimeType)) {
-    mime_types.erase(std::remove_if(mime_types.begin(), mime_types.end(),
-                                    [](const std::string& mime) {
-                                      return mime.find(kTextTypeMimeType) !=
-                                             std::string::npos;
-                                    }),
-                     mime_types.end());
-    mime_types.push_back(kTextWildcardMimeType);
-  }
-
-  apps::IntentFilters intent_filters;
-  intent_filters.push_back(apps_util::CreateFileFilter(
-      {apps_util::kIntentActionView}, mime_types, extension_types,
-      // TODO(crbug/1349974): Remove activity_name when default file handling
-      // preferences for Files App are migrated.
-      /*activity_name=*/apps_util::kGuestOsActivityName));
-
-  return intent_filters;
-}
-
 }  // namespace
 
 namespace apps {
@@ -258,12 +202,6 @@ void CrostiniApps::CreateAppOverrides(
   app->allow_uninstall =
       crostini::IsUninstallable(profile(), registration.app_id());
 
-  app->handles_intents = true;
-  const guest_os::GuestOsMimeTypesService* mime_types_service =
-      guest_os::GuestOsMimeTypesServiceFactory::GetForProfile(profile());
-  app->intent_filters =
-      CreateIntentFilterForCrostini(mime_types_service, registration);
-
   // TODO(crbug.com/1253250): Add other fields for the App struct.
 }