[FileTasks] Support Virtual Tasks in DefaultHandlersForFileExtensions

This CL adds basic support for virtual file tasks to the file handling
policy. Relevant actions are marked using stable identifiers
(VirtualTask/<virtual-task-policy-id>) and then transformed on the
client side with respect to the current id format of virtual tasks.

Bug: b/284800493

Change-Id: I9116319597ce51a6e022075b1590d7a6e839a103
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4834358
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Commit-Queue: Andrew Rayskiy <greengrape@google.com>
Cr-Commit-Position: refs/heads/main@{#1209335}

chrome/browser/apps/app_service/policy_util.cc

@@ -11,6 +11,7 @@
 #include "base/no_destructor.h"
 #include "base/ranges/algorithm.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -19,12 +20,21 @@
 #include "components/services/app_service/public/cpp/app_update.h"
 #include "components/services/app_service/public/cpp/types_util.h"
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+#include "base/containers/map_util.h"
+#include "base/types/optional_util.h"
+#include "chrome/browser/ash/file_manager/office_file_tasks.h"
+#include "chrome/browser/ash/file_manager/virtual_tasks/id_constants.h"
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
 namespace apps_util {
 
 namespace {
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 
+namespace fm_tasks = file_manager::file_tasks;
+
 // This mapping excludes SWAs not included in official builds (like SAMPLE).
 // These app Id constants need to be kept in sync with java/com/
 // google/chrome/cros/policyconverter/ChromePolicySettingsProcessor.java
@@ -57,15 +67,20 @@ constexpr auto kSystemWebAppsMapping =
 
 constexpr ash::SystemWebAppType GetMaxSystemWebAppType() {
   return base::ranges::max(kSystemWebAppsMapping, base::ranges::less{},
-                           [](const auto& systemWebAppMappingPair) {
-                             return systemWebAppMappingPair.second;
-                           })
+                           &decltype(kSystemWebAppsMapping)::value_type::second)
       .second;
 }
 
 static_assert(GetMaxSystemWebAppType() == ash::SystemWebAppType::kMaxValue,
               "Not all SWA types are listed in |system_web_apps_mapping|.");
 
+// These virtual task identifiers are supposed to be a subset of tasks listed in
+// chrome/browser/ash/file_manager/virtual_file_tasks.cc
+constexpr auto kVirtualFileTasksMapping =
+    base::MakeFixedFlatMap<base::StringPiece, base::StringPiece>(
+        {{"install-isolated-web-app", fm_tasks::kActionIdInstallIsolatedWebApp},
+         {"microsoft-office", fm_tasks::kActionIdOpenInOffice}});
+
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
 // Note that this mapping lists only selected Preinstalled Web Apps
@@ -93,8 +108,7 @@ bool IsChromeAppPolicyId(base::StringPiece policy_id) {
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 bool IsArcAppPolicyId(base::StringPiece policy_id) {
-  return policy_id.find('.') != base::StringPiece::npos &&
-         !IsWebAppPolicyId(policy_id);
+  return base::Contains(policy_id, '.') && !IsWebAppPolicyId(policy_id);
 }
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
@@ -115,6 +129,23 @@ bool IsPreinstalledWebAppPolicyId(base::StringPiece policy_id) {
   return base::Contains(kPreinstalledWebAppsMapping, policy_id);
 }
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+bool IsFileManagerVirtualTaskPolicyId(base::StringPiece policy_id) {
+  return GetVirtualTaskIdFromPolicyId(policy_id).has_value();
+}
+
+absl::optional<base::StringPiece> GetVirtualTaskIdFromPolicyId(
+    base::StringPiece policy_id) {
+  if (!base::StartsWith(policy_id, kVirtualTaskPrefix)) {
+    return absl::nullopt;
+  }
+  static constexpr size_t kOffset =
+      std::char_traits<char>::length(kVirtualTaskPrefix);
+  return base::OptionalFromPtr(
+      base::FindOrNull(kVirtualFileTasksMapping, policy_id.substr(kOffset)));
+}
+#endif
+
 std::string TransformRawPolicyId(const std::string& raw_policy_id) {
   if (const GURL raw_policy_id_gurl{raw_policy_id};
       raw_policy_id_gurl.is_valid()) {

chrome/browser/apps/app_service/policy_util.h

@@ -33,6 +33,10 @@ class Profile;
 
 namespace apps_util {
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+constexpr char kVirtualTaskPrefix[] = "VirtualTask/";
+#endif
+
 // Checks whether |policy_id| specifies a Chrome App.
 bool IsChromeAppPolicyId(base::StringPiece policy_id);
 
@@ -52,6 +56,15 @@ bool IsSystemWebAppPolicyId(base::StringPiece policy_id);
 // Checks whether |policy_id| specifies a Preinstalled Web App.
 bool IsPreinstalledWebAppPolicyId(base::StringPiece policy_id);
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+bool IsFileManagerVirtualTaskPolicyId(base::StringPiece policy_id);
+
+// Maps `policy_id` which represents a virtual task to an actual `id` of
+// this virtual task.
+absl::optional<base::StringPiece> GetVirtualTaskIdFromPolicyId(
+    base::StringPiece policy_id);
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
 // Transforms the provided |raw_policy_id| if necessary.
 // For Web Apps, converts it to GURL and returns the spec().
 // Does nothing for other app types.

chrome/browser/ash/BUILD.gn

@@ -1192,6 +1192,7 @@ source_set("ash") {
     "file_manager/url_util.h",
     "file_manager/virtual_file_tasks.cc",
     "file_manager/virtual_file_tasks.h",
+    "file_manager/virtual_tasks/id_constants.h",
     "file_manager/virtual_tasks/install_isolated_web_app_virtual_task.cc",
     "file_manager/virtual_tasks/install_isolated_web_app_virtual_task.h",
     "file_manager/volume.cc",
@@ -4551,6 +4552,8 @@ static_library("test_support") {
     "file_manager/file_manager_test_util.h",
     "file_manager/mount_test_util.cc",
     "file_manager/mount_test_util.h",
+    "file_manager/virtual_tasks/fake_virtual_task.cc",
+    "file_manager/virtual_tasks/fake_virtual_task.h",
     "file_system_provider/fake_extension_provider.cc",
     "file_system_provider/fake_extension_provider.h",
     "file_system_provider/fake_provided_file_system.cc",

chrome/browser/ash/file_manager/app_service_file_tasks.cc

@@ -33,6 +33,7 @@
 #include "chrome/browser/ash/file_manager/filesystem_api_util.h"
 #include "chrome/browser/ash/file_manager/office_file_tasks.h"
 #include "chrome/browser/ash/file_manager/path_util.h"
+#include "chrome/browser/ash/file_manager/virtual_file_tasks.h"
 #include "chrome/browser/ash/fusebox/fusebox_server.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/os_integration/os_integration_manager.h"
@@ -442,18 +443,28 @@ bool ChooseAndSetDefaultTaskFromPolicyPrefs(
   DCHECK_EQ(default_handlers_for_entries.size(), 1U);
   const auto& policy_id = *default_handlers_for_entries.begin();
 
-  std::vector<std::string> app_ids =
-      apps_util::GetAppIdsFromPolicyId(profile, policy_id);
-
   std::vector<FullTaskDescriptor*> filtered_tasks;
-  for (auto& task : resulting_tasks->tasks) {
-    const auto& td = task.task_descriptor;
-    if (base::Contains(app_ids, td.app_id) ||
-        (td.task_type == TASK_TYPE_ARC_APP &&
-         !ash::features::ShouldArcFileTasksUseAppService() &&
-         MatchPolicyIdAgainstLegacyArcAppFormat(td, policy_id))) {
-      filtered_tasks.push_back(&task);
-      continue;
+  // `app_id` matching is not necessary if the policy points to a virtual task.
+  if (absl::optional<base::StringPiece> virtual_task_id =
+          apps_util::GetVirtualTaskIdFromPolicyId(policy_id)) {
+    std::string full_virtual_task_id = ToSwaActionId(*virtual_task_id);
+    for (auto& task : resulting_tasks->tasks) {
+      if (IsVirtualTask(task.task_descriptor) &&
+          task.task_descriptor.action_id == full_virtual_task_id) {
+        filtered_tasks.push_back(&task);
+      }
+    }
+  } else {
+    std::vector<std::string> app_ids =
+        apps_util::GetAppIdsFromPolicyId(profile, policy_id);
+    for (auto& task : resulting_tasks->tasks) {
+      const auto& td = task.task_descriptor;
+      if (base::Contains(app_ids, td.app_id) ||
+          (td.task_type == TASK_TYPE_ARC_APP &&
+           !ash::features::ShouldArcFileTasksUseAppService() &&
+           MatchPolicyIdAgainstLegacyArcAppFormat(td, policy_id))) {
+        filtered_tasks.push_back(&task);
+      }
     }
   }
 

chrome/browser/ash/file_manager/file_tasks.h

@@ -127,7 +127,6 @@ class PrefRegistrySyncable;
 
 namespace file_manager::file_tasks {
 
-constexpr char kActionIdInstallIsolatedWebApp[] = "install-isolated-web-app";
 constexpr char kActionIdView[] = "view";
 constexpr char kActionIdSend[] = "send";
 constexpr char kActionIdSendMultiple[] = "send_multiple";

chrome/browser/ash/file_manager/file_tasks_unittest.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/apps/app_service/app_service_proxy_ash.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/apps/app_service/app_service_test.h"
+#include "chrome/browser/apps/app_service/policy_util.h"
 #include "chrome/browser/ash/crostini/crostini_pref_names.h"
 #include "chrome/browser/ash/crostini/crostini_test_helper.h"
 #include "chrome/browser/ash/crostini/fake_crostini_features.h"
@@ -31,6 +32,8 @@
 #include "chrome/browser/ash/file_manager/app_service_file_tasks.h"
 #include "chrome/browser/ash/file_manager/file_manager_test_util.h"
 #include "chrome/browser/ash/file_manager/path_util.h"
+#include "chrome/browser/ash/file_manager/virtual_file_tasks.h"
+#include "chrome/browser/ash/file_manager/virtual_tasks/fake_virtual_task.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/ash/login/users/scoped_test_user_manager.h"
@@ -256,6 +259,8 @@ class FileManagerFileTaskPolicyDefaultHandlersTest
     CreateAppsAndTasks();
   }
 
+  void TearDown() override { GetTestVirtualTasks().clear(); }
+
  protected:
   void UpdateDefaultHandlersPrefs(
       const std::vector<std::pair<std::string, std::string>>& handlers = {}) {
@@ -270,7 +275,7 @@ class FileManagerFileTaskPolicyDefaultHandlersTest
   ResultingTasks* resulting_tasks() { return resulting_tasks_.get(); }
   std::vector<extensions::EntryInfo>& entries() { return entries_; }
 
-  void CheckCorrectPolicyAssignment(const std::string& default_app_id) {
+  void CheckCorrectPolicyAssignment(base::StringPiece default_app_id) {
     ASSERT_EQ(resulting_tasks()->policy_default_handler_status,
               PolicyDefaultHandlerStatus::kDefaultHandlerAssignedByPolicy);
     ASSERT_EQ(base::ranges::count_if(resulting_tasks()->tasks, &IsDefaultTask),
@@ -280,6 +285,19 @@ class FileManagerFileTaskPolicyDefaultHandlersTest
               default_app_id);
   }
 
+  void CheckCorrectPolicyAssignmentForVirtualTask(
+      base::StringPiece virtual_task_id) {
+    ASSERT_EQ(resulting_tasks()->policy_default_handler_status,
+              PolicyDefaultHandlerStatus::kDefaultHandlerAssignedByPolicy);
+    ASSERT_EQ(base::ranges::count_if(resulting_tasks()->tasks, &IsDefaultTask),
+              1);
+    const auto& task =
+        base::ranges::find_if(resulting_tasks()->tasks, &IsDefaultTask)
+            ->task_descriptor;
+    ASSERT_TRUE(IsVirtualTask(task));
+    ASSERT_THAT(task.action_id, testing::EndsWith(virtual_task_id));
+  }
+
   void CheckConflictingPolicyAssignment() {
     ASSERT_EQ(resulting_tasks()->policy_default_handler_status,
               PolicyDefaultHandlerStatus::kIncorrectAssignment);
@@ -302,6 +320,10 @@ class FileManagerFileTaskPolicyDefaultHandlersTest
   static constexpr char kWebAppUrl[] = "https://web.app";
   static constexpr char kArcAppPackageName[] = "com.package.name";
 
+  // Should be a valid identifier in kVirtualTasksMapping from
+  // chrome/browser/apps/app_service/policy_util.cc.
+  static constexpr char kVirtualTaskActionId[] = "install-isolated-web-app";
+
   static constexpr AppIdPolicyIdPair kAppIdPolicyIdMapping[] = {
       {kWebAppId, kWebAppUrl},
       {kArcAppId, kArcAppPackageName},
@@ -393,6 +415,63 @@ TEST_F(FileManagerFileTaskPolicyDefaultHandlersTest, LegacyArcAppFormat) {
   CheckCorrectPolicyAssignment("com.legacy.package/intentName");
 }
 
+// Check that virtual tasks are handled by the policy.
+TEST_F(FileManagerFileTaskPolicyDefaultHandlersTest, VirtualTask) {
+  auto virtual_task =
+      std::make_unique<FakeVirtualTask>(ToSwaActionId(kVirtualTaskActionId));
+  GetTestVirtualTasks().push_back(virtual_task.get());
+
+  constexpr char kFileName[] = "foo.txt";
+
+  FindVirtualTasks(
+      profile(),
+      {{base::FilePath::FromUTF8Unsafe(kFileName), "text/plain",
+        /*is_directory=*/false}},
+      /*file_urls=*/
+      {GURL(base::StrCat(
+          {"filesystem:chrome://file-manager/external/", kFileName}))},
+      /*dlp_source_urls=*/{}, &resulting_tasks()->tasks);
+
+  UpdateDefaultHandlersPrefs(
+      {{".txt",
+        base::StrCat({apps_util::kVirtualTaskPrefix, kVirtualTaskActionId})}});
+  entries().emplace_back(base::FilePath::FromUTF8Unsafe(kFileName),
+                         "text/plain", false);
+  ASSERT_TRUE(ChooseAndSetDefaultTaskFromPolicyPrefs(profile(), entries(),
+                                                     resulting_tasks()));
+  CheckCorrectPolicyAssignmentForVirtualTask(kVirtualTaskActionId);
+}
+
+// Check that incorrectly assigned virtual tasks are ignored.
+TEST_F(FileManagerFileTaskPolicyDefaultHandlersTest,
+       VirtualTaskIncorrectAssignment) {
+  auto virtual_task =
+      std::make_unique<FakeVirtualTask>(ToSwaActionId(kVirtualTaskActionId));
+  GetTestVirtualTasks().push_back(virtual_task.get());
+
+  constexpr char kFileName[] = "foo.txt";
+
+  FindVirtualTasks(
+      profile(),
+      {{base::FilePath::FromUTF8Unsafe(kFileName), "text/plain",
+        /*is_directory=*/false}},
+      /*file_urls=*/
+      {GURL(base::StrCat(
+          {"filesystem:chrome://file-manager/external/", kFileName}))},
+      /*dlp_source_urls=*/{}, &resulting_tasks()->tasks);
+
+  constexpr char kNonExistentVirtualTaskActionId[] = "incorrect-virtual-id";
+
+  UpdateDefaultHandlersPrefs(
+      {{".txt", base::StrCat({apps_util::kVirtualTaskPrefix,
+                              kNonExistentVirtualTaskActionId})}});
+  entries().emplace_back(base::FilePath::FromUTF8Unsafe(kFileName),
+                         "text/plain", false);
+  ASSERT_FALSE(ChooseAndSetDefaultTaskFromPolicyPrefs(profile(), entries(),
+                                                      resulting_tasks()));
+  CheckNoPolicyAssignment();
+}
+
 class FileManagerFileTaskPolicyDefaultHandlersTestPerAppType
     : public FileManagerFileTaskPolicyDefaultHandlersTest,
       public testing::WithParamInterface<AppIdPolicyIdPair> {