desktop-pwas: Allow same-origin redirects when loading install URL for policy apps.

Web apps installed through external sources (ie. default, policy, or system
installed) could fail to install due to redirects, making the install process
flaky. This was also unexpected for admins setting policy.


Bug: 1015650
Change-Id: I616da7b9f68098591c63818be0467c69428350a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1914139
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716082}

chrome/browser/web_applications/components/external_install_options.cc

@@ -81,6 +81,8 @@ InstallManager::InstallParams ConvertExternalInstallOptionsToParams(
 
   params.user_display_mode = install_options.user_display_mode;
 
+  params.fallback_start_url = install_options.url;
+
   params.add_to_applications_menu = install_options.add_to_applications_menu;
   params.add_to_desktop = install_options.add_to_desktop;
   params.add_to_quick_launch_bar = install_options.add_to_quick_launch_bar;

chrome/browser/web_applications/components/install_manager.h

@@ -102,6 +102,9 @@ class InstallManager {
   struct InstallParams {
     DisplayMode user_display_mode = DisplayMode::kUndefined;
 
+    // URL to be used as start_url if manifest is unavailable.
+    GURL fallback_start_url;
+
     bool add_to_applications_menu = true;
     bool add_to_desktop = true;
     bool add_to_quick_launch_bar = true;

chrome/browser/web_applications/components/manifest_update_manager_browsertest.cc

@@ -154,6 +154,7 @@ class ManifestUpdateManagerBrowserTest : public InProcessBrowserTest {
     AppId app_id;
     base::RunLoop run_loop;
     InstallManager::InstallParams params;
+    params.fallback_start_url = http_server_.GetURL("/fallback-url");
     params.add_to_applications_menu = false;
     params.add_to_desktop = false;
     params.add_to_quick_launch_bar = false;
@@ -467,13 +468,14 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
         auto http_response =
             std::make_unique<net::test_server::BasicHttpResponse>();
         http_response->set_code(net::HTTP_TEMPORARY_REDIRECT);
-        http_response->AddCustomHeader("Location", "/defaultresponse");
+        http_response->AddCustomHeader(
+            "Location", "http://other-origin.com/defaultresponse");
         http_response->set_content("redirect page");
         return std::move(http_response);
       });
 
-  // Install via PendingAppManager, the redirect should cause it to install a
-  // placeholder app.
+  // Install via PendingAppManager, the redirect to a different origin should
+  // cause it to install a placeholder app.
   AppId app_id = InstallPolicyApp();
   EXPECT_TRUE(GetProvider().registrar().IsPlaceholderApp(app_id));
 

chrome/browser/web_applications/components/web_app_url_loader.cc

@@ -19,12 +19,27 @@ namespace web_app {
 constexpr base::TimeDelta WebAppUrlLoader::kSecondsToWaitForWebContentsLoad;
 
 namespace {
-bool EqualsIgnoringQueryAndRef(const GURL& a, const GURL& b) {
+using UrlComparison = WebAppUrlLoader::UrlComparison;
+
+bool EqualsWithComparison(const GURL& a,
+                          const GURL& b,
+                          UrlComparison url_comparison) {
+  DCHECK(a.is_valid());
+  DCHECK(b.is_valid());
   if (a == b)
     return true;
   GURL::Replacements replace;
-  replace.ClearQuery();
-  replace.ClearRef();
+  switch (url_comparison) {
+    case UrlComparison::kExact:
+      return false;
+    case UrlComparison::kSameOrigin:
+      replace.ClearPath();
+      FALLTHROUGH;
+    case UrlComparison::kIgnoreQueryParamsAndRef:
+      replace.ClearQuery();
+      replace.ClearRef();
+      break;
+  }
   return a.ReplaceComponents(replace) == b.ReplaceComponents(replace);
 }
 
@@ -35,8 +50,10 @@ class LoaderTask : public content::WebContentsObserver {
 
   void LoadUrl(const GURL& url,
                content::WebContents* web_contents,
+               UrlComparison url_comparison,
                WebAppUrlLoader::ResultCallback callback) {
     url_ = url;
+    url_comparison_ = url_comparison;
     callback_ = std::move(callback);
     Observe(web_contents);
 
@@ -65,7 +82,7 @@ class LoaderTask : public content::WebContentsObserver {
 
     timer_.Stop();
 
-    if (EqualsIgnoringQueryAndRef(validated_url, url_)) {
+    if (EqualsWithComparison(validated_url, url_, url_comparison_)) {
       PostResultTask(WebAppUrlLoader::Result::kUrlLoaded);
       return;
     }
@@ -109,8 +126,10 @@ class LoaderTask : public content::WebContentsObserver {
         FROM_HERE, base::BindOnce(std::move(callback_), result));
   }
 
-  WebAppUrlLoader::ResultCallback callback_;
   GURL url_;
+  UrlComparison url_comparison_;
+  WebAppUrlLoader::ResultCallback callback_;
+
   base::OneShotTimer timer_;
 
   base::WeakPtrFactory<LoaderTask> weak_ptr_factory_{this};
@@ -126,11 +145,12 @@ WebAppUrlLoader::~WebAppUrlLoader() = default;
 
 void WebAppUrlLoader::LoadUrl(const GURL& url,
                               content::WebContents* web_contents,
+                              UrlComparison url_comparison,
                               ResultCallback callback) {
   auto loader_task = std::make_unique<LoaderTask>();
   auto* loader_task_ptr = loader_task.get();
   loader_task_ptr->LoadUrl(
-      url, web_contents,
+      url, web_contents, url_comparison,
       base::BindOnce(
           [](ResultCallback callback, std::unique_ptr<LoaderTask> task,
              Result result) {

chrome/browser/web_applications/components/web_app_url_loader.h

@@ -19,11 +19,19 @@ namespace web_app {
 // Callback-based wrapper around NavigationController::LoadUrl.
 class WebAppUrlLoader {
  public:
+  // How to compare resolved URL against the given URL for the purpose of
+  // determining a successful load.
+  enum class UrlComparison {
+    kExact,
+    kIgnoreQueryParamsAndRef,
+    kSameOrigin,
+  };
+
   enum class Result {
-    // The provided URL (or one differing only in query params) was loaded.
+    // The provided URL (matched using |UrlComparison|) was loaded.
     kUrlLoaded,
-    // The provided URL redirected to another URL and the final URL
-    // was loaded.
+    // The provided URL redirected to another URL (that did not match using
+    // |UrlComparison|) and the final URL was loaded.
     kRedirectedUrlLoaded,
     kFailedUnknownReason,
     kFailedPageTookTooLong,
@@ -35,9 +43,11 @@ class WebAppUrlLoader {
   WebAppUrlLoader();
   virtual ~WebAppUrlLoader();
 
-  // Navigates |web_contents| to |url| and runs callback with the result code.
+  // Navigates |web_contents| to |url|, compares the resolved URL with
+  // |url_comparison|, and runs callback with the result code.
   virtual void LoadUrl(const GURL& url,
                        content::WebContents* web_contents,
+                       UrlComparison url_comparison,
                        ResultCallback callback);
 
   // Exposed for testing.

chrome/browser/web_applications/components/web_app_url_loader_browsertest.cc

@@ -23,6 +23,7 @@
 namespace web_app {
 
 using Result = WebAppUrlLoader::Result;
+using UrlComparison = WebAppUrlLoader::UrlComparison;
 
 // Returns a redirect response to |dest| URL.
 std::unique_ptr<net::test_server::HttpResponse> HandleServerRedirect(
@@ -69,11 +70,13 @@ class WebAppUrlLoaderTest : public InProcessBrowserTest {
     web_contents_.reset();
   }
 
-  Result LoadUrlAndWait(WebAppUrlLoader* loader, const std::string& path) {
+  Result LoadUrlAndWait(WebAppUrlLoader* loader,
+                        UrlComparison url_comparison,
+                        const std::string& path) {
     base::Optional<Result> result;
     base::RunLoop run_loop;
     loader->LoadUrl(embedded_test_server()->GetURL(path), web_contents(),
-                    base::BindLambdaForTesting([&](Result r) {
+                    url_comparison, base::BindLambdaForTesting([&](Result r) {
                       result = r;
                       run_loop.Quit();
                     }));
@@ -109,25 +112,59 @@ class WebAppUrlLoaderTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, Loaded) {
   ASSERT_TRUE(embedded_test_server()->Start());
   WebAppUrlLoader loader;
-  EXPECT_EQ(Result::kUrlLoaded, LoadUrlAndWait(&loader, "/simple.html"));
+  EXPECT_EQ(Result::kUrlLoaded,
+            LoadUrlAndWait(&loader, UrlComparison::kExact, "/simple.html"));
 }
 
-IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, LoadedWithParamChange) {
+IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, LoadedWithParamChangeIgnored) {
   SetupRedirect("/test-redirect", "/test-redirect?param=stuff");
   ASSERT_TRUE(embedded_test_server()->Start());
   WebAppUrlLoader loader;
 
-  EXPECT_EQ(Result::kUrlLoaded, LoadUrlAndWait(&loader, "/test-redirect"));
+  EXPECT_EQ(Result::kUrlLoaded,
+            LoadUrlAndWait(&loader, UrlComparison::kIgnoreQueryParamsAndRef,
+                           "/test-redirect"));
 }
 
-IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, LoadedWithParamAndRefChange) {
+IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest,
+                       LoadedWithParamAndRefChangeIgnored) {
   // Note we cannot test the ref change in isolation as it is not sent to the
   // server, so we cannot check it in the request handler.
   SetupRedirect("/test-redirect", "/test-redirect?param=foo#ref");
   ASSERT_TRUE(embedded_test_server()->Start());
   WebAppUrlLoader loader;
 
-  EXPECT_EQ(Result::kUrlLoaded, LoadUrlAndWait(&loader, "/test-redirect"));
+  EXPECT_EQ(Result::kUrlLoaded,
+            LoadUrlAndWait(&loader, UrlComparison::kIgnoreQueryParamsAndRef,
+                           "/test-redirect"));
+}
+
+IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, LoadedWithPathChangeIgnored) {
+  SetupRedirect("/test-redirect", "/test-redirect-new-path");
+  ASSERT_TRUE(embedded_test_server()->Start());
+  WebAppUrlLoader loader;
+
+  EXPECT_EQ(
+      Result::kUrlLoaded,
+      LoadUrlAndWait(&loader, UrlComparison::kSameOrigin, "/test-redirect"));
+}
+
+IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, RedirectWithRefChange) {
+  SetupRedirect("/test-redirect", "/test-redirect#ref");
+  ASSERT_TRUE(embedded_test_server()->Start());
+  WebAppUrlLoader loader;
+
+  EXPECT_EQ(Result::kRedirectedUrlLoaded,
+            LoadUrlAndWait(&loader, UrlComparison::kExact, "/test-redirect"));
+}
+
+IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, RedirectWithParamChange) {
+  SetupRedirect("/test-redirect", "/test-redirect?param=stuff");
+  ASSERT_TRUE(embedded_test_server()->Start());
+  WebAppUrlLoader loader;
+
+  EXPECT_EQ(Result::kRedirectedUrlLoaded,
+            LoadUrlAndWait(&loader, UrlComparison::kExact, "/test-redirect"));
 }
 
 IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, RedirectWithPathChange) {
@@ -136,7 +173,8 @@ IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, RedirectWithPathChange) {
   WebAppUrlLoader loader;
 
   EXPECT_EQ(Result::kRedirectedUrlLoaded,
-            LoadUrlAndWait(&loader, "/test-redirect"));
+            LoadUrlAndWait(&loader, UrlComparison::kIgnoreQueryParamsAndRef,
+                           "/test-redirect"));
 }
 
 IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, 302FoundRedirect) {
@@ -144,9 +182,9 @@ IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, 302FoundRedirect) {
   WebAppUrlLoader loader;
 
   const GURL final_url = embedded_test_server()->GetURL("/simple.html");
-  EXPECT_EQ(
-      Result::kRedirectedUrlLoaded,
-      LoadUrlAndWait(&loader, "/server-redirect-302?" + final_url.spec()));
+  EXPECT_EQ(Result::kRedirectedUrlLoaded,
+            LoadUrlAndWait(&loader, UrlComparison::kIgnoreQueryParamsAndRef,
+                           "/server-redirect-302?" + final_url.spec()));
 }
 
 IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, Hung) {
@@ -158,6 +196,7 @@ IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, Hung) {
   base::Optional<Result> result;
 
   loader.LoadUrl(embedded_test_server()->GetURL("/hung"), web_contents(),
+                 UrlComparison::kExact,
                  base::BindLambdaForTesting([&](Result r) { result = r; }));
 
   // Run all pending tasks. The URL should still be loading.
@@ -181,6 +220,7 @@ IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, WebContentsDestroyed) {
 
   base::RunLoop run_loop;
   loader.LoadUrl(embedded_test_server()->GetURL("/hung"), web_contents(),
+                 UrlComparison::kExact,
                  base::BindLambdaForTesting([&](Result r) {
                    result = r;
                    run_loop.Quit();
@@ -218,12 +258,14 @@ IN_PROC_BROWSER_TEST_F(WebAppUrlLoaderTest, MultipleLoadUrlCalls) {
       base::BarrierClosure(2, run_loop.QuitClosure());
 
   loader.LoadUrl(embedded_test_server()->GetURL("/title1.html"),
-                 web_contents1.get(), base::BindLambdaForTesting([&](Result r) {
+                 web_contents1.get(), UrlComparison::kExact,
+                 base::BindLambdaForTesting([&](Result r) {
                    title1_result = r;
                    barrier_closure.Run();
                  }));
   loader.LoadUrl(embedded_test_server()->GetURL("/title2.html"),
-                 web_contents2.get(), base::BindLambdaForTesting([&](Result r) {
+                 web_contents2.get(), UrlComparison::kExact,
+                 base::BindLambdaForTesting([&](Result r) {
                    title2_result = r;
                    barrier_closure.Run();
                  }));

chrome/browser/web_applications/extensions/install_manager_bookmark_app_unittest.cc

@@ -271,6 +271,7 @@ class InstallManagerBookmarkAppTest : public ExtensionServiceTestBase {
           web_app::InstallManager::InstallParams{}) {
     base::RunLoop run_loop;
     web_app::AppId app_id;
+    install_params.fallback_start_url = GURL("https://example.com/fallback");
 
     auto* provider = web_app::WebAppProviderBase::GetProviderBase(profile());
 

chrome/browser/web_applications/pending_app_manager_impl.cc

@@ -219,6 +219,7 @@ void PendingAppManagerImpl::StartInstallationTask(
 
   url_loader_->LoadUrl(current_install_->task->install_options().url,
                        web_contents_.get(),
+                       WebAppUrlLoader::UrlComparison::kSameOrigin,
                        base::BindOnce(&PendingAppManagerImpl::OnUrlLoaded,
                                       weak_ptr_factory_.GetWeakPtr()));
 }

chrome/browser/web_applications/pending_app_manager_impl_browsertest.cc

@@ -25,12 +25,19 @@
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/url_constants.h"
+#include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 
 namespace web_app {
 
 class PendingAppManagerImplBrowserTest : public InProcessBrowserTest {
  protected:
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+    // Allow different origins to be handled by the embedded_test_server.
+    host_resolver()->AddRule("*", "127.0.0.1");
+  }
+
   AppRegistrar& registrar() {
     return WebAppProviderBase::GetProviderBase(browser()->profile())
         ->registrar();
@@ -88,15 +95,63 @@ IN_PROC_BROWSER_TEST_F(PendingAppManagerImplBrowserTest, InstallSucceeds) {
   EXPECT_EQ("Manifest test app", registrar().GetAppShortName(app_id.value()));
 }
 
+// If install URL redirects, install should still succeed.
+IN_PROC_BROWSER_TEST_F(PendingAppManagerImplBrowserTest,
+                       InstallSucceedsWithRedirect) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL start_url =
+      embedded_test_server()->GetURL("/banners/manifest_test_page.html");
+  GURL install_url =
+      embedded_test_server()->GetURL("/server-redirect?" + start_url.spec());
+  InstallApp(CreateInstallOptions(install_url));
+  EXPECT_EQ(InstallResultCode::kSuccessNewInstall, result_code_.value());
+  base::Optional<AppId> app_id =
+      ExternallyInstalledWebAppPrefs(browser()->profile()->GetPrefs())
+          .LookupAppId(install_url);
+  EXPECT_TRUE(app_id.has_value());
+  EXPECT_EQ("Manifest test app", registrar().GetAppShortName(app_id.value()));
+  // Same AppID should be in the registrar using start_url from the manifest.
+  EXPECT_TRUE(registrar().IsLocallyInstalled(start_url));
+  base::Optional<AppId> opt_app_id =
+      registrar().FindAppWithUrlInScope(start_url);
+  EXPECT_TRUE(opt_app_id.has_value());
+  EXPECT_EQ(*opt_app_id, app_id);
+}
+
+// If install URL redirects, install should still succeed.
+IN_PROC_BROWSER_TEST_F(PendingAppManagerImplBrowserTest,
+                       InstallSucceedsWithRedirectNoManifest) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL final_url =
+      embedded_test_server()->GetURL("/banners/no_manifest_test_page.html");
+  GURL install_url =
+      embedded_test_server()->GetURL("/server-redirect?" + final_url.spec());
+  InstallApp(CreateInstallOptions(install_url));
+  EXPECT_EQ(InstallResultCode::kSuccessNewInstall, result_code_.value());
+  base::Optional<AppId> app_id =
+      ExternallyInstalledWebAppPrefs(browser()->profile()->GetPrefs())
+          .LookupAppId(install_url);
+  EXPECT_TRUE(app_id.has_value());
+  EXPECT_EQ("Web app banner test page",
+            registrar().GetAppShortName(app_id.value()));
+  // Same AppID should be in the registrar using install_url.
+  EXPECT_TRUE(registrar().IsLocallyInstalled(install_url));
+  base::Optional<AppId> opt_app_id =
+      registrar().FindAppWithUrlInScope(install_url);
+  ASSERT_TRUE(opt_app_id.has_value());
+  EXPECT_EQ(*opt_app_id, app_id);
+  EXPECT_EQ(registrar().GetAppLaunchURL(*opt_app_id), install_url);
+}
+
 // Installing a placeholder app with shortcuts should succeed.
 IN_PROC_BROWSER_TEST_F(PendingAppManagerImplBrowserTest,
                        PlaceholderInstallSucceedsWithShortcuts) {
   ASSERT_TRUE(embedded_test_server()->Start());
   shortcut_manager().SuppressShortcutsForTesting();
 
-  GURL final_url =
-      embedded_test_server()->GetURL("/banners/manifest_test_page.html");
-  // Add a redirect, so a placeholder is installed.
+  GURL final_url = embedded_test_server()->GetURL(
+      "other.origin.com", "/banners/manifest_test_page.html");
+  // Add a redirect to a different origin, so a placeholder is installed.
   GURL url(
       embedded_test_server()->GetURL("/server-redirect?" + final_url.spec()));
 

chrome/browser/web_applications/pending_app_registration_task.cc

@@ -86,7 +86,9 @@ void PendingAppRegistrationTask::OnDidCheckHasServiceWorker(
 
   // No action is needed when the URL loads.
   // We wait for OnRegistrationCompleted (or registration timeout).
-  url_loader_->LoadUrl(launch_url(), web_contents_, base::DoNothing());
+  url_loader_->LoadUrl(launch_url(), web_contents_,
+                       WebAppUrlLoader::UrlComparison::kExact,
+                       base::DoNothing());
 }
 
 void PendingAppRegistrationTask::OnRegistrationTimeout() {

chrome/browser/web_applications/test/test_web_app_url_loader.cc

@@ -41,6 +41,7 @@ void TestWebAppUrlLoader::SetNextLoadUrlResult(const GURL& url, Result result) {
 
 void TestWebAppUrlLoader::LoadUrl(const GURL& url,
                                   content::WebContents* web_contents,
+                                  UrlComparison url_comparison,
                                   ResultCallback callback) {
   if (should_save_requests_) {
     pending_requests_.emplace(url, std::move(callback));

chrome/browser/web_applications/test/test_web_app_url_loader.h

@@ -32,6 +32,7 @@ class TestWebAppUrlLoader : public WebAppUrlLoader {
   // WebAppUrlLoader
   void LoadUrl(const GURL& url,
                content::WebContents* web_contents,
+               UrlComparison url_comparison,
                ResultCallback callback) override;
 
  private:

chrome/browser/web_applications/web_app_install_manager.cc

@@ -355,6 +355,7 @@ content::WebContents* WebAppInstallManager::EnsureWebContentsCreated() {
 
   // Load about:blank so that the process actually starts.
   url_loader_->LoadUrl(GURL("about:blank"), web_contents_.get(),
+                       WebAppUrlLoader::UrlComparison::kExact,
                        base::BindOnce(&WebAppInstallManager::OnWebContentsReady,
                                       weak_ptr_factory_.GetWeakPtr()));
 

chrome/browser/web_applications/web_app_install_task.cc

@@ -106,6 +106,7 @@ void WebAppInstallTask::LoadWebAppAndCheckInstallability(
 
   url_loader->LoadUrl(
       url, web_contents_ptr,
+      WebAppUrlLoader::UrlComparison::kIgnoreQueryParamsAndRef,
       base::BindOnce(
           &WebAppInstallTask::
               OnWebAppUrlLoadedCheckInstallabilityAndRetrieveManifest,
@@ -170,6 +171,7 @@ void WebAppInstallTask::LoadAndInstallWebAppFromManifestWithFallback(
 
   url_loader->LoadUrl(
       launch_url, contents,
+      WebAppUrlLoader::UrlComparison::kIgnoreQueryParamsAndRef,
       base::BindOnce(&WebAppInstallTask::OnWebAppUrlLoadedGetWebApplicationInfo,
                      weak_ptr_factory_.GetWeakPtr()));
 }
@@ -285,6 +287,7 @@ void WebAppInstallTask::LoadAndRetrieveWebApplicationInfoWithIcons(
   DCHECK(url_loader);
   url_loader->LoadUrl(
       app_url, web_contents(),
+      WebAppUrlLoader::UrlComparison::kIgnoreQueryParamsAndRef,
       base::BindOnce(&WebAppInstallTask::OnWebAppUrlLoadedGetWebApplicationInfo,
                      weak_ptr_factory_.GetWeakPtr()));
 }
@@ -416,8 +419,13 @@ void WebAppInstallTask::OnGetWebApplicationInfo(
   }
 
   bool bypass_service_worker_check = false;
-  if (install_params_)
+  if (install_params_) {
     bypass_service_worker_check = install_params_->bypass_service_worker_check;
+    // Set app_url to fallback_start_url as web_contents may have been
+    // redirected. Will be overridden by manifest values if present.
+    DCHECK(install_params_->fallback_start_url.is_valid());
+    web_app_info->app_url = install_params_->fallback_start_url;
+  }
 
   data_retriever_->CheckInstallabilityAndRetrieveManifest(
       web_contents(), bypass_service_worker_check,

chrome/browser/web_applications/web_app_install_task_unittest.cc

@@ -114,6 +114,14 @@ void TestDeclineDialogCallback(
                                 false /*accept*/, std::move(web_app_info)));
 }
 
+WebAppInstallManager::InstallParams MakeParams(
+    web_app::DisplayMode display_mode = DisplayMode::kUndefined) {
+  WebAppInstallManager::InstallParams params;
+  params.fallback_start_url = GURL("https://example.com/fallback");
+  params.user_display_mode = display_mode;
+  return params;
+}
+
 }  // namespace
 
 class WebAppInstallTaskTest : public WebAppTest {
@@ -1134,8 +1142,7 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppWithParams_GuestProfile) {
 
   base::RunLoop run_loop;
   install_task->InstallWebAppWithParams(
-      web_contents(), InstallManager::InstallParams{},
-      WebappInstallSource::EXTERNAL_DEFAULT,
+      web_contents(), MakeParams(), WebappInstallSource::EXTERNAL_DEFAULT,
       base::BindLambdaForTesting(
           [&](const AppId& app_id, InstallResultCode code) {
             EXPECT_EQ(InstallResultCode::kSuccessNewInstall, code);
@@ -1149,9 +1156,7 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppWithParams_DisplayMode) {
     CreateDataToRetrieve(GURL("https://example.com/"),
                          /*open_as_window*/ false);
 
-    InstallManager::InstallParams params;
-    params.user_display_mode = DisplayMode::kUndefined;
-    auto app_id = InstallWebAppWithParams(params);
+    auto app_id = InstallWebAppWithParams(MakeParams(DisplayMode::kUndefined));
 
     EXPECT_EQ(DisplayMode::kBrowser,
               registrar().GetAppById(app_id)->user_display_mode());
@@ -1159,9 +1164,7 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppWithParams_DisplayMode) {
   {
     CreateDataToRetrieve(GURL("https://example.org/"), /*open_as_window*/ true);
 
-    InstallManager::InstallParams params;
-    params.user_display_mode = DisplayMode::kUndefined;
-    auto app_id = InstallWebAppWithParams(params);
+    auto app_id = InstallWebAppWithParams(MakeParams(DisplayMode::kUndefined));
 
     EXPECT_EQ(DisplayMode::kStandalone,
               registrar().GetAppById(app_id)->user_display_mode());
@@ -1169,9 +1172,7 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppWithParams_DisplayMode) {
   {
     CreateDataToRetrieve(GURL("https://example.au/"), /*open_as_window*/ true);
 
-    InstallManager::InstallParams params;
-    params.user_display_mode = DisplayMode::kBrowser;
-    auto app_id = InstallWebAppWithParams(params);
+    auto app_id = InstallWebAppWithParams(MakeParams(DisplayMode::kBrowser));
 
     EXPECT_EQ(DisplayMode::kBrowser,
               registrar().GetAppById(app_id)->user_display_mode());
@@ -1180,9 +1181,7 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppWithParams_DisplayMode) {
     CreateDataToRetrieve(GURL("https://example.app/"),
                          /*open_as_window*/ false);
 
-    InstallManager::InstallParams params;
-    params.user_display_mode = DisplayMode::kStandalone;
-    auto app_id = InstallWebAppWithParams(params);
+    auto app_id = InstallWebAppWithParams(MakeParams(DisplayMode::kStandalone));
 
     EXPECT_EQ(DisplayMode::kStandalone,
               registrar().GetAppById(app_id)->user_display_mode());