Create a separate component for UrlPatternIndex.

Bug: 713774
Change-Id: I12a70bc0b5caa37470ecf53568d90304c05095eb
Reviewed-on: https://chromium-review.googlesource.com/527445
Commit-Queue: Pavel Kalinnikov <pkalinnikov@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481360}

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

@@ -61,6 +61,7 @@
 #include "components/subresource_filter/core/common/scoped_timers.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "components/subresource_filter/core/common/test_ruleset_utils.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -81,6 +82,8 @@
 
 namespace {
 
+namespace proto = url_pattern_index::proto;
+
 // The path to a multi-frame document used for tests.
 static constexpr const char kTestFrameSetPath[] =
     "/subresource_filter/frame_set.html";

components/BUILD.gn

@@ -151,6 +151,7 @@ test("components_unittests") {
     "//components/upload_list:unit_tests",
     "//components/url_formatter:unit_tests",
     "//components/url_matcher:unit_tests",
+    "//components/url_pattern_index:unit_tests",
     "//components/variations:unit_tests",
     "//components/variations/service:unit_tests",
     "//components/web_resource:unit_tests",

components/subresource_filter/DEPS

@@ -1,5 +1,6 @@
 include_rules = [
   "+components/prefs",
+  "+components/url_pattern_index",
   "+components/variations",
   # subresource_filter is a layered component; subdirectories must explicitly
   # introduce the ability to use non-core layers as appropriate.

components/subresource_filter/content/browser/activation_state_computing_navigation_throttle_unittest.cc

@@ -20,9 +20,9 @@
 #include "components/subresource_filter/content/browser/async_document_subresource_filter_test_utils.h"
 #include "components/subresource_filter/core/common/activation_level.h"
 #include "components/subresource_filter/core/common/activation_state.h"
-#include "components/subresource_filter/core/common/proto/rules.pb.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "components/subresource_filter/core/common/test_ruleset_utils.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/navigation_simulator.h"
@@ -31,6 +31,8 @@
 
 namespace subresource_filter {
 
+namespace proto = url_pattern_index::proto;
+
 class ActivationStateComputingNavigationThrottleTest
     : public content::RenderViewHostTestHarness,
       public content::WebContentsObserver {

components/subresource_filter/content/browser/async_document_subresource_filter.cc

@@ -14,6 +14,7 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "components/subresource_filter/core/common/memory_mapped_ruleset.h"
 #include "components/subresource_filter/core/common/time_measurements.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 
 namespace subresource_filter {
 
@@ -42,12 +43,12 @@ ActivationState ComputeActivationState(
   // TODO(pkalinnikov): Match several activation types in a batch.
   if (matcher.ShouldDisableFilteringForDocument(
           document_url, parent_document_origin,
-          proto::ACTIVATION_TYPE_DOCUMENT)) {
+          url_pattern_index::proto::ACTIVATION_TYPE_DOCUMENT)) {
     activation_state.filtering_disabled_for_document = true;
   } else if (!activation_state.generic_blocking_rules_disabled &&
              matcher.ShouldDisableFilteringForDocument(
                  document_url, parent_document_origin,
-                 proto::ACTIVATION_TYPE_GENERICBLOCK)) {
+                 url_pattern_index::proto::ACTIVATION_TYPE_GENERICBLOCK)) {
     activation_state.generic_blocking_rules_disabled = true;
   }
   return activation_state;
@@ -136,8 +137,9 @@ void AsyncDocumentSubresourceFilter::GetLoadPolicyForSubdocument(
             DCHECK(core);
             DocumentSubresourceFilter* filter = core->filter();
             return filter
-                       ? filter->GetLoadPolicy(subdocument_url,
-                                               proto::ELEMENT_TYPE_SUBDOCUMENT)
+                       ? filter->GetLoadPolicy(
+                             subdocument_url,
+                             url_pattern_index::proto::ELEMENT_TYPE_SUBDOCUMENT)
                        : LoadPolicy::ALLOW;
           },
           core_.get(), subdocument_url),

components/subresource_filter/content/browser/async_document_subresource_filter_unittest.cc

@@ -18,13 +18,15 @@
 #include "components/subresource_filter/content/browser/async_document_subresource_filter_test_utils.h"
 #include "components/subresource_filter/core/common/load_policy.h"
 #include "components/subresource_filter/core/common/memory_mapped_ruleset.h"
-#include "components/subresource_filter/core/common/proto/rules.pb.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "components/subresource_filter/core/common/test_ruleset_utils.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace subresource_filter {
 
+namespace proto = url_pattern_index::proto;
+
 class AsyncDocumentSubresourceFilterTest : public ::testing::Test {
  public:
   AsyncDocumentSubresourceFilterTest() = default;

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc

@@ -21,9 +21,9 @@
 #include "components/subresource_filter/content/common/subresource_filter_messages.h"
 #include "components/subresource_filter/core/common/activation_level.h"
 #include "components/subresource_filter/core/common/activation_state.h"
-#include "components/subresource_filter/core/common/proto/rules.pb.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "components/subresource_filter/core/common/test_ruleset_utils.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/web_contents.h"
@@ -37,6 +37,8 @@
 
 namespace subresource_filter {
 
+namespace proto = url_pattern_index::proto;
+
 const char kTestURLWithActivation[] = "https://www.page-with-activation.com/";
 const char kTestURLWithActivation2[] =
     "https://www.page-with-activation-2.com/";

components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc

@@ -31,6 +31,7 @@
 #include "components/subresource_filter/core/common/activation_state.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "components/subresource_filter/core/common/test_ruleset_utils.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -166,7 +167,7 @@ class SubresourceFilterSafeBrowsingActivationThrottleTest
     Configure();
     test_io_task_runner_ = new base::TestMockTimeTaskRunner();
     // Note: Using NiceMock to allow uninteresting calls and suppress warnings.
-    std::vector<proto::UrlRule> rules;
+    std::vector<url_pattern_index::proto::UrlRule> rules;
     rules.push_back(testing::CreateSuffixRule("disallowed.html"));
     ASSERT_NO_FATAL_FAILURE(test_ruleset_creator_.CreateRulesetWithRules(
         rules, &test_ruleset_pair_));

components/subresource_filter/content/renderer/web_document_subresource_filter_impl.cc

@@ -17,6 +17,8 @@
 
 namespace subresource_filter {
 
+namespace proto = url_pattern_index::proto;
+
 namespace {
 
 using WebLoadPolicy = blink::WebDocumentSubresourceFilter::LoadPolicy;

components/subresource_filter/content/renderer/web_document_subresource_filter_impl.h

@@ -9,7 +9,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/subresource_filter/core/common/document_subresource_filter.h"
-#include "components/subresource_filter/core/common/proto/rules.pb.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 #include "third_party/WebKit/public/platform/WebDocumentSubresourceFilter.h"
 
 namespace subresource_filter {
@@ -44,8 +44,9 @@ class WebDocumentSubresourceFilterImpl
   bool ShouldLogToConsole() override;
 
  private:
-  LoadPolicy getLoadPolicyImpl(const blink::WebURL& url,
-                               proto::ElementType element_type);
+  LoadPolicy getLoadPolicyImpl(
+      const blink::WebURL& url,
+      url_pattern_index::proto::ElementType element_type);
 
   DocumentSubresourceFilter filter_;
   base::OnceClosure first_disallowed_load_callback_;

components/subresource_filter/core/browser/BUILD.gn

@@ -14,7 +14,7 @@ static_library("browser") {
   ]
   deps = [
     "//base",
-    "//components/prefs:prefs",
+    "//components/prefs",
     "//components/subresource_filter/core/common",
     "//components/variations",
     "//third_party/protobuf:protobuf_lite",
@@ -50,7 +50,7 @@ source_set("unit_tests") {
     "//base/test:test_support",
     "//components/prefs:test_support",
     "//components/subresource_filter/core/common:test_support",
-    "//components/subresource_filter/core/common/proto",
+    "//components/url_pattern_index/proto:url_pattern_index",
     "//components/variations",
     "//testing/gmock",
     "//testing/gtest",

components/subresource_filter/core/browser/ruleset_service.cc

@@ -25,11 +25,11 @@
 #include "components/prefs/pref_service.h"
 #include "components/subresource_filter/core/browser/ruleset_service_delegate.h"
 #include "components/subresource_filter/core/browser/subresource_filter_constants.h"
-#include "components/subresource_filter/core/common/copying_file_stream.h"
 #include "components/subresource_filter/core/common/indexed_ruleset.h"
-#include "components/subresource_filter/core/common/proto/rules.pb.h"
 #include "components/subresource_filter/core/common/time_measurements.h"
-#include "components/subresource_filter/core/common/unindexed_ruleset.h"
+#include "components/url_pattern_index/copying_file_stream.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
+#include "components/url_pattern_index/unindexed_ruleset.h"
 #include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl_lite.h"
 
 namespace subresource_filter {
@@ -351,15 +351,16 @@ bool RulesetService::IndexRuleset(base::File unindexed_ruleset_file,
   int64_t unindexed_ruleset_size = unindexed_ruleset_file.GetLength();
   if (unindexed_ruleset_size < 0)
     return false;
-  CopyingFileInputStream copying_stream(std::move(unindexed_ruleset_file));
+  url_pattern_index::CopyingFileInputStream copying_stream(
+      std::move(unindexed_ruleset_file));
   google::protobuf::io::CopyingInputStreamAdaptor zero_copy_stream_adaptor(
       &copying_stream, 4096 /* buffer_size */);
-  UnindexedRulesetReader reader(&zero_copy_stream_adaptor);
+  url_pattern_index::UnindexedRulesetReader reader(&zero_copy_stream_adaptor);
 
   size_t num_unsupported_rules = 0;
-  proto::FilteringRules ruleset_chunk;
+  url_pattern_index::proto::FilteringRules ruleset_chunk;
   while (reader.ReadNextChunk(&ruleset_chunk)) {
-    for (const proto::UrlRule& rule : ruleset_chunk.url_rules()) {
+    for (const auto& rule : ruleset_chunk.url_rules()) {
       if (!indexer->AddUrlRule(rule))
         ++num_unsupported_rules;
     }

components/subresource_filter/core/browser/ruleset_service_unittest.cc

@@ -27,8 +27,8 @@
 #include "build/build_config.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/subresource_filter/core/browser/ruleset_service_delegate.h"
-#include "components/subresource_filter/core/common/proto/rules.pb.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -661,7 +661,7 @@ TEST_F(SubresourceFilteringRulesetServiceTest,
   mock_delegate()->SimulateStartupCompleted();
 
   // The default field values are considered unsupported.
-  proto::UrlRule unfilled_rule;
+  url_pattern_index::proto::UrlRule unfilled_rule;
 
   TestRulesetPair ruleset_with_unsupported_rule;
   ASSERT_NO_FATAL_FAILURE(

components/subresource_filter/core/common/BUILD.gn

@@ -13,43 +13,29 @@ static_library("common") {
     "activation_scope.h",
     "activation_state.cc",
     "activation_state.h",
-    "closed_hash_map.h",
-    "copying_file_stream.cc",
-    "copying_file_stream.h",
     "document_load_statistics.h",
     "document_subresource_filter.cc",
     "document_subresource_filter.h",
     "first_party_origin.cc",
     "first_party_origin.h",
-    "fuzzy_pattern_matching.cc",
-    "fuzzy_pattern_matching.h",
     "indexed_ruleset.cc",
     "indexed_ruleset.h",
     "load_policy.h",
     "memory_mapped_ruleset.cc",
     "memory_mapped_ruleset.h",
-    "ngram_extractor.h",
     "scoped_timers.h",
-    "string_splitter.h",
     "time_measurements.h",
-    "uint64_hasher.h",
-    "unindexed_ruleset.cc",
-    "unindexed_ruleset.h",
-    "url_pattern.cc",
-    "url_pattern.h",
-    "url_pattern_index.cc",
-    "url_pattern_index.h",
   ]
 
   public_deps = [
-    "//components/subresource_filter/core/common/flat:flatbuffer",
-    "//components/subresource_filter/core/common/proto:proto",
+    "//components/subresource_filter/core/common/flat:indexed_ruleset",
+    "//components/url_pattern_index",
   ]
 
   deps = [
     "//base",
     "//net",
-    "//third_party/flatbuffers:flatbuffers",
+    "//third_party/flatbuffers",
     "//third_party/protobuf:protobuf_lite",
     "//url",
   ]
@@ -62,8 +48,6 @@ static_library("test_support") {
     "test_ruleset_creator.h",
     "test_ruleset_utils.cc",
     "test_ruleset_utils.h",
-    "url_rule_test_support.cc",
-    "url_rule_test_support.h",
   ]
   deps = [
     ":common",
@@ -77,23 +61,17 @@ static_library("test_support") {
 source_set("unit_tests") {
   testonly = true
   sources = [
-    "closed_hash_map_unittest.cc",
     "document_subresource_filter_unittest.cc",
     "first_party_origin_unittest.cc",
-    "fuzzy_pattern_matching_unittest.cc",
     "indexed_ruleset_unittest.cc",
-    "ngram_extractor_unittest.cc",
     "scoped_timers_unittest.cc",
-    "string_splitter_unittest.cc",
-    "unindexed_ruleset_unittest.cc",
-    "url_pattern_index_unittest.cc",
-    "url_pattern_unittest.cc",
   ]
   deps = [
     ":common",
     ":test_support",
     "//base",
     "//base/test:test_support",
+    "//components/url_pattern_index:test_support",
     "//testing/gtest",
     "//third_party/protobuf:protobuf_lite",
     "//url",

components/subresource_filter/core/common/PRESUBMIT.py

@@ -11,18 +11,21 @@
 def CheckIndexedRulesetVersion(input_api, output_api):
   """ Checks that IndexedRuleset format version is modified when necessary.
 
-  Whenever a *.fbs or indexed_ruleset.cc file is touched in
-  components/subresource_filter/core/common and kIndexedFormatVersion constant
-  is not changed, this check returns a presubmit warning to make sure the value
-  should not be updated.
+  Whenever any of the following files is changed:
+   - components/subresource_filter/core/common/indexed_ruleset.cc
+   - components/url_pattern_index/flat/*.fbs
+   - components/url_pattern_index/url_pattern_index.cc
+  and kIndexedFormatVersion constant stays intact, this check returns a
+  presubmit warning to make sure the value should not be updated.
   """
 
   indexed_ruleset_changed = False
   indexed_ruleset_version_changed = False
 
   for affected_file in input_api.AffectedFiles():
     path = affected_file.LocalPath()
-    if not 'components/subresource_filter/core/common' in path:
+    if (not 'components/subresource_filter/core/common' in path and
+        not 'components/url_pattern_index/flat' in path):
       continue
     basename = input_api.basename(path)
 
@@ -37,9 +40,9 @@ def CheckIndexedRulesetVersion(input_api, output_api):
 
   if indexed_ruleset_changed and not indexed_ruleset_version_changed:
     return [output_api.PresubmitPromptWarning(
-        'Please make sure that IndexedRuleset modifications in *.fbs and '
-        'indexed_ruleset.cc do not require updating '
-        'RulesetIndexer::kIndexedFormatVersion.')]
+        'Please make sure that UrlPatternIndex/IndexedRuleset modifications in '
+        '*.fbs and url_pattern_index.cc/indexed_ruleset.cc do not require '
+        'updating RulesetIndexer::kIndexedFormatVersion.')]
   return []
 
 def CheckChangeOnUpload(input_api, output_api):

components/subresource_filter/core/common/document_subresource_filter.cc

@@ -33,7 +33,7 @@ DocumentSubresourceFilter::~DocumentSubresourceFilter() = default;
 
 LoadPolicy DocumentSubresourceFilter::GetLoadPolicy(
     const GURL& subresource_url,
-    proto::ElementType subresource_type) {
+    url_pattern_index::proto::ElementType subresource_type) {
   TRACE_EVENT1("loader", "DocumentSubresourceFilter::GetLoadPolicy", "url",
                subresource_url.spec());
 

components/subresource_filter/core/common/document_subresource_filter.h

@@ -17,7 +17,7 @@
 #include "components/subresource_filter/core/common/document_load_statistics.h"
 #include "components/subresource_filter/core/common/indexed_ruleset.h"
 #include "components/subresource_filter/core/common/load_policy.h"
-#include "components/subresource_filter/core/common/proto/rules.pb.h"
+#include "components/url_pattern_index/proto/rules.pb.h"
 
 class GURL;
 
@@ -52,8 +52,9 @@ class DocumentSubresourceFilter {
   // TODO(pkalinnikov): Find a better way to achieve this.
   DocumentLoadStatistics& statistics() { return statistics_; }
 
-  LoadPolicy GetLoadPolicy(const GURL& subresource_url,
-                           proto::ElementType subresource_type);
+  LoadPolicy GetLoadPolicy(
+      const GURL& subresource_url,
+      url_pattern_index::proto::ElementType subresource_type);
 
  private:
   const ActivationState activation_state_;