Unverified Commit 2a6f9ad6 authored by Paul Berry's avatar Paul Berry Committed by GitHub

Replace NavigatorObserver._navigator with a static expando. (#109238)

parent b1dab531
...@@ -619,8 +619,18 @@ abstract class Page<T> extends RouteSettings { ...@@ -619,8 +619,18 @@ abstract class Page<T> extends RouteSettings {
/// An interface for observing the behavior of a [Navigator]. /// An interface for observing the behavior of a [Navigator].
class NavigatorObserver { class NavigatorObserver {
/// The navigator that the observer is observing, if any. /// The navigator that the observer is observing, if any.
NavigatorState? get navigator => _navigator; NavigatorState? get navigator => _navigators[this];
NavigatorState? _navigator;
/// Expando mapping instances of NavigatorObserver to their associated
/// NavigatorState (or `null`, if there is no associated NavigatorState). The
/// reason we don't simply use a private instance field of type
/// `NavigatorState?` is because as part of implementing
/// https://github.com/dart-lang/language/issues/2020, it will soon become a
/// runtime error to invoke a private member that is mocked in another
/// library. By using an expando rather than an instance field, we ensure
/// that a mocked NavigatorObserver can still properly keep track of its
/// associated NavigatorState.
static final Expando<NavigatorState> _navigators = Expando<NavigatorState>();
/// The [Navigator] pushed `route`. /// The [Navigator] pushed `route`.
/// ///
...@@ -3236,7 +3246,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3236,7 +3246,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
}()); }());
for (final NavigatorObserver observer in widget.observers) { for (final NavigatorObserver observer in widget.observers) {
assert(observer.navigator == null); assert(observer.navigator == null);
observer._navigator = this; NavigatorObserver._navigators[observer] = this;
} }
_effectiveObservers = widget.observers; _effectiveObservers = widget.observers;
...@@ -3359,12 +3369,12 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3359,12 +3369,12 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
ServicesBinding.instance.addPostFrameCallback((Duration timestamp) { ServicesBinding.instance.addPostFrameCallback((Duration timestamp) {
// We only check if this navigator still owns the hero controller. // We only check if this navigator still owns the hero controller.
if (_heroControllerFromScope == newHeroController) { if (_heroControllerFromScope == newHeroController) {
final bool hasHeroControllerOwnerShip = _heroControllerFromScope!._navigator == this; final bool hasHeroControllerOwnerShip = _heroControllerFromScope!.navigator == this;
if (!hasHeroControllerOwnerShip || if (!hasHeroControllerOwnerShip ||
previousOwner._heroControllerFromScope == newHeroController) { previousOwner._heroControllerFromScope == newHeroController) {
final NavigatorState otherOwner = hasHeroControllerOwnerShip final NavigatorState otherOwner = hasHeroControllerOwnerShip
? previousOwner ? previousOwner
: _heroControllerFromScope!._navigator!; : _heroControllerFromScope!.navigator!;
FlutterError.reportError( FlutterError.reportError(
FlutterErrorDetails( FlutterErrorDetails(
exception: FlutterError( exception: FlutterError(
...@@ -3386,12 +3396,12 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3386,12 +3396,12 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
} }
return true; return true;
}()); }());
newHeroController._navigator = this; NavigatorObserver._navigators[newHeroController] = this;
} }
// Only unsubscribe the hero controller when it is currently subscribe to // Only unsubscribe the hero controller when it is currently subscribe to
// this navigator. // this navigator.
if (_heroControllerFromScope?._navigator == this) { if (_heroControllerFromScope?.navigator == this) {
_heroControllerFromScope?._navigator = null; NavigatorObserver._navigators[_heroControllerFromScope!] = null;
} }
_heroControllerFromScope = newHeroController; _heroControllerFromScope = newHeroController;
_updateEffectiveObservers(); _updateEffectiveObservers();
...@@ -3440,11 +3450,11 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3440,11 +3450,11 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
}()); }());
if (oldWidget.observers != widget.observers) { if (oldWidget.observers != widget.observers) {
for (final NavigatorObserver observer in oldWidget.observers) { for (final NavigatorObserver observer in oldWidget.observers) {
observer._navigator = null; NavigatorObserver._navigators[observer] = null;
} }
for (final NavigatorObserver observer in widget.observers) { for (final NavigatorObserver observer in widget.observers) {
assert(observer.navigator == null); assert(observer.navigator == null);
observer._navigator = this; NavigatorObserver._navigators[observer] = this;
} }
_updateEffectiveObservers(); _updateEffectiveObservers();
} }
...@@ -3489,7 +3499,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3489,7 +3499,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
@override @override
void deactivate() { void deactivate() {
for (final NavigatorObserver observer in _effectiveObservers) { for (final NavigatorObserver observer in _effectiveObservers) {
observer._navigator = null; NavigatorObserver._navigators[observer] = null;
} }
super.deactivate(); super.deactivate();
} }
...@@ -3499,7 +3509,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3499,7 +3509,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
super.activate(); super.activate();
for (final NavigatorObserver observer in _effectiveObservers) { for (final NavigatorObserver observer in _effectiveObservers) {
assert(observer.navigator == null); assert(observer.navigator == null);
observer._navigator = this; NavigatorObserver._navigators[observer] = this;
} }
} }
...@@ -3512,7 +3522,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3512,7 +3522,7 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
}()); }());
assert(() { assert(() {
for (final NavigatorObserver observer in _effectiveObservers) { for (final NavigatorObserver observer in _effectiveObservers) {
assert(observer._navigator != this); assert(observer.navigator != this);
} }
return true; return true;
}()); }());
......
...@@ -3865,6 +3865,39 @@ void main() { ...@@ -3865,6 +3865,39 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(focusNode.hasFocus, true); expect(focusNode.hasFocus, true);
}); });
testWidgets('class implementing NavigatorObserver can be used without problems', (WidgetTester tester) async {
final _MockNavigatorObserver observer = _MockNavigatorObserver();
Widget build([Key? key]) {
return Directionality(
textDirection: TextDirection.ltr,
child: Navigator(
key: key,
observers: <NavigatorObserver>[observer],
onGenerateRoute: (RouteSettings settings) {
return PageRouteBuilder<void>(
settings: settings,
pageBuilder: (BuildContext _, Animation<double> __, Animation<double> ___) {
return Container();
},
);
},
),
);
}
await tester.pumpWidget(build());
observer._checkInvocations(<Symbol>[#navigator, #didPush]);
await tester.pumpWidget(Container(child: build()));
observer._checkInvocations(<Symbol>[#navigator, #didPush, #navigator]);
await tester.pumpWidget(Container());
observer._checkInvocations(<Symbol>[#navigator]);
final GlobalKey key = GlobalKey();
await tester.pumpWidget(build(key));
observer._checkInvocations(<Symbol>[#navigator, #didPush]);
await tester.pumpWidget(Container(child: build(key)));
observer._checkInvocations(<Symbol>[#navigator, #navigator]);
});
} }
typedef AnnouncementCallBack = void Function(Route<dynamic>?); typedef AnnouncementCallBack = void Function(Route<dynamic>?);
...@@ -4115,3 +4148,18 @@ class ZeroDurationPageRoute extends PageRoute<void> { ...@@ -4115,3 +4148,18 @@ class ZeroDurationPageRoute extends PageRoute<void> {
@override @override
String? get barrierLabel => null; String? get barrierLabel => null;
} }
class _MockNavigatorObserver implements NavigatorObserver {
final List<Symbol> _invocations = <Symbol>[];
void _checkInvocations(List<Symbol> expected) {
expect(_invocations, expected);
_invocations.clear();
}
@override
Object? noSuchMethod(Invocation invocation) {
_invocations.add(invocation.memberName);
return null;
}
}
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