Skip to content

Refactor FXIOS-13944 [Swift 6] Isolate new state and subsequently a lot of redux to the main thread #30301

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+162 −47
Open

Refactor FXIOS-13944 [Swift 6] Isolate new state and subsequently a lot of redux to the main thread #30301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a86abea
isolate new state and subsequently a lot of redux to the main thread
Cramsden Oct 31, 2025
fc64406
fix tests
Cramsden Oct 31, 2025
a0e36e3
Swiftlint fix.
ih-codes Nov 5, 2025
a5734f1
Addressed some PR feedback.
ih-codes Nov 5, 2025
ce303de
Fix unit tests.
ih-codes Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions BrowserKit/Sources/Redux/DispatchStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@ public protocol DispatchStore {
public protocol DefaultDispatchStore<State>: DispatchStore where State: StateType {
associatedtype State

@MainActor
var state: State { get }

@MainActor
func subscribe<S: StoreSubscriber>(_ subscriber: S) where S.SubscriberStateType == State
@MainActor
func subscribe<SubState, S: StoreSubscriber>(
_ subscriber: S,
transform: ((Subscription<State>) -> Subscription<SubState>)?
) where S.SubscriberStateType == SubState
@MainActor
func unsubscribe<S: StoreSubscriber>(_ subscriber: S) where S.SubscriberStateType == State
@MainActor
func unsubscribe(_ subscriber: any StoreSubscriber)
}
8 changes: 8 additions & 0 deletions BrowserKit/Sources/Redux/Store.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Common
public final class Store<State: StateType & Sendable>: DefaultDispatchStore {
Copy link
Collaborator

@ih-codes ih-codes Oct 31, 2025

Choose a reason for hiding this comment

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

Should we just annotate the entire store @MainActor if we're doing so in AppState.swift anyway? 🤔

typealias SubscriptionType = SubscriptionWrapper<State>

@MainActor
public var state: State {
didSet {
subscriptions.forEach {
Expand All @@ -26,12 +27,14 @@ public final class Store<State: StateType & Sendable>: DefaultDispatchStore {

private var reducer: Reducer<State>
private var middlewares: [Middleware<State>]
@MainActor
private var subscriptions: Set<SubscriptionType> = []
private var actionRunning = false
private let logger: Logger
private var actionQueue: [Action] = []
private var isProcessingActions = false

@MainActor
public init(state: State,
reducer: @escaping Reducer<State>,
middlewares: [Middleware<State>] = [],
Expand All @@ -43,11 +46,13 @@ public final class Store<State: StateType & Sendable>: DefaultDispatchStore {
}

/// General subscription to app main state
@MainActor
public func subscribe<S: StoreSubscriber>(_ subscriber: S) where S.SubscriberStateType == State {
subscribe(subscriber, transform: nil)
}

/// Adds support to subscribe to subState parts of the store's state
@MainActor
public func subscribe<SubState, S: StoreSubscriber>(
_ subscriber: S,
transform: ((Subscription<State>) -> Subscription<SubState>)?
Expand All @@ -57,12 +62,14 @@ public final class Store<State: StateType & Sendable>: DefaultDispatchStore {
subscribe(subscriber, mainSubscription: originalSubscription, transformedSubscription: transformedSubscription)
}

@MainActor
public func unsubscribe(_ subscriber: any StoreSubscriber) {
if let index = subscriptions.firstIndex(where: { return $0.subscriber === subscriber }) {
subscriptions.remove(at: index)
}
}

@MainActor
public func unsubscribe<S: StoreSubscriber>(_ subscriber: S) where S.SubscriberStateType == State {
if let index = subscriptions.firstIndex(where: { return $0.subscriber === subscriber }) {
subscriptions.remove(at: index)
Expand Down Expand Up @@ -122,6 +129,7 @@ public final class Store<State: StateType & Sendable>: DefaultDispatchStore {
state = newState
}

@MainActor
private func subscribe<SubState, S: StoreSubscriber>(
_ subscriber: S,
mainSubscription: Subscription<State>,
Expand Down
4 changes: 4 additions & 0 deletions BrowserKit/Sources/Redux/StoreSubscriber.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import Foundation

public protocol AnyStoreSubscriber: AnyObject {
@MainActor
func subscribeToRedux()
@MainActor
func newState(state: Any)
}

Expand All @@ -17,10 +19,12 @@ public protocol StoreSubscriber: AnyStoreSubscriber {

/// Updates the subscriber with a new State for its screen state type.
/// - Parameter state: the changed screen state.
@MainActor
func newState(state: SubscriberStateType)
}

extension StoreSubscriber {
@MainActor
public func newState(state: Any) {
if let typedState = state as? SubscriberStateType {
newState(state: typedState)
Expand Down
8 changes: 5 additions & 3 deletions BrowserKit/Sources/Redux/Subscription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

import Foundation

final class SubscriptionWrapper<State>: Hashable {
@MainActor
final class SubscriptionWrapper<State>: @MainActor Hashable {
private let originalSubscription: Subscription<State>
weak var subscriber: AnyStoreSubscriber?
private let objectIdentifier: ObjectIdentifier
Expand Down Expand Up @@ -41,12 +42,13 @@ final class SubscriptionWrapper<State>: Hashable {
}
}

@MainActor
public final class Subscription<State> {
public var observer: ((State?, State) -> Void)?
public var observer: (@MainActor (State?, State) -> Void)?

init() {}

public init(sink: @escaping (@escaping (State?, State) -> Void) -> Void) {
init(sink: @escaping (@MainActor @escaping (State?, State) -> Void) -> Void) {
sink { old, new in
self.newValues(oldState: old, newState: new)
}
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Frontend/Browser/Toolbars/AddressBarPanGestureHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ final class AddressBarPanGestureHandler: NSObject, StoreSubscriber {
}

// MARK: - Redux
nonisolated func subscribeToRedux() {
func subscribeToRedux() {
let uuid = windowUUID
store.subscribe(self, transform: {
$0.select({ appState in
Expand Down
15 changes: 10 additions & 5 deletions firefox-ios/Client/Frontend/Browser/Toolbars/Models/AddressToolbarContainerModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,14 @@ final class AddressToolbarContainerModel: Equatable {
gestureType: .tap,
windowUUID: self.windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
},
onLongPress: {
let action = ToolbarMiddlewareAction(buttonType: .locationView,
gestureType: .longPress,
windowUUID: self.windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
})
return AddressToolbarConfiguration(
locationViewConfiguration: locationViewConfiguration,
Expand Down Expand Up @@ -190,6 +190,7 @@ final class AddressToolbarContainerModel: Equatable {
)
}

@MainActor
init(
state: ToolbarState,
profile: Profile,
Expand Down Expand Up @@ -252,6 +253,7 @@ final class AddressToolbarContainerModel: Equatable {
return searchEnginesManager.queryForSearchURL(searchURL)
}

@MainActor
private static func mapActions(_ actions: [ToolbarActionConfiguration],
isShowingTopTabs: Bool,
windowUUID: UUID) -> [ToolbarElement] {
Expand Down Expand Up @@ -283,26 +285,29 @@ final class AddressToolbarContainerModel: Equatable {
}
}

@MainActor
private static func getA11yCustomAction(action: ToolbarActionConfiguration, windowUUID: UUID) -> (() -> Void)? {
return action.a11yCustomActionName != nil ? {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.customA11yAction)
store.dispatchLegacy(action)
store.dispatch(action)
} : nil
}

@MainActor
private static func getOnSelected(action: ToolbarActionConfiguration, windowUUID: UUID) -> ((UIButton) -> Void)? {
return { button in
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .tap,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
}
}

@MainActor
private static func getOnLongPress(action: ToolbarActionConfiguration,
windowUUID: UUID,
isShowingTopTabs: Bool) -> ((UIButton) -> Void)? {
Expand All @@ -312,7 +317,7 @@ final class AddressToolbarContainerModel: Equatable {
gestureType: .longPress,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
} : nil
}

Expand Down
10 changes: 7 additions & 3 deletions firefox-ios/Client/Frontend/Browser/Toolbars/Models/NavigationToolbarContainerModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct NavigationToolbarContainerModel: Equatable {
isTranslucencyEnabled: isTranslucent)
}

@MainActor
init(state: ToolbarState, windowUUID: WindowUUID) {
self.displayBorder = state.navigationToolbar.displayBorder
self.canShowNavigationHint = state.canShowNavigationHint
Expand Down Expand Up @@ -51,26 +52,29 @@ struct NavigationToolbarContainerModel: Equatable {
self.windowUUID = windowUUID
}

@MainActor
private static func getA11yCustomAction(action: ToolbarActionConfiguration, windowUUID: WindowUUID) -> (() -> Void)? {
return action.a11yCustomActionName != nil ? {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.customA11yAction)
store.dispatchLegacy(action)
store.dispatch(action)
} : nil
}

@MainActor
private static func getOnSelected(action: ToolbarActionConfiguration, windowUUID: WindowUUID) -> ((UIButton) -> Void)? {
return { button in
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .tap,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
}
}

@MainActor
private static func getOnLongPress(action: ToolbarActionConfiguration,
state: ToolbarState,
windowUUID: WindowUUID) -> ((UIButton) -> Void)? {
Expand All @@ -80,7 +84,7 @@ struct NavigationToolbarContainerModel: Equatable {
gestureType: .longPress,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
} : nil
}
}
Loading