WebApp: Retire FinalizeOptions::force_launch_container argument.

WebApplicationInfo::open_as_window unambiguously determines
launch container.

Bug: 973288
Change-Id: I85ad1c1d556f5dbba2760c21014f58f3bd3162a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1746331
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686324}

chrome/browser/web_applications/components/install_finalizer.h

@@ -9,7 +9,6 @@
 
 #include "base/callback_forward.h"
 #include "chrome/browser/installable/installable_metrics.h"
-#include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 
 struct WebApplicationInfo;
@@ -37,9 +36,6 @@ class InstallFinalizer {
       base::OnceCallback<void(bool shortcuts_created)>;
 
   struct FinalizeOptions {
-    // If |force_launch_container| defined as non-kDefault then the installed
-    // app will launch in |force_launch_container|.
-    LaunchContainer force_launch_container = LaunchContainer::kDefault;
     WebappInstallSource install_source = WebappInstallSource::COUNT;
     bool locally_installed = true;
     bool no_network_install = false;

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.cc

@@ -60,19 +60,6 @@ void BookmarkAppInstallFinalizer::FinalizeInstall(
   extensions::LaunchType launch_type =
       web_app_info.open_as_window ? LAUNCH_TYPE_WINDOW : LAUNCH_TYPE_REGULAR;
 
-  // Override extensions::LaunchType with force web_app::LaunchContainer:
-  switch (options.force_launch_container) {
-    case web_app::LaunchContainer::kDefault:
-      // force_launch_container is not defined, do not override.
-      break;
-    case web_app::LaunchContainer::kTab:
-      launch_type = LAUNCH_TYPE_REGULAR;
-      break;
-    case web_app::LaunchContainer::kWindow:
-      launch_type = LAUNCH_TYPE_WINDOW;
-      break;
-  }
-
   crx_installer->set_installer_callback(base::BindOnce(
       &BookmarkAppInstallFinalizer::OnExtensionInstalled,
       weak_ptr_factory_.GetWeakPtr(), web_app_info.app_url, launch_type,

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer_unittest.cc

@@ -369,38 +369,6 @@ TEST_F(BookmarkAppInstallFinalizerTest, NoNetworkInstallSucceeds) {
   run_loop.Run();
 }
 
-TEST_F(BookmarkAppInstallFinalizerTest, ForceLaunchContainer) {
-  auto info = std::make_unique<WebApplicationInfo>();
-  info->app_url = kWebAppUrl;
-  // The info says extensions::LAUNCH_TYPE_WINDOW needed.
-  info->open_as_window = true;
-
-  web_app::InstallFinalizer::FinalizeOptions options;
-  options.install_source = WebappInstallSource::INTERNAL_DEFAULT;
-  // Force launch as a tab.
-  options.force_launch_container = web_app::LaunchContainer::kTab;
-
-  base::RunLoop run_loop;
-  finalizer().FinalizeInstall(
-      *info, options,
-      base::BindLambdaForTesting([&](const web_app::AppId& installed_app_id,
-                                     web_app::InstallResultCode code) {
-        EXPECT_EQ(web_app::InstallResultCode::kSuccess, code);
-
-        auto* extension =
-            ExtensionRegistry::Get(profile())->GetInstalledExtension(
-                installed_app_id);
-
-        extensions::LaunchType launch_type =
-            GetLaunchType(ExtensionPrefs::Get(profile()), extension);
-        // Not extensions::LAUNCH_TYPE_WINDOW.
-        EXPECT_EQ(launch_type, extensions::LAUNCH_TYPE_REGULAR);
-
-        run_loop.Quit();
-      }));
-  run_loop.Run();
-}
-
 TEST_F(BookmarkAppInstallFinalizerTest, CanSkipAppUpdateForSync) {
   auto info = std::make_unique<WebApplicationInfo>();
   info->app_url = kWebAppUrl;

chrome/browser/web_applications/extensions/pending_app_install_task_unittest.cc

@@ -306,6 +306,11 @@ class PendingAppInstallTaskTest : public ChromeRenderViewHostTestHarness {
 
   TestDataRetriever* data_retriever() { return data_retriever_; }
 
+  const WebApplicationInfo& web_app_info() {
+    DCHECK_EQ(1u, install_finalizer_->web_app_info_list().size());
+    return install_finalizer_->web_app_info_list().at(0);
+  }
+
   const InstallFinalizer::FinalizeOptions& finalize_options() {
     DCHECK_EQ(1u, install_finalizer_->finalize_options_list().size());
     return install_finalizer_->finalize_options_list().at(0);
@@ -372,8 +377,7 @@ TEST_F(PendingAppInstallTaskTest,
         EXPECT_EQ(0u, finalizer()->num_reparent_tab_calls());
         EXPECT_EQ(0u, finalizer()->num_reveal_appshim_calls());
 
-        EXPECT_EQ(LaunchContainer::kDefault,
-                  finalize_options().force_launch_container);
+        EXPECT_FALSE(web_app_info().open_as_window);
         EXPECT_EQ(WebappInstallSource::INTERNAL_DEFAULT,
                   finalize_options().install_source);
 
@@ -510,10 +514,7 @@ TEST_F(PendingAppInstallTaskTest,
       base::BindLambdaForTesting([&](PendingAppInstallTask::Result result) {
         EXPECT_EQ(InstallResultCode::kSuccess, result.code);
         EXPECT_TRUE(result.app_id.has_value());
-
-        EXPECT_EQ(LaunchContainer::kWindow,
-                  finalize_options().force_launch_container);
-
+        EXPECT_TRUE(web_app_info().open_as_window);
         run_loop.Quit();
       }));
 
@@ -533,9 +534,7 @@ TEST_F(PendingAppInstallTaskTest,
       base::BindLambdaForTesting([&](PendingAppInstallTask::Result result) {
         EXPECT_EQ(InstallResultCode::kSuccess, result.code);
         EXPECT_TRUE(result.app_id.has_value());
-
-        EXPECT_EQ(LaunchContainer::kTab,
-                  finalize_options().force_launch_container);
+        EXPECT_FALSE(web_app_info().open_as_window);
         run_loop.Quit();
       }));
 

chrome/browser/web_applications/web_app_install_task.cc

@@ -135,7 +135,7 @@ void WebAppInstallTask::InstallWebAppFromInfo(
   if (no_network_install) {
     options.no_network_install = true;
     // We should only install windowed apps via this method.
-    options.force_launch_container = LaunchContainer::kWindow;
+    web_application_info->open_as_window = true;
   }
 
   install_finalizer_->FinalizeInstall(*web_application_info, options,
@@ -454,8 +454,10 @@ void WebAppInstallTask::OnDialogCompleted(
 
   InstallFinalizer::FinalizeOptions finalize_options;
   finalize_options.install_source = install_source_;
-  if (install_params_) {
-    finalize_options.force_launch_container = install_params_->launch_container;
+  if (install_params_ &&
+      install_params_->launch_container != LaunchContainer::kDefault) {
+    web_app_info_copy.open_as_window =
+        install_params_->launch_container == LaunchContainer::kWindow;
   }
 
   install_finalizer_->FinalizeInstall(

chrome/browser/web_applications/web_app_install_task_unittest.cc

@@ -32,6 +32,7 @@
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_icon_manager.h"
 #include "chrome/browser/web_applications/web_app_install_finalizer.h"
+#include "chrome/browser/web_applications/web_app_install_manager.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -207,6 +208,21 @@ class WebAppInstallTaskTest : public WebAppTest {
     CreateDefaultDataToRetrieve(url, GURL{});
   }
 
+  void CreateDataToRetrieve(const GURL& url, bool open_as_window) {
+    DCHECK(data_retriever_);
+
+    auto renderer_web_app_info = std::make_unique<WebApplicationInfo>();
+    renderer_web_app_info->open_as_window = open_as_window;
+    data_retriever_->SetRendererWebApplicationInfo(
+        std::move(renderer_web_app_info));
+
+    auto manifest = std::make_unique<blink::Manifest>();
+    manifest->start_url = url;
+    data_retriever_->SetManifest(std::move(manifest), /*is_installable=*/true);
+
+    data_retriever_->SetIcons(IconsMap{});
+  }
+
   TestInstallFinalizer& test_install_finalizer() {
     DCHECK(test_install_finalizer_);
     return *test_install_finalizer_;
@@ -246,6 +262,22 @@ class WebAppInstallTaskTest : public WebAppTest {
     return result.app_id;
   }
 
+  AppId InstallWebAppWithParams(
+      const WebAppInstallManager::InstallParams& params) {
+    AppId app_id;
+    base::RunLoop run_loop;
+    install_task_->InstallWebAppWithParams(
+        web_contents(), params, WebappInstallSource::EXTERNAL_DEFAULT,
+        base::BindLambdaForTesting(
+            [&](const AppId& installed_app_id, InstallResultCode code) {
+              ASSERT_EQ(InstallResultCode::kSuccess, code);
+              app_id = installed_app_id;
+              run_loop.Quit();
+            }));
+    run_loop.Run();
+    return app_id;
+  }
+
   void PrepareTestAppInstall() {
     const GURL url{"https://example.com/path"};
     CreateDefaultDataToRetrieve(url);
@@ -724,7 +756,7 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppFromInfo_Success) {
 
   auto web_app_info = std::make_unique<WebApplicationInfo>();
   web_app_info->app_url = url;
-  web_app_info->open_as_window = false;
+  web_app_info->open_as_window = true;
 
   base::RunLoop run_loop;
 
@@ -739,10 +771,10 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppFromInfo_Success) {
                          .finalize_options_list()
                          .at(0)
                          .no_network_install);
-        EXPECT_EQ(LaunchContainer::kDefault, test_install_finalizer()
-                                                 .finalize_options_list()
-                                                 .at(0)
-                                                 .force_launch_container);
+
+        std::unique_ptr<WebApplicationInfo> final_web_app_info =
+            test_install_finalizer().web_app_info();
+        EXPECT_TRUE(final_web_app_info->open_as_window);
 
         run_loop.Quit();
       }));
@@ -768,10 +800,10 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppFromInfo_NoNetworkInstall) {
                             .finalize_options_list()
                             .at(0)
                             .no_network_install);
-            EXPECT_EQ(LaunchContainer::kWindow, test_install_finalizer()
-                                                    .finalize_options_list()
-                                                    .at(0)
-                                                    .force_launch_container);
+
+            std::unique_ptr<WebApplicationInfo> final_web_app_info =
+                test_install_finalizer().web_app_info();
+            EXPECT_TRUE(final_web_app_info->open_as_window);
 
             run_loop.Quit();
           }));
@@ -833,6 +865,8 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppFromInfo_GenerateIcons) {
                 EXPECT_TRUE(icon.url.is_empty());
             }
 
+            EXPECT_TRUE(final_web_app_info->open_as_window);
+
             run_loop.Quit();
           }));
 
@@ -1024,7 +1058,7 @@ TEST_F(WebAppInstallTaskTest, IntentToPlayStore) {
 #endif
 
 // Default apps should be installable for guest profiles.
-TEST_F(WebAppInstallTaskTest, InstallWebAppWithOptions_GuestProfile) {
+TEST_F(WebAppInstallTaskTest, InstallWebAppWithParams_GuestProfile) {
   SetInstallFinalizerForTesting();
 
   TestingProfileManager profile_manager(TestingBrowserProcess::GetGlobal());
@@ -1051,4 +1085,54 @@ TEST_F(WebAppInstallTaskTest, InstallWebAppWithOptions_GuestProfile) {
   run_loop.Run();
 }
 
+TEST_F(WebAppInstallTaskTest, InstallWebAppWithParams_LaunchContainer) {
+  SetInstallFinalizerForTesting();
+  {
+    CreateDataToRetrieve(GURL("https://example.com/"),
+                         /*open_as_window*/ false);
+
+    InstallManager::InstallParams params;
+    params.launch_container = LaunchContainer::kDefault;
+    InstallWebAppWithParams(params);
+
+    std::unique_ptr<WebApplicationInfo> web_app_info =
+        test_install_finalizer().web_app_info();
+    EXPECT_FALSE(web_app_info->open_as_window);
+  }
+  {
+    CreateDataToRetrieve(GURL("https://example.org/"), /*open_as_window*/ true);
+
+    InstallManager::InstallParams params;
+    params.launch_container = LaunchContainer::kDefault;
+    InstallWebAppWithParams(params);
+
+    std::unique_ptr<WebApplicationInfo> web_app_info =
+        test_install_finalizer().web_app_info();
+    EXPECT_TRUE(web_app_info->open_as_window);
+  }
+  {
+    CreateDataToRetrieve(GURL("https://example.au/"), /*open_as_window*/ true);
+
+    InstallManager::InstallParams params;
+    params.launch_container = LaunchContainer::kTab;
+    InstallWebAppWithParams(params);
+
+    std::unique_ptr<WebApplicationInfo> web_app_info =
+        test_install_finalizer().web_app_info();
+    EXPECT_FALSE(web_app_info->open_as_window);
+  }
+  {
+    CreateDataToRetrieve(GURL("https://example.app/"),
+                         /*open_as_window*/ false);
+
+    InstallManager::InstallParams params;
+    params.launch_container = LaunchContainer::kWindow;
+    InstallWebAppWithParams(params);
+
+    std::unique_ptr<WebApplicationInfo> web_app_info =
+        test_install_finalizer().web_app_info();
+    EXPECT_TRUE(web_app_info->open_as_window);
+  }
+}
+
 }  // namespace web_app