WebApps: Make file handling work for applications in tabs.

NavigateApplicationWindow (previously ShowApplicationWindow) has been
refactored to only show the application window, but we still need the
ability to handle files in a tab. This CL lifts the file handling
functionality up so that it works for apps that open in tabs as well as
in windows.

Bug: 829689
Change-Id: I6870d5a399ef993401b35d024bae06fe6269cac2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1961291
Commit-Queue: Robert Woods <robertwoods@google.com>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Jay Harris <harrisjay@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727842}

chrome/browser/ui/extensions/application_launch.cc

@@ -144,14 +144,22 @@ bool IsAllowedToOverrideURL(const extensions::Extension* extension,
 // |override_url|, if non-empty, will be preferred over the extension's
 // launch url.
 GURL UrlForExtension(const extensions::Extension* extension,
-                     const GURL& override_url) {
+                     Profile* profile,
+                     const apps::AppLaunchParams& params) {
   if (!extension)
-    return override_url;
+    return params.override_url;
 
   GURL url;
-  if (!override_url.is_empty()) {
-    DCHECK(IsAllowedToOverrideURL(extension, override_url));
-    url = override_url;
+  if (!params.override_url.is_empty()) {
+    DCHECK(IsAllowedToOverrideURL(extension, params.override_url));
+    url = params.override_url;
+  } else if (extension->from_bookmark()) {
+    web_app::FileHandlerManager& file_handler_manager =
+        web_app::WebAppProviderBase::GetProviderBase(profile)
+            ->file_handler_manager();
+    url = file_handler_manager
+              .GetMatchingFileHandlerURL(params.app_id, params.launch_files)
+              .value_or(extensions::AppLaunchInfo::GetFullLaunchURL(extension));
   } else {
     url = extensions::AppLaunchInfo::GetFullLaunchURL(extension);
   }
@@ -303,7 +311,7 @@ WebContents* OpenEnabledApplication(Profile* profile,
   UMA_HISTOGRAM_ENUMERATION("Extensions.HostedAppLaunchContainer",
                             params.container);
 
-  GURL url = UrlForExtension(extension, params.override_url);
+  GURL url = UrlForExtension(extension, profile, params);
 
   // System Web Apps go through their own launch path.
   base::Optional<web_app::SystemAppType> system_app_type =
@@ -337,6 +345,11 @@ WebContents* OpenEnabledApplication(Profile* profile,
       break;
   }
 
+  if (base::FeatureList::IsEnabled(blink::features::kFileHandlingAPI)) {
+    web_launch::WebLaunchFilesHelper::SetLaunchPaths(tab, url,
+                                                     params.launch_files);
+  }
+
   if (extension->from_bookmark()) {
     UMA_HISTOGRAM_ENUMERATION("Extensions.BookmarkAppLaunchSource",
                               params.source);
@@ -410,24 +423,13 @@ Browser* CreateApplicationWindow(Profile* profile,
 
 WebContents* NavigateApplicationWindow(Browser* browser,
                                        const apps::AppLaunchParams& params,
-                                       const GURL& default_url,
+                                       const GURL& url,
                                        WindowOpenDisposition disposition) {
   const Extension* const extension = GetExtension(browser->profile(), params);
   ui::PageTransition transition =
       (extension ? ui::PAGE_TRANSITION_AUTO_BOOKMARK
                  : ui::PAGE_TRANSITION_AUTO_TOPLEVEL);
 
-  GURL url = default_url;
-
-  if (extension && extension->from_bookmark()) {
-    web_app::FileHandlerManager& file_handler_manager =
-        web_app::WebAppProviderBase::GetProviderBase(browser->profile())
-            ->file_handler_manager();
-    url = file_handler_manager
-              .GetMatchingFileHandlerURL(params.app_id, params.launch_files)
-              .value_or(default_url);
-  }
-
   NavigateParams nav_params(browser, url, transition);
   nav_params.disposition = disposition;
   Navigate(&nav_params);

chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc

@@ -29,17 +29,25 @@ class WebAppFileHandlingBrowserTest
     return https_server()->GetURL("app.com", "/ssl/google.html");
   }
 
-  GURL GetFileHandlerActionURL() {
+  GURL GetTextFileHandlerActionURL() {
     return https_server()->GetURL("app.com", "/ssl/blank_page.html");
   }
 
-  base::FilePath NewTestFilePath() {
+  GURL GetCSVFileHandlerActionURL() {
+    return https_server()->GetURL("app.com", "/ssl/page_with_refs.html");
+  }
+
+  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);
-    return 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;
   }
 
   std::string InstallFileHandlingPWA() {
@@ -50,30 +58,37 @@ class WebAppFileHandlingBrowserTest
     web_app_info->scope = url.GetWithoutFilename();
     web_app_info->title = base::ASCIIToUTF16("A Hosted App");
 
-    blink::Manifest::FileHandler entry;
-    entry.action = GetFileHandlerActionURL();
-    entry.name = base::ASCIIToUTF16("text");
-    entry.accept[base::ASCIIToUTF16("text/*")].push_back(
+    blink::Manifest::FileHandler entry1;
+    entry1.action = GetTextFileHandlerActionURL();
+    entry1.name = base::ASCIIToUTF16("text");
+    entry1.accept[base::ASCIIToUTF16("text/*")].push_back(
         base::ASCIIToUTF16(".txt"));
-    web_app_info->file_handlers.push_back(std::move(entry));
+    web_app_info->file_handlers.push_back(std::move(entry1));
+
+    blink::Manifest::FileHandler entry2;
+    entry2.action = GetCSVFileHandlerActionURL();
+    entry2.name = base::ASCIIToUTF16("csv");
+    entry2.accept[base::ASCIIToUTF16("application/csv")].push_back(
+        base::ASCIIToUTF16(".csv"));
+    web_app_info->file_handlers.push_back(std::move(entry2));
 
     return WebAppControllerBrowserTest::InstallWebApp(std::move(web_app_info));
   }
 
   content::WebContents* LaunchWithFiles(
       const std::string& app_id,
-      const std::vector<base::FilePath>& files) {
+      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, apps::mojom::LaunchContainer::kLaunchContainerWindow,
-        WindowOpenDisposition::NEW_WINDOW,
+        app_id, launch_container, WindowOpenDisposition::NEW_WINDOW,
         apps::mojom::AppLaunchSource::kSourceFileHandler);
     params.launch_files = files;
 
-    content::TestNavigationObserver navigation_observer(
-        GetFileHandlerActionURL());
+    content::TestNavigationObserver navigation_observer(expected_launch_url);
     navigation_observer.StartWatchingNewWebContents();
 
-    params.override_url = GetFileHandlerActionURL();
     content::WebContents* web_contents =
         apps::LaunchService::Get(profile())->OpenApplication(params);
 
@@ -96,18 +111,38 @@ IN_PROC_BROWSER_TEST_P(WebAppFileHandlingBrowserTest, PWAsCanViewLaunchParams) {
   ASSERT_TRUE(https_server()->Start());
 
   const std::string app_id = InstallFileHandlingPWA();
-  content::WebContents* web_contents = LaunchWithFiles(app_id, {});
+  content::WebContents* web_contents =
+      LaunchWithFiles(app_id, GetSecureAppURL(), {});
   EXPECT_EQ(false, content::EvalJs(web_contents, "!!window.launchParams"));
 }
 
-IN_PROC_BROWSER_TEST_P(WebAppFileHandlingBrowserTest,
+using WebAppFileHandlingBrowserTestNoUnifiedWebApp =
+    WebAppFileHandlingBrowserTest;
+
+IN_PROC_BROWSER_TEST_P(WebAppFileHandlingBrowserTestNoUnifiedWebApp,
                        PWAsCanReceiveFileLaunchParams) {
   ASSERT_TRUE(https_server()->Start());
 
   const std::string app_id = InstallFileHandlingPWA();
-  base::FilePath test_file_path = NewTestFilePath();
+  base::FilePath test_file_path = NewTestFilePath(FILE_PATH_LITERAL("txt"));
+  content::WebContents* web_contents =
+      LaunchWithFiles(app_id, GetTextFileHandlerActionURL(), {test_file_path});
+
+  EXPECT_EQ(1,
+            content::EvalJs(web_contents, "window.launchParams.files.length"));
+  EXPECT_EQ(test_file_path.BaseName().value(),
+            content::EvalJs(web_contents, "window.launchParams.files[0].name"));
+}
+
+IN_PROC_BROWSER_TEST_P(WebAppFileHandlingBrowserTestNoUnifiedWebApp,
+                       PWAsCanReceiveFileLaunchParamsInTab) {
+  ASSERT_TRUE(https_server()->Start());
+
+  const std::string app_id = InstallFileHandlingPWA();
+  base::FilePath test_file_path = NewTestFilePath(FILE_PATH_LITERAL("txt"));
   content::WebContents* web_contents =
-      LaunchWithFiles(app_id, {test_file_path});
+      LaunchWithFiles(app_id, GetTextFileHandlerActionURL(), {test_file_path},
+                      apps::mojom::LaunchContainer::kLaunchContainerTab);
 
   EXPECT_EQ(1,
             content::EvalJs(web_contents, "window.launchParams.files.length"));
@@ -115,6 +150,30 @@ IN_PROC_BROWSER_TEST_P(WebAppFileHandlingBrowserTest,
             content::EvalJs(web_contents, "window.launchParams.files[0].name"));
 }
 
+IN_PROC_BROWSER_TEST_P(WebAppFileHandlingBrowserTestNoUnifiedWebApp,
+                       PWAsDispatchOnCorrectFileHandlingURL) {
+  ASSERT_TRUE(https_server()->Start());
+
+  const std::string app_id = InstallFileHandlingPWA();
+
+  // Test that file handler dispatches correct URL based on file extension.
+  LaunchWithFiles(app_id, GetSecureAppURL(), {});
+  LaunchWithFiles(app_id, GetTextFileHandlerActionURL(),
+                  {NewTestFilePath(FILE_PATH_LITERAL("txt"))});
+  LaunchWithFiles(app_id, GetCSVFileHandlerActionURL(),
+                  {NewTestFilePath(FILE_PATH_LITERAL("csv"))});
+
+  // Test as above in a tab.
+  LaunchWithFiles(app_id, GetSecureAppURL(), {},
+                  apps::mojom::LaunchContainer::kLaunchContainerTab);
+  LaunchWithFiles(app_id, GetTextFileHandlerActionURL(),
+                  {NewTestFilePath(FILE_PATH_LITERAL("txt"))},
+                  apps::mojom::LaunchContainer::kLaunchContainerTab);
+  LaunchWithFiles(app_id, GetCSVFileHandlerActionURL(),
+                  {NewTestFilePath(FILE_PATH_LITERAL("csv"))},
+                  apps::mojom::LaunchContainer::kLaunchContainerTab);
+}
+
 INSTANTIATE_TEST_SUITE_P(
     All,
     WebAppFileHandlingBrowserTest,
@@ -123,3 +182,13 @@ INSTANTIATE_TEST_SUITE_P(
         web_app::ControllerType::kUnifiedControllerWithBookmarkApp,
         web_app::ControllerType::kUnifiedControllerWithWebApp),
     web_app::ControllerTypeParamToString);
+
+// Currently web apps don't support getting a list of all file handlers, so some
+// tests cannot be run.
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    WebAppFileHandlingBrowserTestNoUnifiedWebApp,
+    ::testing::Values(
+        web_app::ControllerType::kHostedAppController,
+        web_app::ControllerType::kUnifiedControllerWithBookmarkApp),
+    web_app::ControllerTypeParamToString);

chrome/browser/ui/web_applications/web_app_launch_manager.cc

@@ -139,6 +139,11 @@ content::WebContents* WebAppLaunchManager::OpenApplication(
   // event and will potentially permit a banner to be shown for the site.
   RecordAppBanner(web_contents, url);
 
+  if (base::FeatureList::IsEnabled(blink::features::kFileHandlingAPI)) {
+    web_launch::WebLaunchFilesHelper::SetLaunchPaths(web_contents, url,
+                                                     params.launch_files);
+  }
+
   return web_contents;
 }
 

chrome/browser/web_applications/components/file_handler_manager.cc

@@ -97,6 +97,9 @@ void FileHandlerManager::OnAppRegistrarDestroyed() {
 const base::Optional<GURL> FileHandlerManager::GetMatchingFileHandlerURL(
     const AppId& app_id,
     const std::vector<base::FilePath>& launch_files) {
+  if (!base::FeatureList::IsEnabled(blink::features::kFileHandlingAPI))
+    return base::nullopt;
+
   const std::vector<apps::FileHandlerInfo>* file_handlers =
       GetAllFileHandlers(app_id);
   if (!file_handlers || launch_files.empty())