Skip to content
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.

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

Wait until the sync has stopped before marking a background task as complete. #3564

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 17 additions & 12 deletions ElementX/Sources/Application/AppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -882,12 +882,12 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg

// MARK: - Application State

private func stopSync(isBackgroundTask: Bool) {
private func stopSync(isBackgroundTask: Bool, completion: (() -> Void)? = nil) {
if isBackgroundTask, UIApplication.shared.applicationState == .active {
// Attempt to stop the background task sync loop cleanly, only if the app not already running
return
}
userSession?.clientProxy.stopSync()
userSession?.clientProxy.stopSync(completion: completion)
clientProxyObserver = nil
}

Expand Down Expand Up @@ -968,9 +968,11 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
backgroundTask = appMediator.beginBackgroundTask { [weak self] in
guard let self else { return }

stopSync(isBackgroundTask: true)

if let backgroundTask {
MXLog.info("Background task is about to expire.")
stopSync(isBackgroundTask: true) { [weak self] in
guard let self, let backgroundTask else { return }

MXLog.info("Ending background task.")
appMediator.endBackgroundTask(backgroundTask)
self.backgroundTask = nil
}
Expand Down Expand Up @@ -1026,10 +1028,12 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
scheduleBackgroundAppRefresh()

task.expirationHandler = { [weak self] in
self?.stopSync(isBackgroundTask: true)
MXLog.info("Background app refresh task is about to expire.")

MXLog.info("Background app refresh task expired")
task.setTaskCompleted(success: true)
self?.stopSync(isBackgroundTask: true) {
MXLog.info("Marking Background app refresh task as complete.")
task.setTaskCompleted(success: true)
}
}

guard let userSession else {
Expand All @@ -1047,13 +1051,14 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
.sink(receiveValue: { [weak self] _ in
guard let self else { return }
MXLog.info("Background app refresh finished")
backgroundRefreshSyncObserver?.cancel()

// Make sure we stop the sync loop, otherwise the ongoing request is immediately
// handled the next time the app refreshes, which can trigger timeout failures.
stopSync(isBackgroundTask: true)
backgroundRefreshSyncObserver?.cancel()

task.setTaskCompleted(success: true)
stopSync(isBackgroundTask: true) {
MXLog.info("Marking Background app refresh task as complete.")
task.setTaskCompleted(success: true)
}
})
}
}
35 changes: 35 additions & 0 deletions ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2346,6 +2346,41 @@ class ClientProxyMock: ClientProxyProtocol {
stopSyncCallsCount += 1
stopSyncClosure?()
}
//MARK: - stopSync

var stopSyncCompletionUnderlyingCallsCount = 0
var stopSyncCompletionCallsCount: Int {
get {
if Thread.isMainThread {
return stopSyncCompletionUnderlyingCallsCount
} else {
var returnValue: Int? = nil
DispatchQueue.main.sync {
returnValue = stopSyncCompletionUnderlyingCallsCount
}

return returnValue!
}
}
set {
if Thread.isMainThread {
stopSyncCompletionUnderlyingCallsCount = newValue
} else {
DispatchQueue.main.sync {
stopSyncCompletionUnderlyingCallsCount = newValue
}
}
}
}
var stopSyncCompletionCalled: Bool {
return stopSyncCompletionCallsCount > 0
}
var stopSyncCompletionClosure: (((() -> Void)?) -> Void)?

func stopSync(completion: (() -> Void)?) {
stopSyncCompletionCallsCount += 1
stopSyncCompletionClosure?(completion)
}
//MARK: - accountURL

var accountURLActionUnderlyingCallsCount = 0
Expand Down
2 changes: 1 addition & 1 deletion ElementX/Sources/Services/Client/ClientProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ class ClientProxy: ClientProxyProtocol {
stopSync(completion: nil)
}

private func stopSync(completion: (() -> Void)?) {
func stopSync(completion: (() -> Void)?) {
MXLog.info("Stopping sync")

if restartTask != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol {
func startSync()

func stopSync()
func stopSync(completion: (() -> Void)?) // Hopefully this will become async once we get SE-0371.

func accountURL(action: AccountManagementAction) async -> URL?

Expand Down
Loading