[google_sign_in] PR 2/4 Migrate the plugin class from Objective-C to Swift. by victogomez-cs · Pull Request #12655 · flutter/packages

victogomez-cs · 2026-08-27T17:22:33Z

Migrates FLTGoogleSignInPlugin from Objective-C to Swift (GoogleSignInPlugin.swift). GID SDK wrappers and ViewProvider stay Obj-C for this PR.

Intended as a 1:1 move of configure, restorePreviousSignIn, signIn, refreshedAuthorizationTokens, addScopes, signOut, disconnect, error-code mapping, URL handling, and the iOS view-controller / macOS window presentation paths.

presentingViewController / presentingWindow on the Obj-C wrapper protocol become nullable. That matches existing runtime behavior: FSIViewProvider.viewController was already nullable.

The -6 EMM mapping in pigeonErrorCode(for:) is kept as a numeric code because GIDSignInError.emm is not imported into Swift.

Bumps google_sign_in_ios to 6.3.3.

PR 2/4 of the Obj-C → Swift migration. Depends on PR 1/4 (SPM packaging). Continues flutter/flutter#119103

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

google-cla · 2026-08-27T17:22:50Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request migrates the google_sign_in_ios plugin class from Objective-C to Swift, replacing FLTGoogleSignInPlugin with GoogleSignInPlugin and adding an Objective-C exception catcher helper. Feedback on the changes suggests simplifying the sanitizedUserInfo function by using a non-optional parameter, updating flutterError to handle the optional error mapping, and correcting a signature mismatch in the scene(_:openURLContexts:) delegate method to return Void instead of Bool while reusing the handleURLs helper.

gemini-code-assist · 2026-08-27T17:25:26Z

+private func sanitizedUserInfo(_ value: Any?) -> Any {
+  switch value {
+  case let error as NSError:
+    return [
+      "domain": error.domain,
+      "code": "\(error.code)",
+      "localizedDescription": error.localizedDescription,
+      "userInfo": sanitizedUserInfo(error.userInfo),
+    ]
+  case let string as String:
+    return string
+  case let url as URL:
+    return url.absoluteString
+  case let number as NSNumber:
+    return number
+  case let array as [Any]:
+    return array.map { sanitizedUserInfo($0) }
+  case let dict as [AnyHashable: Any]:
+    var safeValues: [AnyHashable: Any] = [:]
+    safeValues.reserveCapacity(dict.count)
+    for (key, nestedValue) in dict {
+      safeValues[key] = sanitizedUserInfo(nestedValue)
+    }
+    return safeValues
+  default:
+    if let value {
+      return "[Unsupported type: \(String(describing: type(of: value)))]"
+    }
+    return "[Unsupported type: nil]"
+  }
+}


sanitizedUserInfo can be simplified by making its parameter non-optional Any. Since userInfo dictionaries and nested values are typically non-optional when traversed, we can avoid the awkward default case check for nil and make the code more robust.

private func sanitizedUserInfo(_ value: Any) -> Any { switch value { case let error as NSError: return [ "domain": error.domain, "code": "\(error.code)", "localizedDescription": error.localizedDescription, "userInfo": sanitizedUserInfo(error.userInfo), ] case let string as String: return string case let url as URL: return url.absoluteString case let number as NSNumber: return number case let array as [Any]: return array.map { sanitizedUserInfo($0) } case let dict as [AnyHashable: Any]: var safeValues: [AnyHashable: Any] = [:] safeValues.reserveCapacity(dict.count) for (key, nestedValue) in dict { safeValues[key] = sanitizedUserInfo(nestedValue) } return safeValues default: return "[Unsupported type: \(String(describing: type(of: value)))]" } }

Thanks, going to keep Any? here. The Obj-C helper (FSISanitizedUserInfo) takes a nullable id and is called with a nil NSError (and thus nil userInfo) from FSIFlutterErrorForNSError. The default nil branch is that path, not leftover optionality. Tightening this to Any would require changing how nil errors are mapped, which we don't want in this port

gemini-code-assist · 2026-08-27T17:25:26Z

+private func flutterError(from error: NSError?) -> FlutterError {
+  return FlutterError(
+    code: String(format: "%@: %ld", error?.domain ?? "(null)", error?.code ?? 0),
+    message: error?.localizedDescription,
+    details: sanitizedUserInfo(error?.userInfo))
+}


If sanitizedUserInfo is updated to accept a non-optional Any, we can safely map the optional error here to pass its userInfo only when it is non-nil. This also correctly preserves nil details when error is nil instead of returning "[Unsupported type: nil]".

Suggested change

private func flutterError(from error: NSError?) -> FlutterError {

return FlutterError(

code: String(format: "%@: %ld", error?.domain ?? "(null)", error?.code ?? 0),

message: error?.localizedDescription,

details: sanitizedUserInfo(error?.userInfo))

}

private func flutterError(from error: NSError?) -> FlutterError {

return FlutterError(

code: String(format: "%@: %ld", error?.domain ?? "(null)", error?.code ?? 0),

message: error?.localizedDescription,

details: error.map { sanitizedUserInfo($0.userInfo) })

}

This would change the nil-NSError mapping. Obj-C does details:FSISanitizedUserInfo(error.userInfo) even when error is nil, so the Dart side gets a sanitized placeholder string, not nil details. error.map { ... } would send details: nil instead. Leaving the current call as-is to stay aligned with FSIFlutterErrorForNSError

gemini-code-assist · 2026-08-27T17:25:26Z

+    public func scene(_ scene: UIScene, openURLContexts urlContexts: Set<UIOpenURLContext>) -> Bool
+    {
+      var handled = false
+      for context in urlContexts {
+        handled = signIn.handle(context.url) || handled
+      }
+      return handled
+    }


The delegate method scene(_:openURLContexts:) in UISceneDelegate (and FlutterSceneLifeCycleDelegate) does not return a value (it returns Void). Implementing it with -> Bool is a signature mismatch. Additionally, the implementation duplicates the URL handling logic. We can simplify this by calling the existing handleURLs helper method and removing the return type.

public func scene(_ scene: UIScene, openURLContexts urlContexts: Set<UIOpenURLContext>) { handleURLs(urlContexts.map { $0.url }) }

UISceneDelegate.scene(_:openURLContexts:) is Void, but this method is FlutterSceneLifeCycleDelegate, which is - (BOOL)scene:openURLContexts:, Flutter uses the return value to decide whether another plugin should see the URL. The old Obj-C implementation was void and didn't actually match that protocol. Returning Bool here is intentional.

handleURLs is a test helper that deliberately ignores the handle result so we can cover scene URLs without constructing UIOpenURLContext. Routing scene through it would drop the Bool Flutter needs. Leaving this as-is

…bility

…t compatibility

cbracken

Overall looks good -- mostly just nits.

cbracken · 2026-09-03T23:12:18Z

+      for url in urls {
+        _ = signIn.handle(url)
+      }
+    }


Where is this called from? It looks like maybe it was intended to be called from

public func scene(_ scene: UIScene, openURLContexts urlContexts: Set<UIOpenURLContext>) -> Bool

but that code effectively inlines the logic, or rather slightly different logic since this discards the result. You could either update this and use it or delete.

Good catch, it was only used from tests and discarded the handle result. It now returns Bool (true if GIDSignIn handled any URL), and scene(_:openURLContexts:) calls it. Also added a test that the return value matches signIn.handle

cbracken · 2026-09-03T23:19:47Z

+    return .canceled
+  case GIDSignInError.hasNoAuthInKeychain.rawValue:
+    return .noAuthInKeychain
+  case -6:  // kGIDSignInErrorCodeEMM; not imported as a Swift enum case.


GIDSignInError.EMM looks imported to me. Is there any reason we can't use GIDSignInError.EMM.rawValue like the others?

You’re right, .EMM is imported (the tests already used GIDSignInError.EMM.rawValue). Updated the mapping to use that instead of -6

cbracken · 2026-09-03T23:32:20Z

+  ) {
+    if let userID = user.userID {
+      usersByIdentifier[userID] = user
+    }


If userID is nil, we never populate this, but then we default to "" below and tell Dart that sign in succeeded instead of erroring out.

We should do this as:

guard let userID = userID else { completion(nil, ...) } usersByIdentifier[userID] = user

Then no need for the defaulting it below since userID is non-nil.

The old obj-c code would have crashed assigning a nil key to the dictionary, completing with an error is definitely better.

Agreed that completing with an error is better than crashing on a nil dictionary key, and better than reporting success with userId: "". Left this out of this PR because it changes the Dart-visible path (success → error). Happy to do it as a follow-up if you’d rather have it here

cbracken · 2026-09-03T23:48:32Z

+    // Convert expected errors into structured failure return, and everything else
+    // into a generic error.
+    let nsError = error as NSError?
+    if nsError?.domain == kGIDSignInErrorDomain, let nsError {


This works, but prefer writing more idiomatically as:

if let nsError, nsError.domain == kGIDSignInErrorDomain {

Agree and done!

cbracken · 2026-09-03T23:55:25Z

-  }
-}
-
- (void)scene:(UIScene *)scene openURLContexts:(NSSet<UIOpenURLContext *> *)URLContexts {


See my comment on the Swift implementation. Just adding a comment here to make it visible. This was returning void, which was wrong. Gemini already gave feedback which you already responded to, but since this is fixing a bug, we should probably mention the fix in the changelog, since this patch is no longer entirely just a 1:1 port, but also a bugfix.

Added a CHANGELOG bullet for reporting whether the scene URL was handled. The Swift method already returned Bool, this documents that as a bugfix, not just the port

cbracken · 2026-09-04T00:06:33Z

+
+#if os(iOS)
+  extension GoogleSignInPlugin: FlutterSceneLifeCycleDelegate {
+    public func scene(_ scene: UIScene, openURLContexts urlContexts: Set<UIOpenURLContext>) -> Bool


This looks like a fix that should be mentioned in the CHANGELOG. The old code was returning void as discussed elsewhere.

Looking at where this is used:
https://github.com/flutter/flutter/blob/37ad6a38fad3efe9878c73dca64691d9862bb49d/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSceneLifeCycle.mm#L591-L601

If any delegate returns true, the loop immediately stops and no later plugin will see the URL. Previously we were handling it but not informing the embedder which meant that other plugins got a shot at it. If no plugin at all handled it, then we fall back to deep link handling here:

https://github.com/flutter/flutter/blob/37ad6a38fad3efe9878c73dca64691d9862bb49d/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSceneLifeCycle.mm#L300-L306

Given the other plugins probably won't handle this plugin's URLs, it meant that we were incorrectly doing deep-link fallback for all these.

Re: my comment on handleURLs() being unused, we probably should fix it and use it here and ensure it's tested.

Same as above, CHANGELOG updated, and scene now goes through handleURLs so the OR’d handle result is tested

cbracken · 2026-09-04T00:30:32Z

+          error: error,
+          completion: completion)
+      }
+    }


In packages/google_sign_in/google_sign_in_ios/darwin/google_sign_in_ios/Sources/google_sign_in_ios_objc/ExceptionCatcher.m below, the @try is wrapping the Swift closure we're passing to performSignIn whereas the old code only wrapped Obj-C.

Obj-C uses table-driven unwinding, Swift passes errors up through an ordinary register return path and those frames aren't instrumented with the same info Obj-C frames are, which means Obj-C handling won't trigger the stack unwinding that Swift frames need including ARC cleanup, so we'll end up leaking Swift objects, though looking at this issue, they're explicit that this is undefined behaviour: swiftlang/swift#54322

The path to this is:

GoogleSignInCatchException // you are here -> the closure right here // this will get no cleanup -> GoogleSignInPlugin.performSignIn // this will get no cleanup -> GIDSignIn.signIn(withPresenting) // Obj-C throw can happen here

See: https://forums.swift.org/t/is-it-safe-to-throw-objc-exceptions-across-swift-stack-frames/17449

where they note:

it's not safe to throw an exception through Swift stack frames, even if the Swift code is totally unaware of the exception and you have some wrapper ObjC on the other side waiting to catch it.

They mention docs here:
https://developer.apple.com/documentation/swift/handling-cocoa-errors-in-swift#2993730

I don't know that we can eliminate this entirely since the block we're passing in in Swift, but we can probably reduce the exposure by tightening up the handling.

Maybe something like

public func signIn( withScopeHint scopeHint: [String], nonce: String?, completion: @escaping (FSISignInResult?, FlutterError?) -> Void ) { let exception = performSignIn(hint: nil, additionalScopes: scopeHint, nonce: nonce) { [weak self] signInResult, error in self?.handleAuthResult( user: signInResult?.user, serverAuthCode: signInResult?.serverAuthCode, error: error, completion: completion) } if let exception { completion( nil, FlutterError( code: "google_sign_in", message: exception.reason, details: exception.name.rawValue)) } } // 8< ... snip snip snip ... 8< ... update performSignIn below: /// Wraps the iOS and macOS sign in display methods. /// /// Returns any `NSException` raised by the SDK, or nil. The exception catcher wraps only the /// SDK call itself since Obj-C exception unwinding through Swift frames is undefined behaviour. private func performSignIn( hint: String?, additionalScopes: [String]?, nonce: String?, completion: @escaping (FSIGIDSignInResult?, Error?) -> Void ) -> NSException? { #if os(macOS) let presenting = viewProvider.view?.window #else let presenting = topViewController #endif return GoogleSignInCatchException { self.signIn.signIn( withPresenting: presenting, hint: hint, additionalScopes: additionalScopes, nonce: nonce, completion: completion) } }

This same feedback applies for addScopes as well.

Moved the catcher so it only wraps the GID SDK call in performSignIn. The Swift completion/handleAuthResult path is no longer inside @try. Same NSException → FlutterError mapping

cbracken · 2026-09-04T00:41:00Z

+          error: error,
+          completion: completion)
+      }
+    }


Same feedback as above on the Obj-C/Swift exception handling.

Same change in performAddScopes: the catcher now wraps only the GID SDK call, not the Swift completion/handleAuthResult path

… Sign-In - Updated `handleURLs` method to return a boolean indicating if any URLs were handled by Google Sign-In. - Added tests to verify the correct handling of URLs and the return value of `handleURLs`. - Updated CHANGELOG to reflect the changes in URL handling behavior.

flutter-dashboard Bot added the CICD Run CI/CD label Aug 27, 2026

github-actions Bot added p: google_sign_in platform-ios platform-macos labels Aug 27, 2026

victogomez-cs added the triage-ios Should be looked at in iOS triage label Aug 27, 2026

gemini-code-assist Bot reviewed Aug 27, 2026

View reviewed changes

victogomez-cs force-pushed the pr1/google-sign-in-ios-spm-packaging branch from c2332fb to 2cf54ae Compare August 27, 2026 17:37

victogomez-cs force-pushed the pr2/google-sign-in-ios-swift-plugin branch from f4ccb35 to 34f5240 Compare August 27, 2026 17:37

LouiseHsu requested review from cbracken and okorohelijah and removed request for okorohelijah August 27, 2026 21:56

victogomez-cs force-pushed the pr1/google-sign-in-ios-spm-packaging branch from 2cf54ae to b306c56 Compare August 28, 2026 17:22

victogomez-cs force-pushed the pr2/google-sign-in-ios-swift-plugin branch from 34f5240 to 9dce861 Compare August 28, 2026 17:22

victogomez-cs force-pushed the pr1/google-sign-in-ios-spm-packaging branch from b306c56 to a6041e6 Compare September 2, 2026 18:43

victogomez-cs force-pushed the pr2/google-sign-in-ios-swift-plugin branch from 9dce861 to 93fbe9d Compare September 2, 2026 18:43

Base automatically changed from pr1/google-sign-in-ios-spm-packaging to main September 2, 2026 20:42

[google_sign_in_ios] Migrate the plugin class from Objective-C to Swift.

392df80

victogomez-cs force-pushed the pr2/google-sign-in-ios-swift-plugin branch from 93fbe9d to 392df80 Compare September 2, 2026 21:11

victogomez-cs added 2 commits September 3, 2026 12:22

[google_sign_in_ios] Fix ExceptionCatcher.h format for improved reada…

91824b2

…bility

[google_sign_in_ios] Add CocoaPods placeholder header for Swift impor…

d73aa04

…t compatibility

victogomez-cs mentioned this pull request Sep 3, 2026

[google_sign_in] PR 4/4 Convert the Pigeon host API from Objective-C to Swift #12658

Open

11 tasks

cbracken requested changes Sep 4, 2026

View reviewed changes

Uh oh!

Conversation

victogomez-cs commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

google-cla Bot commented Aug 27, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

victogomez-cs commented Aug 27, 2026 •

edited

Loading

cbracken Sep 3, 2026 •

edited

Loading

cbracken Sep 3, 2026 •

edited

Loading

cbracken Sep 4, 2026 •

edited

Loading

cbracken Sep 4, 2026 •

edited

Loading