File Handling multi launch

Translate an app launch with multiple files handled by separate file handlers into multiple launches (i.e. multiple windows), rather than failing the launch if no single file handler accepts all file types present. The behavior on each OS between when a user selects multiple files and a command line for the PWA is crafted is slightly different on each platform --- see doc for details. As such, this primarily affects Linux for now, but will be more impactful when the new manifest member is added that controls whether an app wants to handle many files or just one in each launched context.* *See document linked from bug for more details. Bug: 1304003 Change-Id: Iff88f8dd9be11b31967f9037543fd57ba007ef3f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3503850 Commit-Queue: Evan Stade <estade@chromium.org> Reviewed-by: Daniel Murphy <dmurph@chromium.org> Reviewed-by: Nancy Wang <nancylingwang@chromium.org> Reviewed-by: Greg Thompson <grt@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Reviewed-by: Alan Cutter <alancutter@chromium.org> Reviewed-by: Maggie Cai <mxcai@chromium.org> Reviewed-by: Jiewei Qian <qjw@chromium.org> Cr-Commit-Position: refs/heads/main@{#984626}
chromium · Mar 24, 2022 · bc3ef1b · bc3ef1b
1 parent 4c767a8
commit bc3ef1b
Show file tree

Hide file tree

Showing 24 changed files with 491 additions and 237 deletions.
diff --git a/chrome/browser/apps/app_shim/web_app_shim_manager_delegate_mac.cc b/chrome/browser/apps/app_shim/web_app_shim_manager_delegate_mac.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/web_applications/os_integration/os_integration_manager.h"
+#include "chrome/browser/web_applications/os_integration/web_app_file_handler_manager.h"
 #include "chrome/browser/web_applications/os_integration/web_app_shortcut_mac.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
@@ -36,11 +37,38 @@ web_app::BrowserAppLauncherForTesting& GetBrowserAppLauncherForTesting() {
   return *instance;
 }
 
-// Launches the app specified by `params` in the given `profile`.
-void LaunchAppWithParams(Profile* profile, apps::AppLaunchParams params) {
-  apps::AppServiceProxyFactory::GetForProfile(profile)
-      ->BrowserAppLauncher()
-      ->LaunchAppWithParams(std::move(params));
+// Launches the app specified by `params` and `file_launches` in the given
+// `profile`.
+void LaunchAppWithParams(
+    Profile* profile,
+    apps::AppLaunchParams params,
+    const WebAppFileHandlerManager::LaunchInfos& file_launches) {
+  if (!file_launches.empty()) {
+    for (const auto& [url, files] : file_launches) {
+      apps::AppLaunchParams params_copy(params.app_id, params.container,
+                                        params.disposition,
+                                        params.launch_source);
+      params_copy.override_url = url;
+      params_copy.launch_files = files;
+
+      if (GetBrowserAppLauncherForTesting()) {
+        std::move(GetBrowserAppLauncherForTesting()).Run(params_copy);
+      } else {
+        apps::AppServiceProxyFactory::GetForProfile(profile)
+            ->BrowserAppLauncher()
+            ->LaunchAppWithParams(std::move(params_copy));
+      }
+    }
+    return;
+  }
+
+  if (GetBrowserAppLauncherForTesting()) {
+    std::move(GetBrowserAppLauncherForTesting()).Run(params);
+  } else {
+    apps::AppServiceProxyFactory::GetForProfile(profile)
+        ->BrowserAppLauncher()
+        ->LaunchAppWithParams(std::move(params));
+  }
 }
 
 // Cancels the launch of the app for the given `app_id`, potentially resulting
@@ -51,35 +79,40 @@ void CancelAppLaunch(Profile* profile, const web_app::AppId& app_id) {
 
 // Called after the user's preference has been persisted, and the OS
 // has been notified of the change.
-void OnPersistUserChoiceCompleted(apps::AppLaunchParams params,
-                                  Profile* profile,
-                                  bool allowed) {
+void OnPersistUserChoiceCompleted(
+    apps::AppLaunchParams params,
+    const WebAppFileHandlerManager::LaunchInfos& file_launches,
+    Profile* profile,
+    bool allowed) {
   if (allowed) {
-    LaunchAppWithParams(profile, std::move(params));
+    LaunchAppWithParams(profile, std::move(params), file_launches);
   } else {
     CancelAppLaunch(profile, params.app_id);
   }
 }
 
 // Called after the user has dismissed the WebAppProtocolHandlerIntentPicker
 // dialog.
-void UserChoiceDialogCompleted(apps::AppLaunchParams params,
-                               Profile* profile,
-                               bool allowed,
-                               bool remember_user_choice) {
+void UserChoiceDialogCompleted(
+    apps::AppLaunchParams params,
+    const WebAppFileHandlerManager::LaunchInfos& file_launches,
+    Profile* profile,
+    bool allowed,
+    bool remember_user_choice) {
   absl::optional<GURL> protocol_url = params.protocol_handler_launch_url;
-  std::vector<base::FilePath> launch_files = params.launch_files;
+  const bool is_file_launch = !file_launches.empty();
   web_app::AppId app_id = params.app_id;
 
-  auto persist_done = base::BindOnce(&OnPersistUserChoiceCompleted,
-                                     std::move(params), profile, allowed);
+  auto persist_done =
+      base::BindOnce(&OnPersistUserChoiceCompleted, std::move(params),
+                     file_launches, profile, allowed);
 
   if (remember_user_choice) {
     if (protocol_url) {
       PersistProtocolHandlersUserChoice(profile, app_id, *protocol_url, allowed,
                                         std::move(persist_done));
     } else {
-      DCHECK(!launch_files.empty());
+      DCHECK(is_file_launch);
       PersistFileHandlersUserChoice(profile, app_id, allowed,
                                     std::move(persist_done));
     }
@@ -240,9 +273,15 @@ void WebAppShimManagerDelegate::LaunchApp(
     params.launch_source = apps::mojom::LaunchSource::kFromProtocolHandler;
   }
 
+  WebAppProvider* const provider = WebAppProvider::GetForWebApps(profile);
+  WebAppFileHandlerManager::LaunchInfos file_launches;
+  if (!params.protocol_handler_launch_url) {
+    file_launches = provider->os_integration_manager()
+                        .file_handler_manager()
+                        .GetMatchingFileHandlerUrls(app_id, launch_files);
+  }
   if (GetBrowserAppLauncherForTesting()) {
-    params.launch_files = launch_files;
-    std::move(GetBrowserAppLauncherForTesting()).Run(params);
+    LaunchAppWithParams(profile, std::move(params), file_launches);
     return;
   }
 
@@ -263,23 +302,14 @@ void WebAppShimManagerDelegate::LaunchApp(
       chrome::ShowWebAppProtocolHandlerIntentPicker(
           std::move(protocol_url), profile, app_id,
           base::BindOnce(&UserChoiceDialogCompleted, std::move(params),
-                         profile));
+                         WebAppFileHandlerManager::LaunchInfos(), profile));
       return;
     }
   }
 
-  WebAppProvider* const provider = WebAppProvider::GetForWebApps(profile);
-  if (!launch_files.empty()) {
-    absl::optional<GURL> file_handler_url =
-        provider->os_integration_manager().GetMatchingFileHandlerURL(
-            app_id, launch_files);
-    if (file_handler_url)
-      params.launch_files = launch_files;
-    // If there is no matching file handling URL (such as when the API has been
-    // disabled), fall back to a normal app launch.
-  }
-
-  if (!params.launch_files.empty()) {
+  // If there is no matching file handling URL (such as when the API has been
+  // disabled), fall back to a normal app launch.
+  if (!file_launches.empty()) {
     const WebApp* web_app = provider->registrar().GetAppById(app_id);
     DCHECK(web_app);
 
@@ -288,15 +318,15 @@ void WebAppShimManagerDelegate::LaunchApp(
       chrome::ShowWebAppFileLaunchDialog(
           launch_files, profile, app_id,
           base::BindOnce(&UserChoiceDialogCompleted, std::move(params),
-                         profile));
+                         file_launches, profile));
       return;
     }
 
     DCHECK_EQ(ApiApprovalState::kAllowed,
               web_app->file_handler_approval_state());
   }
 
-  LaunchAppWithParams(profile, std::move(params));
+  LaunchAppWithParams(profile, std::move(params), file_launches);
 }
 
 void WebAppShimManagerDelegate::LaunchShim(

diff --git a/chrome/browser/apps/app_shim/web_app_shim_manager_delegate_mac_unittest.cc b/chrome/browser/apps/app_shim/web_app_shim_manager_delegate_mac_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/test/bind.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/test/fake_os_integration_manager.h"
@@ -19,6 +20,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
+#include "third_party/blink/public/common/features.h"
 #include "url/gurl.h"
 
 namespace web_app {
@@ -105,8 +107,21 @@ class WebAppShimManagerDelegateTest : public WebAppTest {
     web_app::test::AwaitStartWebAppProviderAndSubsystems(profile());
 
     // Install a dummy app
-    app_id_ = web_app::test::InstallDummyWebApp(profile(), "WebAppTest",
-                                                GURL("https://testpwa.com/"));
+    auto web_app_info = std::make_unique<WebAppInstallInfo>();
+    web_app_info->title = u"WebAppTest";
+    web_app_info->start_url = GURL("https://testpwa.com/");
+    web_app_info->scope = GURL("https://testpwa.com/");
+    web_app_info->display_mode = blink::mojom::DisplayMode::kStandalone;
+
+    // Basic plain text format.
+    apps::FileHandler entry1;
+    entry1.action = GURL("https://testpwa.com/files");
+    entry1.accept.emplace_back();
+    entry1.accept[0].mime_type = "text/*";
+    entry1.accept[0].file_extensions.insert(".txt");
+    web_app_info->file_handlers.push_back(std::move(entry1));
+
+    app_id_ = test::InstallWebApp(profile(), std::move(web_app_info));
   }
 
  protected:
@@ -159,6 +174,8 @@ class WebAppShimManagerDelegateTest : public WebAppTest {
 
  private:
   web_app::AppId app_id_;
+  base::test::ScopedFeatureList scoped_feature_list_{
+      blink::features::kFileHandlingAPI};
 };
 
 TEST_F(WebAppShimManagerDelegateTest, LaunchApp) {
@@ -228,9 +245,9 @@ TEST_F(WebAppShimManagerDelegateTest, LaunchApp_ProtocolMailTo) {
 TEST_F(WebAppShimManagerDelegateTest, LaunchApp_ProtocolFile) {
   GURL protocol_handler_launch_url("file:///test_app_path/test_app_file.txt");
 
-  apps::AppLaunchParams expected_results =
-      CreateLaunchParams({base::FilePath("/test_app_path/test_app_file.txt")},
-                         absl::nullopt, absl::nullopt, GURL());
+  apps::AppLaunchParams expected_results = CreateLaunchParams(
+      {base::FilePath("/test_app_path/test_app_file.txt")}, absl::nullopt,
+      absl::nullopt, GURL("https://testpwa.com/files"));
 
   std::unique_ptr<MockDelegate> delegate = std::make_unique<MockDelegate>();
   WebAppShimManagerDelegate shim_manager(std::move(delegate));
@@ -272,7 +289,8 @@ TEST_F(WebAppShimManagerDelegateTest, LaunchApp_FileFullPath) {
   base::FilePath test_path(kTestPath);
 
   apps::AppLaunchParams expected_results =
-      CreateLaunchParams({test_path}, absl::nullopt, absl::nullopt, GURL());
+      CreateLaunchParams({test_path}, absl::nullopt, absl::nullopt,
+                         GURL("https://testpwa.com/files"));
 
   std::unique_ptr<MockDelegate> delegate = std::make_unique<MockDelegate>();
   WebAppShimManagerDelegate shim_manager(std::move(delegate));
@@ -294,7 +312,8 @@ TEST_F(WebAppShimManagerDelegateTest, LaunchApp_FileRelativePath) {
   base::FilePath test_path(kTestPath);
 
   apps::AppLaunchParams expected_results =
-      CreateLaunchParams({test_path}, absl::nullopt, absl::nullopt, GURL());
+      CreateLaunchParams({test_path}, absl::nullopt, absl::nullopt,
+                         GURL("https://testpwa.com/files"));
 
   std::unique_ptr<MockDelegate> delegate = std::make_unique<MockDelegate>();
   WebAppShimManagerDelegate shim_manager(std::move(delegate));
@@ -317,7 +336,7 @@ TEST_F(WebAppShimManagerDelegateTest, LaunchApp_ProtocolAndFileHandlerMixed) {
   base::FilePath test_path(kTestPath);
 
   apps::AppLaunchParams expected_results = CreateLaunchParams(
-      {test_path}, absl::nullopt, protocol_handler_launch_url, GURL());
+      {}, absl::nullopt, protocol_handler_launch_url, GURL());
   expected_results.launch_source =
       apps::mojom::LaunchSource::kFromProtocolHandler;
 
@@ -344,8 +363,7 @@ TEST_F(WebAppShimManagerDelegateTest,
   base::FilePath test_path(kTestPath);
 
   apps::AppLaunchParams expected_results = CreateLaunchParams(
-      {test_path, base::FilePath("/test_app_path/test_app_file.txt")},
-      absl::nullopt, protocol_handler_launch_url, GURL());
+      {}, absl::nullopt, protocol_handler_launch_url, GURL());
   expected_results.launch_source =
       apps::mojom::LaunchSource::kFromProtocolHandler;
 

diff --git a/chrome/browser/ash/file_manager/file_tasks_browsertest.cc b/chrome/browser/ash/file_manager/file_tasks_browsertest.cc
@@ -498,8 +498,10 @@ IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, ExecuteWebApp) {
   handler.display_name = u"activity name";
   apps::FileHandler::AcceptEntry accept_entry1;
   accept_entry1.mime_type = "image/jpeg";
+  accept_entry1.file_extensions.insert(".jpeg");
   handler.accept.push_back(accept_entry1);
   apps::FileHandler::AcceptEntry accept_entry2;
+  accept_entry2.mime_type = "image/png";
   accept_entry2.file_extensions.insert(".png");
   handler.accept.push_back(accept_entry2);
   web_app_info->file_handlers.push_back(std::move(handler));
@@ -529,26 +531,24 @@ IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, ExecuteWebApp) {
   web_app::WebAppLaunchManager::SetOpenApplicationCallbackForTesting(
       base::BindLambdaForTesting(
           [&run_loop](apps::AppLaunchParams&& params) -> content::WebContents* {
-            EXPECT_EQ(params.intent->action, apps_util::kIntentActionView);
             if (GetParam().crosapi_state ==
                 TestProfileParam::CrosapiParam::kDisabled) {
-              EXPECT_EQ(params.intent->activity_name,
+              EXPECT_EQ(params.override_url,
                         "https://www.example.com/handle_file");
             } else {
-              EXPECT_EQ(params.intent->activity_name,
-                        "chrome://media-app/open");
+              EXPECT_EQ(params.override_url, "chrome://media-app/open");
             }
             EXPECT_EQ(params.launch_files.size(), 2U);
             EXPECT_TRUE(base::EndsWith(params.launch_files.at(0).MaybeAsASCII(),
-                                       "foo.jpg"));
+                                       "foo.jpeg"));
             EXPECT_TRUE(base::EndsWith(params.launch_files.at(1).MaybeAsASCII(),
                                        "bar.png"));
             run_loop.Quit();
             return nullptr;
           }));
 
   base::FilePath file1 =
-      util::GetMyFilesFolderForProfile(profile).AppendASCII("foo.jpg");
+      util::GetMyFilesFolderForProfile(profile).AppendASCII("foo.jpeg");
   base::FilePath file2 =
       util::GetMyFilesFolderForProfile(profile).AppendASCII("bar.png");
   GURL url1;