Skip to content

Commit 269eaa4

Browse files
Check to see if previous page is laid out before starting hero flight (#169633)
Fixes #168267 When going back to a page that never rendered with a hero transition, the app was hanging. This adds a check to see if a page needs to layout before starting the hero flight, and if so, adding a frame delay. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- 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
1 parent ff2184f commit 269eaa4

2 files changed

Lines changed: 115 additions & 8 deletions

File tree

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -677,12 +677,11 @@ class _HeroFlight {
677677
final HeroFlightDirection type = initialManifest.type;
678678
switch (type) {
679679
case HeroFlightDirection.pop:
680-
return initial.value == 1.0 && initialManifest.isUserGestureTransition
680+
return initialManifest.isUserGestureTransition
681681
// During user gesture transitions, the animation controller isn't
682-
// driving the reverse transition, but should still be in a previously
683-
// completed stage with the initial value at 1.0.
684-
? initial.status == AnimationStatus.completed
685-
: initial.status == AnimationStatus.reverse;
682+
// driving the reverse transition, so the status is not important.
683+
||
684+
initial.status == AnimationStatus.reverse;
686685
case HeroFlightDirection.push:
687686
return initial.value == 0.0 && initial.status == AnimationStatus.forward;
688687
}
@@ -923,8 +922,15 @@ class HeroController extends NavigatorObserver {
923922

924923
// For pop transitions driven by a user gesture: if the "to" page has
925924
// maintainState = true, then the hero's final dimensions can be measured
926-
// immediately because their page's layout is still valid.
927-
if (isUserGestureTransition && flightType == HeroFlightDirection.pop && toRoute.maintainState) {
925+
// immediately because their page's layout is still valid. Unless due to directly
926+
// adding routes to the pages stack causing the route to never get laid out.
927+
final RenderBox? fromRouteRenderBox = toRoute.subtreeContext?.findRenderObject() as RenderBox?;
928+
final bool hasValidSize =
929+
(fromRouteRenderBox?.hasSize ?? false) && fromRouteRenderBox!.size.isFinite;
930+
if (isUserGestureTransition &&
931+
flightType == HeroFlightDirection.pop &&
932+
toRoute.maintainState &&
933+
hasValidSize) {
928934
_startHeroTransition(fromRoute, toRoute, flightType, isUserGestureTransition);
929935
} else {
930936
// Otherwise, delay measuring until the end of the next frame to allow
@@ -934,7 +940,6 @@ class HeroController extends NavigatorObserver {
934940
// frame completes, we'll know where the heroes in the `to` route are
935941
// going to end up, and the `to` route will go back onstage.
936942
toRoute.offstage = toRoute.animation!.value == 0.0;
937-
938943
WidgetsBinding.instance.addPostFrameCallback((Duration value) {
939944
if (fromRoute.navigator == null || toRoute.navigator == null) {
940945
return;

packages/flutter/test/widgets/heroes_test.dart

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,73 @@ class MyStatefulWidgetState extends State<MyStatefulWidget> {
216216
Widget build(BuildContext context) => Text(widget.value);
217217
}
218218

219+
class DeepLinkApp extends StatefulWidget {
220+
const DeepLinkApp({super.key});
221+
222+
static const CupertinoPage<void> _homeScreen = CupertinoPage<void>(
223+
name: '/',
224+
child: CupertinoPageScaffold(
225+
navigationBar: CupertinoNavigationBar(middle: Text('First')),
226+
child: Center(child: Text('Home Screen')),
227+
),
228+
);
229+
static const CupertinoPage<void> _middleScreen = CupertinoPage<void>(
230+
name: '/middle',
231+
child: CupertinoPageScaffold(
232+
navigationBar: CupertinoNavigationBar(middle: Text('Second')),
233+
child: Center(child: Text('Middle Screen')),
234+
),
235+
);
236+
static const CupertinoPage<void> _lastScreen = CupertinoPage<void>(
237+
name: '/middle/last',
238+
child: CupertinoPageScaffold(
239+
navigationBar: CupertinoNavigationBar(middle: Text('Third')),
240+
child: Center(child: Text('Last Screen')),
241+
),
242+
);
243+
244+
@override
245+
DeepLinkAppState createState() => DeepLinkAppState();
246+
}
247+
248+
class DeepLinkAppState extends State<DeepLinkApp> {
249+
late List<Page<dynamic>> _pages;
250+
251+
@override
252+
void initState() {
253+
super.initState();
254+
_pages = <Page<dynamic>>[DeepLinkApp._homeScreen];
255+
}
256+
257+
void goToDeepScreen() {
258+
setState(() {
259+
_pages = <Page<dynamic>>[
260+
DeepLinkApp._homeScreen,
261+
DeepLinkApp._middleScreen,
262+
DeepLinkApp._lastScreen,
263+
];
264+
});
265+
}
266+
267+
@override
268+
Widget build(BuildContext context) {
269+
return CupertinoApp(
270+
builder: (BuildContext context, Widget? child) {
271+
return Navigator(
272+
pages: _pages,
273+
onDidRemovePage: (Page<Object?> page) {
274+
setState(() {
275+
if (_pages.length > 1) {
276+
_pages = List<Page<dynamic>>.from(_pages)..removeLast();
277+
}
278+
});
279+
},
280+
);
281+
},
282+
);
283+
}
284+
}
285+
219286
Future<void> main() async {
220287
final ui.Image testImage = await createTestImage();
221288

@@ -2984,6 +3051,41 @@ Future<void> main() async {
29843051
}),
29853052
);
29863053

3054+
// Regression test for https://github.com/flutter/flutter/issues/168267.
3055+
testWidgets('Check if previous page is laid out on backswipe gesture before flight', (
3056+
WidgetTester tester,
3057+
) async {
3058+
final GlobalKey<DeepLinkAppState> appKey = GlobalKey();
3059+
await tester.pumpWidget(DeepLinkApp(key: appKey));
3060+
3061+
await tester.pumpAndSettle();
3062+
3063+
expect(find.text('Home Screen'), findsOneWidget);
3064+
expect(find.text('Last Screen'), findsNothing);
3065+
3066+
appKey.currentState?.goToDeepScreen();
3067+
3068+
await tester.pumpAndSettle();
3069+
3070+
expect(find.text('Home Screen'), findsNothing);
3071+
expect(find.text('Last Screen'), findsOneWidget);
3072+
3073+
final TestGesture gesture = await tester.startGesture(const Offset(0.01, 300));
3074+
3075+
await gesture.moveTo(const Offset(10, 300));
3076+
await tester.pump();
3077+
// Should not throw an assert here for size and finite space.
3078+
await gesture.moveTo(const Offset(500, 300));
3079+
await tester.pump();
3080+
3081+
await gesture.up();
3082+
await tester.pumpAndSettle();
3083+
3084+
expect(find.text('Home Screen'), findsNothing);
3085+
expect(find.text('Middle Screen'), findsOneWidget);
3086+
expect(find.text('Last Screen'), findsNothing);
3087+
});
3088+
29873089
// Regression test for https://github.com/flutter/flutter/issues/40239.
29883090
testWidgets(
29893091
'In a pop transition, when fromHero is null, the to hero should eventually become visible',

0 commit comments

Comments
 (0)