Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #6254: Get cosmetic filters on serial queue #7148

Merged
merged 2 commits into from
Apr 1, 2023

Conversation

cuba
Copy link
Contributor

@cuba cuba commented Mar 25, 2023

Summary of Changes

This pull request fixes #6254

This PR ensures that the cosmetic filters are retrieved on the serial queue. This should prevent any possible crashes due to threading. However the engine does allow multiple threads to be used so its more of a extra security than actual fix.

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

  1. Test cosmetic filters. For example. reddit.com should hide cookie consent notices when cookie consent blocking is enabled
  2. Test launch performance (both initial launch and relaunch). Enable all filter lists for extra delays during launch.
  3. Test navigation performance. Just make sure it's not caused by a slow internet connection. best to compare before and after changes and see if there is any visible difference in website loading times that aren't present prior to the changes. Again enabling a lot of filter lists before and after change is a good test.
  4. Ensure that passed: true is enabled for all values on the following site: https://test-pages.privacytests.org/tracking_content.html
  5. Ensure that true is set on all frames on the following site: https://dev-pages.brave.software/filtering/scriptlets.html (note that this may be false on initial launch)

Performance tests

0.918944s with all filter lists enabled. This is similar to what is found in current dev.

Screenshots:

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@cuba cuba requested a review from a team as a code owner March 25, 2023 13:14
@cuba cuba changed the title Js/serial cosmetic fllters Fix #6254: Get cosmetic filters on serial queue Mar 27, 2023
@cuba cuba force-pushed the js/serial-cosmetic-fllters branch from 2270aab to b4c54d1 Compare March 29, 2023 12:41
@iccub iccub force-pushed the js/serial-cosmetic-fllters branch from b4c54d1 to 5d785ab Compare March 31, 2023 19:45
@iccub iccub added this to the 1.50 milestone Mar 31, 2023
Copy link
Contributor

@iccub iccub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AdBlockEngineManagerTests fail now.
I tried to rebase it and merge, i hope i did not mess this up

let's fix then i will merge it

@cuba cuba force-pushed the js/serial-cosmetic-fllters branch from 5d785ab to 2356384 Compare March 31, 2023 23:25
@iccub iccub merged commit 9d19802 into development Apr 1, 2023
@iccub iccub deleted the js/serial-cosmetic-fllters branch April 1, 2023 10:43
Brandon-T pushed a commit that referenced this pull request Apr 11, 2023
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adblock/Cosmetic filters crash
3 participants