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

Use list_catalog.json file in iOS #8729

Closed
wants to merge 3 commits into from
Closed

Use list_catalog.json file in iOS #8729

wants to merge 3 commits into from

Conversation

cuba
Copy link
Contributor

@cuba cuba commented Feb 2, 2024

Summary of Changes

This pull request fixes brave/brave-browser#35210

MUST Be merged in response to brave/brave-core#21883

This actually has a number of benefits:

  1. All lists are now filter lists. so no additional code if we add new hidden lists such as first party list or iOS only lists
  2. Will very likely use this mechanism for any launch only lists too to increase launch performance (but this is not yet done and no such list exists yet)
  3. Brings us one step closer to how desktop functions.

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:

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).

Copy link

github-actions bot commented Feb 9, 2024

[puLL-Merge] - brave/brave-ios@8729

Description

This pull request introduces a set of updates primarily focused on improving the ContentBlockerManager logging, the handling of filter lists, and integrating signpost IDs for performance logging. It includes the refinement of data models and the implementation of guard checks across various functionalities to ensure a smoother and more reliable runtime behavior.

Changes

Changes

BVC+WKNavigationDelegate.swift

  • Addition of signpost IDs and logging:
    • Added signpost ID generation and performance logging for various methods within decidePolicyFor navigation actions.
    • This allows for better performance measurement and debugging.
  • Code cleanup and minor refactorings:
    • Removed unnecessary whitespace and improved code readability.

LaunchHelper.swift

  • Enhanced logging during the launch phase:
    • Introduced debug logging for loading blocking data and loaded blocking launch data for better insight during startup.

UserScriptType.swift

  • Syntax correction:
    • Fixed a missing closing parenthesis in the gpc case's debug description.

FilterListsView.swift

  • UI adjustments for filter lists:
    • Modified the filter list view to hide certain filter lists based on their properties, improving UI clarity.

ContentBlockerManager.swift

  • Logging and performance measurement:
    • Introduced signpost ID usage for the cleanup of invalid rule lists, aiding in performance analysis.

CustomFilterListStorage.swift

  • Extension for enabled sources:
    • Added a new extension to provide source representations for all enabled custom filter lists.

FilterList.swift

  • Data model adjustments:
    • Refinements in the filter list structure, including default behavior adjustments and removing redundant code.

FilterListCustomURLDownloader.swift

  • Downloader adjustments:
    • Removed unnecessary do-catch block and improved structure for fetching resources.

Security Hotspots

  1. FilterListResourceDownloader.swift:

    • Risk: The method compileFilterListEngineIfNeeded directly checks for file existence which could potentially introduce race conditions if files are modified or removed between the check and usage - Mitigation: Ensure atomicity or use more robust mechanisms to handle resource integrity.
  2. CachedAdBlockEngine.swift:

    • Risk: Direct usage of FileManager to check for file existence within cosmeticFilterModel. Similar to above, this introduces possible race conditions - Mitigation: Implement safer file handling practices.
  3. CustomFilterListStorage.swift (enabledSources extension):

    • Risk: Usage of compactMap on filter lists with optional safety checks may skip error handling for unexpected nil values, leading to skipped filter lists without notice - Mitigation: Consider explicit handling or logging of nil cases to ensure all expected filter lists are loaded without silently failing.

@cuba
Copy link
Contributor Author

cuba commented Feb 13, 2024

moved to brave-core

@cuba cuba closed this Feb 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose the first party list in iOS
1 participant