DEV/flutter_flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2026-02-20 02:29:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fixes crash when adding and removing mulitple page-based route (#177338)

Browse Source 
<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

fixes https://github.com/flutter/flutter/issues/177322

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
chunhtai 2025-10-22 12:08:09 -07:00 committed by GitHub
parent 2b7e563149
commit 6bafbf8f7e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 78 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
packages/flutter/lib/src/widgets/navigator.dart
View File

@ -184,6 +184,9 @@ abstract class Route<T> extends _RoutePlaceholder {
NavigatorState? get navigator => _navigator;
NavigatorState? _navigator;
bool get _installed => _navigator != null;
bool _isInstalledIn(NavigatorState state) => _navigator == state;
/// The settings for this route.
///
/// See [RouteSettings] for details.
@ -220,7 +223,7 @@ abstract class Route<T> extends _RoutePlaceholder {
void _updateSettings(RouteSettings newSettings) {
if (_settings != newSettings) {
_settings = newSettings;
if (_navigator != null) {
if (_installed) {
changedInternalState();
}
}
@ -579,7 +582,7 @@ abstract class Route<T> extends _RoutePlaceholder {
///
/// If this is true, then [isActive] is also true.
bool get isCurrent {
if (_navigator == null) {
if (!_installed) {
return false;
}
final _RouteEntry? currentRouteEntry = _navigator!._lastRouteEntryWhereOrNull(
@ -596,7 +599,7 @@ abstract class Route<T> extends _RoutePlaceholder {
/// If [isFirst] and [isCurrent] are both true then this is the only route on
/// the navigator (and [isActive] will also be true).
bool get isFirst {
if (_navigator == null) {
if (!_installed) {
return false;
}
final _RouteEntry? currentRouteEntry = _navigator!._firstRouteEntryWhereOrNull(
@ -611,7 +614,7 @@ abstract class Route<T> extends _RoutePlaceholder {
/// Whether there is at least one active route underneath this route.
@protected
bool get hasActiveRouteBelow {
if (_navigator == null) {
if (!_installed) {
return false;
}
for (final _RouteEntry entry in _navigator!._history) {
@ -3213,7 +3216,7 @@ class _RouteEntry extends RouteTransitionRecord {
);
assert(navigator._debugLocked);
assert(
route._navigator == null,
!route._installed,
'The pushed route has already been used. When pushing a route, a new '
'Route object must be provided.',
);
@ -3292,7 +3295,7 @@ class _RouteEntry extends RouteTransitionRecord {
/// refuses to be popped.
bool handlePop({required NavigatorState navigator, required Route<dynamic>? previousPresent}) {
assert(navigator._debugLocked);
assert(route._navigator == navigator);
assert(route._isInstalledIn(navigator));
currentState = _RouteLifecycle.popping;
if (route._popCompleter.isCompleted) {
// This is a page-based route popped through the Navigator.pop. The
@ -3322,7 +3325,7 @@ class _RouteEntry extends RouteTransitionRecord {
required Route<dynamic>? previousPresent,
}) {
assert(navigator._debugLocked);
if (route._navigator == navigator) {
if (route._isInstalledIn(navigator)) {
currentState = _RouteLifecycle.removing;
} else {
// This route is still waiting to be added while a top-most push or pop
@ -3336,7 +3339,7 @@ class _RouteEntry extends RouteTransitionRecord {
}
void didAdd({required NavigatorState navigator, required bool isNewFirst}) {
assert(route._navigator == null);
assert(!route._installed);
route._navigator = navigator;
route.install();
assert(route.overlayEntries.isNotEmpty);
@ -3436,7 +3439,7 @@ class _RouteEntry extends RouteTransitionRecord {
if (!navigator._entryWaitingForSubTreeDisposal.remove(this)) {
// This route must have been destroyed as a result of navigator
// force dispose.
assert(route._navigator == null && !navigator.mounted);
assert(!route._installed && !navigator.mounted);
return;
}
assert(currentState == _RouteLifecycle.disposing);
@ -4484,9 +4487,12 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
assert(entry.currentState == _RouteLifecycle.remove);
continue;
case _RouteLifecycle.remove:
if (!seenTopActiveRoute) {
// Routes that are not yet added (_installed == false) will exit as if
// they have never been on the navigator. They shouldn't announce
// their presence.
if (!seenTopActiveRoute && entry.route._installed) {
if (poppedRoute != null) {
entry.route.didPopNext(poppedRoute);
entry.handleDidPopNext(poppedRoute);
}
poppedRoute = null;
}
@ -5049,7 +5055,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
_debugLocked = true;
return true;
}());
assert(entry.route._navigator == null);
assert(!entry.route._installed);
assert(entry.currentState == _RouteLifecycle.push);
_history.add(entry);
_flushHistoryUpdates();
@ -5127,7 +5133,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
Route<T> newRoute, {
TO? result,
}) {
assert(newRoute._navigator == null);
assert(!newRoute._installed);
_pushReplacementEntry(
_RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.pushReplace),
result,
@ -5181,7 +5187,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
_debugLocked = true;
return true;
}());
assert(entry.route._navigator == null);
assert(!entry.route._installed);
assert(_history.isNotEmpty);
assert(
_history.any(_RouteEntry.isPresentPredicate),
@ -5228,7 +5234,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
/// restored during state restoration.
@optionalTypeArgs
Future<T?> pushAndRemoveUntil<T extends Object?>(Route<T> newRoute, RoutePredicate predicate) {
assert(newRoute._navigator == null);
assert(!newRoute._installed);
assert(newRoute.overlayEntries.isEmpty);
_pushEntryAndRemoveUntil(
_RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.push),
@ -5282,7 +5288,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
_debugLocked = true;
return true;
}());
assert(entry.route._navigator == null);
assert(!entry.route._installed);
assert(entry.route.overlayEntries.isEmpty);
assert(entry.currentState == _RouteLifecycle.push);
int index = _history.length - 1;
@ -5315,7 +5321,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
@optionalTypeArgs
void replace<T extends Object?>({required Route<dynamic> oldRoute, required Route<T> newRoute}) {
assert(!_debugLocked);
assert(oldRoute._navigator == this);
assert(oldRoute._isInstalledIn(this));
_replaceEntry(
_RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.replace),
oldRoute,
@ -5337,7 +5343,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
required RestorableRouteBuilder<T> newRouteBuilder,
Object? arguments,
}) {
assert(oldRoute._navigator == this);
assert(oldRoute._isInstalledIn(this));
assert(
_debugIsStaticCallback(newRouteBuilder),
'The provided routeBuilder must be a static function.',
@ -5365,7 +5371,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
return true;
}());
assert(entry.currentState == _RouteLifecycle.replace);
assert(entry.route._navigator == null);
assert(!entry.route._installed);
final int index = _history.indexWhere(_RouteEntry.isRoutePredicate(oldRoute));
assert(index >= 0, 'This Navigator does not contain the specified oldRoute.');
assert(
@ -5401,8 +5407,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
required Route<dynamic> anchorRoute,
required Route<T> newRoute,
}) {
assert(newRoute._navigator == null);
assert(anchorRoute._navigator == this);
assert(!newRoute._installed);
assert(anchorRoute._isInstalledIn(this));
_replaceEntryBelow(
_RouteEntry(newRoute, pageBased: false, initialState: _RouteLifecycle.replace),
anchorRoute,
@ -5425,7 +5431,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
required RestorableRouteBuilder<T> newRouteBuilder,
Object? arguments,
}) {
assert(anchorRoute._navigator == this);
assert(anchorRoute._isInstalledIn(this));
assert(
_debugIsStaticCallback(newRouteBuilder),
'The provided routeBuilder must be a static function.',
@ -5515,7 +5521,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
if (lastEntry == null) {
return false;
}
assert(lastEntry.route._navigator == this);
assert(lastEntry.route._isInstalledIn(this));
// TODO(justinmc): When the deprecated willPop method is removed, delete
// this code and use only popDisposition, below.
@ -5636,7 +5642,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
_debugLocked = true;
return true;
}());
assert(route._navigator == this);
assert(route._isInstalledIn(this));
final bool wasCurrent = route.isCurrent;
final _RouteEntry entry = _history.firstWhere(_RouteEntry.isRoutePredicate(route));
entry.complete(result, isReplaced: false, imperativeRemoval: true);
@ -5661,7 +5667,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
_debugLocked = true;
return true;
}());
assert(anchorRoute._navigator == this);
assert(anchorRoute._isInstalledIn(this));
final int anchorIndex = _history.indexWhere(_RouteEntry.isRoutePredicate(anchorRoute));
assert(anchorIndex >= 0, 'This Navigator does not contain the specified anchorRoute.');
assert(

47
packages/flutter/test/widgets/navigator_test.dart
View File

@ -280,6 +280,7 @@ void main() {
expect(tester.widget<Overlay>(find.byType(Overlay)).clipBehavior, Clip.none);
});
// Regression test for https://github.com/flutter/flutter/issues/170250.
testWidgets('Can remove page before they are added', (WidgetTester tester) async {
final GlobalKey<NavigatorState> key = GlobalKey<NavigatorState>();
Future<void> buildPages(List<Page<void>> pages) {
@ -337,6 +338,52 @@ void main() {
expect(tester.takeException(), isNull);
});
// Regression test for https://github.com/flutter/flutter/issues/177322.
testWidgets('Can remove page before they are added - case 2', (WidgetTester tester) async {
final GlobalKey<NavigatorState> key = GlobalKey<NavigatorState>();
Future<void> buildPages(List<Page<void>> pages) {
return tester.pumpWidget(
MediaQuery(
data: MediaQueryData.fromView(tester.view),
child: Directionality(
textDirection: TextDirection.ltr,
child: Navigator(key: key, pages: pages, onDidRemovePage: (_) {}),
),
),
);
}
const MaterialPage<void> page = MaterialPage<void>(
key: ValueKey<String>('page'),
child: Text('page'),
);
const MaterialPage<void> page1 = MaterialPage<void>(
key: ValueKey<String>('page1'),
child: Text('page1'),
);
const MaterialPage<void> page2 = MaterialPage<void>(
key: ValueKey<String>('page2'),
child: Text('page2'),
);
const MaterialPage<void> page3 = MaterialPage<void>(
key: ValueKey<String>('page3'),
child: Text('page3'),
);
await buildPages(<Page<void>>[page]);
expect(find.text('page'), findsOneWidget);
await buildPages(<Page<void>>[page, page1, page2, page3]);
// In mid transition;
await tester.pump(const Duration(microseconds: 150));
// Cause page1 and page2 to be removed before they are added.
await buildPages(<Page<void>>[page]);
await tester.pumpAndSettle();
expect(find.text('page'), findsOneWidget);
expect(tester.takeException(), isNull);
});
testWidgets('Zero transition page-based route correctly notifies observers when it is popped', (
WidgetTester tester,
) async {