Multiple blocking rule lists #53

SlayterDev · 2022-01-20T22:21:09Z

Task/Issue URL: https://app.asana.com/0/0/1201694797383792/f
Tech Design URL:
CC: @bwaresiak @brindy @ladamski

Description:
This PR adds support for multiple content blocking rules

Steps to test this PR:

See testing in FB click to load PR

OS Testing:

Internal references:

Software Engineering Expectations
Technical Design Template

…e-blocking-rule-lists

mallexxx

Looks good to me (see minor data race comment inline)
CompilationTask concept still looks over-complicated, but this to be targeted by my next ContentRules PR

mallexxx · 2022-01-24T09:19:49Z

Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift

+        sourceManager.compilationFailed(for: model, with: error)
+        let newModel = sourceManager.makeModel()


I think this should go inside self.workQueue.async { .. } as sourceManager.makeModel() usage is made from the workQueue and the compilationFailed callback may be called by the compileContentRuleList from the main thread

It's not like there's a bug - as operations here are done sequentially and are encapsulated to a single class instance - but this is something I know as an author of this class :) and it isn't necessary a good thing. I agree that having this in a async block is more clear to the reader and possibly less error prone in case of any refactoring - going to change this.

bwaresiak · 2022-01-24T15:20:12Z

@mallexxx I'm happy to chat how to simplify this in the future :)

When looking at clock to load PR I've noticed that there is a possible problem if we are not careful with providing a embedded (fallback) list and it does not compile, so based on https://github.com/more-duckduckgo-org/macos-browser/pull/329/files#r790736026 I've updated the logic so that:

BSK now contains the test to ensure embedded TDS compiles.
We kill the app only in case main TDS does not compile, ignoring other rule lists.
Non-tds rule lists are allowed to fail compilation - test case for that has been added to test suite.

ladamski · 2022-01-25T18:01:55Z

Are we good to merge? Looks like it but want to check in before I pull the trigger :)

bwaresiak · 2022-01-25T19:49:29Z

@ladamski I believe so :)

* Initial implementation to support multiple rules lists * Make API open for inheritance * Fix few bugs, update tests * CTL delegate WIP * Update ContentBlockerRulesUserScript.swift * Update ContentBlockerRulesUserScript.swift * Refactor Content Blocker User Script * Add missing change information * Expand error reporting API * Unit tests for error reporting * Workaround for missing TDS at startup issue * Move model processing to work queue * Kill the app only in case main TDS rule list fails to compile * Allow for non-TDS rule lists to fail compilation in case setup fails * Add check for embedded TDS compilation Co-authored-by: Bartek Waresiak <bartek@duckduckgo.com> Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com>

* Secure Vault autofill Pixel support (#26) * add delegate calls after autofilling * upd with AutofillType * Secure Vault additions (#28) * Finish note support. * Note database migration * Begin working on identities * Further work on Identities * Consolidate database migrations. * Fill out remaining credit card functions. * Remove unneeded credit card properties. * Further autofill API work. * Support cardholder name. * Fix unit test compilation failures. * Remove an incorrect marker comment. * Add better credit card validation. * Bump the Autofill commit. * Add tests for the new autofill Get methods. * Clean up the database access calls. * Clean up a number of SwiftLint warnings and errors. * Update the Autofill commit. * Fill in the autofill type delegate methods. * Ignore the release script. * Bump autofill Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill to version 3.3.0 (#29) * Bump autofill to version 3.3.0 Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Update the duckduckgo-autofill submodule commit. Co-authored-by: Sam Symons <sam@samsymons.com> * Add requiresRunInPageContentWorld to run UserScripts in page world (#30) * Ensure that the WKContentWorld usage compiles on iOS. (#32) * Fix lastName in identity (#33) * Update the credit card table (#34) * Upgrade the credit card number field to L2. * Correctly provide the last name for Identity autofill. * Allow the SecureVaultFactory to return the crypto classes only. * Ensure that addressStreet2 is encoded and read correctly. * Update the migration to remove the old column. * Generate a display title and subtitle for notes. * Add a new error type for if no L1 key is available. * Move content blocking code to BSK (#35) * Initial refactoring of content blocking to enable sharing between platforms * Extracted Content Rules script * Tweaks based on Mac OS X implementation * Expand API to enable checking for result of the requested compilation * Add option to obtain PrivacyFeature settings dictionary * Add Surrogates user script * API Tweaks * Fix tests * Content Blocking reference tests * More Pixels * Add tests for User Scripts and Tracker allowlist * Reuse sha256 code in tests * Better Privacy Config tests * Add autoconsent as a configurable feature (#39) * Remove unused TrackingProtectionStats dead code (#41) * Add identities autofill (#38) * Bump autofill pointer Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill revision Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill to latest version Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill revision (#46) Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Support content scope as a user script (#45) * Remove content scope files causing warnings (#49) * Navigator credentials move (#47) * Fix unprotected domains calculation for about:blank urls (#51) * Add Resolver tests and fix cname resolving (#50) * Multiple blocking rule lists (#53) * Initial implementation to support multiple rules lists * Make API open for inheritance * Fix few bugs, update tests * CTL delegate WIP * Update ContentBlockerRulesUserScript.swift * Update ContentBlockerRulesUserScript.swift * Refactor Content Blocker User Script * Add missing change information * Expand error reporting API * Unit tests for error reporting * Workaround for missing TDS at startup issue * Move model processing to work queue * Kill the app only in case main TDS rule list fails to compile * Allow for non-TDS rule lists to fail compilation in case setup fails * Add check for embedded TDS compilation Co-authored-by: Bartek Waresiak <bartek@duckduckgo.com> Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> * Add platform.name to BrowserServices kit (#48) * Add platform.name to BrowserServices kit * be explicit about supported platform names * add clickToPlay config (#56) Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> * Enable unit tests via Xcode (#57) * Have all bundle resources copied to the root directory. * Remove a duplicate file. * Move embedded TDS to platform code (#58) * Filtering various suggestion types from Top Hits section of suggestions (#55) * Filtering various suggestion types from Top Hits section of suggestions * Suggesting bookmarks instead of history entries with the same URL * Allowing bookmarks to be in the Top Hits if they replaced history entry * isDownload flag removed * Edge case with bookmark not allowed in Top Hits resolved * Array renamed to all * Fix Privcy Config API to correcty take into account feature type (#62) * Update config to reflect iOS features (#64) * Update Autofill submodule pointer with iOS fix (#65) * Update the duckduckgo-autofill submodule commit. * Resolve package warnings Co-authored-by: Bartek Waresiak <bartek@duckduckgo.com> Co-authored-by: Alexey Martemyanov <mallexxx@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Jonathan Kingston <jkingston@duckduckgo.com> Co-authored-by: bwaresiak <bartek@duckduckgo.com> Co-authored-by: Sam Macbeth <sammacbeth@users.noreply.github.com> Co-authored-by: Brad Slayter <SlayterDev@users.noreply.github.com> Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> Co-authored-by: Shane Osbourne <shane.osbourne8@gmail.com> Co-authored-by: Lucas Adamski <ladamski@users.noreply.github.com> Co-authored-by: Tomas Strba <57389842+tomasstrba@users.noreply.github.com>

* Secure Vault autofill Pixel support (#26) * add delegate calls after autofilling * upd with AutofillType * Secure Vault additions (#28) * Finish note support. * Note database migration * Begin working on identities * Further work on Identities * Consolidate database migrations. * Fill out remaining credit card functions. * Remove unneeded credit card properties. * Further autofill API work. * Support cardholder name. * Fix unit test compilation failures. * Remove an incorrect marker comment. * Add better credit card validation. * Bump the Autofill commit. * Add tests for the new autofill Get methods. * Clean up the database access calls. * Clean up a number of SwiftLint warnings and errors. * Update the Autofill commit. * Fill in the autofill type delegate methods. * Ignore the release script. * Bump autofill Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill to version 3.3.0 (#29) * Bump autofill to version 3.3.0 Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Update the duckduckgo-autofill submodule commit. Co-authored-by: Sam Symons <sam@samsymons.com> * Add requiresRunInPageContentWorld to run UserScripts in page world (#30) * Ensure that the WKContentWorld usage compiles on iOS. (#32) * Fix lastName in identity (#33) * Update the credit card table (#34) * Upgrade the credit card number field to L2. * Correctly provide the last name for Identity autofill. * Allow the SecureVaultFactory to return the crypto classes only. * Ensure that addressStreet2 is encoded and read correctly. * Update the migration to remove the old column. * Generate a display title and subtitle for notes. * Add a new error type for if no L1 key is available. * Move content blocking code to BSK (#35) * Initial refactoring of content blocking to enable sharing between platforms * Extracted Content Rules script * Tweaks based on Mac OS X implementation * Expand API to enable checking for result of the requested compilation * Add option to obtain PrivacyFeature settings dictionary * Add Surrogates user script * API Tweaks * Fix tests * Content Blocking reference tests * More Pixels * Add tests for User Scripts and Tracker allowlist * Reuse sha256 code in tests * Better Privacy Config tests * Add autoconsent as a configurable feature (#39) * Remove unused TrackingProtectionStats dead code (#41) * Add identities autofill (#38) * Bump autofill pointer Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill revision Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill to latest version Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill revision (#46) Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Support content scope as a user script (#45) * Remove content scope files causing warnings (#49) * Navigator credentials move (#47) * Fix unprotected domains calculation for about:blank urls (#51) * Add Resolver tests and fix cname resolving (#50) * Multiple blocking rule lists (#53) * Initial implementation to support multiple rules lists * Make API open for inheritance * Fix few bugs, update tests * CTL delegate WIP * Update ContentBlockerRulesUserScript.swift * Update ContentBlockerRulesUserScript.swift * Refactor Content Blocker User Script * Add missing change information * Expand error reporting API * Unit tests for error reporting * Workaround for missing TDS at startup issue * Move model processing to work queue * Kill the app only in case main TDS rule list fails to compile * Allow for non-TDS rule lists to fail compilation in case setup fails * Add check for embedded TDS compilation Co-authored-by: Bartek Waresiak <bartek@duckduckgo.com> Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> * Add platform.name to BrowserServices kit (#48) * Add platform.name to BrowserServices kit * be explicit about supported platform names * add clickToPlay config (#56) Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> * Enable unit tests via Xcode (#57) * Have all bundle resources copied to the root directory. * Remove a duplicate file. * Move embedded TDS to platform code (#58) * Pass config to Autofill script * lint * Parentheses * Fix tests * Fix tests * Update Autofill * Fix var injection * Refactor Autofill script to use SourceProvider * Cache result * Fix test * Make constructor public * Remove debug code * Update autofill * Update autofill to release branch * Update Autofill Co-authored-by: Alexey Martemyanov <mallexxx@gmail.com> Co-authored-by: Sam Symons <sam@samsymons.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Jonathan Kingston <jkingston@duckduckgo.com> Co-authored-by: bwaresiak <bartek@duckduckgo.com> Co-authored-by: Sam Macbeth <sammacbeth@users.noreply.github.com> Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> Co-authored-by: Shane Osbourne <shane.osbourne8@gmail.com> Co-authored-by: Lucas Adamski <ladamski@users.noreply.github.com>

* Secure Vault autofill Pixel support (#26) * add delegate calls after autofilling * upd with AutofillType * Secure Vault additions (#28) * Finish note support. * Note database migration * Begin working on identities * Further work on Identities * Consolidate database migrations. * Fill out remaining credit card functions. * Remove unneeded credit card properties. * Further autofill API work. * Support cardholder name. * Fix unit test compilation failures. * Remove an incorrect marker comment. * Add better credit card validation. * Bump the Autofill commit. * Add tests for the new autofill Get methods. * Clean up the database access calls. * Clean up a number of SwiftLint warnings and errors. * Update the Autofill commit. * Fill in the autofill type delegate methods. * Ignore the release script. * Bump autofill Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill to version 3.3.0 (#29) * Bump autofill to version 3.3.0 Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Update the duckduckgo-autofill submodule commit. Co-authored-by: Sam Symons <sam@samsymons.com> * Add requiresRunInPageContentWorld to run UserScripts in page world (#30) * Ensure that the WKContentWorld usage compiles on iOS. (#32) * Fix lastName in identity (#33) * Update the credit card table (#34) * Upgrade the credit card number field to L2. * Correctly provide the last name for Identity autofill. * Allow the SecureVaultFactory to return the crypto classes only. * Ensure that addressStreet2 is encoded and read correctly. * Update the migration to remove the old column. * Generate a display title and subtitle for notes. * Add a new error type for if no L1 key is available. * Move content blocking code to BSK (#35) * Initial refactoring of content blocking to enable sharing between platforms * Extracted Content Rules script * Tweaks based on Mac OS X implementation * Expand API to enable checking for result of the requested compilation * Add option to obtain PrivacyFeature settings dictionary * Add Surrogates user script * API Tweaks * Fix tests * Content Blocking reference tests * More Pixels * Add tests for User Scripts and Tracker allowlist * Reuse sha256 code in tests * Better Privacy Config tests * Add autoconsent as a configurable feature (#39) * Remove unused TrackingProtectionStats dead code (#41) * Add identities autofill (#38) * Bump autofill pointer Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill revision Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill to latest version Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Bump autofill revision (#46) Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Support content scope as a user script (#45) * Remove content scope files causing warnings (#49) * Navigator credentials move (#47) * Fix unprotected domains calculation for about:blank urls (#51) * Add Resolver tests and fix cname resolving (#50) * Multiple blocking rule lists (#53) * Initial implementation to support multiple rules lists * Make API open for inheritance * Fix few bugs, update tests * CTL delegate WIP * Update ContentBlockerRulesUserScript.swift * Update ContentBlockerRulesUserScript.swift * Refactor Content Blocker User Script * Add missing change information * Expand error reporting API * Unit tests for error reporting * Workaround for missing TDS at startup issue * Move model processing to work queue * Kill the app only in case main TDS rule list fails to compile * Allow for non-TDS rule lists to fail compilation in case setup fails * Add check for embedded TDS compilation Co-authored-by: Bartek Waresiak <bartek@duckduckgo.com> Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> * Compiled Content Rules caching * Add platform.name to BrowserServices kit (#48) * Add platform.name to BrowserServices kit * be explicit about supported platform names * add clickToPlay config (#56) Co-authored-by: ladamski@duckduckgo.com <ladamski@duckduckgo.com> * Invert ContentBlockerRules dependency injection * Have all bundle resources copied to the root directory. * Remove a duplicate file. * fix tests * Add ContentBlockerRulesManager cache tests * fix linter issues * ContentBlockerRulesManager.Rules, UpdateEvent public init for tests * Enable unit tests via Xcode (#57) * Have all bundle resources copied to the root directory. * Remove a duplicate file. * Move embedded TDS to platform code (#58) * Filtering various suggestion types from Top Hits section of suggestions (#55) * Filtering various suggestion types from Top Hits section of suggestions * Suggesting bookmarks instead of history entries with the same URL * Allowing bookmarks to be in the Top Hits if they replaced history entry * isDownload flag removed * Edge case with bookmark not allowed in Top Hits resolved * Array renamed to all * Fix Privcy Config API to correcty take into account feature type (#62) * report CBR compilation time on success * fix tests * measure whole rules compilation time * add copyright note * minor readability improvement

bwaresiak and others added 12 commits December 17, 2021 19:08

Initial implementation to support multiple rules lists

438ecb0

Make API open for inheritance

f5b32df

Fix few bugs, update tests

5697394

CTL delegate WIP

01a9e6b

Update ContentBlockerRulesUserScript.swift

cb682d7

Update ContentBlockerRulesUserScript.swift

9d180b6

Merge branch 'release/friends-and-family' into feature/bartek/multipl…

1c9f4bf

…e-blocking-rule-lists

Refactor Content Blocker User Script

69e854a

Add missing change information

c1aed68

Expand error reporting API

c8f7774

Unit tests for error reporting

cef5aac

Workaround for missing TDS at startup issue

73b2e4d

SlayterDev requested a review from brindy January 20, 2022 22:21

SlayterDev requested review from bwaresiak, mallexxx, samsymons, THISISDINOSAUR and tomasstrba as code owners January 20, 2022 22:21

SlayterDev changed the base branch from main to release/friends-and-family January 21, 2022 15:26

mallexxx self-assigned this Jan 24, 2022

mallexxx approved these changes Jan 24, 2022

View reviewed changes

mallexxx assigned SlayterDev and unassigned mallexxx Jan 24, 2022

mallexxx mentioned this pull request Jan 24, 2022

Brad/multiple rule lists #42

Closed

5 tasks

bwaresiak added 4 commits January 24, 2022 12:43

Move model processing to work queue

3ed7e99

Kill the app only in case main TDS rule list fails to compile

c9a6400

Allow for non-TDS rule lists to fail compilation in case setup fails

db138b9

Add check for embedded TDS compilation

981bd61

ladamski merged commit da3f840 into release/friends-and-family Jan 25, 2022

ladamski deleted the feature/bartek/multiple-blocking-rule-lists branch January 25, 2022 22:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple blocking rule lists #53

Multiple blocking rule lists #53

SlayterDev commented Jan 20, 2022

mallexxx left a comment

mallexxx Jan 24, 2022

bwaresiak Jan 24, 2022

bwaresiak commented Jan 24, 2022

ladamski commented Jan 25, 2022

bwaresiak commented Jan 25, 2022

		sourceManager.compilationFailed(for: model, with: error)
		let newModel = sourceManager.makeModel()

Multiple blocking rule lists #53

Multiple blocking rule lists #53

Conversation

SlayterDev commented Jan 20, 2022

Internal references:

mallexxx left a comment

Choose a reason for hiding this comment

mallexxx Jan 24, 2022

Choose a reason for hiding this comment

bwaresiak Jan 24, 2022

Choose a reason for hiding this comment

bwaresiak commented Jan 24, 2022

ladamski commented Jan 25, 2022

bwaresiak commented Jan 25, 2022