Unverified Commit 2c3db435 authored by Kostia Sokolovskyi's avatar Kostia Sokolovskyi Committed by GitHub

_RouterState should dispose created _RestorableRouteInformation. (#136556)

### Description
- Fixes https://github.com/flutter/flutter/issues/134205.

### Tests
- Removes ignoring the `_RestorableRouteInformation` leak from `cupertino/app_test.dart`;
- Removes ignoring the `_RestorableRouteInformation` leak from `material/app_test.dart`;
- Removes ignoring the `_RestorableRouteInformation` leak from `widgets/app_test.dart`;
- Removes ignoring the `_RestorableRouteInformation` leak from `widgets/route_notification_messages_test.dart`;
- Removes ignoring the `_RestorableRouteInformation` leak from `widgets/router_restoration_test.dart`;
- Updates `widgets/router_test.dart` to use `testWidgetsWithLeakTracking`.
parent 5b7e9d60
...@@ -632,6 +632,10 @@ class _RouterState<T> extends State<Router<T>> with RestorationMixin { ...@@ -632,6 +632,10 @@ class _RouterState<T> extends State<Router<T>> with RestorationMixin {
} }
void _reportRouteInformation(Duration timestamp) { void _reportRouteInformation(Duration timestamp) {
if (!mounted) {
return;
}
assert(_routeInformationReportingTaskScheduled); assert(_routeInformationReportingTaskScheduled);
_routeInformationReportingTaskScheduled = false; _routeInformationReportingTaskScheduled = false;
...@@ -726,6 +730,7 @@ class _RouterState<T> extends State<Router<T>> with RestorationMixin { ...@@ -726,6 +730,7 @@ class _RouterState<T> extends State<Router<T>> with RestorationMixin {
@override @override
void dispose() { void dispose() {
_routeInformation.dispose();
widget.routeInformationProvider?.removeListener(_handleRouteInformationProviderNotification); widget.routeInformationProvider?.removeListener(_handleRouteInformationProviderNotification);
widget.backButtonDispatcher?.removeCallback(_handleBackButtonDispatcherNotification); widget.backButtonDispatcher?.removeCallback(_handleBackButtonDispatcherNotification);
widget.routerDelegate.removeListener(_handleRouterDelegateNotification); widget.routerDelegate.removeListener(_handleRouterDelegateNotification);
......
...@@ -178,12 +178,7 @@ void main() { ...@@ -178,12 +178,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/134205
leakTrackingTestConfig: const LeakTrackingTestConfig(
allowAllNotDisposed: true,
));
testWidgetsWithLeakTracking('CupertinoApp.router route information parser is optional', (WidgetTester tester) async { testWidgetsWithLeakTracking('CupertinoApp.router route information parser is optional', (WidgetTester tester) async {
final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate( final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate(
...@@ -209,12 +204,7 @@ void main() { ...@@ -209,12 +204,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/134205
leakTrackingTestConfig: const LeakTrackingTestConfig(
allowAllNotDisposed: true,
));
testWidgetsWithLeakTracking('CupertinoApp.router throw if route information provider is provided but no route information parser', (WidgetTester tester) async { testWidgetsWithLeakTracking('CupertinoApp.router throw if route information provider is provided but no route information parser', (WidgetTester tester) async {
final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate( final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate(
...@@ -300,12 +290,7 @@ void main() { ...@@ -300,12 +290,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/134205
leakTrackingTestConfig: const LeakTrackingTestConfig(
allowAllNotDisposed: true,
));
testWidgetsWithLeakTracking('CupertinoApp has correct default ScrollBehavior', (WidgetTester tester) async { testWidgetsWithLeakTracking('CupertinoApp has correct default ScrollBehavior', (WidgetTester tester) async {
late BuildContext capturedContext; late BuildContext capturedContext;
......
...@@ -1114,12 +1114,7 @@ void main() { ...@@ -1114,12 +1114,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/134205
leakTrackingTestConfig: const LeakTrackingTestConfig(
allowAllNotDisposed: true,
));
testWidgetsWithLeakTracking('MaterialApp.router route information parser is optional', (WidgetTester tester) async { testWidgetsWithLeakTracking('MaterialApp.router route information parser is optional', (WidgetTester tester) async {
final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate( final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate(
...@@ -1145,12 +1140,7 @@ void main() { ...@@ -1145,12 +1140,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/134205
leakTrackingTestConfig: const LeakTrackingTestConfig(
allowAllNotDisposed: true,
));
testWidgetsWithLeakTracking('MaterialApp.router throw if route information provider is provided but no route information parser', (WidgetTester tester) async { testWidgetsWithLeakTracking('MaterialApp.router throw if route information provider is provided but no route information parser', (WidgetTester tester) async {
final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate( final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate(
...@@ -1236,12 +1226,7 @@ void main() { ...@@ -1236,12 +1226,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/134205
leakTrackingTestConfig: const LeakTrackingTestConfig(
allowAllNotDisposed: true,
));
testWidgetsWithLeakTracking('MaterialApp.builder can build app without a Navigator', (WidgetTester tester) async { testWidgetsWithLeakTracking('MaterialApp.builder can build app without a Navigator', (WidgetTester tester) async {
Widget? builderChild; Widget? builderChild;
......
...@@ -304,12 +304,7 @@ void main() { ...@@ -304,12 +304,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(ksokolovskyi): remove after fixing
// https://github.com/flutter/flutter/issues/134205
notDisposedAllowList: <String, int?> {'_RestorableRouteInformation': 1},
));
testWidgetsWithLeakTracking('WidgetsApp.router route information parser is optional', (WidgetTester tester) async { testWidgetsWithLeakTracking('WidgetsApp.router route information parser is optional', (WidgetTester tester) async {
final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate( final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate(
...@@ -336,12 +331,7 @@ void main() { ...@@ -336,12 +331,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(ksokolovskyi): remove after fixing
// https://github.com/flutter/flutter/issues/134205
notDisposedAllowList: <String, int?> {'_RestorableRouteInformation': 1},
));
testWidgetsWithLeakTracking('WidgetsApp.router throw if route information provider is provided but no route information parser', (WidgetTester tester) async { testWidgetsWithLeakTracking('WidgetsApp.router throw if route information provider is provided but no route information parser', (WidgetTester tester) async {
final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate( final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate(
...@@ -432,12 +422,7 @@ void main() { ...@@ -432,12 +422,7 @@ void main() {
await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { }); await tester.binding.defaultBinaryMessenger.handlePlatformMessage('flutter/navigation', message, (_) { });
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('popped'), findsOneWidget); expect(find.text('popped'), findsOneWidget);
}, });
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(ksokolovskyi): remove after fixing
// https://github.com/flutter/flutter/issues/134205
notDisposedAllowList: <String, int?> {'_RestorableRouteInformation': 1},
));
testWidgetsWithLeakTracking('WidgetsApp.router has correct default', (WidgetTester tester) async { testWidgetsWithLeakTracking('WidgetsApp.router has correct default', (WidgetTester tester) async {
final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate( final SimpleNavigatorRouterDelegate delegate = SimpleNavigatorRouterDelegate(
...@@ -453,12 +438,7 @@ void main() { ...@@ -453,12 +438,7 @@ void main() {
color: const Color(0xFF123456), color: const Color(0xFF123456),
)); ));
expect(find.text('/'), findsOneWidget); expect(find.text('/'), findsOneWidget);
}, });
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(ksokolovskyi): remove after fixing
// https://github.com/flutter/flutter/issues/134205
notDisposedAllowList: <String, int?> {'_RestorableRouteInformation': 1},
));
testWidgetsWithLeakTracking('WidgetsApp has correct default ScrollBehavior', (WidgetTester tester) async { testWidgetsWithLeakTracking('WidgetsApp has correct default ScrollBehavior', (WidgetTester tester) async {
late BuildContext capturedContext; late BuildContext capturedContext;
......
...@@ -316,12 +316,7 @@ void main() { ...@@ -316,12 +316,7 @@ void main() {
'replace': false, 'replace': false,
}), }),
]); ]);
}, });
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(ksokolovskyi): remove after fixing
// https://github.com/flutter/flutter/issues/134205
notDisposedAllowList: <String, int?> {'_RestorableRouteInformation': 1},
));
} }
typedef SimpleRouterDelegateBuilder = Widget Function(BuildContext, RouteInformation); typedef SimpleRouterDelegateBuilder = Widget Function(BuildContext, RouteInformation);
......
...@@ -40,12 +40,7 @@ void main() { ...@@ -40,12 +40,7 @@ void main() {
expect(find.text('Current config: /foo'), findsOneWidget); expect(find.text('Current config: /foo'), findsOneWidget);
expect(delegate().newRoutePaths, isEmpty); expect(delegate().newRoutePaths, isEmpty);
expect(delegate().restoredRoutePaths, <String>['/foo', '/foo']); expect(delegate().restoredRoutePaths, <String>['/foo', '/foo']);
}, });
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(ksokolovskyi): remove after fixing
// https://github.com/flutter/flutter/issues/134205
notDisposedAllowList: <String, int?> {'_RestorableRouteInformation': 2},
));
testWidgets('Router state restoration with RouteInformationProvider', (WidgetTester tester) async { testWidgets('Router state restoration with RouteInformationProvider', (WidgetTester tester) async {
final UniqueKey router = UniqueKey(); final UniqueKey router = UniqueKey();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment