From dcf8df6980ede9160900e97f1e5aa6028a0e5f13 Mon Sep 17 00:00:00 2001 From: Jamil Date: Wed, 1 Jan 2025 11:47:44 -0800 Subject: [PATCH] refactor(apple): Clean up use of DispatchQueue.main (#7612) In several places we were unnecessarily wrapping code in either a `Task` or `DispatchQueue.main.async` block. The former is only needed if the API we're calling itself exposes an `async` function or we're running some long-running task. The latter is typically used for UI updates. At all other times it is preferable to simply use blocking functions and let the caller of these best determine how to execute them. --- .../FirezoneKit/Models/WebAuthSession.swift | 2 +- .../Sources/FirezoneKit/Stores/Store.swift | 16 +++++----------- .../FirezoneKit/Views/GrantVPNView.swift | 8 +++++++- .../Sources/FirezoneKit/Views/MenuBar.swift | 8 ++++++-- .../FirezoneKit/Views/WelcomeView.swift | 18 ++---------------- .../FirezoneKit/Views/iOSNavigationView.swift | 2 +- 6 files changed, 22 insertions(+), 32 deletions(-) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift index 3ed78f9b7..14c481483 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift @@ -15,7 +15,7 @@ import AuthenticationServices struct WebAuthSession { private static let scheme = "firezone-fd0020211111" - static func signIn(store: Store) async { + static func signIn(store: Store) { guard let authURL = store.authURL(), let authClient = try? AuthClient(authURL: authURL), let url = try? authClient.build() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift index 8d70a6e06..e207394e2 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift @@ -50,7 +50,7 @@ public final class Store: ObservableObject { private func initNotifications() { // Finish initializing notification binding sessionNotification.signInHandler = { - Task { await WebAuthSession.signIn(store: self) } + WebAuthSession.signIn(store: self) } sessionNotification.$decision @@ -93,17 +93,11 @@ public final class Store: ObservableObject { #endif } - func createVPNProfile() { - Task { - try await TunnelManager.shared.create() + func createVPNProfile() async throws { + try await TunnelManager.shared.create() - DispatchQueue.main.async { [weak self] in - guard let self else { return } - - // Load the new settings and bind observers - self.loadTunnelManager() - } - } + // Load the new settings and bind observers + self.loadTunnelManager() } func authURL() -> URL? { diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift index e552b7a05..7253aec62 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift @@ -17,7 +17,13 @@ final class GrantVPNViewModel: ObservableObject { func grantPermissionButtonTapped() { Log.log("\(#function)") - store.createVPNProfile() + Task { + do { + try await store.createVPNProfile() + } catch { + Log.error("\(#function): \(error)") + } + } } } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift index d6c07c77c..cae9d3b7a 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift @@ -273,7 +273,7 @@ public final class MenuBar: NSObject, ObservableObject { @objc private func signInButtonTapped() { NSApp.activate(ignoringOtherApps: true) - Task { await WebAuthSession.signIn(store: model.store) } + WebAuthSession.signIn(store: model.store) } @objc private func signOutButtonTapped() { @@ -290,7 +290,11 @@ public final class MenuBar: NSObject, ObservableObject { @objc private func grantVPNPermissionButtonTapped() { Task { - model.store.createVPNProfile() + do { + try await model.store.createVPNProfile() + } catch { + Log.error("\(#function): \(error)") + } } } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/WelcomeView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/WelcomeView.swift index d5288de2b..b374bc445 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/WelcomeView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/WelcomeView.swift @@ -17,16 +17,13 @@ final class WelcomeViewModel: ObservableObject { } func signInButtonTapped() { - Task { await WebAuthSession.signIn(store: store) } + WebAuthSession.signIn(store: store) } } struct WelcomeView: View { @ObservedObject var model: WelcomeViewModel - // Debounce button taps - @State private var tapped = false - var body: some View { VStack( alignment: .center, @@ -44,19 +41,8 @@ struct WelcomeView: View { """).multilineTextAlignment(.center) .padding(.bottom, 10) Button("Sign in") { - if !tapped { - tapped = true - - DispatchQueue.main.async { - model.signInButtonTapped() - } - - DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { - tapped = false - } - } + model.signInButtonTapped() } - .disabled(tapped) .buttonStyle(.borderedProminent) .controlSize(.large) Spacer() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/iOSNavigationView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/iOSNavigationView.swift index 881cc49d3..77f42b377 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/iOSNavigationView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/iOSNavigationView.swift @@ -54,7 +54,7 @@ struct iOSNavigationView: View { } } else { Button(action: { - Task { await WebAuthSession.signIn(store: model.store) } + WebAuthSession.signIn(store: model.store) }) { Label("Sign in", systemImage: "person.crop.circle.fill.badge.plus") }