-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[google_maps_flutter] Raise MapUsedAfterWidgetDisposedError when map controller used after map disposed
#9242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
54f9753
438ce1f
6dbdf3c
6f0d63e
4b5395d
fb7a11f
90ceed1
33d3973
4e64944
cff40b6
244fec4
71cbd5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,6 +264,7 @@ class GoogleMapController { | |
| /// in-memory cache of tiles. If you want to cache tiles for longer, you | ||
| /// should implement an on-disk cache. | ||
| Future<void> clearTileCache(TileOverlayId tileOverlayId) async { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.clearTileCache( | ||
| tileOverlayId, | ||
| mapId: mapId, | ||
|
|
@@ -278,6 +279,7 @@ class GoogleMapController { | |
| /// The returned [Future] completes after the change has been started on the | ||
| /// platform side. | ||
| Future<void> animateCamera(CameraUpdate cameraUpdate, {Duration? duration}) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.animateCameraWithConfiguration( | ||
| cameraUpdate, | ||
| CameraUpdateAnimationConfiguration(duration: duration), | ||
|
|
@@ -290,6 +292,7 @@ class GoogleMapController { | |
| /// The returned [Future] completes after the change has been made on the | ||
| /// platform side. | ||
| Future<void> moveCamera(CameraUpdate cameraUpdate) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.moveCamera( | ||
| cameraUpdate, | ||
| mapId: mapId, | ||
|
|
@@ -311,6 +314,7 @@ class GoogleMapController { | |
| /// style reference for more information regarding the supported styles. | ||
| @Deprecated('Use GoogleMap.style instead.') | ||
| Future<void> setMapStyle(String? mapStyle) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.setMapStyle( | ||
| mapStyle, | ||
| mapId: mapId, | ||
|
|
@@ -319,11 +323,13 @@ class GoogleMapController { | |
|
|
||
| /// Returns the last style error, if any. | ||
| Future<String?> getStyleError() { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.getStyleError(mapId: mapId); | ||
| } | ||
|
|
||
| /// Return [LatLngBounds] defining the region that is visible in a map. | ||
| Future<LatLngBounds> getVisibleRegion() { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.getVisibleRegion(mapId: mapId); | ||
| } | ||
|
|
||
|
|
@@ -333,6 +339,7 @@ class GoogleMapController { | |
| /// Screen location is in screen pixels (not display pixels) with respect to the top left corner | ||
| /// of the map, not necessarily of the whole screen. | ||
| Future<ScreenCoordinate> getScreenCoordinate(LatLng latLng) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.getScreenCoordinate( | ||
| latLng, | ||
| mapId: mapId, | ||
|
|
@@ -344,6 +351,7 @@ class GoogleMapController { | |
| /// Returned [LatLng] corresponds to a screen location. The screen location is specified in screen | ||
| /// pixels (not display pixels) relative to the top left of the map, not top left of the whole screen. | ||
| Future<LatLng> getLatLng(ScreenCoordinate screenCoordinate) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.getLatLng( | ||
| screenCoordinate, | ||
| mapId: mapId, | ||
|
|
@@ -359,6 +367,7 @@ class GoogleMapController { | |
| /// * [hideMarkerInfoWindow] to hide the Info Window. | ||
| /// * [isMarkerInfoWindowShown] to check if the Info Window is showing. | ||
| Future<void> showMarkerInfoWindow(MarkerId markerId) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.showMarkerInfoWindow( | ||
| markerId, | ||
| mapId: mapId, | ||
|
|
@@ -374,6 +383,7 @@ class GoogleMapController { | |
| /// * [showMarkerInfoWindow] to show the Info Window. | ||
| /// * [isMarkerInfoWindowShown] to check if the Info Window is showing. | ||
| Future<void> hideMarkerInfoWindow(MarkerId markerId) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.hideMarkerInfoWindow( | ||
| markerId, | ||
| mapId: mapId, | ||
|
|
@@ -389,6 +399,7 @@ class GoogleMapController { | |
| /// * [showMarkerInfoWindow] to show the Info Window. | ||
| /// * [hideMarkerInfoWindow] to hide the Info Window. | ||
| Future<bool> isMarkerInfoWindowShown(MarkerId markerId) { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.isMarkerInfoWindowShown( | ||
| markerId, | ||
| mapId: mapId, | ||
|
|
@@ -397,11 +408,13 @@ class GoogleMapController { | |
|
|
||
| /// Returns the current zoom level of the map | ||
| Future<double> getZoomLevel() { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.getZoomLevel(mapId: mapId); | ||
| } | ||
|
|
||
| /// Returns the image bytes of the map | ||
| Future<Uint8List?> takeSnapshot() { | ||
| _checkWidgetMountedOrThrow(); | ||
| return GoogleMapsFlutterPlatform.instance.takeSnapshot(mapId: mapId); | ||
| } | ||
|
|
||
|
|
@@ -414,4 +427,40 @@ class GoogleMapController { | |
| _streamSubscriptions.clear(); | ||
| GoogleMapsFlutterPlatform.instance.dispose(mapId: mapId); | ||
| } | ||
|
|
||
| /// It is relatively easy to mistakenly call a method on the controller | ||
| /// after the [GoogleMap] widget has already been disposed. | ||
| /// Historically, this led to Platform-side errors such as | ||
| /// `MissingPluginException` or `Unable to establish connection on channel` | ||
| /// errors. | ||
| /// | ||
| /// To facilitate debugging, this guard function | ||
| /// raises [MapUsedAfterWidgetDisposedError]. | ||
| void _checkWidgetMountedOrThrow() { | ||
| if (!_googleMapState.mounted) { | ||
| throw MapUsedAfterWidgetDisposedError(mapId: mapId); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Error thrown when any [GoogleMapController] method is called after | ||
| /// its associated [GoogleMap] widget has been disposed. | ||
| /// | ||
| /// To avoid this error: | ||
| /// | ||
| /// 1. Set the map controller field to `null` in your widget state's | ||
| /// `dispose()` method, or | ||
| /// 2. Check the [State.mounted] state before each use of the controller. | ||
| class MapUsedAfterWidgetDisposedError extends Error { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a compelling reason to create a new
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought it nicer to have a separate class at the time, but I'm happy to just use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
| /// Creates the use-after-disposed error for the provided [mapId]. | ||
| MapUsedAfterWidgetDisposedError({required this.mapId}); | ||
|
|
||
| /// The map ID of the map for which this error is being raised. | ||
| final int mapId; | ||
|
|
||
| @override | ||
| String toString() { | ||
| return 'GoogleMapController for map ID $mapId was used after ' | ||
| 'the associated GoogleMap widget had already been disposed.'; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| 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_google_maps_flutter_platform.dart'; | ||
|
|
||
| void main() { | ||
| late FakeGoogleMapsFlutterPlatform platform; | ||
|
|
||
| setUp(() { | ||
| platform = FakeGoogleMapsFlutterPlatform(); | ||
| GoogleMapsFlutterPlatform.instance = platform; | ||
| }); | ||
|
|
||
| testWidgets('onMapCreated is called with controller', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| GoogleMapController? controller; | ||
|
|
||
| await tester.pumpWidget( | ||
| Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: GoogleMap( | ||
| initialCameraPosition: const CameraPosition(target: LatLng(0.0, 0.0)), | ||
| onMapCreated: (GoogleMapController value) => controller = value, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| expect(controller, isNotNull); | ||
| await expectLater(controller?.getZoomLevel(), isNotNull); | ||
| }); | ||
|
|
||
| testWidgets('controller throws when used after dispose', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| GoogleMapController? controller; | ||
|
|
||
| await tester.pumpWidget( | ||
| Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: GoogleMap( | ||
| initialCameraPosition: const CameraPosition(target: LatLng(0.0, 0.0)), | ||
| onMapCreated: (GoogleMapController value) => controller = value, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| // Now dispose of the map... | ||
| await tester.pumpWidget(Container()); | ||
|
|
||
| await expectLater( | ||
| () => controller?.getZoomLevel(), | ||
| throwsA(isA<MapUsedAfterWidgetDisposedError>()), | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New API is a minor version bump, per semver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.