system-web-apps: add support for File Handling origin trial

This CL adds support so File Handling origin trial can be used for
System Web Apps.

Under the hood, SystemWebAppManager force enables file handlers after
Apps are installed (in on_apps_synchronized event). FileHandlerManager
treats force enabled file handling origin trial to never expire
(technically, it expires at the max time that can be persisted in
preferences).

Fixed: 1045297
Change-Id: Iaa55601c3ecbe37c543beb1ee6f495b62fac6617
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2093959
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749541}

chrome/browser/chromeos/web_applications/media_app_integration_browsertest.cc

@@ -26,6 +26,7 @@
 #include "extensions/browser/entry_info.h"
 #include "extensions/browser/extension_system.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/features.h"
 
 using platform_util::OpenOperationResult;
 using web_app::SystemAppType;
@@ -51,7 +52,10 @@ constexpr char kFileVideoVP9[] = "world.webm";
 class MediaAppIntegrationTest : public SystemWebAppIntegrationTest {
  public:
   MediaAppIntegrationTest() {
-    scoped_feature_list_.InitWithFeatures({chromeos::features::kMediaApp}, {});
+    scoped_feature_list_.InitWithFeatures(
+        {chromeos::features::kMediaApp, blink::features::kNativeFileSystemAPI,
+         blink::features::kFileHandlingAPI},
+        {});
   }
 
  private:

chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc

@@ -379,6 +379,28 @@ IN_PROC_BROWSER_TEST_P(WebAppFileHandlingOriginTrialBrowserTest,
   EXPECT_EQ(0, file_handler_manager().TriggerFileHandlerCleanupForTesting());
 }
 
+IN_PROC_BROWSER_TEST_P(WebAppFileHandlingOriginTrialBrowserTest,
+                       DisableForceEnabledFileHandlingOriginTrial) {
+  InstallFileHandlingPWA();
+  SetUpInterceptorNavigateToAppAndMaybeWait();
+
+  ASSERT_TRUE(file_handler_manager().AreFileHandlersEnabled(app_id()));
+  ASSERT_TRUE(file_handler_manager().GetEnabledFileHandlers(app_id()));
+
+  // Calling this on non-force-enabled origin trial should have no effect.
+  file_handler_manager().DisableForceEnabledFileHandlingOriginTrial(app_id());
+  EXPECT_TRUE(file_handler_manager().AreFileHandlersEnabled(app_id()));
+  EXPECT_TRUE(file_handler_manager().GetEnabledFileHandlers(app_id()));
+
+  // Force enables file handling.
+  file_handler_manager().ForceEnableFileHandlingOriginTrial(app_id());
+
+  // Calling this on force enabled origin trial should remove file handlers.
+  file_handler_manager().DisableForceEnabledFileHandlingOriginTrial(app_id());
+  EXPECT_FALSE(file_handler_manager().AreFileHandlersEnabled(app_id()));
+  EXPECT_EQ(nullptr, file_handler_manager().GetEnabledFileHandlers(app_id()));
+}
+
 namespace {
 static constexpr char kBaseDataDir[] = "chrome/test/data/web_app_file_handling";
 

chrome/browser/web_applications/components/file_handler_manager.cc

@@ -20,6 +20,21 @@
 
 namespace web_app {
 
+namespace {
+// Use a large double that can be safely saved in prefs, and be  safely
+// represented in JS timestamp (milliseconds from epoch). base::Time::Max() does
+// not work here, it returns +Infinity (which is invalid and can not be
+// represented in JSON).
+//
+// This value is `floor((2^53 - 1) / 1000)` because base::Time::FromDoubleT()
+// accepts time offset in seconds. In reality, it means 287396-10-12 08:58:59
+// UTC, which is a long distant future (long after File Handling goes out of
+// origin trial or be deprecated).
+//
+// Do not change this value, because it is persisted to disk.
+const double kMaxOriginTrialExpiryTime = 9007199254740;
+}  // namespace
+
 bool FileHandlerManager::disable_automatic_file_handler_cleanup_for_testing_ =
     false;
 
@@ -109,11 +124,29 @@ void FileHandlerManager::UpdateFileHandlingOriginTrialExpiry(
   DCHECK(expiry_service);
 
   auto* raw = expiry_service.get();
+
+  // Here we need to pass the |expiry_service| Mojom remote interface, so it is
+  // not destroyed before we get a reply.
   raw->RequestOriginTrialExpiryTime(base::BindOnce(
       &FileHandlerManager::OnOriginTrialExpiryTimeReceived,
       weak_ptr_factory_.GetWeakPtr(), std::move(expiry_service), app_id));
 }
 
+void FileHandlerManager::ForceEnableFileHandlingOriginTrial(
+    const AppId& app_id) {
+  UpdateFileHandlersForOriginTrialExpiryTime(
+      app_id, base::Time::FromDoubleT(kMaxOriginTrialExpiryTime));
+}
+
+void FileHandlerManager::DisableForceEnabledFileHandlingOriginTrial(
+    const AppId& app_id) {
+  double pref_expiry_time = GetDoubleWebAppPref(
+      profile()->GetPrefs(), app_id, kFileHandlingOriginTrialExpiryTime);
+  if (pref_expiry_time == kMaxOriginTrialExpiryTime) {
+    UpdateFileHandlersForOriginTrialExpiryTime(app_id, base::Time());
+  }
+}
+
 const std::vector<apps::FileHandlerInfo>*
 FileHandlerManager::GetEnabledFileHandlers(const AppId& app_id) {
   if (AreFileHandlersEnabled(app_id) && IsFileHandlingAPIAvailable(app_id))
@@ -137,6 +170,12 @@ void FileHandlerManager::OnOriginTrialExpiryTimeReceived(
     mojo::AssociatedRemote<blink::mojom::FileHandlingExpiry> /*interface*/,
     const AppId& app_id,
     base::Time expiry_time) {
+  UpdateFileHandlersForOriginTrialExpiryTime(app_id, expiry_time);
+}
+
+void FileHandlerManager::UpdateFileHandlersForOriginTrialExpiryTime(
+    const AppId& app_id,
+    const base::Time& expiry_time) {
   web_app::UpdateDoubleWebAppPref(profile_->GetPrefs(), app_id,
                                   kFileHandlingOriginTrialExpiryTime,
                                   expiry_time.ToDoubleT());

chrome/browser/web_applications/components/file_handler_manager.h

@@ -78,6 +78,14 @@ class FileHandlerManager : public AppRegistrarObserver {
   void UpdateFileHandlingOriginTrialExpiry(content::WebContents* web_contents,
                                            const AppId& app_id);
 
+  // Force enables File Handling origin trial. This will register the App's file
+  // handlers even if the App does not have a valid origin trial token.
+  void ForceEnableFileHandlingOriginTrial(const AppId& app_id);
+
+  // Disable a force enabled File Handling origin trial. This will unregister
+  // App's file handlers.
+  void DisableForceEnabledFileHandlingOriginTrial(const AppId& app_id);
+
   // Gets all enabled file handlers for |app_id|. |nullptr| if the app has no
   // enabled file handlers. Note: The lifetime of the file handlers are tied to
   // the app they belong to.
@@ -115,6 +123,10 @@ class FileHandlerManager : public AppRegistrarObserver {
       const AppId& app_id,
       base::Time expiry_time);
 
+  void UpdateFileHandlersForOriginTrialExpiryTime(
+      const AppId& app_id,
+      const base::Time& expiry_time);
+
   // Removes file handlers whose origin trials have expired (assuming
   // kFileHandlingAPI isn't enabled). Returns the number of apps that had file
   // handlers unregistered, for use in tests.

chrome/browser/web_applications/extensions/system_web_app_manager_unittest.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/web_applications/components/pending_app_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/test/test_app_registrar.h"
+#include "chrome/browser/web_applications/test/test_file_handler_manager.h"
 #include "chrome/browser/web_applications/test/test_pending_app_manager.h"
 #include "chrome/browser/web_applications/test/test_system_web_app_manager.h"
 #include "chrome/browser/web_applications/test/test_web_app_provider.h"
@@ -88,6 +89,11 @@ class SystemWebAppManagerTest : public ChromeRenderViewHostTestHarness {
     test_pending_app_manager_ = test_pending_app_manager.get();
     provider->SetPendingAppManager(std::move(test_pending_app_manager));
 
+    auto test_file_handler_manager =
+        std::make_unique<TestFileHandlerManager>(profile());
+    test_file_handler_manager_ = test_file_handler_manager.get();
+    provider->SetFileHandlerManager(std::move(test_file_handler_manager));
+
     auto system_web_app_manager =
         std::make_unique<TestSystemWebAppManager>(profile());
     system_web_app_manager_ = system_web_app_manager.get();
@@ -124,6 +130,7 @@ class SystemWebAppManagerTest : public ChromeRenderViewHostTestHarness {
   base::test::ScopedFeatureList scoped_feature_list_;
   TestAppRegistrar* test_app_registrar_ = nullptr;
   TestPendingAppManager* test_pending_app_manager_ = nullptr;
+  TestFileHandlerManager* test_file_handler_manager_ = nullptr;
   TestSystemWebAppManager* system_web_app_manager_ = nullptr;
   TestWebAppUiManager* ui_manager_ = nullptr;
 

chrome/browser/web_applications/system_web_app_manager.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
+#include "chrome/browser/web_applications/components/file_handler_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
 #include "chrome/browser/web_applications/components/web_app_ui_manager.h"
@@ -51,6 +52,10 @@ namespace web_app {
 
 namespace {
 
+// Copy the origin trial name from runtime_enabled_features.json5, to avoid
+// complex dependencies.
+const char kFileHandlingOriginTrial[] = "FileHandling";
+
 // Use #if defined to avoid compiler error on unused function.
 #if defined(OS_CHROMEOS) && !defined(OFFICIAL_BUILD)
 
@@ -238,12 +243,15 @@ void SystemWebAppManager::Shutdown() {
   shutting_down_ = true;
 }
 
-void SystemWebAppManager::SetSubsystems(PendingAppManager* pending_app_manager,
-                                        AppRegistrar* registrar,
-                                        WebAppUiManager* ui_manager) {
+void SystemWebAppManager::SetSubsystems(
+    PendingAppManager* pending_app_manager,
+    AppRegistrar* registrar,
+    WebAppUiManager* ui_manager,
+    FileHandlerManager* file_handler_manager) {
   pending_app_manager_ = pending_app_manager;
   registrar_ = registrar;
   ui_manager_ = ui_manager;
+  file_handler_manager_ = file_handler_manager;
 }
 
 void SystemWebAppManager::Start() {
@@ -358,6 +366,13 @@ const std::vector<std::string>* SystemWebAppManager::GetEnabledOriginTrials(
   return &iter_trials->second;
 }
 
+bool SystemWebAppManager::AppHasFileHandlingOriginTrial(SystemAppType type) {
+  const auto& info = system_app_infos_.at(type);
+  const std::vector<std::string>* trials =
+      GetEnabledOriginTrials(type, info.install_url);
+  return trials && base::Contains(*trials, kFileHandlingOriginTrial);
+}
+
 void SystemWebAppManager::OnReadyToCommitNavigation(
     const AppId& app_id,
     content::NavigationHandle* navigation_handle) {
@@ -486,6 +501,23 @@ void SystemWebAppManager::OnAppsSynchronized(
     const base::TimeTicks& install_start_time,
     std::map<GURL, InstallResultCode> install_results,
     std::map<GURL, bool> uninstall_results) {
+  // TODO(crbug.com/1053371): Clean up File Handler install. We install SWA file
+  // handlers here, because the code that registers file handlers for regular
+  // Web Apps, does not run when for apps installed in the background.
+  for (const auto& it : system_app_infos_) {
+    const SystemAppType& type = it.first;
+    base::Optional<AppId> app_id = GetAppIdForSystemApp(type);
+    if (!app_id)
+      continue;
+
+    if (AppHasFileHandlingOriginTrial(type)) {
+      file_handler_manager_->ForceEnableFileHandlingOriginTrial(app_id.value());
+    } else {
+      file_handler_manager_->DisableForceEnabledFileHandlingOriginTrial(
+          app_id.value());
+    }
+  }
+
   const base::TimeDelta install_duration =
       install_start_time - base::TimeTicks::Now();
 

chrome/browser/web_applications/system_web_app_manager.h

@@ -38,6 +38,7 @@ class Profile;
 namespace web_app {
 
 class WebAppUiManager;
+class FileHandlerManager;
 
 // An enum that lists the different System Apps that exist. Can be used to
 // retrieve the App ID from the underlying Web App system.
@@ -121,7 +122,8 @@ class SystemWebAppManager {
 
   void SetSubsystems(PendingAppManager* pending_app_manager,
                      AppRegistrar* registrar,
-                     WebAppUiManager* ui_manager);
+                     WebAppUiManager* ui_manager,
+                     FileHandlerManager* file_handler_manager);
 
   void Start();
 
@@ -198,6 +200,8 @@ class SystemWebAppManager {
   const std::vector<std::string>* GetEnabledOriginTrials(SystemAppType type,
                                                          const GURL& url);
 
+  bool AppHasFileHandlingOriginTrial(SystemAppType type);
+
   void OnAppsSynchronized(const base::TimeTicks& install_start_time,
                           std::map<GURL, InstallResultCode> install_results,
                           std::map<GURL, bool> uninstall_results);
@@ -230,6 +234,8 @@ class SystemWebAppManager {
 
   WebAppUiManager* ui_manager_ = nullptr;
 
+  FileHandlerManager* file_handler_manager_ = nullptr;
+
   base::WeakPtrFactory<SystemWebAppManager> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SystemWebAppManager);

chrome/browser/web_applications/system_web_app_manager_browsertest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/web_applications/system_web_app_manager_browsertest.h"
 
 #include <string>
+#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -45,10 +46,7 @@ namespace web_app {
 
 SystemWebAppManagerBrowserTestBase::SystemWebAppManagerBrowserTestBase(
     bool install_mock) {
-  scoped_feature_list_.InitWithFeatures(
-      {features::kSystemWebApps, blink::features::kNativeFileSystemAPI,
-       blink::features::kFileHandlingAPI},
-      {});
+  scoped_feature_list_.InitWithFeatures({features::kSystemWebApps}, {});
   if (install_mock) {
     maybe_installation_ =
         TestSystemWebAppInstallation::SetUpStandaloneSingleWindowApp();
@@ -193,10 +191,54 @@ IN_PROC_BROWSER_TEST_P(SystemWebAppManagerBrowserTest,
   EXPECT_TRUE(app_browser->app_controller()->ShouldShowCustomTabBar());
 }
 
+class SystemWebAppManagerFileHandlingBrowserTestBase
+    : public SystemWebAppManagerBrowserTestBase,
+      public testing::WithParamInterface<std::tuple<bool, bool>> {
+ public:
+  using IncludeLaunchDirectory =
+      TestSystemWebAppInstallation::IncludeLaunchDirectory;
+  explicit SystemWebAppManagerFileHandlingBrowserTestBase(
+      IncludeLaunchDirectory include_launch_directory)
+      : SystemWebAppManagerBrowserTestBase(/*install_mock=*/false) {
+    bool enable_origin_scoped_permission_context;
+    bool enable_desktop_pwas_without_extensions;
+    std::tie(enable_origin_scoped_permission_context,
+             enable_desktop_pwas_without_extensions) = GetParam();
+
+    scoped_feature_permission_context_.InitWithFeatureState(
+        features::kNativeFileSystemOriginScopedPermissions,
+        enable_origin_scoped_permission_context);
+    scoped_feature_web_app_provider_type_.InitWithFeatureState(
+        features::kDesktopPWAsWithoutExtensions,
+        enable_desktop_pwas_without_extensions);
+    scoped_feature_blink_api_.InitWithFeatures(
+        {blink::features::kNativeFileSystemAPI,
+         blink::features::kFileHandlingAPI},
+        {});
+
+    maybe_installation_ =
+        TestSystemWebAppInstallation::SetUpAppThatReceivesLaunchFiles(
+            include_launch_directory);
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_permission_context_;
+  base::test::ScopedFeatureList scoped_feature_web_app_provider_type_;
+  base::test::ScopedFeatureList scoped_feature_blink_api_;
+};
+
+class SystemWebAppManagerLaunchFilesBrowserTest
+    : public SystemWebAppManagerFileHandlingBrowserTestBase {
+ public:
+  SystemWebAppManagerLaunchFilesBrowserTest()
+      : SystemWebAppManagerFileHandlingBrowserTestBase(
+            IncludeLaunchDirectory::kNo) {}
+};
+
 // Check launch files are passed to application.
 // Note: This test uses ExecuteScriptXXX instead of ExecJs and EvalJs because of
 // some quirks surrounding origin trials and content security policies.
-IN_PROC_BROWSER_TEST_P(SystemWebAppManagerBrowserTest,
+IN_PROC_BROWSER_TEST_P(SystemWebAppManagerLaunchFilesBrowserTest,
                        LaunchFilesForSystemWebApp) {
   WaitForTestSystemAppInstall();
   apps::AppLaunchParams params = LaunchParamsForApp(GetMockAppType());
@@ -271,26 +313,19 @@ IN_PROC_BROWSER_TEST_P(SystemWebAppManagerBrowserTest,
   EXPECT_EQ(temp_file_path2.BaseName().AsUTF8Unsafe(), file_name);
 }
 
-class SystemWebAppManagerLaunchFilesBrowserTest
-    : public SystemWebAppManagerBrowserTestBase,
-      public testing::WithParamInterface<std::vector<base::Feature>> {
+class SystemWebAppManagerLaunchDirectoryBrowserTest
+    : public SystemWebAppManagerFileHandlingBrowserTestBase {
  public:
-  SystemWebAppManagerLaunchFilesBrowserTest()
-      : SystemWebAppManagerBrowserTestBase(/*install_mock=*/false) {
-    scoped_feature_list_.InitWithFeatures(GetParam(), {});
-    maybe_installation_ =
-        TestSystemWebAppInstallation::SetUpAppThatReceivesLaunchDirectory();
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
+  SystemWebAppManagerLaunchDirectoryBrowserTest()
+      : SystemWebAppManagerFileHandlingBrowserTestBase(
+            IncludeLaunchDirectory::kYes) {}
 };
 
 // Launching behavior for apps that do not want to received launch directory are
 // tested in |SystemWebAppManagerBrowserTestBase.LaunchFilesForSystemWebApp|.
 // Note: This test uses ExecuteScriptXXX instead of ExecJs and EvalJs because of
 // some quirks surrounding origin trials and content security policies.
-IN_PROC_BROWSER_TEST_P(SystemWebAppManagerLaunchFilesBrowserTest,
+IN_PROC_BROWSER_TEST_P(SystemWebAppManagerLaunchDirectoryBrowserTest,
                        LaunchDirectoryForSystemWebApp) {
   WaitForTestSystemAppInstall();
   apps::AppLaunchParams params = LaunchParamsForApp(GetMockAppType());
@@ -456,6 +491,58 @@ IN_PROC_BROWSER_TEST_P(SystemWebAppManagerLaunchFilesBrowserTest,
   EXPECT_FALSE(base::PathExists(temp_file_path2));
 }
 
+class SystemWebAppManagerFileHandlingOriginTrialsBrowserTest
+    : public SystemWebAppManagerBrowserTest {
+ public:
+  SystemWebAppManagerFileHandlingOriginTrialsBrowserTest()
+      : SystemWebAppManagerBrowserTest(/*install_mock=*/false) {
+    maybe_installation_ =
+        TestSystemWebAppInstallation::SetUpAppWithEnabledOriginTrials(
+            OriginTrialsMap({{GetOrigin(GURL("chrome://test-system-app/")),
+                              {"NativeFileSystem2", "FileHandling"}}}));
+  }
+
+  ~SystemWebAppManagerFileHandlingOriginTrialsBrowserTest() override = default;
+
+ private:
+  url::Origin GetOrigin(const GURL& url) { return url::Origin::Create(url); }
+};
+
+IN_PROC_BROWSER_TEST_P(SystemWebAppManagerFileHandlingOriginTrialsBrowserTest,
+                       FileHandlingWorks) {
+  WaitForTestSystemAppInstall();
+  apps::AppLaunchParams params = LaunchParamsForApp(GetMockAppType());
+  params.source = apps::mojom::AppLaunchSource::kSourceChromeInternal;
+
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  base::ScopedTempDir temp_directory;
+  ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
+  base::FilePath temp_file_path;
+  ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_directory.GetPath(),
+                                             &temp_file_path));
+
+  const GURL& launch_url = WebAppProvider::Get(browser()->profile())
+                               ->registrar()
+                               .GetAppLaunchURL(params.app_id);
+
+  params.launch_files = {temp_file_path};
+  content::TestNavigationObserver navigation_observer(launch_url);
+  navigation_observer.StartWatchingNewWebContents();
+  content::WebContents* web_contents =
+      apps::LaunchService::Get(browser()->profile())->OpenApplication(params);
+  navigation_observer.Wait();
+
+  // Wait for the Promise to resolve.
+  bool promise_resolved = false;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
+      web_contents,
+      "launchQueue.setConsumer(launchParams => {"
+      "  domAutomationController.send(true);"
+      "});",
+      &promise_resolved));
+  EXPECT_TRUE(promise_resolved);
+}
+
 class SystemWebAppManagerNotShownInLauncherTest
     : public SystemWebAppManagerBrowserTest {
  public:
@@ -720,19 +807,18 @@ INSTANTIATE_TEST_SUITE_P(All,
                          ProviderTypeParamToString);
 
 INSTANTIATE_TEST_SUITE_P(
-    PermissionContext,
+    All,
     SystemWebAppManagerLaunchFilesBrowserTest,
-    testing::Values(
-        /*default_enabled_permission_context*/ std::vector<base::Feature>(),
-        /*origin_scoped_permission_context*/
-        std::vector<base::Feature>(
-            {features::kNativeFileSystemOriginScopedPermissions}),
-        /*default_enabled_permission_context*/
-        std::vector<base::Feature>({features::kDesktopPWAsWithoutExtensions}),
-        /*origin_scoped_permission_context*/
-        std::vector<base::Feature>(
-            {features::kNativeFileSystemOriginScopedPermissions,
-             features::kDesktopPWAsWithoutExtensions})));
+    testing::Combine(
+        /* enable_origin_scoped_permission_context */ testing::Bool(),
+        /* enable_pwas_without_extensions */ testing::Bool()));
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    SystemWebAppManagerLaunchDirectoryBrowserTest,
+    testing::Combine(
+        /* enable_origin_scoped_permission_context */ testing::Bool(),
+        /* enable_desktop_pwas_without_extensions */ testing::Bool()));
 
 INSTANTIATE_TEST_SUITE_P(All,
                          SystemWebAppManagerNotShownInLauncherTest,
@@ -758,4 +844,10 @@ INSTANTIATE_TEST_SUITE_P(All,
                                            ProviderType::kWebApps),
                          ProviderTypeParamToString);
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         SystemWebAppManagerFileHandlingOriginTrialsBrowserTest,
+                         ::testing::Values(ProviderType::kBookmarkApps,
+                                           ProviderType::kWebApps),
+                         ProviderTypeParamToString);
+
 }  // namespace web_app

chrome/browser/web_applications/test/test_system_web_app_installation.cc

@@ -125,10 +125,16 @@ TestSystemWebAppInstallation::SetUpStandaloneSingleWindowApp() {
 
 // static
 std::unique_ptr<TestSystemWebAppInstallation>
-TestSystemWebAppInstallation::SetUpAppThatReceivesLaunchDirectory() {
+TestSystemWebAppInstallation::SetUpAppThatReceivesLaunchFiles(
+    IncludeLaunchDirectory include_launch_directory) {
   SystemAppInfo media_system_app_info(
       "Media", GURL("chrome://test-system-app/pwa.html"));
-  media_system_app_info.include_launch_directory = true;
+
+  if (include_launch_directory == IncludeLaunchDirectory::kYes)
+    media_system_app_info.include_launch_directory = true;
+  else
+    media_system_app_info.include_launch_directory = false;
+
   return base::WrapUnique(new TestSystemWebAppInstallation(
       SystemAppType::MEDIA, media_system_app_info));
 }

chrome/browser/web_applications/test/test_system_web_app_installation.h

@@ -22,12 +22,15 @@ namespace web_app {
 // WaitForAppInstall() to finish the installation.
 class TestSystemWebAppInstallation {
  public:
+  enum IncludeLaunchDirectory { kYes, kNo };
+
   static std::unique_ptr<TestSystemWebAppInstallation>
   SetUpTabbedMultiWindowApp();
   static std::unique_ptr<TestSystemWebAppInstallation>
   SetUpStandaloneSingleWindowApp();
   static std::unique_ptr<TestSystemWebAppInstallation>
-  SetUpAppThatReceivesLaunchDirectory();
+  SetUpAppThatReceivesLaunchFiles(
+      IncludeLaunchDirectory include_launch_directory);
   static std::unique_ptr<TestSystemWebAppInstallation>
   SetUpAppWithEnabledOriginTrials(const OriginTrialsMap& origin_to_trials);
   static std::unique_ptr<TestSystemWebAppInstallation>

chrome/browser/web_applications/web_app_provider.cc

@@ -242,7 +242,8 @@ void WebAppProvider::ConnectSubsystems() {
       ui_manager_.get(), install_finalizer_.get());
   external_web_app_manager_->SetSubsystems(pending_app_manager_.get());
   system_web_app_manager_->SetSubsystems(pending_app_manager_.get(),
-                                         registrar_.get(), ui_manager_.get());
+                                         registrar_.get(), ui_manager_.get(),
+                                         file_handler_manager_.get());
   web_app_policy_manager_->SetSubsystems(pending_app_manager_.get());
   file_handler_manager_->SetSubsystems(registrar_.get());
   shortcut_manager_->SetSubsystems(registrar_.get());

chrome/browser/web_applications/web_app_tab_helper.cc

@@ -106,7 +106,10 @@ void WebAppTabHelper::DOMContentLoaded(
   if (app_id_.empty())
     return;
 
-  // Update when the file handling origin trial expires for this app.
+  // Ordinary Web Apps need to hold valid origin trial tokens to continue using
+  // File Handling API.
+  if (provider_->system_web_app_manager().IsSystemWebApp(app_id_))
+    return;
   provider_->file_handler_manager().UpdateFileHandlingOriginTrialExpiry(
       web_contents(), app_id_);
 }
@@ -135,8 +138,16 @@ void WebAppTabHelper::OnWebAppInstalled(const AppId& installed_app_id) {
 
   SetAppId(app_id);
 
-  // When the app is installed record when its File Handling origin trial
-  // expires, so it can be removed.
+  // Ordinary Web Apps need valid origin trial tokens to use File Handling API.
+  // When an App is installed, record its origin trial expiry time.
+  //
+  // TODO(crbug.com/1053371): Clean up where we install file handlers.
+  // The following check is not necessary. This tab helper is not attached to
+  // the WebContents we used to install the WebApp. When a System Web App is
+  // installed, there is no associated tab and we early return at `app_id !=
+  // installed_app_id`.
+  if (provider_->system_web_app_manager().IsSystemWebApp(installed_app_id))
+    return;
   provider_->file_handler_manager().UpdateFileHandlingOriginTrialExpiry(
       web_contents(), installed_app_id);
 }