[google_map_flutter] Fix map object regression due to async changes (#4171)

In #4067 I changed several implicitly unawaited futures to `await`, not noticing that doing so would change the timing on updating fields that were used to compute diffs for future updates. Since the update flow is unawaited at higher levels, this meant that multiple calls to update map objects in rapid succession would compute incorrect diffs due to using the wrong base objects. This fixes that by restoring the old flow using `unawaited`.

Adds new tests that introduce synthetic awaits into the fake platform interface object and perform multiple updates in a row that should have different base object sets, which fail without the fix.

In order to simplify adding that behavior to the fake, and to pay down technical debt in the unit tests, I replaced the old test fake that is based on method channel interception (which predates the federation of the plugin and is implicitly relying on the method channel implementation from a different package never changing) with a fake platform interface implementation (substantially simplifying the fake).

Fixes flutter/flutter#128042

Author: stuartmorgan-g

packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md

@@ -1,5 +1,7 @@
-## NEXT
+## 2.3.1
 
+* Fixes a regression from 2.2.8 that could cause incorrect handling of a
+  rapid series of map object updates.
 * Fixes stale ignore: prefer_const_constructors.
 * Updates minimum supported SDK version to Flutter 3.10/Dart 3.0.
 

packages/google_maps_flutter/google_maps_flutter/lib/src/google_map.dart

@@ -360,41 +360,41 @@ class _GoogleMapState extends State<GoogleMap> {
       return;
     }
     final GoogleMapController controller = await _controller.future;
-    await controller._updateMapConfiguration(updates);
+    unawaited(controller._updateMapConfiguration(updates));
     _mapConfiguration = newConfig;
   }
 
   Future<void> _updateMarkers() async {
     final GoogleMapController controller = await _controller.future;
-    await controller._updateMarkers(
-        MarkerUpdates.from(_markers.values.toSet(), widget.markers));
+    unawaited(controller._updateMarkers(
+        MarkerUpdates.from(_markers.values.toSet(), widget.markers)));
     _markers = keyByMarkerId(widget.markers);
   }
 
   Future<void> _updatePolygons() async {
     final GoogleMapController controller = await _controller.future;
-    await controller._updatePolygons(
-        PolygonUpdates.from(_polygons.values.toSet(), widget.polygons));
+    unawaited(controller._updatePolygons(
+        PolygonUpdates.from(_polygons.values.toSet(), widget.polygons)));
     _polygons = keyByPolygonId(widget.polygons);
   }
 
   Future<void> _updatePolylines() async {
     final GoogleMapController controller = await _controller.future;
-    await controller._updatePolylines(
-        PolylineUpdates.from(_polylines.values.toSet(), widget.polylines));
+    unawaited(controller._updatePolylines(
+        PolylineUpdates.from(_polylines.values.toSet(), widget.polylines)));
     _polylines = keyByPolylineId(widget.polylines);
   }
 
   Future<void> _updateCircles() async {
     final GoogleMapController controller = await _controller.future;
-    await controller._updateCircles(
-        CircleUpdates.from(_circles.values.toSet(), widget.circles));
+    unawaited(controller._updateCircles(
+        CircleUpdates.from(_circles.values.toSet(), widget.circles)));
     _circles = keyByCircleId(widget.circles);
   }
 
   Future<void> _updateTileOverlays() async {
     final GoogleMapController controller = await _controller.future;
-    await controller._updateTileOverlays(widget.tileOverlays);
+    unawaited(controller._updateTileOverlays(widget.tileOverlays));
   }
 
   Future<void> onPlatformViewCreated(int id) async {

packages/google_maps_flutter/google_maps_flutter/pubspec.yaml

@@ -2,7 +2,7 @@ name: google_maps_flutter
 description: A Flutter plugin for integrating Google Maps in iOS and Android applications.
 repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
-version: 2.3.0
+version: 2.3.1
 
 environment:
   sdk: ">=3.0.0 <4.0.0"

packages/google_maps_flutter/google_maps_flutter/test/circle_updates_test.dart

@@ -2,12 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'package:flutter/services.dart';
 import 'package:flutter/widgets.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:google_maps_flutter/google_maps_flutter.dart';
+import 'package:google_maps_flutter_platform_interface/google_maps_flutter_platform_interface.dart';
 
-import 'fake_maps_controllers.dart';
+import 'fake_google_maps_flutter_platform.dart';
 
 Widget _mapWithCircles(Set<Circle> circles) {
   return Directionality(
@@ -20,36 +20,24 @@ Widget _mapWithCircles(Set<Circle> circles) {
 }
 
 void main() {
-  TestWidgetsFlutterBinding.ensureInitialized();
-
-  final FakePlatformViewsController fakePlatformViewsController =
-      FakePlatformViewsController();
-
-  setUpAll(() {
-    _ambiguate(TestDefaultBinaryMessengerBinding.instance)!
-        .defaultBinaryMessenger
-        .setMockMethodCallHandler(
-          SystemChannels.platform_views,
-          fakePlatformViewsController.fakePlatformViewsMethodHandler,
-        );
-  });
+  late FakeGoogleMapsFlutterPlatform platform;
 
   setUp(() {
-    fakePlatformViewsController.reset();
+    platform = FakeGoogleMapsFlutterPlatform();
+    GoogleMapsFlutterPlatform.instance = platform;
   });
 
   testWidgets('Initializing a circle', (WidgetTester tester) async {
     const Circle c1 = Circle(circleId: CircleId('circle_1'));
     await tester.pumpWidget(_mapWithCircles(<Circle>{c1}));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
-    expect(platformGoogleMap.circlesToAdd.length, 1);
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
+    expect(map.circleUpdates.last.circlesToAdd.length, 1);
 
-    final Circle initializedCircle = platformGoogleMap.circlesToAdd.first;
+    final Circle initializedCircle = map.circleUpdates.last.circlesToAdd.first;
     expect(initializedCircle, equals(c1));
-    expect(platformGoogleMap.circleIdsToRemove.isEmpty, true);
-    expect(platformGoogleMap.circlesToChange.isEmpty, true);
+    expect(map.circleUpdates.last.circleIdsToRemove.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToChange.isEmpty, true);
   });
 
   testWidgets('Adding a circle', (WidgetTester tester) async {
@@ -59,16 +47,15 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(<Circle>{c1}));
     await tester.pumpWidget(_mapWithCircles(<Circle>{c1, c2}));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
-    expect(platformGoogleMap.circlesToAdd.length, 1);
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
+    expect(map.circleUpdates.last.circlesToAdd.length, 1);
 
-    final Circle addedCircle = platformGoogleMap.circlesToAdd.first;
+    final Circle addedCircle = map.circleUpdates.last.circlesToAdd.first;
     expect(addedCircle, equals(c2));
 
-    expect(platformGoogleMap.circleIdsToRemove.isEmpty, true);
+    expect(map.circleUpdates.last.circleIdsToRemove.isEmpty, true);
 
-    expect(platformGoogleMap.circlesToChange.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToChange.isEmpty, true);
   });
 
   testWidgets('Removing a circle', (WidgetTester tester) async {
@@ -77,13 +64,12 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(<Circle>{c1}));
     await tester.pumpWidget(_mapWithCircles(<Circle>{}));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
-    expect(platformGoogleMap.circleIdsToRemove.length, 1);
-    expect(platformGoogleMap.circleIdsToRemove.first, equals(c1.circleId));
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
+    expect(map.circleUpdates.last.circleIdsToRemove.length, 1);
+    expect(map.circleUpdates.last.circleIdsToRemove.first, equals(c1.circleId));
 
-    expect(platformGoogleMap.circlesToChange.isEmpty, true);
-    expect(platformGoogleMap.circlesToAdd.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToChange.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToAdd.isEmpty, true);
   });
 
   testWidgets('Updating a circle', (WidgetTester tester) async {
@@ -93,13 +79,12 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(<Circle>{c1}));
     await tester.pumpWidget(_mapWithCircles(<Circle>{c2}));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
-    expect(platformGoogleMap.circlesToChange.length, 1);
-    expect(platformGoogleMap.circlesToChange.first, equals(c2));
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
+    expect(map.circleUpdates.last.circlesToChange.length, 1);
+    expect(map.circleUpdates.last.circlesToChange.first, equals(c2));
 
-    expect(platformGoogleMap.circleIdsToRemove.isEmpty, true);
-    expect(platformGoogleMap.circlesToAdd.isEmpty, true);
+    expect(map.circleUpdates.last.circleIdsToRemove.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToAdd.isEmpty, true);
   });
 
   testWidgets('Updating a circle', (WidgetTester tester) async {
@@ -109,11 +94,10 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(<Circle>{c1}));
     await tester.pumpWidget(_mapWithCircles(<Circle>{c2}));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
-    expect(platformGoogleMap.circlesToChange.length, 1);
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
+    expect(map.circleUpdates.last.circlesToChange.length, 1);
 
-    final Circle update = platformGoogleMap.circlesToChange.first;
+    final Circle update = map.circleUpdates.last.circlesToChange.first;
     expect(update, equals(c2));
     expect(update.radius, 10);
   });
@@ -129,12 +113,11 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(prev));
     await tester.pumpWidget(_mapWithCircles(cur));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
 
-    expect(platformGoogleMap.circlesToChange, cur);
-    expect(platformGoogleMap.circleIdsToRemove.isEmpty, true);
-    expect(platformGoogleMap.circlesToAdd.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToChange, cur);
+    expect(map.circleUpdates.last.circleIdsToRemove.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToAdd.isEmpty, true);
   });
 
   testWidgets('Multi Update', (WidgetTester tester) async {
@@ -150,16 +133,15 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(prev));
     await tester.pumpWidget(_mapWithCircles(cur));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
 
-    expect(platformGoogleMap.circlesToChange.length, 1);
-    expect(platformGoogleMap.circlesToAdd.length, 1);
-    expect(platformGoogleMap.circleIdsToRemove.length, 1);
+    expect(map.circleUpdates.last.circlesToChange.length, 1);
+    expect(map.circleUpdates.last.circlesToAdd.length, 1);
+    expect(map.circleUpdates.last.circleIdsToRemove.length, 1);
 
-    expect(platformGoogleMap.circlesToChange.first, equals(c2));
-    expect(platformGoogleMap.circlesToAdd.first, equals(c1));
-    expect(platformGoogleMap.circleIdsToRemove.first, equals(c3.circleId));
+    expect(map.circleUpdates.last.circlesToChange.first, equals(c2));
+    expect(map.circleUpdates.last.circlesToAdd.first, equals(c1));
+    expect(map.circleUpdates.last.circleIdsToRemove.first, equals(c3.circleId));
   });
 
   testWidgets('Partial Update', (WidgetTester tester) async {
@@ -173,12 +155,11 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(prev));
     await tester.pumpWidget(_mapWithCircles(cur));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
 
-    expect(platformGoogleMap.circlesToChange, <Circle>{c3});
-    expect(platformGoogleMap.circleIdsToRemove.isEmpty, true);
-    expect(platformGoogleMap.circlesToAdd.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToChange, <Circle>{c3});
+    expect(map.circleUpdates.last.circleIdsToRemove.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToAdd.isEmpty, true);
   });
 
   testWidgets('Update non platform related attr', (WidgetTester tester) async {
@@ -190,17 +171,42 @@ void main() {
     await tester.pumpWidget(_mapWithCircles(prev));
     await tester.pumpWidget(_mapWithCircles(cur));
 
-    final FakePlatformGoogleMap platformGoogleMap =
-        fakePlatformViewsController.lastCreatedView!;
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
 
-    expect(platformGoogleMap.circlesToChange.isEmpty, true);
-    expect(platformGoogleMap.circleIdsToRemove.isEmpty, true);
-    expect(platformGoogleMap.circlesToAdd.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToChange.isEmpty, true);
+    expect(map.circleUpdates.last.circleIdsToRemove.isEmpty, true);
+    expect(map.circleUpdates.last.circlesToAdd.isEmpty, true);
   });
-}
 
-/// This allows a value of type T or T? to be treated as a value of type T?.
-///
-/// We use this so that APIs that have become non-nullable can still be used
-/// with `!` and `?` on the stable branch.
-T? _ambiguate<T>(T? value) => value;
+  testWidgets('multi-update with delays', (WidgetTester tester) async {
+    platform.simulatePlatformDelay = true;
+
+    const Circle c1 = Circle(circleId: CircleId('circle_1'));
+    const Circle c2 = Circle(circleId: CircleId('circle_2'));
+    const Circle c3 = Circle(circleId: CircleId('circle_3'), radius: 1);
+    const Circle c3updated = Circle(circleId: CircleId('circle_3'), radius: 10);
+
+    // First remove one and add another, then update the new one.
+    await tester.pumpWidget(_mapWithCircles(<Circle>{c1, c2}));
+    await tester.pumpWidget(_mapWithCircles(<Circle>{c1, c3}));
+    await tester.pumpWidget(_mapWithCircles(<Circle>{c1, c3updated}));
+
+    final PlatformMapStateRecorder map = platform.lastCreatedMap;
+
+    expect(map.circleUpdates.length, 3);
+
+    expect(map.circleUpdates[0].circlesToChange.isEmpty, true);
+    expect(map.circleUpdates[0].circlesToAdd, <Circle>{c1, c2});
+    expect(map.circleUpdates[0].circleIdsToRemove.isEmpty, true);
+
+    expect(map.circleUpdates[1].circlesToChange.isEmpty, true);
+    expect(map.circleUpdates[1].circlesToAdd, <Circle>{c3});
+    expect(map.circleUpdates[1].circleIdsToRemove, <CircleId>{c2.circleId});
+
+    expect(map.circleUpdates[2].circlesToChange, <Circle>{c3updated});
+    expect(map.circleUpdates[2].circlesToAdd.isEmpty, true);
+    expect(map.circleUpdates[2].circleIdsToRemove.isEmpty, true);
+
+    await tester.pumpAndSettle();
+  });
+}