Skip to content

Commit 42bb347

Browse files
[google_maps_flutter] Fixes exception when dispose is called while asynchronous update from didUpdateWidget is executed (#9227)
This PR fixes an exception in Google Maps Flutter which occurs when dispose is called while asynchronous code is executed from didUpdateWidget. flutter/flutter#43785 I've done the work in 2 commits, the first one just used mount checks on after each awaited retrieval of the controller. The second reuses the controller to limit the number of awaited async calls. I am a little unsure of the expected execution order of all the unawaited futures which update the controller in regards to whether dispose could still be called in between their executions. Testing locally I haven't been able to reproduce the exception with a single awaited controller. I bumped the version 2.12.2 as no feature behaviour has changed. No new tests in this PR, I'm not sure that you can test this change using widget tests. Existing test coverage ensures that the values are still being updated correctly. ## Pre-Review Checklist - [/] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [/] 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 updated `pubspec.yaml` with an appropriate new version according to the [pub versioning philosophy], or I have commented below to indicate which [version change exemption] this PR falls under[^1]. - [/] I updated `CHANGELOG.md` to add a description of the change, [following repository CHANGELOG style], or I have commented below to indicate which [CHANGELOG exemption] this PR falls under[^1]. - [/] 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[^1]. - [/] All existing and new tests are passing. [^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. ## Code Sample This sample app can be used to trigger the error, just repeatedly tapping the button at the bottom of the screen to reload the map. I did the testing in Chrome using the google_maps_flutter_web example project and the javascript key in web/index.html ``` import 'package:flutter/material.dart'; import 'package:google_maps_flutter/google_maps_flutter.dart'; void main() { runApp(const MapToggleWidget()); } class MapToggleWidget extends StatefulWidget { const MapToggleWidget({Key? key}) : super(key: key); @OverRide _MapToggleWidgetState createState() => _MapToggleWidgetState(); } class _MapToggleWidgetState extends State<MapToggleWidget> { bool _showMap = true; @OverRide Widget build(BuildContext context) { return MaterialApp( home: Scaffold( appBar: AppBar(title: const Text('Map Toggle Example')), body: Column( children: [ Expanded(child: _showMap ? buildGoogleMap() : buildNoMapContainer()), Padding( padding: const EdgeInsets.all(8.0), child: ElevatedButton( onPressed: () => setState(() => _showMap = !_showMap), child: Text(_showMap ? 'Hide Map' : 'Show Map'), ), ), ], ), ), ); } Container buildNoMapContainer() { return Container( color: Colors.grey[300], child: const Center( child: Text( 'Map is hidden', style: TextStyle(fontSize: 18), ), ), ); } GoogleMap buildGoogleMap() { return const GoogleMap( mapType: MapType.satellite, initialCameraPosition: CameraPosition( target: LatLng(-38.9000, 175.8000), zoom: 10.0, ), ); } } ```
1 parent dc48f5c commit 42bb347

File tree

5 files changed

+105
-33
lines changed

5 files changed

+105
-33
lines changed

packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 2.13.1
2+
3+
* Fixes exception when dispose is called while asynchronous update from
4+
`didUpdateWidget` is executed.
5+
16
## 2.13.0
27

38
* Adds support for camera control button on web.

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

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -448,30 +448,38 @@ class _GoogleMapState extends State<GoogleMap> {
448448
@override
449449
void didUpdateWidget(GoogleMap oldWidget) {
450450
super.didUpdateWidget(oldWidget);
451-
_updateOptions();
452-
_updateClusterManagers();
453-
_updateMarkers();
454-
_updatePolygons();
455-
_updatePolylines();
456-
_updateCircles();
457-
_updateHeatmaps();
458-
_updateTileOverlays();
459-
_updateGroundOverlays();
451+
452+
_refreshStateFromWidget();
453+
}
454+
455+
Future<void> _refreshStateFromWidget() async {
456+
final GoogleMapController controller = await _controller.future;
457+
if (!mounted) {
458+
return;
459+
}
460+
461+
_updateOptions(controller);
462+
_updateClusterManagers(controller);
463+
_updateMarkers(controller);
464+
_updatePolygons(controller);
465+
_updatePolylines(controller);
466+
_updateCircles(controller);
467+
_updateHeatmaps(controller);
468+
_updateTileOverlays(controller);
469+
_updateGroundOverlays(controller);
460470
}
461471

462-
Future<void> _updateOptions() async {
472+
void _updateOptions(GoogleMapController controller) {
463473
final MapConfiguration newConfig = _configurationFromMapWidget(widget);
464474
final MapConfiguration updates = newConfig.diffFrom(_mapConfiguration);
465475
if (updates.isEmpty) {
466476
return;
467477
}
468-
final GoogleMapController controller = await _controller.future;
469478
unawaited(controller._updateMapConfiguration(updates));
470479
_mapConfiguration = newConfig;
471480
}
472481

473-
Future<void> _updateMarkers() async {
474-
final GoogleMapController controller = await _controller.future;
482+
void _updateMarkers(GoogleMapController controller) {
475483
unawaited(
476484
controller._updateMarkers(
477485
MarkerUpdates.from(_markers.values.toSet(), widget.markers),
@@ -480,8 +488,7 @@ class _GoogleMapState extends State<GoogleMap> {
480488
_markers = keyByMarkerId(widget.markers);
481489
}
482490

483-
Future<void> _updateClusterManagers() async {
484-
final GoogleMapController controller = await _controller.future;
491+
void _updateClusterManagers(GoogleMapController controller) {
485492
unawaited(
486493
controller._updateClusterManagers(
487494
ClusterManagerUpdates.from(
@@ -493,8 +500,7 @@ class _GoogleMapState extends State<GoogleMap> {
493500
_clusterManagers = keyByClusterManagerId(widget.clusterManagers);
494501
}
495502

496-
Future<void> _updateGroundOverlays() async {
497-
final GoogleMapController controller = await _controller.future;
503+
void _updateGroundOverlays(GoogleMapController controller) {
498504
unawaited(
499505
controller._updateGroundOverlays(
500506
GroundOverlayUpdates.from(
@@ -506,8 +512,7 @@ class _GoogleMapState extends State<GoogleMap> {
506512
_groundOverlays = keyByGroundOverlayId(widget.groundOverlays);
507513
}
508514

509-
Future<void> _updatePolygons() async {
510-
final GoogleMapController controller = await _controller.future;
515+
void _updatePolygons(GoogleMapController controller) {
511516
unawaited(
512517
controller._updatePolygons(
513518
PolygonUpdates.from(_polygons.values.toSet(), widget.polygons),
@@ -516,8 +521,7 @@ class _GoogleMapState extends State<GoogleMap> {
516521
_polygons = keyByPolygonId(widget.polygons);
517522
}
518523

519-
Future<void> _updatePolylines() async {
520-
final GoogleMapController controller = await _controller.future;
524+
void _updatePolylines(GoogleMapController controller) {
521525
unawaited(
522526
controller._updatePolylines(
523527
PolylineUpdates.from(_polylines.values.toSet(), widget.polylines),
@@ -526,8 +530,7 @@ class _GoogleMapState extends State<GoogleMap> {
526530
_polylines = keyByPolylineId(widget.polylines);
527531
}
528532

529-
Future<void> _updateCircles() async {
530-
final GoogleMapController controller = await _controller.future;
533+
void _updateCircles(GoogleMapController controller) {
531534
unawaited(
532535
controller._updateCircles(
533536
CircleUpdates.from(_circles.values.toSet(), widget.circles),
@@ -536,8 +539,7 @@ class _GoogleMapState extends State<GoogleMap> {
536539
_circles = keyByCircleId(widget.circles);
537540
}
538541

539-
Future<void> _updateHeatmaps() async {
540-
final GoogleMapController controller = await _controller.future;
542+
void _updateHeatmaps(GoogleMapController controller) {
541543
unawaited(
542544
controller._updateHeatmaps(
543545
HeatmapUpdates.from(_heatmaps.values.toSet(), widget.heatmaps),
@@ -546,8 +548,7 @@ class _GoogleMapState extends State<GoogleMap> {
546548
_heatmaps = keyByHeatmapId(widget.heatmaps);
547549
}
548550

549-
Future<void> _updateTileOverlays() async {
550-
final GoogleMapController controller = await _controller.future;
551+
void _updateTileOverlays(GoogleMapController controller) {
551552
unawaited(controller._updateTileOverlays(widget.tileOverlays));
552553
}
553554

@@ -558,10 +559,12 @@ class _GoogleMapState extends State<GoogleMap> {
558559
this,
559560
);
560561
_controller.complete(controller);
561-
unawaited(_updateTileOverlays());
562-
final MapCreatedCallback? onMapCreated = widget.onMapCreated;
563-
if (onMapCreated != null) {
564-
onMapCreated(controller);
562+
if (mounted) {
563+
_updateTileOverlays(controller);
564+
final MapCreatedCallback? onMapCreated = widget.onMapCreated;
565+
if (onMapCreated != null) {
566+
onMapCreated(controller);
567+
}
565568
}
566569
}
567570

packages/google_maps_flutter/google_maps_flutter/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: google_maps_flutter
22
description: A Flutter plugin for integrating Google Maps in iOS and Android applications.
33
repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter
44
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
5-
version: 2.13.0
5+
version: 2.13.1
66

77
environment:
88
sdk: ^3.7.0

packages/google_maps_flutter/google_maps_flutter/test/fake_google_maps_flutter_platform.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,11 @@ class FakeGoogleMapsFlutterPlatform extends GoogleMapsFlutterPlatform {
3737
final StreamController<MapEvent<dynamic>> mapEventStreamController =
3838
StreamController<MapEvent<dynamic>>.broadcast();
3939

40+
// Overrides completion of the init.
41+
Completer<void>? initCompleter;
42+
4043
@override
41-
Future<void> init(int mapId) async {}
44+
Future<void> init(int mapId) async => initCompleter?.future;
4245

4346
@override
4447
Future<void> updateMapConfiguration(

packages/google_maps_flutter/google_maps_flutter/test/google_map_test.dart

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'dart:async';
6+
57
import 'package:flutter/widgets.dart';
68
import 'package:flutter_test/flutter_test.dart';
79
import 'package:google_maps_flutter/google_maps_flutter.dart';
@@ -598,4 +600,63 @@ void main() {
598600

599601
expect(map.mapConfiguration.style, '');
600602
});
603+
604+
testWidgets('Update state from widget only when mounted', (
605+
WidgetTester tester,
606+
) async {
607+
await tester.pumpWidget(
608+
const Directionality(
609+
textDirection: TextDirection.ltr,
610+
child: GoogleMap(
611+
initialCameraPosition: CameraPosition(target: LatLng(10.0, 15.0)),
612+
),
613+
),
614+
);
615+
616+
final State<StatefulWidget> googleMapState = tester.state(
617+
find.byType(GoogleMap),
618+
);
619+
620+
await tester.pumpWidget(Container());
621+
622+
// This is done to force the update path while the widget is not mounted.
623+
// ignore:invalid_use_of_protected_member
624+
googleMapState.didUpdateWidget(
625+
GoogleMap(
626+
initialCameraPosition: const CameraPosition(target: LatLng(10.0, 15.0)),
627+
circles: <Circle>{const Circle(circleId: CircleId('circle'))},
628+
),
629+
);
630+
631+
await tester.pumpAndSettle();
632+
633+
final PlatformMapStateRecorder map = platform.lastCreatedMap;
634+
635+
expect(map.circleUpdates.length, 1);
636+
});
637+
638+
testWidgets('Update state after map is initialized only when mounted', (
639+
WidgetTester tester,
640+
) async {
641+
platform.initCompleter = Completer<void>();
642+
643+
await tester.pumpWidget(
644+
const Directionality(
645+
textDirection: TextDirection.ltr,
646+
child: GoogleMap(
647+
initialCameraPosition: CameraPosition(target: LatLng(10.0, 15.0)),
648+
),
649+
),
650+
);
651+
652+
await tester.pumpWidget(Container());
653+
654+
platform.initCompleter!.complete();
655+
656+
await tester.pumpAndSettle();
657+
658+
final PlatformMapStateRecorder map = platform.lastCreatedMap;
659+
660+
expect(map.tileOverlaySets.length, 1);
661+
});
601662
}

0 commit comments

Comments
 (0)