system-web-app: fix regression for launch_files for System Web App launches

Passthrough AppLaunchParams to LaunchSystemWebApp

Bug: 996088
Change-Id: I0b435d0656054af7b66ebda58b007278d83157a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1910984
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#717998}

chrome/browser/ui/extensions/application_launch.cc

@@ -308,7 +308,7 @@ WebContents* OpenEnabledApplication(Profile* profile,
       web_app::GetSystemWebAppTypeForAppId(profile, extension->id());
   if (system_app_type) {
     Browser* browser =
-        web_app::LaunchSystemWebApp(profile, *system_app_type, url);
+        web_app::LaunchSystemWebApp(profile, *system_app_type, url, params);
     return browser->tab_strip_model()->GetActiveWebContents();
   }
 
@@ -471,8 +471,7 @@ void OpenApplicationWithReenablePrompt(Profile* profile,
   OpenEnabledApplication(profile, params);
 }
 
-WebContents* OpenAppShortcutWindow(Profile* profile,
-                                   const GURL& url) {
+WebContents* OpenAppShortcutWindow(Profile* profile, const GURL& url) {
   apps::AppLaunchParams launch_params(
       std::string(),  // this is a URL app. No app id.
       extensions::LaunchContainer::kLaunchContainerWindow,

chrome/browser/ui/web_applications/system_web_app_ui_utils.cc

@@ -13,6 +13,7 @@
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -60,16 +61,6 @@ Browser* LaunchSystemWebApp(Profile* profile,
   if (did_create)
     *did_create = false;
 
-  Browser* browser = FindSystemWebAppBrowser(profile, app_type);
-  if (browser) {
-    content::WebContents* web_contents =
-        browser->tab_strip_model()->GetWebContentsAt(0);
-    if (web_contents && web_contents->GetURL() == url) {
-      browser->window()->Show();
-      return browser;
-    }
-  }
-
   base::Optional<AppId> app_id = GetAppIdForSystemWebApp(profile, app_type);
   // TODO(calamity): Queue a task to launch app after it is installed.
   if (!app_id)
@@ -86,6 +77,29 @@ Browser* LaunchSystemWebApp(Profile* profile,
       display::kInvalidDisplayId);
   params.override_url = url;
 
+  return LaunchSystemWebApp(profile, app_type, url, params, did_create);
+}
+
+Browser* LaunchSystemWebApp(Profile* profile,
+                            SystemAppType app_type,
+                            const GURL& url,
+                            const apps::AppLaunchParams& params,
+                            bool* did_create) {
+  if (did_create)
+    *did_create = false;
+
+  // TODO(https://crbug.com/1027030): Better understand and improve SWA launch
+  // behavior. Early exit here skips logic in ShowApplicationWindow.
+  Browser* browser = FindSystemWebAppBrowser(profile, app_type);
+  if (browser) {
+    content::WebContents* web_contents =
+        browser->tab_strip_model()->GetWebContentsAt(0);
+    if (web_contents && web_contents->GetURL() == url) {
+      browser->window()->Show();
+      return browser;
+    }
+  }
+
   if (!browser) {
     if (did_create)
       *did_create = true;

chrome/browser/ui/web_applications/system_web_app_ui_utils.h

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/optional.h"
+#include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "url/gurl.h"
@@ -36,6 +37,11 @@ Browser* LaunchSystemWebApp(Profile* profile,
                             SystemAppType app_type,
                             const GURL& url = GURL(),
                             bool* did_create = nullptr);
+Browser* LaunchSystemWebApp(Profile* profile,
+                            SystemAppType app_type,
+                            const GURL& url,
+                            const apps::AppLaunchParams& params,
+                            bool* did_create = nullptr);
 
 // Returns a browser that is hosting the given system app type, or nullptr if
 // not found.

chrome/browser/ui/web_applications/web_app_launch_manager.cc

@@ -96,19 +96,21 @@ content::WebContents* WebAppLaunchManager::OpenApplication(
   if (!provider_->registrar().IsInstalled(params.app_id))
     return nullptr;
 
+  const GURL url = params.override_url.is_empty()
+                       ? provider_->registrar().GetAppLaunchURL(params.app_id)
+                       : params.override_url;
+
   // System Web Apps go through their own launch path.
   base::Optional<SystemAppType> system_app_type =
       GetSystemWebAppTypeForAppId(profile(), params.app_id);
   if (system_app_type) {
-    Browser* browser = LaunchSystemWebApp(profile(), *system_app_type, GURL());
+    Browser* browser =
+        LaunchSystemWebApp(profile(), *system_app_type, url, params);
     return browser->tab_strip_model()->GetActiveWebContents();
   }
 
   Browser* browser = CreateWebApplicationWindow(profile(), params.app_id);
 
-  const GURL url = params.override_url.is_empty()
-                       ? provider_->registrar().GetAppLaunchURL(params.app_id)
-                       : params.override_url;
   content::WebContents* web_contents =
       ShowWebApplicationWindow(params, params.app_id, url, browser,
                                WindowOpenDisposition::NEW_FOREGROUND_TAB);

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.cc

@@ -4,15 +4,20 @@
 
 #include "chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h"
 
+#include <string>
+#include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/apps/app_service/app_launch_params.h"
+#include "chrome/browser/apps/launch_service/launch_service.h"
 #include "chrome/browser/extensions/browsertest_util.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/test/test_system_web_app_manager.h"
@@ -28,9 +33,11 @@
 #include "content/public/browser/web_ui_controller_factory.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/features.h"
 #include "url/gurl.h"
 
 namespace web_app {
@@ -131,7 +138,10 @@ class TestWebUIControllerFactory : public content::WebUIControllerFactory {
 
 SystemWebAppManagerBrowserTest::SystemWebAppManagerBrowserTest(
     bool install_mock) {
-  scoped_feature_list_.InitWithFeatures({features::kSystemWebApps}, {});
+  scoped_feature_list_.InitWithFeatures(
+      {features::kSystemWebApps, blink::features::kNativeFileSystemAPI,
+       blink::features::kFileHandlingAPI},
+      {});
   if (install_mock) {
     factory_ = std::make_unique<TestWebUIControllerFactory>();
     test_web_app_provider_creator_ =
@@ -185,8 +195,7 @@ SystemWebAppManagerBrowserTest::CreateWebAppProvider(Profile* profile) {
   return provider;
 }
 
-Browser* SystemWebAppManagerBrowserTest::WaitForSystemAppInstallAndLaunch(
-    SystemAppType system_app_type) {
+void SystemWebAppManagerBrowserTest::WaitForTestSystemAppInstall() {
   // Wait for the System Web Apps to install.
   if (factory_) {
     base::RunLoop run_loop;
@@ -195,6 +204,11 @@ Browser* SystemWebAppManagerBrowserTest::WaitForSystemAppInstallAndLaunch(
   } else {
     GetManager().InstallSystemAppsForTesting();
   }
+}
+
+Browser* SystemWebAppManagerBrowserTest::WaitForSystemAppInstallAndLaunch(
+    SystemAppType system_app_type) {
+  WaitForTestSystemAppInstall();
 
   base::Optional<AppId> app_id =
       GetManager().GetAppIdForSystemApp(system_app_type);
@@ -255,4 +269,51 @@ IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest,
   EXPECT_TRUE(app_browser->app_controller()->ShouldShowCustomTabBar());
 }
 
+// Check launch files are passed to application.
+IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest,
+                       LaunchFilesForSystemWebApp) {
+  WaitForTestSystemAppInstall();
+
+  base::Optional<AppId> app_id =
+      GetManager().GetAppIdForSystemApp(SystemAppType::SETTINGS);
+  ASSERT_TRUE(app_id.has_value());
+
+  apps::AppLaunchParams params(
+      app_id.value(), apps::mojom::LaunchContainer::kLaunchContainerWindow,
+      WindowOpenDisposition::NEW_WINDOW,
+      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));
+
+  params.launch_files = std::vector<base::FilePath>{temp_file_path};
+
+  const GURL& launch_url = WebAppProvider::Get(browser()->profile())
+                               ->registrar()
+                               .GetAppLaunchURL(app_id.value());
+  content::TestNavigationObserver navigation_observer(launch_url);
+  navigation_observer.StartWatchingNewWebContents();
+
+  content::WebContents* web_contents =
+      OpenApplication(browser()->profile(), params);
+
+  navigation_observer.Wait();
+
+  auto result =
+      content::EvalJs(web_contents,
+                      "new Promise(resolve => {"
+                      "  launchQueue.setConsumer(launchParams => {"
+                      "    resolve(launchParams.files.length);"
+                      "  });"
+                      "});",
+                      content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, 1 /*world_id*/);
+
+  // Files array should be populated with one file.
+  EXPECT_EQ(1, result.value.GetInt());
+}
+
 }  // namespace web_app

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h

@@ -44,6 +44,7 @@ class SystemWebAppManagerBrowserTest : public InProcessBrowserTest {
   // TestSystemWebAppManager if initialized with |install_mock| true.
   SystemWebAppManager& GetManager();
 
+  void WaitForTestSystemAppInstall();
   Browser* WaitForSystemAppInstallAndLaunch(SystemAppType system_app_type);
 
  private: