Skip to content

[google_maps_flutter] Adopt Pigeon's Kotlin 'suspend' support - #12980

Merged
auto-submit[bot] merged 9 commits into
flutter:mainfrom
stuartmorgan-g:maps-async-kotlin-pigeon
Sep 28, 2026
Merged

[google_maps_flutter] Adopt Pigeon's Kotlin 'suspend' support#12980
auto-submit[bot] merged 9 commits into
flutter:mainfrom
stuartmorgan-g:maps-async-kotlin-pigeon

Conversation

@stuartmorgan-g

@stuartmorgan-g stuartmorgan-g commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Converts the Pigeon generation from the older callback system to the new Kotlin suspend support. While this is not any simpler while the code is Java, it has several benefits:

  • It makes future pigeon mass updates much simpler, since this is the new default generation mode.
  • It simplifies adoption of the JNI-based Pigeon backend, which always uses the suspend style.
  • It will simplify the tests when converting them to Kotlin.

Part of flutter/flutter#192417

Pre-Review Checklist

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. ↩ ↩2

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Sep 22, 2026
@stuartmorgan-g

stuartmorgan-g commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

The non-Pigeon changes were mostly Gemini-generated, but I reviewed them and they all look reasonable to me.

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 22, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the google_maps_flutter_android package to use Pigeon's new 'suspend' support for platform communication, refactoring asynchronous callback-based methods to Kotlin suspend functions and updating the generated Pigeon messages. Feedback on these changes identifies critical issues in ClusterManagersControllerTest and MarkersControllerTest where Mockito verification calls incorrectly use null instead of matchers, which will cause test failures. Additionally, a simplification is suggested in ResultUtils.kt to streamline exception resumption on continuations without requiring unchecked casts.

Comment thread ...droid/android/src/test/java/io/flutter/plugins/googlemaps/ClusterManagersControllerTest.java
Comment on lines 201 to +202
Mockito.verify(flutterApi)
.onClusterTap(
eq(Convert.clusterToPigeon(clusterManagerId, cluster)), ArgumentMatchers.any());
.onClusterTap(eq(Convert.clusterToPigeon(clusterManagerId, cluster)), null);

@gemini-code-assist gemini-code-assist Bot Sep 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using null here will cause Mockito to throw an InvalidUseOfMatchersException because you cannot mix matchers (like eq()) with raw values (like null) in a verify call. Additionally, the actual argument passed in production is ResultUtilsKt.emptyContinuation(), which is a non-null object, so verifying with null would fail the test. Use any() instead.

Suggested change
Mockito.verify(flutterApi)
.onClusterTap(
eq(Convert.clusterToPigeon(clusterManagerId, cluster)), ArgumentMatchers.any());
.onClusterTap(eq(Convert.clusterToPigeon(clusterManagerId, cluster)), null);
Mockito.verify(flutterApi)
.onClusterTap(eq(Convert.clusterToPigeon(clusterManagerId, cluster)), any());

@stuartmorgan-g stuartmorgan-g Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using null here will cause Mockito to throw an InvalidUseOfMatchersException because you cannot mix matchers (like eq()) with raw values (like null) in a verify call.

It doesn't.

Additionally, the actual argument passed in production is ResultUtilsKt.emptyContinuation(), which is a non-null object, so verifying with null would fail the test.

The part about failing is also not correct. The part about passing null to a non-null parameter is correct, and causes a warning in the IDE, but...

Use any() instead.

... this doesn't actually work. Local Gemini had this explanation in the summary when it made the changes:

Adjusted Mockito.verify calls to pass null for the synthetic Continuation parameter instead of a matcher to avoid Mockito's Kotlin suspend argument count mismatch.

I was curious about that, so I had tried passing any() instead and sure enough:

Invalid use of argument matchers!
2 matchers expected, 3 recorded:

Since the workaround works, and we don't actually care about the continuation since it's a no-op, this seems fine. The real fix is to just convert these test to Kotlin, at which point we won't have this problem in the first place, because this synthetic parameter is a Kotlin/Java compatibility artifact.

Comment thread ...utter_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java
Comment thread ...utter_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java
Comment thread ...utter_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java
Comment thread ...le_maps_flutter_android/android/src/main/kotlin/io/flutter/plugins/googlemaps/ResultUtils.kt
reidbaker
reidbaker approved these changes Sep 28, 2026
View reviewed changes

@reidbaker reidbaker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most code looks generated. I spot checked a couple of classes and the changes appear reasonable. changlog looks good.

@stuartmorgan-g
stuartmorgan-g force-pushed the maps-async-kotlin-pigeon branch from 4156963 to 8b17d92 Compare September 28, 2026 18:58
@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 28, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 28, 2026
@auto-submit

auto-submit Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/packages/12980, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 28, 2026
@auto-submit
auto-submit Bot merged commit e4ade63 into flutter:main Sep 28, 2026
13 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 29, 2026
Merged
pull Bot pushed a commit to g-star1024/flutter that referenced this pull request Sep 29, 2026
@engine-flutter-autoroll
Roll Packages from ba0364a650af to 3c6ce5905441 (14 revisions) (flutt…
3e4289c 
…er#193507)

flutter/packages@ba0364a...3c6ce59

2026-09-29 [email protected] [cupertino_icons] Migrate to
`cupertino_ui` (flutter/packages#12649)
2026-09-29 [email protected] Manual roll Flutter from
4fcd90b to c5a061b (222 revisions) (flutter/packages#13060)
2026-09-28 [email protected] Enable Built-in Kotlin in all Flutter
Android Plugin Example Apps (flutter/packages#12883)
2026-09-28 [email protected] Enable Built-in Kotlin for
`interactive_media_ads` (flutter/packages#12912)
2026-09-28 [email protected] [google_maps_flutter] Adopt Pigeon's
Kotlin 'suspend' support (flutter/packages#12980)
2026-09-28 [email protected] [material_ui] Fix floating-point
rounding errors for discrete Slider and RangeSlider
(flutter/packages#13022)
2026-09-28 [email protected] [google_sign_in] PR 4/4 Convert
the Pigeon host API from Objective-C to Swift (flutter/packages#12658)
2026-09-28 [email protected]
[google_maps_flutter] Disable flaky web tests blocking latest flutter ->
packages roll (flutter/packages#13047)
2026-09-28 [email protected] [material_ui] Add fontFeatures and
fontVariations to TextTheme.apply() (flutter/packages#12879)
2026-09-28 [email protected]
[in_app_purchase_storekit] Disable flaky tests blocking latest flutter
-> packages roll (flutter/packages#13048)
2026-09-28 [email protected] Sync release-material_ui-1.5.0 to
main (flutter/packages#13051)
2026-09-28 [email protected] Sync release-go_router-18.0.2 to
main (flutter/packages#13050)
2026-09-28 [email protected] [google_fonts] Migrate to material_ui &
cupertino_ui (flutter/packages#13006)
2026-09-28 [email protected] [go_router_builder] Reland material_ui
migration (flutter/packages#12995)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reidbaker reidbaker reidbaker approved these changes
@mboetger mboetger Awaiting requested review from mboetger
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD p: google_maps_flutter platform-android

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stuartmorgan-g @reidbaker