Split the ScopedSubresourceFilterFeatureToggle into two helper classes.

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

 // Copyright 2016 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 <map>
 #include <memory>
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 152 matching lines @@
 GURL GetURLWithFragment(const GURL& url, base::StringPiece fragment) {
   GURL::Replacements replacements;
   replacements.SetRefStr(fragment);
   return url.ReplaceComponents(replacements);
 }
 
 }  // namespace
 
 namespace subresource_filter {
 
-using subresource_filter::testing::ScopedSubresourceFilterFeatureToggle;
+using subresource_filter::testing::ScopedSubresourceFilterConfigurator;
 using subresource_filter::testing::TestRulesetPublisher;
 using subresource_filter::testing::TestRulesetCreator;
 using subresource_filter::testing::TestRulesetPair;
 using ActivationDecision =
     ContentSubresourceFilterDriverFactory::ActivationDecision;
 
 // SubresourceFilterDisabledBrowserTest ---------------------------------------
 
 class SubresourceFilterDisabledByDefaultBrowserTest
     : public InProcessBrowserTest {
@@ skipping 13 matching lines @@
 // The RulesetService should not even be instantiated when the feature is
 // disabled, which should be the default state unless there is an override
 // specified in the field trial configuration.
 IN_PROC_BROWSER_TEST_F(SubresourceFilterDisabledByDefaultBrowserTest,
                        RulesetServiceNotCreated) {
   EXPECT_FALSE(g_browser_process->subresource_filter_ruleset_service());
 }
 
 // SubresourceFilterBrowserTest -----------------------------------------------
 
-class SubresourceFilterBrowserTestImpl : public InProcessBrowserTest {
+class SubresourceFilterBrowserTest : public InProcessBrowserTest {
  public:
-  explicit SubresourceFilterBrowserTestImpl(bool measure_performance,
-                                            bool whitelist_site_on_reload)
-      : measure_performance_(measure_performance),
-        whitelist_site_on_reload_(whitelist_site_on_reload) {}
-
-  ~SubresourceFilterBrowserTestImpl() override {}
+  SubresourceFilterBrowserTest() {}
+  ~SubresourceFilterBrowserTest() override {}
 
  protected:
-  // It would be too late to enable the feature in SetUpOnMainThread(), as it is
-  // called after ChromeBrowserMainParts::PreBrowserStart(), which instantiates
-  // the RulesetService.
-  //
-  // On the other hand, setting up field trials in this method would be too
-  // early, as it is called before BrowserMain, which expects no FieldTrialList
-  // singleton to exist. There are no other hooks we could use either.
-  //
-  // As a workaround, enable the feature here, then enable the feature once
-  // again + set up the field trials later.
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitchASCII(
         switches::kEnableFeatures,
         base::JoinString(
             {kSafeBrowsingSubresourceFilter.name, "SafeBrowsingV4OnlyEnabled",
              kSafeBrowsingSubresourceFilterExperimentalUI.name},
             ","));
   }
 
   void SetUp() override {
@@ skipping 21 matching lines @@
     InProcessBrowserTest::TearDown();
     // Unregister test factories after InProcessBrowserTest::TearDown
     // (which destructs SafeBrowsingService).
     safe_browsing::V4GetHashProtocolManager::RegisterFactory(nullptr);
     safe_browsing::V4Database::RegisterDatabaseFactoryForTest(nullptr);
     safe_browsing::V4Database::RegisterStoreFactoryForTest(nullptr);
     safe_browsing::SafeBrowsingService::RegisterFactory(nullptr);
   }
 
   void SetUpOnMainThread() override {
-    SetUpActivationFeature();
     base::FilePath test_data_dir;
     PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
     embedded_test_server()->ServeFilesFromDirectory(test_data_dir);
     host_resolver()->AddSimulatedFailure("host-with-dns-lookup-failure");
     host_resolver()->AddRule("*", "127.0.0.1");
     content::SetupCrossSiteRedirector(embedded_test_server());
     ASSERT_TRUE(embedded_test_server()->Start());
-  }
-
-  virtual void SetUpActivationFeature() {
-    ToggleFeatures(base::MakeUnique<ScopedSubresourceFilterFeatureToggle>(
-        base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
-        kActivationScopeActivationList, kActivationListPhishingInterstitial,
-        measure_performance_ ? "1" : "0", "" /* suppress_notifications */,
-        whitelist_site_on_reload_ ? "true" : "false"));
+    ResetConfigurationToEnableOnPhishingSites();
   }
 
   GURL GetTestUrl(const std::string& relative_url) {
     return embedded_test_server()->base_url().Resolve(relative_url);
   }
 
   void MarkUrlAsMatchingListWithId(
       const GURL& bad_url,
       const safe_browsing::ListIdentifier& list_id,
       safe_browsing::ThreatPatternType threat_pattern_type) {
@@ skipping 77 matching lines @@
         test_ruleset_publisher_.SetRuleset(test_ruleset_pair.unindexed));
   }
 
   void SetRulesetWithRules(const std::vector<proto::UrlRule>& rules) {
     TestRulesetPair test_ruleset_pair;
     ruleset_creator_.CreateRulesetWithRules(rules, &test_ruleset_pair);
     ASSERT_NO_FATAL_FAILURE(
         test_ruleset_publisher_.SetRuleset(test_ruleset_pair.unindexed));
   }
 
-  void ToggleFeatures(
-      std::unique_ptr<ScopedSubresourceFilterFeatureToggle> features) {
-    scoped_feature_toggle_ = std::move(features);
+  void ResetConfiguration(Configuration config) {
+    scoped_configuration_.ResetConfiguration(std::move(config));
+  }
+
+  void ResetConfigurationToEnableOnPhishingSites(
+      bool measure_performance = false,
+      bool whitelist_site_on_reload = false) {
+    Configuration config(
+        subresource_filter::ActivationLevel::ENABLED,
+        subresource_filter::ActivationScope::ACTIVATION_LIST,
+        subresource_filter::ActivationList::PHISHING_INTERSTITIAL);
+    config.performance_measurement_rate = measure_performance ? 1.0 : 0.0;
+    config.should_whitelist_site_on_reload = whitelist_site_on_reload;
+    ResetConfiguration(std::move(config));
   }
 
  private:
   TestRulesetCreator ruleset_creator_;
-
-  std::unique_ptr<ScopedSubresourceFilterFeatureToggle> scoped_feature_toggle_;
+  ScopedSubresourceFilterConfigurator scoped_configuration_;
   TestRulesetPublisher test_ruleset_publisher_;
-  const bool measure_performance_;
-  const bool whitelist_site_on_reload_;
 
   std::unique_ptr<safe_browsing::TestSafeBrowsingServiceFactory> sb_factory_;
   // Owned by the V4Database.
   safe_browsing::TestV4DatabaseFactory* v4_db_factory_;
   // Owned by the V4GetHashProtocolManager.
   safe_browsing::TestV4GetHashProtocolManagerFactory* v4_get_hash_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(SubresourceFilterBrowserTestImpl);
-};
-
-class SubresourceFilterBrowserTest : public SubresourceFilterBrowserTestImpl {
- public:
-  SubresourceFilterBrowserTest()
-      : SubresourceFilterBrowserTestImpl(false, false) {}
-};
-
-// TODO(pkalinnikov): It should be possible to have only one fixture, i.e.,
-// SubresourceFilterBrowserTest, unsetting |measure_performance| by default, and
-// providing a method to switch it on. However, the current implementation of
-// ScopedSubresourceFilterFeatureToggle in use pollutes the global
-// FieldTrialList in such a way that variation parameters can not be reassigned.
-class SubresourceFilterWithPerformanceMeasurementBrowserTest
-    : public SubresourceFilterBrowserTestImpl {
- public:
-  SubresourceFilterWithPerformanceMeasurementBrowserTest()
-      : SubresourceFilterBrowserTestImpl(true, false) {}
-};
-
-class SubresourceFilterWhitelistSiteOnReloadBrowserTest
-    : public SubresourceFilterBrowserTestImpl {
- public:
-  SubresourceFilterWhitelistSiteOnReloadBrowserTest()
-      : SubresourceFilterBrowserTestImpl(false, true) {}
+  DISALLOW_COPY_AND_ASSIGN(SubresourceFilterBrowserTest);
 };
 
 enum WebSocketCreationPolicy {
   IN_MAIN_FRAME,
   IN_WORKER,
 };
 class SubresourceFilterWebSocketBrowserTest
-    : public SubresourceFilterBrowserTestImpl,
+    : public SubresourceFilterBrowserTest,
       public ::testing::WithParamInterface<WebSocketCreationPolicy> {
  public:
-  SubresourceFilterWebSocketBrowserTest()
-      : SubresourceFilterBrowserTestImpl(false, false) {}
+  SubresourceFilterWebSocketBrowserTest() {}
 
   void SetUpOnMainThread() override {
-    SubresourceFilterBrowserTestImpl::SetUpOnMainThread();
+    SubresourceFilterBrowserTest::SetUpOnMainThread();
     websocket_test_server_ = base::MakeUnique<net::SpawnedTestServer>(
         net::SpawnedTestServer::TYPE_WS, net::SpawnedTestServer::kLocalhost,
         net::GetWebSocketTestDataDirectory());
     ASSERT_TRUE(websocket_test_server_->Start());
   }
 
   net::SpawnedTestServer* websocket_test_server() {
     return websocket_test_server_.get();
   }
 
@@ skipping 10 matching lines @@
         browser()->tab_strip_model()->GetActiveWebContents(),
         base::StringPrintf("connectWebSocket('%s');", url.spec().c_str()),
         &websocket_connection_succeeded));
     EXPECT_EQ(expect_connection_success, websocket_connection_succeeded);
   }
 
  private:
   std::unique_ptr<net::SpawnedTestServer> websocket_test_server_;
 };
 
-#if defined(GOOGLE_CHROME_BUILD)
-class SubresourceFilterListBrowserTest
-    : public SubresourceFilterBrowserTestImpl {
- public:
-  SubresourceFilterListBrowserTest()
-      : SubresourceFilterBrowserTestImpl(false, false) {}
-
-  void SetUpActivationFeature() override {
-    ToggleFeatures(base::MakeUnique<ScopedSubresourceFilterFeatureToggle>(
-        base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
-        kActivationScopeActivationList, kActivationListSubresourceFilter, "0",
-        "" /* suppress_notifications */, "false"));
-  }
-};
-#endif
 
 // Tests -----------------------------------------------------------------------
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, MainFrameActivation) {
   GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
   ConfigureAsPhishingURL(url);
   ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
       "suffix-that-does-not-match-anything"));
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   // The main frame document should never be filtered.
   SetRulesetToDisallowURLsWithPathSuffix("frame_with_included_script.html");
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 }
 
 #if defined(GOOGLE_CHROME_BUILD)
-IN_PROC_BROWSER_TEST_F(SubresourceFilterListBrowserTest, MainFrameActivation) {
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       MainFrameActivation_SubresourceFilterList) {
   GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
   ConfigureAsSubresourceFilterOnlyURL(url);
   ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
       "suffix-that-does-not-match-anything"));
+
+  Configuration config(subresource_filter::ActivationLevel::ENABLED,
+                       subresource_filter::ActivationScope::ACTIVATION_LIST,
+                       subresource_filter::ActivationList::SUBRESOURCE_FILTER);
+  ResetConfiguration(std::move(config));
+
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   // The main frame document should never be filtered.
   SetRulesetToDisallowURLsWithPathSuffix("frame_with_included_script.html");
@@ skipping 574 matching lines @@
 
   // Force a navigation to another page, which flushes page load metrics for the
   // previous page load.
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
 
   // Make sure that pages that have subresource filtered in child frames are
   // also counted.
   tester.ExpectTotalCount(internal::kHistogramSubresourceFilterCount, 1);
 }
 
-IN_PROC_BROWSER_TEST_F(SubresourceFilterWithPerformanceMeasurementBrowserTest,
-                       ExpectHistogramsAreRecorded) {
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       ExpectPerformanceHistogramsAreRecorded) {
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
+  ResetConfigurationToEnableOnPhishingSites(
+      true /* measure_performance */, false /* whitelist_site_on_reload */);
   const GURL url = GetTestUrl(kTestFrameSetPath);
   ConfigureAsPhishingURL(url);
 
   base::HistogramTester tester;
   ui_test_utils::NavigateToURL(browser(), url);
 
   ExpectHistogramsAreRecordedForTestFrameSet(
       tester, true /* expect_performance_measurements */);
 }
 
@@ skipping 63 matching lines @@
       internal::kHistogramSubresourceFilterActivationDecision,
       static_cast<int>(ActivationDecision::ACTIVATED), 2);
 
   tester.ExpectTotalCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload, 1);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload,
       static_cast<int>(ActivationDecision::ACTIVATED), 1);
 }
 
-IN_PROC_BROWSER_TEST_F(SubresourceFilterWhitelistSiteOnReloadBrowserTest,
-                       ActivationDisabledOnReload) {
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       WhiteliseSiteOnReload_ActivationDisabledOnReload) {
   GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
   ConfigureAsPhishingURL(url);
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
-
+  ResetConfigurationToEnableOnPhishingSites(
+      false /* measure_performance */, true /* whitelist_site_on_reload */);
   base::HistogramTester tester;
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   content::TestNavigationObserver observer(
       browser()->tab_strip_model()->GetActiveWebContents(),
       content::MessageLoopRunner::QuitMode::DEFERRED);
   chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB);
   observer.Wait();
   EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   tester.ExpectTotalCount(
       internal::kHistogramSubresourceFilterActivationDecision, 2);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecision,
       static_cast<int>(ActivationDecision::ACTIVATED), 1);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecision,
       static_cast<int>(ActivationDecision::URL_WHITELISTED), 1);
 
   tester.ExpectTotalCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload, 1);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload,
       static_cast<int>(ActivationDecision::URL_WHITELISTED), 1);
 }
 
-IN_PROC_BROWSER_TEST_F(SubresourceFilterWhitelistSiteOnReloadBrowserTest,
-                       ActivationDisabledOnReloadFromScript) {
+IN_PROC_BROWSER_TEST_F(
+    SubresourceFilterBrowserTest,
+    WhiteliseSiteOnReload_ActivationDisabledOnReloadFromScript) {
   GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
   ConfigureAsPhishingURL(url);
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
-
+  ResetConfigurationToEnableOnPhishingSites(
+      false /* measure_performance */, true /* whitelist_site_on_reload */);
   base::HistogramTester tester;
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   content::TestNavigationObserver observer(
       browser()->tab_strip_model()->GetActiveWebContents(),
       content::MessageLoopRunner::QuitMode::DEFERRED);
   ASSERT_TRUE(content::ExecuteScript(
       browser()->tab_strip_model()->GetActiveWebContents(),
       "location.reload();"));
   observer.Wait();
   EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   tester.ExpectTotalCount(
       internal::kHistogramSubresourceFilterActivationDecision, 2);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecision,
       static_cast<int>(ActivationDecision::ACTIVATED), 1);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecision,
       static_cast<int>(ActivationDecision::URL_WHITELISTED), 1);
 
   tester.ExpectTotalCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload, 1);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload,
       static_cast<int>(ActivationDecision::URL_WHITELISTED), 1);
 }
 
-IN_PROC_BROWSER_TEST_F(SubresourceFilterWhitelistSiteOnReloadBrowserTest,
-                       ActivationDisabledOnNavigationToSameURL) {
+IN_PROC_BROWSER_TEST_F(
+    SubresourceFilterBrowserTest,
+    WhiteliseSiteOnReload_ActivationDisabledOnNavigationToSameURL) {
   GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
   ConfigureAsPhishingURL(url);
   ASSERT_NO_FATAL_FAILURE(
       SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
-
+  ResetConfigurationToEnableOnPhishingSites(
+      false /* measure_performance */, true /* whitelist_site_on_reload */);
   base::HistogramTester tester;
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
 
   content::TestNavigationObserver observer(
       browser()->tab_strip_model()->GetActiveWebContents(),
       content::MessageLoopRunner::QuitMode::DEFERRED);
   std::string nav_frame_script = "location.href = '" + url.spec() + "';";
   ASSERT_TRUE(content::ExecuteScript(
       browser()->tab_strip_model()->GetActiveWebContents(), nav_frame_script));
@@ skipping 10 matching lines @@
       static_cast<int>(ActivationDecision::URL_WHITELISTED), 1);
 
   tester.ExpectTotalCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload, 1);
   tester.ExpectBucketCount(
       internal::kHistogramSubresourceFilterActivationDecisionReload,
       static_cast<int>(ActivationDecision::URL_WHITELISTED), 1);
 }
 
 #if defined(GOOGLE_CHROME_BUILD)
-// This test is only enabled when GOOGLE_CHROME_BUILD is true because the store
-// that this test uses is only populated on GOOGLE_CHROME_BUILD builds.
+// These tests are only enabled when GOOGLE_CHROME_BUILD is true because the
+// store that this test uses is only populated on GOOGLE_CHROME_BUILD builds.
 IN_PROC_BROWSER_TEST_F(
     SubresourceFilterBrowserTest,
     ExpectRedirectPatternHistogramsAreRecordedForSubresourceFilterOnlyMatch) {
   ASSERT_NO_FATAL_FAILURE(SetRulesetToDisallowURLsWithPathSuffix(
       "suffix-that-does-not-match-anything"));
 
   GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
   ConfigureAsSubresourceFilterOnlyURL(url);
 
   base::HistogramTester tester;
@@ skipping 27 matching lines @@
                                    std::string(kSubresourceFilterOnlySuffix)),
               ::testing::IsEmpty());
 
   EXPECT_THAT(tester.GetAllSamples(std::string(kNavigationChainSize) +
                                    std::string(kSubresourceFilterOnlySuffix)),
               ::testing::IsEmpty());
 }
 #endif
 
 }  // namespace subresource_filter