WebApps: always bind WebLaunchService for File Handling

Prior to this CL, WebLaunchServiceImpl is behind a FileHandlingEnabled
check on the document. However, at the time of this check, the actual
document is not loaded (we get a blank document without origin trial
tokens). So FileHandlingEnabled check always return false, unless we
turn on runtime flags.

This check cause the problem: when a PWA with File Handling origin
trial is launched as a file handler, the app will not receive the
launch files.

This CL fixes the problem by always bind WebLaunchServiceImpl, this is
okay because the DOM (JavaScript) side is still behind a origin trial
check, we will not be exposing extra APIs to webpages.

Fixed: 1053819
Bug: 1045297
Change-Id: I19966d26afe97dfd3115f142343c6eaacdfe0eab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063196
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Reviewed-by: Olivier Yiptong <oyiptong@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743789}

chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc

@@ -4,6 +4,7 @@
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
@@ -14,9 +15,11 @@
 #include "chrome/browser/web_applications/components/file_handler_manager.h"
 #include "chrome/browser/web_applications/components/web_app_prefs_utils.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/web_application_info.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/url_loader_interceptor.h"
 #include "mojo/public/cpp/bindings/associated_receiver.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
@@ -112,6 +115,56 @@ class WebAppFileHandlingTestBase : public web_app::WebAppControllerBrowserTest {
   web_app::AppId app_id_;
 };
 
+namespace {
+
+base::FilePath NewTestFilePath(const base::FilePath::CharType* extension) {
+  // CreateTemporaryFile blocks, temporarily allow blocking.
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  // In order to test file handling, we need to be able to supply a file
+  // extension for the temp file.
+  base::FilePath test_file_path;
+  base::CreateTemporaryFile(&test_file_path);
+  base::FilePath new_file_path = test_file_path.AddExtension(extension);
+  EXPECT_TRUE(base::ReplaceFile(test_file_path, new_file_path, nullptr));
+  return new_file_path;
+}
+
+content::WebContents* LaunchApplication(
+    Profile* profile,
+    const std::string& app_id,
+    const GURL& expected_launch_url,
+    const apps::mojom::LaunchContainer launch_container =
+        apps::mojom::LaunchContainer::kLaunchContainerWindow,
+    const apps::mojom::AppLaunchSource launch_source =
+        apps::mojom::AppLaunchSource::kSourceTest,
+    const std::vector<base::FilePath>& files = std::vector<base::FilePath>()) {
+  apps::AppLaunchParams params(app_id, launch_container,
+                               WindowOpenDisposition::NEW_WINDOW,
+                               launch_source);
+
+  if (files.size())
+    params.launch_files = files;
+
+  content::TestNavigationObserver navigation_observer(expected_launch_url);
+  navigation_observer.StartWatchingNewWebContents();
+
+  content::WebContents* web_contents =
+      apps::LaunchService::Get(profile)->OpenApplication(params);
+
+  navigation_observer.Wait();
+
+  // Attach the launchParams to the window so we can inspect them easily.
+  auto result = content::EvalJs(web_contents,
+                                "launchQueue.setConsumer(launchParams => {"
+                                "  window.launchParams = launchParams;"
+                                "});");
+
+  return web_contents;
+}
+
+}  // namespace
+
 class WebAppFileHandlingBrowserTest : public WebAppFileHandlingTestBase {
  public:
   WebAppFileHandlingBrowserTest() {
@@ -120,46 +173,15 @@ class WebAppFileHandlingBrowserTest : public WebAppFileHandlingTestBase {
          blink::features::kFileHandlingAPI},
         {});
   }
-
-  base::FilePath NewTestFilePath(const base::FilePath::CharType* extension) {
-    // CreateTemporaryFile blocks, temporarily allow blocking.
-    base::ScopedAllowBlockingForTesting allow_blocking;
-
-    // In order to test file handling, we need to be able to supply a file
-    // extension for the temp file.
-    base::FilePath test_file_path;
-    base::CreateTemporaryFile(&test_file_path);
-    base::FilePath new_file_path = test_file_path.AddExtension(extension);
-    EXPECT_TRUE(base::ReplaceFile(test_file_path, new_file_path, nullptr));
-    return new_file_path;
-  }
-
-  virtual content::WebContents* LaunchWithFiles(
+  content::WebContents* LaunchWithFiles(
       const std::string& app_id,
       const GURL& expected_launch_url,
       const std::vector<base::FilePath>& files,
       const apps::mojom::LaunchContainer launch_container =
           apps::mojom::LaunchContainer::kLaunchContainerWindow) {
-    apps::AppLaunchParams params(
-        app_id, launch_container, WindowOpenDisposition::NEW_WINDOW,
-        apps::mojom::AppLaunchSource::kSourceFileHandler);
-    params.launch_files = files;
-
-    content::TestNavigationObserver navigation_observer(expected_launch_url);
-    navigation_observer.StartWatchingNewWebContents();
-
-    content::WebContents* web_contents =
-        apps::LaunchService::Get(profile())->OpenApplication(params);
-
-    navigation_observer.Wait();
-
-    // Attach the launchParams to the window so we can inspect them easily.
-    auto result = content::EvalJs(web_contents,
-                                  "launchQueue.setConsumer(launchParams => {"
-                                  "  window.launchParams = launchParams;"
-                                  "});");
-
-    return web_contents;
+    return LaunchApplication(
+        profile(), app_id, expected_launch_url, launch_container,
+        apps::mojom::AppLaunchSource::kSourceFileHandler, files);
   }
 
  private:
@@ -357,6 +379,93 @@ IN_PROC_BROWSER_TEST_P(WebAppFileHandlingOriginTrialBrowserTest,
   EXPECT_EQ(0, file_handler_manager().TriggerFileHandlerCleanupForTesting());
 }
 
+namespace {
+static constexpr char kBaseDataDir[] = "chrome/test/data/web_app_file_handling";
+
+// This is the public key of tools/origin_trials/eftest.key, used to validate
+// origin trial tokens generated by tools/origin_trials/generate_token.py.
+static constexpr char kOriginTrialPublicKeyForTesting[] =
+    "dRCs+TocuKkocNKa0AtZ4awrt9XKH2SQCI6o4FY6BNA=";
+
+// Create a URLLoaderInterceptor that serves a test Web App (with origin trial
+// token) at the given url.
+std::unique_ptr<content::URLLoaderInterceptor>
+CreateURLLoaderInterceptorAndServeTestApp(const GURL& app_url) {
+  return std::make_unique<content::URLLoaderInterceptor>(
+      base::BindLambdaForTesting(
+          [&](content::URLLoaderInterceptor::RequestParams* params) -> bool {
+            // Only serve this App's url.
+            if (params->url_request.url != app_url)
+              return false;
+
+            content::URLLoaderInterceptor::WriteResponse(
+                base::StrCat(
+                    {kBaseDataDir, params->url_request.url.path_piece()}),
+                params->client.get());
+            return true;
+          }));
+}
+}  // namespace
+
+class WebAppFileHandlingOriginTrialTest
+    : public web_app::WebAppControllerBrowserTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    WebAppControllerBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(switches::kOriginTrialPublicKey,
+                                    kOriginTrialPublicKeyForTesting);
+  }
+
+ protected:
+  web_app::AppId InstallFileHandlingWebApp(const GURL& app_url) {
+    auto web_app_info = std::make_unique<WebApplicationInfo>();
+    web_app_info->app_url = app_url;
+    web_app_info->scope = app_url.GetWithoutFilename();
+    web_app_info->title = base::ASCIIToUTF16("A Web App");
+
+    blink::Manifest::FileHandler entry1;
+    entry1.action = app_url;
+    entry1.name = base::ASCIIToUTF16("text");
+    entry1.accept[base::ASCIIToUTF16("text/*")].push_back(
+        base::ASCIIToUTF16(".txt"));
+    web_app_info->file_handlers.push_back(std::move(entry1));
+
+    web_app::AppId app_id =
+        WebAppControllerBrowserTest::InstallWebApp(std::move(web_app_info));
+
+    // Here we need first launch the App, so it can update the origin trial
+    // expiry time in prefs. This is needed because the above InstallWebApp
+    // invocation bypassed the normal Web App install pipeline.
+    content::WebContents* web_content =
+        LaunchApplication(profile(), app_id, app_url);
+    web_content->Close();
+
+    return app_id;
+  }
+};
+
+IN_PROC_BROWSER_TEST_P(WebAppFileHandlingOriginTrialTest,
+                       LaunchParamsArePassedCorrectly) {
+  const GURL app_url = GURL("https://file-handling-pwa/index.html");
+
+  // We need to use URLLoaderInterceptor (rather than a EmbeddedTestServer),
+  // because origin trial token is associated with a fixed origin, whereas
+  // EmbeddedTestServer serves content on a random port.
+  std::unique_ptr<content::URLLoaderInterceptor> url_loader_interceptor_ =
+      CreateURLLoaderInterceptorAndServeTestApp(app_url);
+
+  const web_app::AppId app_id = InstallFileHandlingWebApp(app_url);
+  base::FilePath test_file_path = NewTestFilePath(FILE_PATH_LITERAL("txt"));
+  content::WebContents* web_content = LaunchApplication(
+      profile(), app_id, app_url,
+      apps::mojom::LaunchContainer::kLaunchContainerWindow,
+      apps::mojom::AppLaunchSource::kSourceFileHandler, {test_file_path});
+  EXPECT_EQ(1,
+            content::EvalJs(web_content, "window.launchParams.files.length"));
+  EXPECT_EQ(test_file_path.BaseName().AsUTF8Unsafe(),
+            content::EvalJs(web_content, "window.launchParams.files[0].name"));
+}
+
 INSTANTIATE_TEST_SUITE_P(
     All,
     WebAppFileHandlingBrowserTest,
@@ -374,3 +483,12 @@ INSTANTIATE_TEST_SUITE_P(
         web_app::ControllerType::kUnifiedControllerWithBookmarkApp,
         web_app::ControllerType::kUnifiedControllerWithWebApp),
     web_app::ControllerTypeParamToString);
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    WebAppFileHandlingOriginTrialTest,
+    ::testing::Values(
+        web_app::ControllerType::kHostedAppController,
+        web_app::ControllerType::kUnifiedControllerWithBookmarkApp,
+        web_app::ControllerType::kUnifiedControllerWithWebApp),
+    web_app::ControllerTypeParamToString);

chrome/test/data/web_app_file_handling/index.html

+<html>
+
+<head>
+  <!--
+    NOTE: these tokens expires at 2033-05-18 03:33:20 UTC, we don't use larger
+    values because origin trial token internally parse expiry timestamp as an
+    int.
+  -->
+
+  <!-- tools/origin_trials/generate_token.py https://file-handling-pwa NativeFileSystem --expire-timestamp 2000000000 -->
+  <meta http-equiv="origin-trial" name="native-file-system" content="Arkp12LJOMKVzm21vq1QuXCr5Q3+FX15nAfKwFNFSipg8msSrT+d4ZfK0WPrCg891VIAHgf3J1ga/jA9H9V3IgYAAABceyJvcmlnaW4iOiAiaHR0cHM6Ly9maWxlLWhhbmRsaW5nLXB3YTo0NDMiLCAiZmVhdHVyZSI6ICJGaWxlSGFuZGxpbmciLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0=">
+
+  <!-- tools/origin_trials/generate_token.py https://file-handling-pwa FileHandling --expire-timestamp 2000000000 -->
+  <meta http-equiv="origin-trial" name="file-handling" content="Arkp12LJOMKVzm21vq1QuXCr5Q3+FX15nAfKwFNFSipg8msSrT+d4ZfK0WPrCg891VIAHgf3J1ga/jA9H9V3IgYAAABceyJvcmlnaW4iOiAiaHR0cHM6Ly9maWxlLWhhbmRsaW5nLXB3YTo0NDMiLCAiZmVhdHVyZSI6ICJGaWxlSGFuZGxpbmciLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0=">
+</head>
+
+<body></body>
+
+</html>

third_party/blink/renderer/modules/modules_initializer.cc

@@ -163,10 +163,8 @@ void ModulesInitializer::InitLocalFrame(LocalFrame& frame) const {
     frame.GetInterfaceRegistry()->AddInterface(WTF::BindRepeating(
         &DocumentMetadataServer::BindMojoReceiver, WrapWeakPersistent(&frame)));
   }
-  if (RuntimeEnabledFeatures::FileHandlingEnabled(frame.GetDocument())) {
-    frame.GetInterfaceRegistry()->AddAssociatedInterface(WTF::BindRepeating(
-        &WebLaunchServiceImpl::Create, WrapWeakPersistent(&frame)));
-  }
+  frame.GetInterfaceRegistry()->AddAssociatedInterface(WTF::BindRepeating(
+      &WebLaunchServiceImpl::Create, WrapWeakPersistent(&frame)));
   frame.GetInterfaceRegistry()->AddAssociatedInterface(WTF::BindRepeating(
       &FileHandlingExpiryImpl::Create, WrapWeakPersistent(&frame)));