Skip to content

Commit afbb80b

Browse files
authored
fix navigator observer announcement order (flutter#57605)
* fix navigator observer announcement order * addressing review comments * update * fix analyzer
1 parent 3e273b1 commit afbb80b

File tree

2 files changed

+348
-46
lines changed

2 files changed

+348
-46
lines changed

packages/flutter/lib/src/widgets/navigator.dart

Lines changed: 101 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
import 'dart:async';
6+
import 'dart:collection';
67
import 'dart:convert';
78
import 'dart:developer' as developer;
89

@@ -2335,7 +2336,7 @@ class _RouteEntry extends RouteTransitionRecord {
23352336
return page.canUpdate(routePage);
23362337
}
23372338

2338-
void handleAdd({ @required NavigatorState navigator}) {
2339+
void handleAdd({ @required NavigatorState navigator, @required Route<dynamic> previousPresent }) {
23392340
assert(currentState == _RouteLifecycle.add);
23402341
assert(navigator != null);
23412342
assert(navigator._debugLocked);
@@ -2344,6 +2345,9 @@ class _RouteEntry extends RouteTransitionRecord {
23442345
route.install();
23452346
assert(route.overlayEntries.isNotEmpty);
23462347
currentState = _RouteLifecycle.adding;
2348+
navigator._observedRouteAdditions.add(
2349+
_NavigatorPushObservation(route, previousPresent)
2350+
);
23472351
}
23482352

23492353
void handlePush({ @required NavigatorState navigator, @required bool isNewFirst, @required Route<dynamic> previous, @required Route<dynamic> previousPresent }) {
@@ -2377,12 +2381,14 @@ class _RouteEntry extends RouteTransitionRecord {
23772381
}
23782382

23792383
if (previousState == _RouteLifecycle.replace || previousState == _RouteLifecycle.pushReplace) {
2380-
for (final NavigatorObserver observer in navigator.widget.observers)
2381-
observer.didReplace(newRoute: route, oldRoute: previousPresent);
2384+
navigator._observedRouteAdditions.add(
2385+
_NavigatorReplaceObservation(route, previousPresent)
2386+
);
23822387
} else {
23832388
assert(previousState == _RouteLifecycle.push);
2384-
for (final NavigatorObserver observer in navigator.widget.observers)
2385-
observer.didPush(route, previousPresent);
2389+
navigator._observedRouteAdditions.add(
2390+
_NavigatorPushObservation(route, previousPresent)
2391+
);
23862392
}
23872393
}
23882394

@@ -2396,8 +2402,9 @@ class _RouteEntry extends RouteTransitionRecord {
23962402
assert(navigator._debugLocked);
23972403
assert(route._navigator == navigator);
23982404
currentState = _RouteLifecycle.popping;
2399-
for (final NavigatorObserver observer in navigator.widget.observers)
2400-
observer.didPop(route, previousPresent);
2405+
navigator._observedRouteDeletions.add(
2406+
_NavigatorPopObservation(route, previousPresent)
2407+
);
24012408
}
24022409

24032410
void handleRemoval({ @required NavigatorState navigator, @required Route<dynamic> previousPresent }) {
@@ -2406,21 +2413,20 @@ class _RouteEntry extends RouteTransitionRecord {
24062413
assert(route._navigator == navigator);
24072414
currentState = _RouteLifecycle.removing;
24082415
if (_reportRemovalToObserver) {
2409-
for (final NavigatorObserver observer in navigator.widget.observers)
2410-
observer.didRemove(route, previousPresent);
2416+
navigator._observedRouteDeletions.add(
2417+
_NavigatorRemoveObservation(route, previousPresent)
2418+
);
24112419
}
24122420
}
24132421

24142422
bool doingPop = false;
24152423

2416-
void didAdd({ @required NavigatorState navigator, @required bool isNewFirst, @required Route<dynamic> previous, @required Route<dynamic> previousPresent }) {
2424+
void didAdd({ @required NavigatorState navigator, @required bool isNewFirst}) {
24172425
route.didAdd();
24182426
currentState = _RouteLifecycle.idle;
24192427
if (isNewFirst) {
24202428
route.didChangeNext(null);
24212429
}
2422-
for (final NavigatorObserver observer in navigator.widget.observers)
2423-
observer.didPush(route, previousPresent);
24242430
}
24252431

24262432
void pop<T>(T result) {
@@ -2579,10 +2585,71 @@ class _RouteEntry extends RouteTransitionRecord {
25792585
}
25802586
}
25812587

2588+
abstract class _NavigatorObservation {
2589+
_NavigatorObservation(
2590+
this.primaryRoute,
2591+
this.secondaryRoute,
2592+
);
2593+
final Route<dynamic> primaryRoute;
2594+
final Route<dynamic> secondaryRoute;
2595+
2596+
void notify(NavigatorObserver observer);
2597+
}
2598+
2599+
class _NavigatorPushObservation extends _NavigatorObservation {
2600+
_NavigatorPushObservation(
2601+
Route<dynamic> primaryRoute,
2602+
Route<dynamic> secondaryRoute
2603+
) : super(primaryRoute, secondaryRoute);
2604+
2605+
@override
2606+
void notify(NavigatorObserver observer) {
2607+
observer.didPush(primaryRoute, secondaryRoute);
2608+
}
2609+
}
2610+
2611+
class _NavigatorPopObservation extends _NavigatorObservation {
2612+
_NavigatorPopObservation(
2613+
Route<dynamic> primaryRoute,
2614+
Route<dynamic> secondaryRoute
2615+
) : super(primaryRoute, secondaryRoute);
2616+
2617+
@override
2618+
void notify(NavigatorObserver observer) {
2619+
observer.didPop(primaryRoute, secondaryRoute);
2620+
}
2621+
}
2622+
2623+
class _NavigatorRemoveObservation extends _NavigatorObservation {
2624+
_NavigatorRemoveObservation(
2625+
Route<dynamic> primaryRoute,
2626+
Route<dynamic> secondaryRoute
2627+
) : super(primaryRoute, secondaryRoute);
2628+
2629+
@override
2630+
void notify(NavigatorObserver observer) {
2631+
observer.didRemove(primaryRoute, secondaryRoute);
2632+
}
2633+
}
2634+
2635+
class _NavigatorReplaceObservation extends _NavigatorObservation {
2636+
_NavigatorReplaceObservation(
2637+
Route<dynamic> primaryRoute,
2638+
Route<dynamic> secondaryRoute
2639+
) : super(primaryRoute, secondaryRoute);
2640+
2641+
@override
2642+
void notify(NavigatorObserver observer) {
2643+
observer.didReplace(newRoute: primaryRoute, oldRoute: secondaryRoute);
2644+
}
2645+
}
2646+
25822647
/// The state for a [Navigator] widget.
25832648
class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
25842649
final GlobalKey<OverlayState> _overlayKey = GlobalKey<OverlayState>();
25852650
List<_RouteEntry> _history = <_RouteEntry>[];
2651+
final Queue<_NavigatorObservation> _observedRouteAdditions = Queue<_NavigatorObservation>();
2652+
final Queue<_NavigatorObservation> _observedRouteDeletions = Queue<_NavigatorObservation>();
25862653

25872654
/// The [FocusScopeNode] for the [FocusScope] that encloses the routes.
25882655
final FocusScopeNode focusScopeNode = FocusScopeNode(debugLabel: 'Navigator Scope');
@@ -2984,15 +3051,14 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
29843051
assert(rearrangeOverlay);
29853052
entry.handleAdd(
29863053
navigator: this,
3054+
previousPresent: _getRouteBefore(index - 1, _RouteEntry.isPresentPredicate)?.route,
29873055
);
29883056
assert(entry.currentState == _RouteLifecycle.adding);
29893057
continue;
29903058
case _RouteLifecycle.adding:
29913059
if (canRemoveOrAdd || next == null) {
29923060
entry.didAdd(
29933061
navigator: this,
2994-
previous: previous?.route,
2995-
previousPresent: _getRouteBefore(index - 1, _RouteEntry.isPresentPredicate)?.route,
29963062
isNewFirst: next == null
29973063
);
29983064
assert(entry.currentState == _RouteLifecycle.idle);
@@ -3079,6 +3145,10 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
30793145
entry = previous;
30803146
previous = index > 0 ? _history[index - 1] : null;
30813147
}
3148+
3149+
// Informs navigator observers about route changes.
3150+
_flushObserverNotifications();
3151+
30823152
// Now that the list is clean, send the didChangeNext/didChangePrevious
30833153
// notifications.
30843154
_flushRouteAnnouncement();
@@ -3102,6 +3172,23 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
31023172
overlay?.rearrange(_allRouteOverlayEntries);
31033173
}
31043174

3175+
void _flushObserverNotifications() {
3176+
if (widget.observers.isEmpty) {
3177+
_observedRouteDeletions.clear();
3178+
_observedRouteAdditions.clear();
3179+
return;
3180+
}
3181+
while (_observedRouteAdditions.isNotEmpty) {
3182+
final _NavigatorObservation observation = _observedRouteAdditions.removeLast();
3183+
widget.observers.forEach(observation.notify);
3184+
}
3185+
3186+
while (_observedRouteDeletions.isNotEmpty) {
3187+
final _NavigatorObservation observation = _observedRouteDeletions.removeFirst();
3188+
widget.observers.forEach(observation.notify);
3189+
}
3190+
}
3191+
31053192
void _flushRouteAnnouncement() {
31063193
int index = _history.length - 1;
31073194
while (index >= 0) {

0 commit comments

Comments
 (0)