New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 900292 link

Starred by 1 user
Status: Untriaged
Owner:
█ ericrobinson@google.com
Last visit > 30 days ago
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Merge ContentRulesetService and RulesetService

Project Member Reported by ericrobinson@google.com, Oct 30 
See: go/chromerulesetservicemerge

These classes handle the publishing of the ruleset to the renderer processes and the saving/storing/receiving of the ruleset data respectively.  Both are currently visible to our codebase for historical reasons, but only the RulesetService should be (publishing should happen behind the scenes).

This should be handled in 3 distinct steps:
1. Merge the two files under content.
2. Change external references to refer only to RulesetService.
3. (Optional) Remove the delegate virtual class, having RulesetService inherit its virtual members and have ContentRulesetService inherit from RulesetService.

Note that the third step is optional because a stronger separation of concerns might be desirable for testing and implementation purposes.

Comment 1 by ericrobinson@google.com, Oct 30

Owner: ericrobinson@google.com
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 2 

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

commit efb3ea55c70bb35d0b52c782105b3c8c40213e3a
Author: Eric Robinson <ericrobinson@chromium.org>
Date: Fri Nov 02 18:24:32 2018

Remove non-testing references to ContentRulesetService

See: go/chromerulesetservicemerge

This change removes references to ContentRulesetService in our codebase
preferring RulesetService.  It changes the direction of ownership of
these classes, having the RulesetService own the ContentRulesetService.
The publishing/distribution functionality is now called via the
RulesetService itself, which makes calls to the virtual
RulesetServiceDelegate implemented by the ContentRulesetService.

Bug: 900292
Change-Id: I30420f0b36107e65d59d1911b506242c58886ecd
Reviewed-on: https://chromium-review.googlesource.com/c/1294299
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: Eric Robinson <ericrobinson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604988}
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/android/subresource_filter/test_subresource_filter_publisher.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/browser_process.h
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/browser_process_impl.h
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/component_updater/subresource_filter_component_installer.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/component_updater/subresource_filter_component_installer_unittest.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/subresource_filter/ruleset_browsertest.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/subresource_filter/subresource_filter_browser_test_harness.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/subresource_filter/subresource_filter_test_harness.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/browser/subresource_filter/test_ruleset_publisher.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/components/subresource_filter/content/browser/ruleset_service.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/components/subresource_filter/content/browser/ruleset_service.h
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/components/subresource_filter/content/browser/ruleset_service_unittest.cc
[modify] https://crrev.com/efb3ea55c70bb35d0b52c782105b3c8c40213e3a/components/subresource_filter/core/browser/ruleset_service_delegate.h
►

Sign in to add a comment