Desktop PWAs: WebAppControllerBrowserTestBase cleanup

WebAppControllerBrowserTestBase is no longer parameterized on
ProviderType.

HostedAppEngagementBrowserTest is retired, tests are moved across to
WebAppEngagementBrowserTest.


Bug: 1012171,1065748
Change-Id: I1b65973fe1e300c99570d0e9b67102b2e9a79bf9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454774
Auto-Submit: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814572}

chrome/browser/ui/web_applications/web_app_controller_browsertest.cc

@@ -22,15 +22,7 @@
 
 namespace web_app {
 
-WebAppControllerBrowserTestBase::WebAppControllerBrowserTestBase() {
-  if (GetParam() == ProviderType::kWebApps) {
-    scoped_feature_list_.InitWithFeatures(
-        {features::kDesktopPWAsWithoutExtensions}, {});
-  } else {
-    scoped_feature_list_.InitWithFeatures(
-        {}, {features::kDesktopPWAsWithoutExtensions});
-  }
-}
+WebAppControllerBrowserTestBase::WebAppControllerBrowserTestBase() = default;
 
 WebAppControllerBrowserTestBase::~WebAppControllerBrowserTestBase() = default;
 
@@ -106,8 +98,15 @@ base::Optional<AppId> WebAppControllerBrowserTestBase::FindAppWithUrlInScope(
 
 WebAppControllerBrowserTest::WebAppControllerBrowserTest()
     : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
-  scoped_feature_list_.InitWithFeatures(
-      {}, {predictors::kSpeculativePreconnectFeature});
+  if (GetParam() == ProviderType::kWebApps) {
+    scoped_feature_list_.InitWithFeatures(
+        {features::kDesktopPWAsWithoutExtensions},
+        {predictors::kSpeculativePreconnectFeature});
+  } else {
+    scoped_feature_list_.InitWithFeatures(
+        {}, {features::kDesktopPWAsWithoutExtensions,
+             predictors::kSpeculativePreconnectFeature});
+  }
 }
 
 WebAppControllerBrowserTest::~WebAppControllerBrowserTest() = default;

chrome/browser/ui/web_applications/web_app_controller_browsertest.h

@@ -31,15 +31,14 @@ class WebAppProviderBase;
 // ProviderType selects between use of BMO web apps and
 // Extensions-based bookmark apps.
 class WebAppControllerBrowserTestBase
-    : public extensions::ExtensionBrowserTest,
-      public ::testing::WithParamInterface<ProviderType> {
+    : public extensions::ExtensionBrowserTest {
  public:
   WebAppControllerBrowserTestBase();
   WebAppControllerBrowserTestBase(const WebAppControllerBrowserTestBase&) =
       delete;
   WebAppControllerBrowserTestBase& operator=(
       const WebAppControllerBrowserTestBase&) = delete;
-  ~WebAppControllerBrowserTestBase() = 0;
+  ~WebAppControllerBrowserTestBase() override = 0;
 
   WebAppProviderBase& provider();
 
@@ -66,13 +65,11 @@ class WebAppControllerBrowserTestBase
   Browser* NavigateInNewWindowAndAwaitInstallabilityCheck(const GURL&);
 
   base::Optional<AppId> FindAppWithUrlInScope(const GURL& url);
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-
 };
 
-class WebAppControllerBrowserTest : public WebAppControllerBrowserTestBase {
+class WebAppControllerBrowserTest
+    : public WebAppControllerBrowserTestBase,
+      public ::testing::WithParamInterface<ProviderType> {
  public:
   WebAppControllerBrowserTest();
   ~WebAppControllerBrowserTest() = 0;

chrome/browser/ui/web_applications/web_app_engagement_browsertest.cc

@@ -27,7 +27,6 @@
 #include "chrome/browser/web_applications/components/pending_app_manager.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/test/web_app_test.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/web_application_info.h"
@@ -225,14 +224,7 @@ class WebAppEngagementBrowserTest : public WebAppControllerBrowserTestBase {
 
 };
 
-// TODO(crbug.com/1012171): Migrate all to WebAppEngagementBrowserTest.
-class HostedAppEngagementBrowserTest : public WebAppEngagementBrowserTest {
- public:
-  HostedAppEngagementBrowserTest() = default;
-  ~HostedAppEngagementBrowserTest() override = default;
-};
-
-IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, AppInWindow) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, AppInWindow) {
   base::HistogramTester tester;
 
   const GURL example_url = GURL("http://example.org/");
@@ -258,7 +250,7 @@ IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, AppInWindow) {
   ExpectLaunchCounts(tester, /*windowLaunches=*/1, /*tabLaunches=*/0);
 }
 
-IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, AppInTab) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, AppInTab) {
   base::HistogramTester tester;
 
   const GURL example_url = GURL("http://example.org/");
@@ -283,7 +275,7 @@ IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, AppInTab) {
   ExpectLaunchCounts(tester, /*windowLaunches=*/0, /*tabLaunches=*/1);
 }
 
-IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, AppWithoutScope) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, AppWithoutScope) {
   base::HistogramTester tester;
 
   const GURL example_url = GURL("http://example.org/");
@@ -312,7 +304,7 @@ IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, AppWithoutScope) {
   ExpectLaunchCounts(tester, /*windowLaunches=*/1, /*tabLaunches=*/0);
 }
 
-IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, TwoApps) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, TwoApps) {
   base::HistogramTester tester;
 
   const GURL example_url1 = GURL("http://example.org/");
@@ -357,7 +349,7 @@ IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, TwoApps) {
   ExpectLaunchCounts(tester, /*windowLaunches=*/3, /*tabLaunches=*/0);
 }
 
-IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, ManyUserApps) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, ManyUserApps) {
   base::HistogramTester tester;
 
   // More than 3 user-installed apps:
@@ -402,7 +394,7 @@ IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, ManyUserApps) {
                      /*tabLaunches=*/0);
 }
 
-IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, DefaultApp) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, DefaultApp) {
   base::HistogramTester tester;
 
   ASSERT_TRUE(embedded_test_server()->Start());
@@ -428,7 +420,7 @@ IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, DefaultApp) {
   ExpectLaunchCounts(tester, /*windowLaunches=*/1, /*tabLaunches=*/0);
 }
 
-IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, NavigateAwayFromAppTab) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, NavigateAwayFromAppTab) {
   base::HistogramTester tester;
   const GURL start_url = GURL("http://example.org/app/");
   const GURL outer_url = GURL("http://example.org/");
@@ -461,7 +453,7 @@ IN_PROC_BROWSER_TEST_P(HostedAppEngagementBrowserTest, NavigateAwayFromAppTab) {
   ExpectLaunchCounts(tester, /*windowLaunches=*/0, /*tabLaunches=*/1);
 }
 
-IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, RecordedForNonApps) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, RecordedForNonApps) {
   base::HistogramTester tester;
   CountUserInstalledApps();
 
@@ -480,7 +472,7 @@ IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, RecordedForNonApps) {
   TestEngagementEventsAfterLaunch(histograms, browser());
 }
 
-IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, CommandLineWindow) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, CommandLineWindow) {
   base::HistogramTester tester;
   ASSERT_TRUE(embedded_test_server()->Start());
 
@@ -535,7 +527,7 @@ IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, CommandLineWindow) {
   EXPECT_EQ(expected_tabs, app_browser->tab_strip_model()->count());
 }
 
-IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, CommandLineTab) {
+IN_PROC_BROWSER_TEST_F(WebAppEngagementBrowserTest, CommandLineTab) {
   base::HistogramTester tester;
   ASSERT_TRUE(embedded_test_server()->Start());
 
@@ -588,15 +580,4 @@ IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, CommandLineTab) {
   EXPECT_EQ(expected_tabs, browser()->tab_strip_model()->count());
 }
 
-INSTANTIATE_TEST_SUITE_P(All,
-                         WebAppEngagementBrowserTest,
-                         ::testing::Values(ProviderType::kBookmarkApps,
-                                           ProviderType::kWebApps),
-                         ProviderTypeParamToString);
-
-INSTANTIATE_TEST_SUITE_P(All,
-                         HostedAppEngagementBrowserTest,
-                         ::testing::Values(ProviderType::kBookmarkApps),
-                         ProviderTypeParamToString);
-
 }  // namespace web_app

chrome/browser/ui/web_applications/web_app_navigate_browsertest.cc

@@ -12,7 +12,6 @@
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
 #include "chrome/browser/ui/web_applications/web_app_controller_browsertest.h"
 #include "chrome/browser/web_applications/components/web_app_id.h"
-#include "chrome/browser/web_applications/test/web_app_test.h"
 #include "content/public/test/browser_test.h"
 #include "ui/base/page_transition_types.h"
 #include "ui/base/window_open_disposition.h"
@@ -33,7 +32,7 @@ class WebAppNavigateBrowserTest : public WebAppControllerBrowserTestBase {
 
 // This test verifies that navigating with "open_pwa_window_if_possible = true"
 // opens a new app window if there is an installed Web App for the URL.
-IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest,
+IN_PROC_BROWSER_TEST_F(WebAppNavigateBrowserTest,
                        AppInstalled_OpenAppWindowIfPossible_True) {
   InstallPWA(GetGoogleURL());
 
@@ -51,7 +50,7 @@ IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest,
 // This test verifies that navigating with "open_pwa_window_if_possible = false"
 // opens a new foreground tab even if there is an installed Web App for the
 // URL.
-IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest,
+IN_PROC_BROWSER_TEST_F(WebAppNavigateBrowserTest,
                        AppInstalled_OpenAppWindowIfPossible_False) {
   InstallPWA(GetGoogleURL());
 
@@ -68,7 +67,7 @@ IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest,
 
 // This test verifies that navigating with "open_pwa_window_if_possible = true"
 // opens a new foreground tab when there is no app installed for the URL.
-IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest,
+IN_PROC_BROWSER_TEST_F(WebAppNavigateBrowserTest,
                        NoAppInstalled_OpenAppWindowIfPossible) {
   int num_tabs = browser()->tab_strip_model()->count();
 
@@ -81,7 +80,7 @@ IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest,
   EXPECT_EQ(++num_tabs, browser()->tab_strip_model()->count());
 }
 
-IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest, NewPopup) {
+IN_PROC_BROWSER_TEST_F(WebAppNavigateBrowserTest, NewPopup) {
   BrowserList* const browser_list = BrowserList::GetInstance();
   InstallPWA(GetGoogleURL());
 
@@ -131,10 +130,4 @@ IN_PROC_BROWSER_TEST_P(WebAppNavigateBrowserTest, NewPopup) {
   }
 }
 
-INSTANTIATE_TEST_SUITE_P(All,
-                         WebAppNavigateBrowserTest,
-                         ::testing::Values(ProviderType::kBookmarkApps,
-                                           ProviderType::kWebApps),
-                         ProviderTypeParamToString);
-
 }  // namespace web_app