[stuartmorgan-g]

Switches the async authenticate method from the old callback system to Pigeon's new suspend support. Because the underlying SDK doesn't have a suspend-style method currently, the plugin bridges the existing intrenal callback to a suspend coroutine.

This will simplify later conversion to JNI-backed Pigeon, and also simplifies unit tests.

Part of flutter/flutter#192417

Pre-Review Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the AI contribution guidelines and understand my responsibilities, or I am not using AI tools.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter.
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I followed the version and CHANGELOG instructions, using semantic versioning and the repository CHANGELOG style, or I have commented below to indicate which documented exception this PR falls under¹.
• I updated/added any relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under¹.
• All existing and new tests are passing.

Footnotes

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

[gemini-code-assist]

Code Review

This pull request updates the internal platform API of the Android local authentication plugin from callbacks to Kotlin coroutines (suspend functions), updating the Pigeon-generated messages and modifying the corresponding unit tests to run within runTest. A review comment identifies a potential issue in createAuthCompletionHandler where the suspended authenticate coroutine could hang indefinitely if stopAuthentication() is called, and suggests using a local AtomicBoolean to guarantee the continuation is resumed exactly once.

[gemini-code-assist]

If stopAuthentication() is called, it sets authInProgress to false and cancels the authentication. This asynchronously triggers onAuthenticationError in AuthenticationHelper, which invokes this completion handler.\n\nHowever, because authInProgress has already been set to false by stopAuthentication(), the check authInProgress.compareAndSet(true, false) will fail, and continuation.resume(authResult) will never be called. This causes the suspended authenticate coroutine to hang indefinitely, leaking resources and preventing the Dart Future from completing.\n\nUsing a local AtomicBoolean to ensure the continuation is resumed exactly once, while safely attempting to clear the global authInProgress state, resolves this hang and avoids potential race conditions with subsequent authentication attempts.

  internal fun createAuthCompletionHandler(
      continuation: Continuation<AuthResult>
  ): (AuthResult) -> Unit {
    val resumed = AtomicBoolean(false)
    return { authResult ->
      if (resumed.compareAndSet(false, true)) {
        authInProgress.compareAndSet(true, false)
        continuation.resume(authResult)
      }
    }
  }

[stuartmorgan-g]

This was bothering me when I was touching the code as well, so I had checked the history and it's pretty much always worked this way so I let it go. But it does seem like it if works it's because it happens not to actually be async, which doesn't seem like a good thing to rely on. I updated the PR to remove the authInProgress.set(false) line from cancel, so that it will be set to false by the callback actually triggering instead. (I verified locally with a synthetic timed cancel that this works as expected.)

[stuartmorgan-g]

The test changes here were Gemini-authored, but (with the caveat that I'm not familiar with the specific test utilities involved here) looked good to me, and had the simplifications of directly using returned values that I was expecting from the refactor.

[mboetger]

Sorry I didn't get to this before your last day - happy to take over from here if you want. Just let me know.

[stuartmorgan]

@mboetger You'll need to do a CLA override on this; the automagic group stuff has updated such that my old @google.com address fails the CLA now. We've run into this before, and I always just put something like "Author was employed at Google at time the PR was opened".

[mboetger]

@mboetger You'll need to do a CLA override on this; the automagic group stuff has updated such that my old @google.com address fails the CLA now. We've run into this before, and I always just put something like "Author was employed at Google at time the PR was opened".

Done.