[google_maps_flutter] Convert simple object controllers to Swift by stuartmorgan-g · Pull Request #12618 · flutter/packages

stuartmorgan-g · 2026-08-25T17:12:48Z

This converts CircleController, PolygonController, and PolylineController to Swift in the _sdk* packages.

The remaining Obj-C code will be migrated in a series of follow-up PRs to keep the scope of each PR reasonable for review.

The conversion process was:

Initial conversion via Gemini, with explicit instruction to keep the structure the same.
Side-by-side manual review of the old and new versions of the files.
Manual fixes and improvements.

This includes test changes beyond what we would normally expect for a language conversion PR (e.g., name changes from dropping the prefix) because the conversion to Swift exposed a latent issue in the tests. Several tests had a pattern where a helper method was creating a local mapView instance, using it to construct the subcontroller under test, and then returning that subcontroller—at which point the map view goes out of scope. The subcontrollers have weak references to the map view, so it can be deallocated before the test actually runs, and doing so causes the tests to fail (as many operations in the subcontroller no-op when the map view is nil). In cases where the helper is only used once, it can be inlined; in cases where it's used repeatedly we can instead return multiple values and keep the map view alive.

Part of flutter/flutter#119108

Pre-Review Checklist

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 converts the circle, polygon, and polyline controllers from Objective-C to Swift in the iOS Google Maps plugin packages. The Objective-C implementation files are removed and replaced with new Swift controllers (CircleController, PolygonController, and PolylineController), with corresponding updates to bridging headers, unit tests, and GoogleMapController. Feedback on these changes suggests simplifying the initializers of the new Swift controllers by using default parameterless initializers and removing redundant parameters, which also allows for the simplification of the add methods in the collection controllers. Additionally, it is recommended to declare the polyline property as a constant.

gemini-code-assist · 2026-08-25T17:14:50Z

+  init(circle: FGMPlatformCircle, mapView: GMSMapView) {
+    let center = FGMGetCoordinateForPigeonLatLng(circle.center)
+    self.circle = GMSCircle(position: center, radius: circle.radius)
+    self.mapView = mapView
+    self.circle.userData = [circle.circleId]
+    super.init()
+    CircleController.update(self.circle, from: circle, with: mapView)
+  }


The initializer redundantly calculates the coordinate and sets the position and radius on GMSCircle, which are immediately overwritten when CircleController.update is called. We can simplify this by using the default parameterless initializer GMSCircle() and letting update handle the property assignment.

init(circle: FGMPlatformCircle, mapView: GMSMapView) { self.circle = GMSCircle() self.mapView = mapView self.circle.userData = [circle.circleId] super.init() CircleController.update(self.circle, from: circle, with: mapView) }

Seems like a reasonable cleanup to do even as part of a language migration; done.

gemini-code-assist · 2026-08-25T17:14:50Z

+  init(path: GMSMutablePath, identifier: String, mapView: GMSMapView) {
+    self.polygon = GMSPolygon(path: path)
+    self.mapView = mapView
+    self.polygon.userData = [identifier]
+    super.init()
+  }


The initializer takes a path parameter and sets it on GMSPolygon, but this path is immediately recalculated and overwritten when update(from:) is called in PolygonsController.add. We can simplify the initializer by using the default parameterless initializer GMSPolygon() and removing the unused path parameter.

Suggested change

init(path: GMSMutablePath, identifier: String, mapView: GMSMapView) {

self.polygon = GMSPolygon(path: path)

self.mapView = mapView

self.polygon.userData = [identifier]

super.init()

}

init(identifier: String, mapView: GMSMapView) {

self.polygon = GMSPolygon()

self.mapView = mapView

self.polygon.userData = [identifier]

super.init()

}

(Also, I have no idea why Circle's add/init/update structure is different from Polygon/Polyline. They should probably all match Circle, but that's out of scope so I'll just leave a TODO suggesting it.)

gemini-code-assist · 2026-08-25T17:14:50Z

+  private(set) var polyline: GMSPolyline
+  private weak var mapView: GMSMapView?
+
+  init(path: GMSMutablePath, identifier: String, mapView: GMSMapView) {
+    self.polyline = GMSPolyline(path: path)
+    self.mapView = mapView
+    self.polyline.userData = [identifier]
+    super.init()
+  }


The polyline property is never reassigned, so it can be declared as a constant let instead of private(set) var. Additionally, the initializer takes a path parameter and sets it on GMSPolyline, but this path is immediately recalculated and overwritten when update(from:) is called in PolylinesController.add. We can simplify the initializer by using the default parameterless initializer GMSPolyline() and removing the unused path parameter.

Suggested change

private(set) var polyline: GMSPolyline

private weak var mapView: GMSMapView?

init(path: GMSMutablePath, identifier: String, mapView: GMSMapView) {

self.polyline = GMSPolyline(path: path)

self.mapView = mapView

self.polyline.userData = [identifier]

super.init()

}

let polyline: GMSPolyline

private weak var mapView: GMSMapView?

init(identifier: String, mapView: GMSMapView) {

self.polyline = GMSPolyline()

self.mapView = mapView

self.polyline.userData = [identifier]

super.init()

}

vashworth · 2026-08-26T15:25:13Z

+
+  func change(_ circles: [FGMPlatformCircle]) {
+    for circle in circles {
+      circleIdToController[circle.circleId]?.update(from: circle)


This doesn't check if a nullable mapView? Does it need to?

See comment below.

vashworth · 2026-08-26T15:25:41Z

+  ///
+  /// Setting the circle to visible will set its map to the given mapView.
+  func update(from platformCircle: FGMPlatformCircle) {
+    if let mapView = mapView {


I'm a little uncertain on the nullability here. In the ObjC, if mapView is null, it would still call update, wouldn't it?

packages/packages/google_maps_flutter/google_maps_flutter_ios_shared_code/ios/google_maps_flutter_ios/Sources/google_maps_flutter_ios_objc/FGMCircleController.m

Lines 37 to 39 in 2c65fb5

[FGMCircleController updateCircle:self.circle

fromPlatformCircle:platformCircle

withMapView:self.mapView];

Maybe there's no implications, just a discrepancy to point out

Basically, if mapView is null a) nothing matters, and b) these methods probably can't even be called in the first place.

So in the Obj-C code, the declarations in the test headers said that the mapView was non-nullable, so the Obj-C code referenced above was "wrong" (but nullability in Obj-C is a sign, not a cop), which is why the code changed; I kept that nullability annotation, and fixed the call site that was technically violating the requirements.

We could also fix the Swift compilation just by making the mapView parameter nullable instead, and removing all the checks. It was kind of a coin flip because of the "nothing matters" bit: mapView can never be set externally, and is nullable only because it's weak. If mapView is null, than means the map has gone away and we are somehow processing a message that Dart sent us to make updates to a map that doesn't exist any more. I don't think this can happen in the first place because the communication channel should have been torn down with the map, but even if it does happen, it means the map is gone. So we could still convert all the Pigeon data into a map object if we wanted to, and then at the last step we would just... not add that object to a map. And we never will because the map view can't become non-null again later.

So how we should handle the null map view is basically a philosophical question: if a circle is created in a map plugin, but never shown on a map, does it still have a radius? 🤔

I came down on the side of short-circuiting if somehow we would be trying to convert objects in a pointless scenario, but there's also a perfectly good argument for simplifying the code by making update take a nullable map view so we don't need checks, so if you think it'll be easier to understand that way I'm happy to change it.

Okay that makes sense. I was thinking it probably didn't matter, but just wanted to check what you thought

vashworth · 2026-08-26T15:27:10Z

+  }
+
+  func add(_ circles: [FGMPlatformCircle]) {
+    guard let mapView = mapView else { return }


Again, the ObjC version doesn't check if the mapView is nullable

vashworth

LGTM other than a couple questions

…tter#12618) This converts CircleController, PolygonController, and PolylineController to Swift in the `_sdk*` packages. The remaining Obj-C code will be migrated in a series of follow-up PRs to keep the scope of each PR reasonable for review. The conversion process was: - Initial conversion via Gemini, with explicit instruction to keep the structure the same. - Side-by-side manual review of the old and new versions of the files. - Manual fixes and improvements. This includes test changes beyond what we would normally expect for a language conversion PR (e.g., name changes from dropping the prefix) because the conversion to Swift exposed a latent issue in the tests. Several tests had a pattern where a helper method was creating a local `mapView` instance, using it to construct the subcontroller under test, and then returning that subcontroller—at which point the map view goes out of scope. The subcontrollers have weak references to the map view, so it can be deallocated before the test actually runs, and doing so causes the tests to fail (as many operations in the subcontroller no-op when the map view is nil). In cases where the helper is only used once, it can be inlined; in cases where it's used repeatedly we can instead return multiple values and keep the map view alive. Part of flutter/flutter#119108 ## Pre-Review Checklist [^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.

…r#191886) flutter/packages@740f093...bd3cbc1 2026-08-26 engine-flutter-autoroll@skia.org Roll Flutter from 9a82789 to 15d8908 (37 revisions) (flutter/packages#12619) 2026-08-26 stuartmorgan@google.com [google_maps_flutter] Convert simple object controllers to Swift (flutter/packages#12618) 2026-08-26 21270878+elliette@users.noreply.github.com [material_ui] `gen_defaults` should follow new versioning strategy (flutter/packages#12627) 2026-08-25 10687576+bparrishMines@users.noreply.github.com [material_ui] Fixes analysis_options migration from calling `flutter pub get` on pacakage (flutter/packages#12626) 2026-08-25 jason-simmons@users.noreply.github.com [material_ui] Declare Flutter SDK dependencies in the pubspec.yaml for tool/gen_defaults (flutter/packages#12623) 2026-08-25 21270878+elliette@users.noreply.github.com [material_ui] Set-up `tool/gen_defaults` sub-directory (flutter/packages#12469) 2026-08-25 stuartmorgan@google.com [google_maps_flutter] Convert top-level classes to Swift in `_sdk*` packages (flutter/packages#12307) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com 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

…tter#12618) This converts CircleController, PolygonController, and PolylineController to Swift in the `_sdk*` packages. The remaining Obj-C code will be migrated in a series of follow-up PRs to keep the scope of each PR reasonable for review. The conversion process was: - Initial conversion via Gemini, with explicit instruction to keep the structure the same. - Side-by-side manual review of the old and new versions of the files. - Manual fixes and improvements. This includes test changes beyond what we would normally expect for a language conversion PR (e.g., name changes from dropping the prefix) because the conversion to Swift exposed a latent issue in the tests. Several tests had a pattern where a helper method was creating a local `mapView` instance, using it to construct the subcontroller under test, and then returning that subcontroller—at which point the map view goes out of scope. The subcontrollers have weak references to the map view, so it can be deallocated before the test actually runs, and doing so causes the tests to fail (as many operations in the subcontroller no-op when the map view is nil). In cases where the helper is only used once, it can be inlined; in cases where it's used repeatedly we can instead return multiple values and keep the map view alive. Part of flutter/flutter#119108 ## Pre-Review Checklist [^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.

stuartmorgan-g added 5 commits August 25, 2026 13:01

Initial Gemini conversions

774c944

Minor cleanup

6a53593

Fix references and tests

d9a3057

Sync changes

04013e9

Version bump

16bc76a

stuartmorgan-g requested a review from vashworth August 25, 2026 17:12

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

gemini-code-assist Bot reviewed Aug 25, 2026

View reviewed changes

github-actions Bot added p: google_maps_flutter platform-ios labels Aug 25, 2026

stuartmorgan-g added 2 commits August 25, 2026 14:23

GCAfGH review

e1bb423

Conditionalize imports

a02d361

vashworth reviewed Aug 26, 2026

View reviewed changes

vashworth approved these changes Aug 26, 2026

View reviewed changes

stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2026

auto-submit Bot merged commit 4797ae2 into flutter:main Aug 26, 2026
13 checks passed

stuartmorgan-g mentioned this pull request Aug 27, 2026

iOS: Key the platform view factory registries by NSString flutter/flutter#191855

Merged

10 tasks done

engine-flutter-autoroll mentioned this pull request Aug 27, 2026

Roll Packages from 740f09361ccc to bd3cbc1b8c3f (7 revisions) flutter/flutter#191886

Merged

	[FGMCircleController updateCircle:self.circle
	fromPlatformCircle:platformCircle
	withMapView:self.mapView];

Conversation

stuartmorgan-g commented Aug 25, 2026

Pre-Review Checklist

Footnotes

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 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 25, 2026

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!

Uh oh!

gemini-code-assist Bot Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

vashworth Aug 26, 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!

stuartmorgan-g Aug 26, 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!

vashworth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vashworth Aug 26, 2026 •

edited

Loading

stuartmorgan-g Aug 26, 2026 •

edited

Loading