New issue
Advanced search Search tips

Issue 843830 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Move subresource filter logic out of url_pattern_index.

Project Member Reported by karandeepb@chromium.org, May 16 2018

Issue description

url_pattern_index shouldn't know about things specific to subresource filter like proto::UrlRule.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6

commit 51fe5e65414d20e2a1a15bda7b765e7fe87a6af6
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Fri May 18 19:43:54 2018

URLPatternIndex: Move UnindexedRuleset from url_pattern_index to subresource_filter.

url_pattern_index doesn't need to know about UnindexedRuleset. Move it to the
subresource_filter component.

BUG=843830

Change-Id: I17e4d07d7e9347d47895ed71bc276e96b14b2deb
Reviewed-on: https://chromium-review.googlesource.com/1056266
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559994}
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/chrome/browser/android/subresource_filter/test_subresource_filter_publisher.cc
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/core/browser/ruleset_service.cc
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/core/common/BUILD.gn
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/core/common/test_ruleset_creator.cc
[rename] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/core/common/unindexed_ruleset.cc
[rename] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/core/common/unindexed_ruleset.h
[rename] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/core/common/unindexed_ruleset_unittest.cc
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/tools/indexing_tool.cc
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/tools/ruleset_converter/BUILD.gn
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/tools/ruleset_converter/rule_stream.cc
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/subresource_filter/tools/ruleset_converter/ruleset_format.h
[modify] https://crrev.com/51fe5e65414d20e2a1a15bda7b765e7fe87a6af6/components/url_pattern_index/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4c240479415924192c908dd6ea969c65c996696

commit f4c240479415924192c908dd6ea969c65c996696
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Fri May 18 20:53:57 2018

URLPatternIndex: Move copying_file_stream* from url_pattern_index to subresource_filter.

url_pattern_index doesn't need to know about copying_file_stream*. Move it to the
subresource_filter component.

BUG=843830

Change-Id: I1151b57dfe630acd857e1f83d95814d40599ee54
Reviewed-on: https://chromium-review.googlesource.com/1056788
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560022}
[modify] https://crrev.com/f4c240479415924192c908dd6ea969c65c996696/components/subresource_filter/core/browser/BUILD.gn
[rename] https://crrev.com/f4c240479415924192c908dd6ea969c65c996696/components/subresource_filter/core/browser/copying_file_stream.cc
[rename] https://crrev.com/f4c240479415924192c908dd6ea969c65c996696/components/subresource_filter/core/browser/copying_file_stream.h
[modify] https://crrev.com/f4c240479415924192c908dd6ea969c65c996696/components/subresource_filter/core/browser/ruleset_service.cc
[modify] https://crrev.com/f4c240479415924192c908dd6ea969c65c996696/components/subresource_filter/tools/BUILD.gn
[modify] https://crrev.com/f4c240479415924192c908dd6ea969c65c996696/components/subresource_filter/tools/indexing_tool.cc
[modify] https://crrev.com/f4c240479415924192c908dd6ea969c65c996696/components/url_pattern_index/BUILD.gn

Hey Karan, it looks like some of the moved files are still in namespace url_pattern_index. Sorry I missed this in review.
Oh missed that, will issue a patch. Thanks for catching.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/542072f8c026c5497f3fdaba4f7de912e43258e4

commit 542072f8c026c5497f3fdaba4f7de912e43258e4
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Fri Jun 01 18:51:01 2018

Correct namespace for CopyingFileStream and UnindexedRuleset.

r559994 and r560022 moved UnindexedRuleset and CopyingFileStream respectively
from the url_pattern_index to the subresource_filter component. This CL moves
the affected classes to the subresource_filter namespace (from the
url_pattern_index namespace).

BUG=843830

Change-Id: Ibff1e062e24643ae6264ae52805967a5a2cb73a3
Reviewed-on: https://chromium-review.googlesource.com/1081514
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563739}
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/chrome/browser/android/subresource_filter/test_subresource_filter_publisher.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/browser/copying_file_stream.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/browser/copying_file_stream.h
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/browser/ruleset_service.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/common/indexed_ruleset_fuzzer.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/common/test_ruleset_creator.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/common/unindexed_ruleset.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/common/unindexed_ruleset.h
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/core/common/unindexed_ruleset_unittest.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/tools/indexing_tool.cc
[modify] https://crrev.com/542072f8c026c5497f3fdaba4f7de912e43258e4/components/subresource_filter/tools/ruleset_converter/rule_stream.cc

Sign in to add a comment