-
Notifications
You must be signed in to change notification settings - Fork 864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement unified adblock catalog #19946
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
d75a32a
rename AdBlockRegionalServiceManager -> AdBlockComponentServiceManager
antonok-edm e3bd426
support new fields in FilterListCatalogEntry
antonok-edm dca87e9
support `hidden` field
antonok-edm 5656145
support `first_party_protections` field
antonok-edm 7104202
simplify state saved for griffin-enabled lists
antonok-edm 4027404
simplify logic for selecting enabled regional lists
antonok-edm 7c4424e
support `default_enabled` field
antonok-edm d74e31a
remove uppercasing from FindAdBlockFilterListByUUID
antonok-edm cf197f2
let the component service manager manage the default list component
antonok-edm 35bd3ee
remove LoadDAT method in favor of calling LoadDATBuffer directly
antonok-edm b2d9c36
remove DAT deserialization from AdBlockFiltersProvider interface
antonok-edm 4043bb3
fix debug name
antonok-edm e375d63
expose FilterSet in adblock FFI
antonok-edm 91fd4a4
reduce code duplication in SourceProviderObserver
antonok-edm 9656930
reload adblock engines via FilterSet
antonok-edm 1a6ad1f
remove hacky empty dat_buf_ workaround
antonok-edm bd1a6f7
add methods for combinating FilterSets in all FiltersProviders
antonok-edm 2ec4c7c
remove the now unused LoadDAT family of methods
antonok-edm 45a8b34
support `permission_mask` field
antonok-edm ca37c45
seemingly unrelated formatting fixes
antonok-edm e274e6a
fix DebounceBrowserTest.DebounceBeforeDomainBlock
antonok-edm cfe8a82
use std::unique_ptr instead of absl::optional
antonok-edm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
336 changes: 202 additions & 134 deletions
336
browser/brave_shields/ad_block_service_browsertest.cc
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
component
is already so overloaded, is there a better name we can use here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😅 It is at least managing all the filter lists fetched via the component updater now. Previously it managed a weird subset of them, and the original "regional" naming was never 100% accurate for as long as I can remember.
Would
CRXServiceManager
be better?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see DM, I think maybe it's better to not expose this in the public api at all. I don't think callers of these methods need to care that there is a ComponentServiceManager that handles certain providers and a subscription manager that handles others, they just want to check
IsFilterListAvailable
or whatever the case may be