From 3e93c54061f5f365447944e539446f416027eca7 Mon Sep 17 00:00:00 2001 From: Filip Hracek Date: Mon, 12 May 2025 15:25:35 +0200 Subject: [PATCH 1/3] Add guard function --- .../lib/src/controller.dart | 50 +++++++++++++ .../test/google_map_controller_test.dart | 74 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart diff --git a/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart b/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart index 50b0641f68ac..6b3ce86b6cb6 100644 --- a/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart +++ b/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart @@ -236,6 +236,7 @@ class GoogleMapController { /// in-memory cache of tiles. If you want to cache tiles for longer, you /// should implement an on-disk cache. Future clearTileCache(TileOverlayId tileOverlayId) async { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .clearTileCache(tileOverlayId, mapId: mapId); } @@ -248,6 +249,7 @@ class GoogleMapController { /// The returned [Future] completes after the change has been started on the /// platform side. Future animateCamera(CameraUpdate cameraUpdate, {Duration? duration}) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance.animateCameraWithConfiguration( cameraUpdate, CameraUpdateAnimationConfiguration(duration: duration), mapId: mapId); @@ -258,6 +260,7 @@ class GoogleMapController { /// The returned [Future] completes after the change has been made on the /// platform side. Future moveCamera(CameraUpdate cameraUpdate) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .moveCamera(cameraUpdate, mapId: mapId); } @@ -277,17 +280,20 @@ class GoogleMapController { /// style reference for more information regarding the supported styles. @Deprecated('Use GoogleMap.style instead.') Future setMapStyle(String? mapStyle) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .setMapStyle(mapStyle, mapId: mapId); } /// Returns the last style error, if any. Future getStyleError() { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance.getStyleError(mapId: mapId); } /// Return [LatLngBounds] defining the region that is visible in a map. Future getVisibleRegion() { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance.getVisibleRegion(mapId: mapId); } @@ -297,6 +303,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 getScreenCoordinate(LatLng latLng) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .getScreenCoordinate(latLng, mapId: mapId); } @@ -306,6 +313,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 getLatLng(ScreenCoordinate screenCoordinate) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .getLatLng(screenCoordinate, mapId: mapId); } @@ -319,6 +327,7 @@ class GoogleMapController { /// * [hideMarkerInfoWindow] to hide the Info Window. /// * [isMarkerInfoWindowShown] to check if the Info Window is showing. Future showMarkerInfoWindow(MarkerId markerId) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .showMarkerInfoWindow(markerId, mapId: mapId); } @@ -332,6 +341,7 @@ class GoogleMapController { /// * [showMarkerInfoWindow] to show the Info Window. /// * [isMarkerInfoWindowShown] to check if the Info Window is showing. Future hideMarkerInfoWindow(MarkerId markerId) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .hideMarkerInfoWindow(markerId, mapId: mapId); } @@ -345,17 +355,20 @@ class GoogleMapController { /// * [showMarkerInfoWindow] to show the Info Window. /// * [hideMarkerInfoWindow] to hide the Info Window. Future isMarkerInfoWindowShown(MarkerId markerId) { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance .isMarkerInfoWindowShown(markerId, mapId: mapId); } /// Returns the current zoom level of the map Future getZoomLevel() { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance.getZoomLevel(mapId: mapId); } /// Returns the image bytes of the map Future takeSnapshot() { + _checkWidgetMountedOrThrow(); return GoogleMapsFlutterPlatform.instance.takeSnapshot(mapId: mapId); } @@ -368,4 +381,41 @@ 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 { + /// Creates the use-after-disposed error for the provided [mapId] + /// and with the provided [diagnosticString]. + 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.'; + } } diff --git a/packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart b/packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart new file mode 100644 index 000000000000..12615651a4ab --- /dev/null +++ b/packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart @@ -0,0 +1,74 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// 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); + }); + + testWidgets('controller can be used to access underlying map', + (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, + ), + ), + ); + + 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())); + }); +} From 9648d823e0fcd2afa13e92f1eab548bbed7fc54e Mon Sep 17 00:00:00 2001 From: Filip Hracek Date: Mon, 12 May 2025 15:33:11 +0200 Subject: [PATCH 2/3] Simplify unit test --- .../test/google_map_controller_test.dart | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart b/packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart index 12615651a4ab..551370d18ef8 100644 --- a/packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter/test/google_map_controller_test.dart @@ -32,22 +32,6 @@ void main() { ); expect(controller, isNotNull); - }); - - testWidgets('controller can be used to access underlying map', - (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, - ), - ), - ); - await expectLater(controller?.getZoomLevel(), isNotNull); }); From 3b53dc2bf91471a2014dcf17c9ac2509eb04c84f Mon Sep 17 00:00:00 2001 From: Filip Hracek Date: Mon, 12 May 2025 15:40:29 +0200 Subject: [PATCH 3/3] Bump version and add CHANGELOG --- .../google_maps_flutter/google_maps_flutter/CHANGELOG.md | 5 +++++ .../google_maps_flutter/google_maps_flutter/pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md b/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md index edc282394190..e3e042fb5f04 100644 --- a/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md +++ b/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.12.3 + +* Adds a check that raises `MapUsedAfterWidgetDisposedError` + when map controller is used after its widget has been disposed. + ## 2.12.2 * Fixes memory leak by disposing stream subscriptions in `GoogleMapController`. diff --git a/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml b/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml index 974bbaa21014..c7126a047b48 100644 --- a/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml +++ b/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.12.2 +version: 2.12.3 environment: sdk: ^3.6.0