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/+/2252561Reviewed-by: Aleks Totic <atotic@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779872}

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

@@ -16,6 +16,7 @@
 #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"
@@ -33,7 +34,6 @@
 #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,11 +44,13 @@
 #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;
 
@@ -170,10 +172,33 @@ class AppServiceWrapperTest : public ::testing::TestWithParam<ProviderType> {
 
     if (base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions) &&
         app_id.app_type() == apps::mojom::AppType::kWeb) {
-      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());
+      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();
       task_environment_.RunUntilIdle();
       return;
     }

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

@@ -17,6 +17,7 @@
 #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"
@@ -34,11 +35,13 @@
 #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"
@@ -60,6 +63,7 @@
 #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 {
@@ -710,6 +714,43 @@ 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_;
 };
@@ -719,11 +760,7 @@ TEST_F(AppSearchProviderWebAppTest, WebApp) {
       apps::AppServiceProxyFactory::GetForProfile(testing_profile());
   proxy->FlushMojoCallsForTesting();
 
-  const web_app::AppId app_id = web_app::InstallDummyWebApp(
-      testing_profile(), kWebAppName, GURL(kWebAppUrl));
-
-  // Allow async callbacks to run.
-  base::RunLoop().RunUntilIdle();
+  const web_app::AppId app_id = InstallWebApp(kWebAppName, GURL(kWebAppUrl));
 
   CreateSearch();
   EXPECT_EQ("WebApp1", RunQuery("WebA"));

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

@@ -10,7 +10,6 @@
 #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"
@@ -41,12 +40,14 @@
 #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"
@@ -63,6 +64,7 @@
 
 using crostini::CrostiniTestHelper;
 
+using web_app::InstallResultCode;
 using web_app::ProviderType;
 
 namespace {
@@ -221,6 +223,42 @@ 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 =
@@ -707,8 +745,7 @@ TEST_P(ShelfContextMenuWebAppTest, WebApp) {
   constexpr char kWebAppName[] = "WebApp1";
 
   app_service_test().FlushMojoCalls();
-  const web_app::AppId app_id =
-      web_app::InstallDummyWebApp(profile(), kWebAppName, GURL(kWebAppUrl));
+  const web_app::AppId app_id = InstallWebApp(kWebAppName, GURL(kWebAppUrl));
 
   controller()->PinAppWithID(app_id);
   const ash::ShelfItem* item = controller()->GetItem(ash::ShelfID(app_id));

chrome/browser/web_applications/BUILD.gn

@@ -157,8 +157,6 @@ 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,6 +346,23 @@ 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()) {