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

Fix #8762: Isolate Tab to MainActor and Update all Script Injection to be Async-Await #8763

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

Brandon-T
Copy link
Collaborator

Summary of Changes

  • Added @MainActor to Tab and all places that use it
  • Made script injection use async-await and isolate to @MainActor when necessary
  • Note: Scripts have NOT changed (There is no changes to Javascript and the like). This is entirely just switching from completion blocks/callbacks to async-await.

This pull request fixes #8762

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:

  • Test that reader-mode and playlist work
  • Test brave-skus for VPN still works
  • Test night-mode still works
  • Test that switching tabs and tab-switcher still works
  • Test that favicons still work

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

@Brandon-T Brandon-T added this to the 1.63 milestone Feb 10, 2024
@Brandon-T Brandon-T self-assigned this Feb 10, 2024
@Brandon-T Brandon-T requested a review from a team as a code owner February 10, 2024 16:47
Copy link

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

Description

This PR introduces several enhancements and bug fixes to the Brave iOS app. The changes mainly focus on improving the asynchronous handling of tasks, utilizing Swift's async/await and @MainActor annotations to ensure UI-related operations are performed on the main thread. These changes aim to make the code cleaner, more efficient, and eliminate potential issues related to threading.

Changes

Changes

App/Client.xcodeproj

  • Removed the askForAppToLaunch flag in the ActionExtension.xcscheme, implying that the app no longer prompts which app to launch upon triggering the action extension.

Sources/Brave/BraveSkus/BraveSkusManager.swift

  • Refactored various functions, such as refreshOrder, fetchOrderCredentials, and prepareCredentialsPresentation, to use Swift concurrency (async/await), making the functions cleaner and easier to read.

Sources/Brave/Frontend/Browser

  • Various Swift files, such as BraveGetUA.swift, BVC+ReaderMode.swift, and others, have been updated to include @MainActor annotations, ensuring that UI updates are performed on the main thread. Many functions have been updated to be asynchronous using Swift's concurrency model.

Sources/Brave/Frontend/Browser/UserScripts

  • Updated script handlers, such as BraveTalkScriptHandler.swift and ContentBlockerHelper.swift, to support async/await for asynchronous code execution.

Sources/Brave/Frontend/Browser/Playlist

  • PlaylistViewController.swift and other Playlist related files have been updated with Swift's concurrency features to improve handling of asynchronous operations.

Sources/BrowserViewController/BVC+TabManagerDelegate.swift

  • Added @MainActor annotations and updated function signatures to support Swift's concurrency model, ensuring UI-related operations are run on the main thread.

Sources/Favicon/FaviconFetcher.swift

  • Updated loadIcon and monogramIcon functions to be nonisolated, allowing them to be called from contexts adhering to Swift's actor model.

Sources/Playlist/PlaylistMediaStreamer.swift

  • Updated to ensure webLoader stops and is removed from its superview after task completion, preventing potential memory leaks.

Security Hotspots

  1. Sources/Brave/Frontend/Browser/BraveSkus/BraveSkusManager.swift (High Risk)

    • Changes in BraveSkusManager.swift involve network requests and handling sensitive information, such as credentials. Ensure that all data is properly sanitized and that error handling is robust to prevent leaking sensitive information.
  2. Sources/Brave/Frontend/Browser/UserScripts and Sources/Favicon/FaviconFetcher.swift (Medium Risk)

    • Updates involve executing and fetching scripts or icons, which can be potential vectors for injecting malicious content. Validate all URLs and content thoroughly.
  3. Sources/Brave/Frontend/Browser/Playlist (Low Risk)

    • Changes related to Playlist functionality should be reviewed to ensure that concurrency does not introduce race conditions, especially where user-generated content is involved.

In summary, this PR significantly refactors the Brave iOS app to utilize modern Swift concurrency features for better performance, readability, and maintainability.

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.

Tab and UI functions should only be accessed on main
3 participants