[google_maps_flutter] Convert top-level classes to Swift in `_sdk*` packages by stuartmorgan-g · Pull Request #12307 · flutter/packages

stuartmorgan-g · 2026-07-29T15:31:47Z

This converts GoogleMapsPlugin, GoogleMapFactory, and GoogleMapController to Swift, and restructures the source code to typical migration state of having the Swift code be the primary target, depending on a new *_objc target that contains the unmigrated code. Because google_maps_flutter_ios cannot adopt Swift (see discussion in linked issue), this removes that package from the code sharing. Going forward, development will be limited to the _sdk* packages.

For reasons explained here, the Pigeon API layer is still Objective-C for now, which does lead to some temporary non-ideal code in GoogleMapController. That will be cleaned up in the last step when we are able to switch to Swift Pigeon generation.

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.

The last step was more extensive than usual because the init method had longstanding design issues (flutter/flutter#104121) that became hard errors under Swift. There was some refactoring of helper methods and reordering of initialization steps to make the code work with the strict two-phase init process.

Part of flutter/flutter#119108
Fixes flutter/flutter#104121

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 parts of the iOS Google Maps implementation to Swift across the SDK 9, SDK 10, and shared code packages, replacing several Objective-C controllers and plugins. The build configurations, synchronization tools, and tests are updated to support the new Swift files. Feedback on these changes includes correcting a regular expression typo in the utility script, addressing typos in documentation and comments, avoiding unsafe force-unwrapping and force-casting in the map factory, and resolving an inconsistency in how uninitialized map views are handled.

gemini-code-assist · 2026-07-29T15:35:22Z

+    -> NSNumber?
+  {
+    guard let mapView = controller?.mapView else {
+      return false


Returning false when mapView is nil is inconsistent with other uninitialized map checks in the same class (such as currentZoomLevel at line 723, which returns nil). It is safer and more consistent to return nil to represent the uninitialized state.

Suggested change

return false

return nil

This is intentional; if the map isn't present, it doesn't support advanced markers.

Is this a Swift thing? Like the return type is NSNumber?, but this is returning false. Does that automatically become 0?

It automatically becomes an NSNumber whose boolValue is false; Swift will essentially auto-box literals that are in an NSNumber context (vs Obj-C where we would have to write @(NO)).

stuartmorgan-g · 2026-07-29T15:36:40Z

We'll want to hold off on landing this until 3.47 reaches stable, since that's when nobody on stable will have an actual need to keep using google_maps_flutter_ios, making it reasonable to stop adding features to it. I wanted to have it reviewed and ready to go at that point though, so I'm opening it now.

@vashworth per offline discussion, this punts on the question of whether we switch the endorsed implementation to _sdk9. We can do that in a follow-up if we decide to go that route.

For review, it may be helpful to look at some of the individual commits. 5a27e87 in particular, which is where I reworked the initialization.

stuartmorgan-g · 2026-07-29T15:47:53Z

    #expect(mapView.frameObserverCount == 0)
  }

-  @Test func mapsServiceSync() {


This is removed because in the Swift version, all the custom logic for calling this exactly once was replaced by us being able to use Swift's lazy static handling, and since that's a core language behavior rather than custom logic, there's no value in testing it.

stuartmorgan-g · 2026-07-29T15:48:30Z

    )
  }
+
+  @Test func frameObserverRemovedOnDeinitIfNeverFired() {


Added to cover an issue found by Gemini in local review of the conversion.

stuartmorgan-g · 2026-07-29T15:48:57Z

+    #expect(mapView.frameObserverCount == 0)
+  }
+
+  @Test func styleErrorPersistsAcrossConfigUpdates() {


Added to cover a case I accidentally regressed when reworking init, also found by Gemini in a local review.

vashworth · 2026-08-20T18:54:16Z

  s.xcconfig = {
-    'LIBRARY_SEARCH_PATHS' => '$(inherited) $(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)/ $(SDKROOT)/usr/lib/swift',
-    'LD_RUNPATH_SEARCH_PATHS' => '$(inherited) /usr/lib/swift',
+    'LIBRARY_SEARCH_PATHS' => '$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)/ $(SDKROOT)/usr/lib/swift',


Why did $(inherited) need to be removed?

Our tooling's regex didn't handle it, and it seemed easier to make this match all of our other plugins than to change the tooling (given that I don't know of any reason this plugin would need to be different).

vashworth · 2026-08-21T17:22:15Z

+
+    mapView.delegate = self
+    isObservingFrame = true
+    mapView.addObserver(self, forKeyPath: "frame", options: [], context: nil)


In ObjC, options is 0, I'm assuming [] is the swift equivalent? Is the API different in swift?

Yes, this is the very nice auto-bridging from NS_OPTIONS to https://developer.apple.com/documentation/swift/optionset.

vashworth · 2026-08-21T19:15:40Z

+    if styleUpdateAttempted {
+      styleError = errorString
+    }
+    if let trackCameraPosition = config.trackCameraPosition {
+      self.trackCameraPosition = trackCameraPosition.boolValue
+    }


Question: Why does trackCameraPosition need to be accessed via self, but styleError doesn't?

Like Dart, in Swift the convention is to only use self when necessary to disambiguate from a local. Line 39 creates a local that shadows the ivar, but nothing shadows styleError.

Ah I see, thanks for explaining!

vashworth · 2026-08-21T19:26:02Z

+
+class MapCallHandler: NSObject, FGMMapsApi {
+  weak var controller: GoogleMapController?
+  let messenger: FlutterBinaryMessenger


In ObjC, this uses copy, is this going to maintain a strong reference?

copy was always wrong and only really worked by accident 😬 Strong refs to the messenger should be fine; we use a proxy object with an internal weak ref in the engine to avoid retain loops with the engine.

vashworth · 2026-08-21T19:30:15Z

+    controller?.markersController.add(toAdd)
+    controller?.markersController.change(toChange)
+    controller?.markersController.removeMarkers(withIdentifiers: idsToRemove)
+    controller?.clusterManagersController.invokeClusteringForEachClusterManager()


Add back comment

Suggested change

controller?.clusterManagersController.invokeClusteringForEachClusterManager()

// Invoke clustering after markers are added.

controller?.clusterManagersController.invokeClusteringForEachClusterManager()

Thanks, I thought I caught all of these and added the back. For some reason, Gemini loves removing implementation comments when doing conversions to Swift.

vashworth · 2026-08-21T20:06:17Z

+  weak var controller: GoogleMapController?
+  let messenger: FlutterBinaryMessenger
+  let pigeonSuffix: String
+  var transactionWrapper: FGMCATransactionProtocol


In ObjC, this is in a test header? Do we not need to do that in Swift?

This class isn't public, so we shouldn't need to worry about visibility of its members.

vashworth · 2026-08-21T20:14:49Z

+  // initialization.
+  var styleError: String?
+  /// Whether we are currently observing the "frame" key path on `mapView`.
+  private var isObservingFrame = false


This seems like new logic. It doesn't appear to be part of the self/init fixes. Is this fixing an issue? Perhaps add a comment somewhere on why it's needed

Yes, this is fixing an issue the local Gemini review flagged. We were removing the observer when it fired the first time, but if it never did we didn't remove the observer on dealloc, which is a potentially serious bug (since unlike notification center observers, docs don't indicate that Apple made this automagically safe). Added a better comment.

vashworth · 2026-08-21T20:19:50Z

+    )
+  }
+
+  public init(


I don't think this was public in the Objc version? It was in a Test category

packages/packages/google_maps_flutter/google_maps_flutter_ios_shared_code/ios/google_maps_flutter_ios/Sources/google_maps_flutter_ios/include/google_maps_flutter_ios/FGMGoogleMapController_Test.h

Lines 42 to 60 in 959f062

@interface FGMGoogleMapController (Test)

/// Initializes a map controller with a concrete map view.

///

/// @param mapView A map view that will be displayed by the controller

/// @param viewId A unique identifier for the controller.

/// @param creationParameters Parameters for initialising the map view.

/// @param assetProvider The asset provider to use for looking up assets.

/// @param binaryMessenger The binary messenger to use for sending messages to Dart.

- (instancetype)initWithMapView:(GMSMapView *)mapView

viewIdentifier:(int64_t)viewId

creationParameters:(FGMPlatformMapViewCreationParams *)creationParameters

assetProvider:(NSObject<FGMAssetProvider> *)assetProvider

binaryMessenger:(NSObject<FlutterBinaryMessenger> *)binaryMessenger;

// The main Pigeon API implementation.

@property(nonatomic, strong, readonly) FGMMapCallHandler *callHandler;

@end

Good catch! I didn't even register that in my review since public init seems natural.

vashworth · 2026-08-21T20:20:37Z

+    SetUpFGMMapsInspectorApiWithSuffix(inspector.messenger, nil, inspector.pigeonSuffix)
+  }
+
+  public func view() -> UIView {


Is this intentionally public? I don't think it was public in objc

As far as I can tell there's no such thing in Swift as non-public conformance to a public protocol, and as currently architected, GoogleMapController is the FlutterPlatformView, so must implement this method.

If you want, I can separate out a minimal GoogleMapPluginView to implement FlutterPlatformView and then make that create and own GoogleMapController. It would still need to expose this, but it would be on a different object with a clearer scope.

vashworth · 2026-08-21T20:29:13Z

+    return mapView
+  }
+
+  override public func observeValue(


Is this intentionally public?

It has to be, because the NSObject method is public.

vashworth · 2026-08-25T15:12:27Z

LGTM, but tests are failing on formatting stuff I think

stuartmorgan-g · 2026-08-25T15:22:33Z

Yep, I forgot to check for formatter behavior changes due to the min SDK bump.

stuartmorgan-g · 2026-08-25T15:26:24Z

Actually, I can just revert the min SDK bumps; these packages already required higher versions of iOS, so we don't need to bind the implementation changes in them to the SDK. I was just thinking about the overall change of freezing the SDK 8 version being something we needed to wait for 3.47 for, and then translated that into "bump the min SDK" in my mind at some point.

The only package where we would need an SDK bump would be the app-facing package, if/when we switch to _sdk9 as the default implementation.

This reverts commit cdba7da.

This reverts commit 6623dd8.

…ackages (flutter#12307) This converts GoogleMapsPlugin, GoogleMapFactory, and GoogleMapController to Swift, and restructures the source code to typical migration state of having the Swift code be the primary target, depending on a new `*_objc` target that contains the unmigrated code. Because `google_maps_flutter_ios` cannot adopt Swift (see discussion in linked issue), this removes that package from the code sharing. Going forward, development will be limited to the `_sdk*` packages. For reasons explained [here](flutter/flutter#119108 (comment)), the Pigeon API layer is still Objective-C for now, which does lead to some temporary non-ideal code in GoogleMapController. That will be cleaned up in the last step when we are able to switch to Swift Pigeon generation. 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. The last step was more extensive than usual because the init method had longstanding design issues (flutter/flutter#104121) that became hard errors under Swift. There was some refactoring of helper methods and reordering of initialization steps to make the code work with the strict two-phase init process. Part of flutter/flutter#119108 Fixes flutter/flutter#104121 ## 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

…ackages (flutter#12307) This converts GoogleMapsPlugin, GoogleMapFactory, and GoogleMapController to Swift, and restructures the source code to typical migration state of having the Swift code be the primary target, depending on a new `*_objc` target that contains the unmigrated code. Because `google_maps_flutter_ios` cannot adopt Swift (see discussion in linked issue), this removes that package from the code sharing. Going forward, development will be limited to the `_sdk*` packages. For reasons explained [here](flutter/flutter#119108 (comment)), the Pigeon API layer is still Objective-C for now, which does lead to some temporary non-ideal code in GoogleMapController. That will be cleaned up in the last step when we are able to switch to Swift Pigeon generation. 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. The last step was more extensive than usual because the init method had longstanding design issues (flutter/flutter#104121) that became hard errors under Swift. There was some refactoring of helper methods and reordering of initialization steps to make the code work with the strict two-phase init process. Part of flutter/flutter#119108 Fixes flutter/flutter#104121 ## 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 14 commits July 21, 2026 16:00

Detach google_maps_flutter_ios from the shared source

0e6566c

Move plugin and factory to Swift, move Obj-C code to new module

ef24c9c

Initial Gemini conversion of controller

c0b9512

Initial conversion cleanup

45b837c

Fix initialization ordering issues

5a27e87

Minor unit test adjustments

8866d41

Sync to other packages

289cfea

Adjustments to SDK 10 copies of unshared files

dca7980

Fix and test issues found in Gemini review

267914a

Minor fixes/improvements

b77ac82

Sync new changes

39266b1

Version bump

ac204d9

Finish removing code sharing from legacy package

5af484d

Add an initial README note to the default implementation package

0d5777a

stuartmorgan-g requested review from LongCatIsLooong and vashworth July 29, 2026 15:31

flutter-dashboard Bot added the CICD Run CI/CD label Jul 29, 2026

github-actions Bot added p: google_maps_flutter platform-ios labels Jul 29, 2026

gemini-code-assist Bot reviewed Jul 29, 2026

View reviewed changes

stuartmorgan-g added 2 commits July 29, 2026 11:36

Merge branch 'main' into maps-swift-top-level

7d60037

Gemini review

7874277

stuartmorgan-g commented Jul 29, 2026

View reviewed changes

stuartmorgan-g added 3 commits July 29, 2026 12:59

Adjust xcconfig value to match tool expectations

cfe4f10

Fix CocoaPods compilation

f9149fb

Sync CP fix

9f5b9a9

This comment was marked as off-topic.

Sign in to view

stuartmorgan-g mentioned this pull request Aug 17, 2026

Ergonomics of re-runs + autosubmit are worse under unified checks flutter/flutter#191005

Open

1 of 4 tasks

stuartmorgan-g added 2 commits August 17, 2026 17:03

Fix conditional import renaming

d28ede5

Bump min SDK

829d0b6

vashworth reviewed Aug 21, 2026

View reviewed changes

stuartmorgan-g added 2 commits August 25, 2026 10:49

Review fixes

6f8fbca

Merge branch 'main' into maps-swift-top-level

df89030

vashworth approved these changes Aug 25, 2026

View reviewed changes

stuartmorgan-g added 2 commits August 25, 2026 11:20

Add minimal 3.47 handling

6623dd8

autoformat

cdba7da

stuartmorgan-g added 6 commits August 25, 2026 11:26

Revert "autoformat"

6044d8a

This reverts commit cdba7da.

Revert "Add minimal 3.47 handling"

17b0d42

This reverts commit 6623dd8.

Revert SDK bump

62eb55a

Autoformat

c04499c

Revert SDK bumps in examples

53c52c1

Autoformat

a224b00

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

auto-submit Bot merged commit d1b2518 into flutter:main Aug 25, 2026
13 checks passed

RyanHolanda mentioned this pull request Aug 25, 2026

[google_maps_flutter] Add background color for unloaded tiles #12357

Open

11 tasks done

	@interface FGMGoogleMapController (Test)

	/// Initializes a map controller with a concrete map view.
	///
	/// @param mapView A map view that will be displayed by the controller
	/// @param viewId A unique identifier for the controller.
	/// @param creationParameters Parameters for initialising the map view.
	/// @param assetProvider The asset provider to use for looking up assets.
	/// @param binaryMessenger The binary messenger to use for sending messages to Dart.
	- (instancetype)initWithMapView:(GMSMapView *)mapView
	viewIdentifier:(int64_t)viewId
	creationParameters:(FGMPlatformMapViewCreationParams *)creationParameters
	assetProvider:(NSObject<FGMAssetProvider> *)assetProvider
	binaryMessenger:(NSObject<FlutterBinaryMessenger> *)binaryMessenger;

	// The main Pigeon API implementation.
	@property(nonatomic, strong, readonly) FGMMapCallHandler *callHandler;

	@end

Conversation

stuartmorgan-g commented Jul 29, 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Jul 29, 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!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Jul 29, 2026

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!

This comment was marked as off-topic.

This comment was marked as off-topic.

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!

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!

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

Choose a reason for hiding this comment

Uh oh!

vashworth commented Aug 25, 2026

Uh oh!

stuartmorgan-g commented Aug 25, 2026

Uh oh!

vashworth Aug 21, 2026 •

edited

Loading