Reland "Desktop PWAs: Unit test helper method InstallWebApp"

This reverts commit 359f3700.

Reason for revert: Broke build due to dependent CLs existing.

Original change's description:
> Revert "Desktop PWAs: Unit test helper method InstallWebApp"
>
> This reverts commit 7db3de6c.
>
> Reason for revert: MSAN failure, crbug.com/1096592
>
> Original change's description:
> > Desktop PWAs: Unit test helper method InstallWebApp
> >
> > Extract unit test helper method InstallWebApp from unit tests to
> > web_app_unittest_util.h/cc and retire duplicate implementations.
> >
> > Drive-by: retire unused method FinalizeInstall
> > in web_app_install_manager_unittest.cc
> >
> > TBR=agawronska@chromium.org
> >
> > Change-Id: I5f7574ef89dfe1543727c2376d8002e2e6fce822
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247991
> > Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
> > Reviewed-by: Alexey Baskakov <loyso@chromium.org>
> > Reviewed-by: Dominick Ng <dominickn@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#779607}
>
> TBR=ericwilligers@chromium.org,loyso@chromium.org,dominickn@chromium.org
>
> Change-Id: I1b50d2ed80fbe29b9e742c7a533d3a0b65e0df7e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252561
> Reviewed-by: Aleks Totic <atotic@chromium.org>
> Commit-Queue: Aleks Totic <atotic@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#779872}

TBR=ericwilligers@chromium.org,loyso@chromium.org,atotic@chromium.org,dominickn@chromium.org

No-Presubmit: true
No-Try: true
No-Tree-Checks: true
Change-Id: I73d0c4893bd9f1f3dd397818faeaa743b0c9a97e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252643
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779884}

chrome/browser/chromeos/child_accounts/time_limits/app_service_wrapper_unittest.cc

@@ -16,7 +16,6 @@
 #include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/strings/strcat.h"
-#include "base/strings/utf_string_conversions.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/values.h"
@@ -34,6 +33,7 @@
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/browser/web_applications/test/test_system_web_app_manager.h"
 #include "chrome/browser/web_applications/test/test_web_app_provider.h"
+#include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/test/base/testing_profile.h"
@@ -44,13 +44,11 @@
 #include "components/services/app_service/public/mojom/types.mojom.h"
 #include "content/public/test/browser_task_environment.h"
 #include "extensions/common/constants.h"
-#include "extensions/common/manifest_constants.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 using web_app::GenerateAppIdFromURL;
-using web_app::InstallResultCode;
 using web_app::ProviderType;
 using web_app::WebAppProviderBase;
 
@@ -172,33 +170,10 @@ class AppServiceWrapperTest : public ::testing::TestWithParam<ProviderType> {
 
     if (base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions) &&
         app_id.app_type() == apps::mojom::AppType::kWeb) {
-      WebApplicationInfo web_app_info;
-
-      web_app_info.app_url = GURL(*url);
-      web_app_info.scope = web_app_info.app_url;
-      web_app_info.title = base::UTF8ToUTF16(app_name);
-      web_app_info.description = base::UTF8ToUTF16(app_name);
-      web_app_info.open_as_window = true;
-
-      web_app::InstallFinalizer::FinalizeOptions options;
-      options.install_source = WebappInstallSource::EXTERNAL_DEFAULT;
-
-      // In unit tests, we do not have Browser or WebContents instances.
-      // Hence we use FinalizeInstall instead of InstallWebAppFromManifest
-      // to install the web app.
-      base::RunLoop run_loop;
-      WebAppProviderBase::GetProviderBase(&profile_)
-          ->install_finalizer()
-          .FinalizeInstall(
-              web_app_info, options,
-              base::BindLambdaForTesting(
-                  [&](const web_app::AppId& installed_app_id,
-                      InstallResultCode code) {
-                    EXPECT_EQ(installed_app_id, app_id.app_id());
-                    EXPECT_EQ(code, InstallResultCode::kSuccessNewInstall);
-                    run_loop.Quit();
-                  }));
-      run_loop.Run();
+      DCHECK(url.has_value());
+      const web_app::AppId installed_app_id =
+          web_app::InstallDummyWebApp(&profile_, app_name, GURL(url.value()));
+      EXPECT_EQ(installed_app_id, app_id.app_id());
       task_environment_.RunUntilIdle();
       return;
     }

chrome/browser/ui/app_list/search/tests/app_search_provider_unittest.cc

@@ -17,7 +17,6 @@
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/simple_test_clock.h"
 #include "base/time/time.h"
@@ -35,13 +34,11 @@
 #include "chrome/browser/ui/app_list/search/search_result_ranker/ranking_item_util.h"
 #include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h"
 #include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
-#include "chrome/browser/web_applications/components/install_finalizer.h"
-#include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
+#include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/web_application_info.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/arc/test/fake_app_instance.h"
 #include "components/crx_file/id_util.h"
@@ -63,7 +60,6 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using web_app::InstallResultCode;
 using web_app::ProviderType;
 
 namespace app_list {
@@ -714,43 +710,6 @@ class AppSearchProviderWebAppTest : public AppSearchProviderTest {
 
   ~AppSearchProviderWebAppTest() override = default;
 
-  web_app::AppId InstallWebApp(const std::string& app_name,
-                               const GURL& app_url) {
-    DCHECK(
-        base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions));
-    const web_app::AppId app_id = web_app::GenerateAppIdFromURL(app_url);
-
-    WebApplicationInfo web_app_info;
-
-    web_app_info.app_url = app_url;
-    web_app_info.scope = app_url;
-    web_app_info.title = base::UTF8ToUTF16(app_name);
-    web_app_info.description = base::UTF8ToUTF16(app_name);
-    web_app_info.open_as_window = true;
-
-    web_app::InstallFinalizer::FinalizeOptions options;
-    options.install_source = WebappInstallSource::EXTERNAL_DEFAULT;
-
-    // In unit tests, we do not have Browser or WebContents instances.
-    // Hence we use FinalizeInstall instead of InstallWebAppFromManifest
-    // to install the web app.
-    base::RunLoop run_loop;
-    web_app::WebAppProviderBase::GetProviderBase(profile_.get())
-        ->install_finalizer()
-        .FinalizeInstall(web_app_info, options,
-                         base::BindLambdaForTesting(
-                             [&](const web_app::AppId& installed_app_id,
-                                 InstallResultCode code) {
-                               EXPECT_EQ(installed_app_id, app_id);
-                               EXPECT_EQ(code,
-                                         InstallResultCode::kSuccessNewInstall);
-                               run_loop.Quit();
-                             }));
-    run_loop.Run();
-    base::RunLoop().RunUntilIdle();
-    return app_id;
-  }
-
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 };
@@ -760,7 +719,11 @@ TEST_F(AppSearchProviderWebAppTest, WebApp) {
       apps::AppServiceProxyFactory::GetForProfile(testing_profile());
   proxy->FlushMojoCallsForTesting();
 
-  const web_app::AppId app_id = InstallWebApp(kWebAppName, GURL(kWebAppUrl));
+  const web_app::AppId app_id = web_app::InstallDummyWebApp(
+      testing_profile(), kWebAppName, GURL(kWebAppUrl));
+
+  // Allow async callbacks to run.
+  base::RunLoop().RunUntilIdle();
 
   CreateSearch();
   EXPECT_EQ("WebApp1", RunQuery("WebA"));

chrome/browser/ui/ash/launcher/shelf_context_menu_unittest.cc

@@ -10,6 +10,7 @@
 #include "ash/public/cpp/app_menu_constants.h"
 #include "ash/public/cpp/shelf_item.h"
 #include "ash/public/cpp/shelf_model.h"
+#include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
@@ -40,14 +41,12 @@
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/extension_shelf_context_menu.h"
 #include "chrome/browser/ui/ash/launcher/internal_app_shelf_context_menu.h"
-#include "chrome/browser/web_applications/components/install_finalizer.h"
-#include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/browser/web_applications/test/test_system_web_app_manager.h"
 #include "chrome/browser/web_applications/test/test_web_app_provider.h"
+#include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/web_application_info.h"
 #include "chrome/test/base/chrome_ash_test_base.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/arc/metrics/arc_metrics_constants.h"
@@ -64,7 +63,6 @@
 
 using crostini::CrostiniTestHelper;
 
-using web_app::InstallResultCode;
 using web_app::ProviderType;
 
 namespace {
@@ -223,42 +221,6 @@ class ShelfContextMenuTest
     arc_test_.app_instance()->SendTaskCreated(task_id, info, std::string());
   }
 
-  web_app::AppId InstallWebApp(const std::string& app_name,
-                               const GURL& app_url) {
-    DCHECK(
-        base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions));
-    const web_app::AppId app_id = web_app::GenerateAppIdFromURL(app_url);
-
-    WebApplicationInfo web_app_info;
-
-    web_app_info.app_url = app_url;
-    web_app_info.scope = app_url;
-    web_app_info.title = base::UTF8ToUTF16(app_name);
-    web_app_info.description = base::UTF8ToUTF16(app_name);
-    web_app_info.open_as_window = true;
-
-    web_app::InstallFinalizer::FinalizeOptions options;
-    options.install_source = WebappInstallSource::EXTERNAL_DEFAULT;
-
-    // In unit tests, we do not have Browser or WebContents instances.
-    // Hence we use FinalizeInstall instead of InstallWebAppFromManifest
-    // to install the web app.
-    base::RunLoop run_loop;
-    web_app::WebAppProviderBase::GetProviderBase(&profile_)
-        ->install_finalizer()
-        .FinalizeInstall(web_app_info, options,
-                         base::BindLambdaForTesting(
-                             [&](const web_app::AppId& installed_app_id,
-                                 InstallResultCode code) {
-                               EXPECT_EQ(installed_app_id, app_id);
-                               EXPECT_EQ(code,
-                                         InstallResultCode::kSuccessNewInstall);
-                               run_loop.Quit();
-                             }));
-    run_loop.Run();
-    return app_id;
-  }
-
  private:
   void ConfigureWebAppProvider() {
     auto system_web_app_manager =
@@ -745,7 +707,8 @@ TEST_P(ShelfContextMenuWebAppTest, WebApp) {
   constexpr char kWebAppName[] = "WebApp1";
 
   app_service_test().FlushMojoCalls();
-  const web_app::AppId app_id = InstallWebApp(kWebAppName, GURL(kWebAppUrl));
+  const web_app::AppId app_id =
+      web_app::InstallDummyWebApp(profile(), kWebAppName, GURL(kWebAppUrl));
 
   controller()->PinAppWithID(app_id);
   const ash::ShelfItem* item = controller()->GetItem(ash::ShelfID(app_id));

chrome/browser/web_applications/BUILD.gn

@@ -157,6 +157,8 @@ source_set("web_applications_test_support") {
     "test/web_app_icon_test_utils.h",
     "test/web_app_install_observer.cc",
     "test/web_app_install_observer.h",
+    "test/web_app_install_test_utils.cc",
+    "test/web_app_install_test_utils.h",
     "test/web_app_registration_waiter.cc",
     "test/web_app_registration_waiter.h",
     "test/web_app_test.cc",

chrome/browser/web_applications/test/web_app_install_test_utils.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
+
+#include "base/feature_list.h"
+#include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/web_applications/components/install_finalizer.h"
+#include "chrome/browser/web_applications/components/web_app_constants.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/components/web_app_provider_base.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/common/web_application_info.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace web_app {
+
+AppId InstallDummyWebApp(Profile* profile,
+                         const std::string& app_name,
+                         const GURL& app_url) {
+  DCHECK(base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions));
+  const AppId app_id = GenerateAppIdFromURL(app_url);
+  WebApplicationInfo web_app_info;
+
+  web_app_info.app_url = app_url;
+  web_app_info.scope = app_url;
+  web_app_info.title = base::UTF8ToUTF16(app_name);
+  web_app_info.description = base::UTF8ToUTF16(app_name);
+  web_app_info.open_as_window = true;
+
+  InstallFinalizer::FinalizeOptions options;
+  options.install_source = WebappInstallSource::EXTERNAL_DEFAULT;
+
+  // In unit tests, we do not have Browser or WebContents instances.
+  // Hence we use FinalizeInstall instead of InstallWebAppFromManifest
+  // to install the web app.
+  base::RunLoop run_loop;
+  WebAppProviderBase::GetProviderBase(profile)
+      ->install_finalizer()
+      .FinalizeInstall(
+          web_app_info, options,
+          base::BindLambdaForTesting(
+              [&](const AppId& installed_app_id, InstallResultCode code) {
+                EXPECT_EQ(installed_app_id, app_id);
+                EXPECT_EQ(code, InstallResultCode::kSuccessNewInstall);
+                run_loop.Quit();
+              }));
+  run_loop.Run();
+
+  return app_id;
+}
+
+}  // namespace web_app

chrome/browser/web_applications/test/web_app_install_test_utils.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_TEST_WEB_APP_INSTALL_TEST_UTILS_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_WEB_APP_INSTALL_TEST_UTILS_H_
+
+#include <string>
+
+#include "chrome/browser/web_applications/components/web_app_id.h"
+
+class GURL;
+class Profile;
+
+namespace web_app {
+
+AppId InstallDummyWebApp(Profile* profile,
+                         const std::string& app_name,
+                         const GURL& app_url);
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_TEST_WEB_APP_INSTALL_TEST_UTILS_H_

chrome/browser/web_applications/web_app_install_manager_unittest.cc

@@ -346,23 +346,6 @@ class WebAppInstallManagerTest : public WebAppTest {
     return result;
   }
 
-  InstallResult FinalizeInstall(
-      const WebApplicationInfo& web_app_info,
-      const InstallFinalizer::FinalizeOptions& options) {
-    InstallResult result;
-    base::RunLoop run_loop;
-    finalizer().FinalizeInstall(
-        web_app_info, options,
-        base::BindLambdaForTesting(
-            [&](const AppId& app_id, InstallResultCode code) {
-              result.app_id = app_id;
-              result.code = code;
-              run_loop.Quit();
-            }));
-    run_loop.Run();
-    return result;
-  }
-
   int GetNumFullyInstalledApps() const {
     int num_apps = 0;
     for (const WebApp& app : test_registry_controller_->registrar().AllApps()) {