Deflake SafeBrowsing triggered popup test

In general, features should not be changed in the test body. My guess
is that InitFromCommandLine is clobbering all features in a non-safe
way.

This CL stops all feature setting in all test bodies in favor of just
using test subclasses.

Bug: 836215
Change-Id: I4ffb6f9d16f87359ed1d9df9966faa345f4a72a3
Reviewed-on: https://chromium-review.googlesource.com/1025640Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553529}

chrome/browser/ui/blocked_content/safe_browsing_triggered_popup_blocker_browsertest.cc

@@ -131,6 +131,7 @@ class SafeBrowsingTriggeredPopupBlockerBrowserTest
   ~SafeBrowsingTriggeredPopupBlockerBrowserTest() override {}
 
   void SetUp() override {
+    FinalizeFeatures();
     database_helper_ = CreateTestDatabase();
     EXPECT_CALL(provider_, IsInitializationComplete(testing::_))
         .WillRepeatedly(testing::Return(true));
@@ -152,6 +153,8 @@ class SafeBrowsingTriggeredPopupBlockerBrowserTest
     database_helper_.reset();
   }
 
+  virtual void FinalizeFeatures() {}
+
   content::WebContents* web_contents() {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
@@ -200,7 +203,22 @@ class SafeBrowsingTriggeredPopupBlockerBrowserTest
 // The boolean parameter is whether to ignore sublists.
 class SafeBrowsingTriggeredPopupBlockerParamBrowserTest
     : public SafeBrowsingTriggeredPopupBlockerBrowserTest,
-      public testing::WithParamInterface<bool> {};
+      public testing::WithParamInterface<bool> {
+  void FinalizeFeatures() override {
+    std::string ignore_param = GetParam() ? "true" : "false";
+    scoped_feature_list_.InitAndEnableFeatureWithParameters(
+        kAbusiveExperienceEnforce, {{"ignore_sublists", ignore_param}});
+  }
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+class SafeBrowsingTriggeredPopupBlockerDisabledTest
+    : public SafeBrowsingTriggeredPopupBlockerBrowserTest {
+  void FinalizeFeatures() override {
+    scoped_feature_list_.InitAndDisableFeature(kAbusiveExperienceEnforce);
+  }
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
 
 // This test harness does not mock the safe browsing v4 hash protocol manager.
 // Instead, it mocks actual HTTP responses from the v4 server by redirecting
@@ -256,15 +274,8 @@ class SafeBrowsingTriggeredInterceptingBrowserTest
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingTriggeredInterceptingBrowserTest);
 };
 
-IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
+IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerDisabledTest,
                        NoFeature_AllowCreatingNewWindows) {
-  // Disable Abusive enforcement.
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndDisableFeature(kAbusiveExperienceEnforce);
-  // Open a new tab to ensure it has the right variation params.
-  chrome::NewTab(browser());
-  ASSERT_NO_FATAL_FAILURE(content::WaitForLoadStop(web_contents()));
-
   const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
   GURL a_url(embedded_test_server()->GetURL("a.com", kWindowOpenPath));
   ConfigureAsAbusive(a_url);
@@ -281,6 +292,28 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
                    ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
 }
 
+IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerDisabledTest,
+                       NoFeature_NoMessages) {
+  const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
+  GURL a_url(embedded_test_server()->GetURL("a.com", kWindowOpenPath));
+  ConfigureAsAbusiveWarn(a_url);
+
+  // Navigate to a_url, should not log any warning messages.
+  ScopedLoggingObserver log_observer;
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  bool opened_window = false;
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_TRUE(content::ExecuteScriptAndExtractBool(web_contents, "openWindow()",
+                                                   &opened_window));
+  EXPECT_TRUE(opened_window);
+  EXPECT_FALSE(TabSpecificContentSettings::FromWebContents(web_contents)
+                   ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
+
+  log_observer.RoundTripAndVerifyLogMessages(
+      web_contents, {}, {kAbusiveWarnMessage, kAbusiveEnforceMessage});
+}
+
 IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
                        DrivenByEnterprisePolicy) {
   // Disable Abusive experience intervention policy.
@@ -332,43 +365,6 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
                   ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
 }
 
-// Disabled due to https://crbug.com/836215
-#if defined(OS_WIN)
-#define MAYBE_NoFeature_NoMessages DISABLED_NoFeature_NoMessages
-#else
-#define MAYBE_NoFeature_NoMessages NoFeature_NoMessages
-#endif
-IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
-                       MAYBE_NoFeature_NoMessages) {
-  // Disable Abusive enforcement, but keep the notifications coming from the
-  // SubresourceFilter.
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitFromCommandLine("SubresourceFilter",
-                                          "AbusiveExperienceEnforce");
-  // Open a new tab to ensure it has the right variation params.
-  chrome::NewTab(browser());
-  ASSERT_NO_FATAL_FAILURE(content::WaitForLoadStop(web_contents()));
-
-  const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
-  GURL a_url(embedded_test_server()->GetURL("a.com", kWindowOpenPath));
-  ConfigureAsAbusiveWarn(a_url);
-
-  // Navigate to a_url, should not log any warning messages.
-  ScopedLoggingObserver log_observer;
-  ui_test_utils::NavigateToURL(browser(), a_url);
-  bool opened_window = false;
-  content::WebContents* web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  EXPECT_TRUE(content::ExecuteScriptAndExtractBool(web_contents, "openWindow()",
-                                                   &opened_window));
-  EXPECT_TRUE(opened_window);
-  EXPECT_FALSE(TabSpecificContentSettings::FromWebContents(web_contents)
-                   ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
-
-  log_observer.RoundTripAndVerifyLogMessages(
-      web_contents, {}, {kAbusiveWarnMessage, kAbusiveEnforceMessage});
-}
-
 IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
                        NoList_AllowCreatingNewWindows) {
   const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
@@ -611,15 +607,6 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SafeBrowsingTriggeredPopupBlockerParamBrowserTest,
                        IgnoreSublist) {
-  std::string ignore_param = GetParam() ? "true" : "false";
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
-      kAbusiveExperienceEnforce, {{"ignore_sublists", ignore_param}});
-
-  // Use a new tab to make sure the tab helper gets the proper variation params.
-  chrome::NewTab(browser());
-  ASSERT_NO_FATAL_FAILURE(content::WaitForLoadStop(web_contents()));
-
   const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
   GURL url(embedded_test_server()->GetURL("a.com", kWindowOpenPath));