[Subresource Filter] Componentize TestRulesetPublisher

This class will be reused in //weblayer browsertests. The
componentization requires passing in the RulesetService rather than
obtaining it internally from the //chrome-level BrowserProcess global.
This in turn causes a slight hiccup in //chrome's subresource filter
browsertest harness: the browser process global is not available until
SubresourceFilterBrowserTest::SetUpInMainThread(), but many subclasses
of SubresourceFilterBrowserTest don't invoke
SubresourceFilterBrowserTest::SetUpInMainThread() from their own
SetUpInMainThread() methods, and can't be made to in a straightforward
way due to dependency issues (some of the code in the subclass'
SetUpInMainThread would need to run before SubresourceFilterBrowserTest'
and some after). I left that yak to shave for another day and turned
TestRulesetPublisher into a local variable in the two
SubresourceFilterBrowserTest methods that need it.

Bug: 1116095
Change-Id: I453b277e1c3290b169170383aa6531d34db32cd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527054Reviewed-by: Eric Robinson <ericrobinson@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826819}

chrome/android/BUILD.gn

@@ -1573,7 +1573,7 @@ chrome_common_shared_library("libchromefortest") {
     "//base/test:test_support",
     "//chrome:chrome_android_core",
     "//chrome/browser/password_manager/android_test_helpers:android_wrappers",
-    "//chrome/browser/subresource_filter:test_support",
+    "//chrome/browser/subresource_filter:android_test_support",
     "//components/autofill_assistant/browser:test_support",
     "//components/crash/android:crash_android",
     "//components/minidump_uploader",

chrome/browser/BUILD.gn

@@ -6582,7 +6582,6 @@ static_library("test_support") {
   deps = [
     "//chrome/app/theme:theme_resources",
     "//chrome/browser",
-    "//chrome/browser/subresource_filter:test_support",
     "//chrome/common",
     "//chrome/common/safe_browsing:proto",
     "//components/consent_auditor:test_support",

chrome/browser/subresource_filter/BUILD.gn

@@ -41,27 +41,16 @@ if (is_android) {
       "//third_party/junit",
     ]
   }
-}
-
-source_set("test_support") {
-  testonly = true
-  sources = [
-    "test_ruleset_publisher.cc",
-    "test_ruleset_publisher.h",
-  ]
-  deps = [
-    "//chrome/browser",
-    "//components/subresource_filter/content/browser:browser",
-    "//components/subresource_filter/core/browser:browser",
-    "//components/subresource_filter/core/common:test_support",
-    "//net",
-    "//services/network/public/mojom",
-    "//testing/gtest:gtest",
-    "//third_party/protobuf:protobuf_lite",
-  ]
 
-  if (is_android) {
-    sources += [ "android_test_ruleset_publisher.cc" ]
-    deps += [ ":jni_headers" ]
+  source_set("android_test_support") {
+    testonly = true
+    sources = [ "android_test_ruleset_publisher.cc" ]
+    deps = [
+      ":jni_headers",
+      "//base",
+      "//chrome/browser",
+      "//components/subresource_filter/content/browser",
+      "//components/subresource_filter/core/common:test_support",
+    ]
   }
 }

chrome/browser/subresource_filter/ruleset_browsertest.cc

@@ -109,7 +109,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, InvalidRuleset_Checksum) {
       g_browser_process->subresource_filter_ruleset_service();
 
   // Publish the good ruleset.
-  TestRulesetPublisher publisher;
+  TestRulesetPublisher publisher(service);
   publisher.SetRuleset(test_ruleset_pair.unindexed);
 
   // Now corrupt it by flipping one entry.  This can only be detected

chrome/browser/subresource_filter/subresource_filter_browser_test_harness.cc

@@ -19,7 +19,6 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_database_helper.h"
 #include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
-#include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/chrome_test_utils.h"
 #include "components/blocked_content/safe_browsing_triggered_popup_blocker.h"
@@ -28,6 +27,7 @@
 #include "components/safe_browsing/core/features.h"
 #include "components/subresource_filter/content/browser/ruleset_service.h"
 #include "components/subresource_filter/content/browser/subresource_filter_profile_context.h"
+#include "components/subresource_filter/content/browser/test_ruleset_publisher.h"
 #include "components/subresource_filter/core/common/common_features.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_paths.h"
@@ -206,16 +206,22 @@ void SubresourceFilterBrowserTest::SetRulesetToDisallowURLsWithPathSuffix(
   TestRulesetPair test_ruleset_pair;
   ruleset_creator_.CreateRulesetToDisallowURLsWithPathSuffix(
       suffix, &test_ruleset_pair);
+
+  TestRulesetPublisher test_ruleset_publisher(
+      g_browser_process->subresource_filter_ruleset_service());
   ASSERT_NO_FATAL_FAILURE(
-      test_ruleset_publisher_.SetRuleset(test_ruleset_pair.unindexed));
+      test_ruleset_publisher.SetRuleset(test_ruleset_pair.unindexed));
 }
 
 void SubresourceFilterBrowserTest::SetRulesetWithRules(
     const std::vector<proto::UrlRule>& rules) {
   TestRulesetPair test_ruleset_pair;
   ruleset_creator_.CreateRulesetWithRules(rules, &test_ruleset_pair);
+
+  TestRulesetPublisher test_ruleset_publisher(
+      g_browser_process->subresource_filter_ruleset_service());
   ASSERT_NO_FATAL_FAILURE(
-      test_ruleset_publisher_.SetRuleset(test_ruleset_pair.unindexed));
+      test_ruleset_publisher.SetRuleset(test_ruleset_pair.unindexed));
 }
 
 

chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h

@@ -12,9 +12,9 @@
 #include "base/macros.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
-#include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
 #include "components/safe_browsing/core/db/util.h"
 #include "components/subresource_filter/content/browser/subresource_filter_profile_context.h"
+#include "components/subresource_filter/content/browser/test_ruleset_publisher.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features_test_support.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "components/url_pattern_index/proto/rules.pb.h"
@@ -121,7 +121,6 @@ class SubresourceFilterBrowserTest : public PlatformBrowserTest {
 
   TestRulesetCreator ruleset_creator_;
   ScopedSubresourceFilterConfigurator scoped_configuration_;
-  TestRulesetPublisher test_ruleset_publisher_;
 
   std::unique_ptr<TestSafeBrowsingDatabaseHelper> database_helper_;
 

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

@@ -29,7 +29,6 @@
 #include "chrome/browser/safe_browsing/test_safe_browsing_database_helper.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
-#include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -46,6 +45,7 @@
 #include "components/subresource_filter/content/browser/async_document_subresource_filter_test_utils.h"
 #include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
 #include "components/subresource_filter/content/browser/ruleset_service.h"
+#include "components/subresource_filter/content/browser/test_ruleset_publisher.h"
 #include "components/subresource_filter/core/browser/subresource_filter_constants.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features_test_support.h"

chrome/browser/subresource_filter/subresource_filter_test_harness.cc

@@ -16,7 +16,6 @@
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
 #include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
 #include "chrome/browser/subresource_filter/subresource_filter_test_harness.h"
-#include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/browser/page_specific_content_settings.h"
@@ -25,6 +24,7 @@
 #include "components/subresource_filter/content/browser/subresource_filter_content_settings_manager.h"
 #include "components/subresource_filter/content/browser/subresource_filter_observer_test_utils.h"
 #include "components/subresource_filter/content/browser/subresource_filter_profile_context.h"
+#include "components/subresource_filter/content/browser/test_ruleset_publisher.h"
 #include "components/subresource_filter/core/common/activation_decision.h"
 #include "components/subresource_filter/core/common/activation_list.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
@@ -98,7 +98,8 @@ void SubresourceFilterTestHarness::SetUp() {
        subresource_filter::testing::CreateAllowlistSuffixRule(
            kDefaultAllowedSuffix)},
       &test_ruleset_pair);
-  subresource_filter::testing::TestRulesetPublisher test_ruleset_publisher;
+  subresource_filter::testing::TestRulesetPublisher test_ruleset_publisher(
+      g_browser_process->subresource_filter_ruleset_service());
   ASSERT_NO_FATAL_FAILURE(
       test_ruleset_publisher.SetRuleset(test_ruleset_pair.unindexed));
 

components/subresource_filter/content/browser/BUILD.gn

@@ -82,11 +82,14 @@ static_library("test_support") {
     "subframe_navigation_test_utils.h",
     "subresource_filter_observer_test_utils.cc",
     "subresource_filter_observer_test_utils.h",
+    "test_ruleset_publisher.cc",
+    "test_ruleset_publisher.h",
   ]
   deps = [
     ":browser",
     "//base/test:test_support",
     "//components/subresource_filter/core/common",
+    "//components/subresource_filter/core/common:test_support",
     "//content/public/browser",
     "//content/test:test_support",
     "//net",

chrome/browser/subresource_filter/test_ruleset_publisher.cc → components/subresource_filter/content/browser/test_ruleset_publisher.cc

@@ -2,12 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
+#include "components/subresource_filter/content/browser/test_ruleset_publisher.h"
 
 #include "base/hash/hash.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
-#include "chrome/browser/browser_process.h"
 #include "components/subresource_filter/content/browser/ruleset_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -18,8 +17,8 @@ namespace {
 
 class RulesetDistributionListener {
  public:
-  RulesetDistributionListener()
-      : service_(g_browser_process->subresource_filter_ruleset_service()) {
+  explicit RulesetDistributionListener(RulesetService* service)
+      : service_(service) {
     service_->SetRulesetPublishedCallbackForTesting(run_loop_.QuitClosure());
   }
 
@@ -38,7 +37,9 @@ class RulesetDistributionListener {
 
 }  // namespace
 
-TestRulesetPublisher::TestRulesetPublisher() = default;
+TestRulesetPublisher::TestRulesetPublisher(RulesetService* ruleset_service)
+    : ruleset_service_(ruleset_service) {}
+
 TestRulesetPublisher::~TestRulesetPublisher() = default;
 
 void TestRulesetPublisher::SetRuleset(const TestRuleset& unindexed_ruleset) {
@@ -48,9 +49,9 @@ void TestRulesetPublisher::SetRuleset(const TestRuleset& unindexed_ruleset) {
   subresource_filter::UnindexedRulesetInfo unindexed_ruleset_info;
   unindexed_ruleset_info.content_version = test_ruleset_content_version;
   unindexed_ruleset_info.ruleset_path = unindexed_ruleset.path;
-  RulesetDistributionListener listener;
-  g_browser_process->subresource_filter_ruleset_service()
-      ->IndexAndStoreAndPublishRulesetIfNeeded(unindexed_ruleset_info);
+  RulesetDistributionListener listener(ruleset_service_);
+  ruleset_service_->IndexAndStoreAndPublishRulesetIfNeeded(
+      unindexed_ruleset_info);
   listener.AwaitDistribution();
 }
 

chrome/browser/subresource_filter/test_ruleset_publisher.h → components/subresource_filter/content/browser/test_ruleset_publisher.h

@@ -2,20 +2,23 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_SUBRESOURCE_FILTER_TEST_RULESET_PUBLISHER_H_
-#define CHROME_BROWSER_SUBRESOURCE_FILTER_TEST_RULESET_PUBLISHER_H_
+#ifndef COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_TEST_RULESET_PUBLISHER_H_
+#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_TEST_RULESET_PUBLISHER_H_
 
 #include "base/macros.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 
 namespace subresource_filter {
+
+class RulesetService;
+
 namespace testing {
 
 // Helper class to create testing rulesets during browser tests, as well as to
 // get them indexed and published to renderers by the RulesetService.
 class TestRulesetPublisher {
  public:
-  TestRulesetPublisher();
+  explicit TestRulesetPublisher(RulesetService* ruleset_service);
   ~TestRulesetPublisher();
 
   // Indexes the |unindexed_ruleset| and publishes it to all renderers
@@ -23,6 +26,7 @@ class TestRulesetPublisher {
   void SetRuleset(const TestRuleset& unindexed_ruleset);
 
  private:
+  RulesetService* ruleset_service_;
 
   DISALLOW_COPY_AND_ASSIGN(TestRulesetPublisher);
 };
@@ -30,4 +34,4 @@ class TestRulesetPublisher {
 }  // namespace testing
 }  // namespace subresource_filter
 
-#endif  // CHROME_BROWSER_SUBRESOURCE_FILTER_TEST_RULESET_PUBLISHER_H_
+#endif  // COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_TEST_RULESET_PUBLISHER_H_