Skip to content

Add locks for browser tabs manager state changes #1690

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 1 commit into
base: develop
Choose a base branch
from
Open

Add locks for browser tabs manager state changes #1690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions ...Modules/DApp/DAppBrowser/Tabs/DAppBrowserTabManager/DAppBrowserTabManager+Singleton.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ extension DAppBrowserTabManager {
logger: logger
)

let workingQueue = DispatchQueue.global(qos: .userInitiated)

return DAppBrowserTabManager(
fileRepository: renderFilesRepository,
tabsSubscriptionFactory: tabsSubscriptionFactory,
walletListLocalSubscriptionFactory: walletListLocalSubscriptionFactory,
repository: AnyDataProviderRepository(coreDataRepository),
workingQueue: workingQueue,
operationQueue: operationQueue,
logger: logger
)
Expand Down
97 changes: 81 additions & 16 deletions novawallet/Modules/DApp/DAppBrowser/Tabs/DAppBrowserTabManager/DAppBrowserTabManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,21 @@ final class DAppBrowserTabManager {
private let fileRepository: WebViewRenderFilesOperationFactoryProtocol
private let repository: AnyDataProviderRepository<DAppBrowserTab.PersistenceModel>
private let operationQueue: OperationQueue
private let workingQueue: DispatchQueue
private let observerQueue: DispatchQueue

private let logger: LoggerProtocol

private let transportStates: InMemoryCache<UUID, [DAppTransportState]> = .init()

private let callStore = CancellableCallStore()
private let mutex = NSLock()

private var metaAccount: MetaAccountModel?

private var browserTabProvider: StreamableProvider<DAppBrowserTab.PersistenceModel>?
private var selectedWalletProvider: StreamableProvider<ManagedMetaAccountModel>?

private var transportStates: InMemoryCache<UUID, [DAppTransportState]> = .init()
private var observableTabs: DAppBrowserTabsObservable = .init(state: .init())

init(
Expand All @@ -28,6 +33,7 @@ final class DAppBrowserTabManager {
walletListLocalSubscriptionFactory: WalletListLocalSubscriptionFactoryProtocol,
repository: AnyDataProviderRepository<DAppBrowserTab.PersistenceModel>,
observerQueue: DispatchQueue = .main,
workingQueue: DispatchQueue,
operationQueue: OperationQueue,
logger: LoggerProtocol
) {
Expand All @@ -36,6 +42,7 @@ final class DAppBrowserTabManager {
self.walletListLocalSubscriptionFactory = walletListLocalSubscriptionFactory
self.fileRepository = fileRepository
self.operationQueue = operationQueue
self.workingQueue = workingQueue
self.observerQueue = observerQueue
self.logger = logger

Expand All @@ -51,7 +58,7 @@ private extension DAppBrowserTabManager {
}

func clearInMemory() {
transportStates = .init()
transportStates.removeAllValues()
observableTabs.state.removeAllValues()
}

Expand Down Expand Up @@ -285,6 +292,38 @@ private extension DAppBrowserTabManager {

return tab
}

func handleWalletChange(_ managedMetaAccount: ManagedMetaAccountModel?) {
let walletChangeInfo = evaluateWalletChange(managedMetaAccount)

guard walletChangeInfo.didChange else {
return
}

performWalletTransition(to: walletChangeInfo.newMetaAccount)
}

func evaluateWalletChange(_ managedMetaAccount: ManagedMetaAccountModel?) -> WalletChangeInfo {
let newMetaId = managedMetaAccount?.info.metaId
let currentMetaId = metaAccount?.metaId

guard currentMetaId != newMetaId else {
return WalletChangeInfo(didChange: false, newMetaAccount: nil)
}

let newMetaAccount = managedMetaAccount?.info
metaAccount = newMetaAccount

return WalletChangeInfo(didChange: true, newMetaAccount: newMetaAccount)
}

func performWalletTransition(to newMetaAccount: MetaAccountModel?) {
observableTabs.state.removeAllValues()

guard let newMetaAccount else { return }

browserTabProvider = subscribeToBrowserTabs(newMetaAccount.metaId)
}
}

// MARK: DAppBrowserTabLocalSubscriber
Expand All @@ -293,6 +332,9 @@ extension DAppBrowserTabManager: DAppBrowserTabLocalSubscriber, DAppBrowserTabLo
func handleBrowserTabs(
result: Result<[DataProviderChange<DAppBrowserTab.PersistenceModel>], any Error>
) {
mutex.lock()
defer { mutex.unlock() }

switch result {
case let .success(changes):
apply(changes)
Expand All @@ -308,18 +350,12 @@ extension DAppBrowserTabManager: WalletListLocalStorageSubscriber, WalletListLoc
func handleSelectedWallet(
result: Result<ManagedMetaAccountModel?, any Error>
) {
mutex.lock()
defer { mutex.unlock() }

switch result {
case let .success(managedMetaAccount):
guard metaAccount?.metaId != managedMetaAccount?.info.metaId else {
return
}

observableTabs.state.removeAllValues()
metaAccount = managedMetaAccount?.info

guard let metaAccount else { return }

browserTabProvider = subscribeToBrowserTabs(metaAccount.metaId)
handleWalletChange(managedMetaAccount)
case let .failure(error):
logger.error("Failed on WalletList local subscription with error: \(error.localizedDescription)")
}
Expand All @@ -330,10 +366,16 @@ extension DAppBrowserTabManager: WalletListLocalStorageSubscriber, WalletListLoc

extension DAppBrowserTabManager: DAppBrowserTabManagerProtocol {
func retrieveTab(with id: UUID) -> CompoundOperationWrapper<DAppBrowserTab?> {
retrieveWrapper(for: id)
mutex.lock()
defer { mutex.unlock() }

return retrieveWrapper(for: id)
}

func getAllTabs(for metaIds: Set<MetaAccountModel.Id>?) -> CompoundOperationWrapper<[DAppBrowserTab]> {
mutex.lock()
defer { mutex.unlock() }

if let metaIds {
return tabsFetchWrapper(for: metaIds)
} else {
Expand All @@ -359,7 +401,7 @@ extension DAppBrowserTabManager: DAppBrowserTabManagerProtocol {
execute(
wrapper: wrapper,
inOperationQueue: operationQueue,
runningCallbackIn: .main,
runningCallbackIn: workingQueue,
callbackClosure: { [weak self] _ in
self?.transportStates.removeValue(for: id)
}
Expand All @@ -376,6 +418,9 @@ extension DAppBrowserTabManager: DAppBrowserTabManagerProtocol {
with id: UUID,
render: DAppBrowserTabRenderProtocol
) -> CompoundOperationWrapper<Void> {
mutex.lock()
defer { mutex.unlock() }

guard let tab = observableTabs.state.fetchValue(for: id) else {
return .createWithResult(())
}
Expand Down Expand Up @@ -411,30 +456,41 @@ extension DAppBrowserTabManager: DAppBrowserTabManagerProtocol {
}

func removeAll(for metaIds: Set<MetaAccountModel.Id>?) {
mutex.lock()
defer { mutex.unlock() }

let wrapper = removeAllWrapper(metaIds)

execute(
wrapper: wrapper,
inOperationQueue: operationQueue,
runningCallbackIn: .main
runningCallbackIn: workingQueue
) { [weak self] result in
switch result {
case .success:
self?.mutex.lock()
self?.clearInMemory()
self?.mutex.unlock()
case .failure:
self?.logger.warning("\(String(describing: self)) Failed on tabs deletion operation")
}
}
}

func removeAllWrapper(for metaIds: Set<MetaAccountModel.Id>?) -> CompoundOperationWrapper<Set<UUID>> {
removeAllWrapper(metaIds)
mutex.lock()
defer { mutex.unlock() }

return removeAllWrapper(metaIds)
}

func addObserver(
_ observer: DAppBrowserTabsObserver,
sendOnSubscription: Bool
) {
mutex.lock()
defer { mutex.unlock() }

observableTabs.addObserver(
with: observer,
sendStateOnSubscription: sendOnSubscription,
Expand All @@ -448,3 +504,12 @@ extension DAppBrowserTabManager: DAppBrowserTabManagerProtocol {
}
}
}

// MARK: - Private types

private extension DAppBrowserTabManager {
struct WalletChangeInfo {
let didChange: Bool
let newMetaAccount: MetaAccountModel?
}
}