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
This commit is contained in:
Mitchell Goodwin 2025-06-02 10:36:15 -07:00 committed by GitHub
parent ff2184fc8a
commit 269eaa4521
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 115 additions and 8 deletions

View File

@ -677,12 +677,11 @@ class _HeroFlight {
final HeroFlightDirection type = initialManifest.type;
switch (type) {
case HeroFlightDirection.pop:
return initial.value == 1.0 && initialManifest.isUserGestureTransition
return initialManifest.isUserGestureTransition
// During user gesture transitions, the animation controller isn't
// driving the reverse transition, but should still be in a previously
// completed stage with the initial value at 1.0.
? initial.status == AnimationStatus.completed
: initial.status == AnimationStatus.reverse;
// driving the reverse transition, so the status is not important.
||
initial.status == AnimationStatus.reverse;
case HeroFlightDirection.push:
return initial.value == 0.0 && initial.status == AnimationStatus.forward;
}
@ -923,8 +922,15 @@ class HeroController extends NavigatorObserver {
// For pop transitions driven by a user gesture: if the "to" page has
// maintainState = true, then the hero's final dimensions can be measured
// immediately because their page's layout is still valid.
if (isUserGestureTransition && flightType == HeroFlightDirection.pop && toRoute.maintainState) {
// immediately because their page's layout is still valid. Unless due to directly
// adding routes to the pages stack causing the route to never get laid out.
final RenderBox? fromRouteRenderBox = toRoute.subtreeContext?.findRenderObject() as RenderBox?;
final bool hasValidSize =
(fromRouteRenderBox?.hasSize ?? false) && fromRouteRenderBox!.size.isFinite;
if (isUserGestureTransition &&
flightType == HeroFlightDirection.pop &&
toRoute.maintainState &&
hasValidSize) {
_startHeroTransition(fromRoute, toRoute, flightType, isUserGestureTransition);
} else {
// Otherwise, delay measuring until the end of the next frame to allow
@ -934,7 +940,6 @@ class HeroController extends NavigatorObserver {
// frame completes, we'll know where the heroes in the `to` route are
// going to end up, and the `to` route will go back onstage.
toRoute.offstage = toRoute.animation!.value == 0.0;
WidgetsBinding.instance.addPostFrameCallback((Duration value) {
if (fromRoute.navigator == null || toRoute.navigator == null) {
return;

View File

@ -216,6 +216,73 @@ class MyStatefulWidgetState extends State<MyStatefulWidget> {
Widget build(BuildContext context) => Text(widget.value);
}
class DeepLinkApp extends StatefulWidget {
const DeepLinkApp({super.key});
static const CupertinoPage<void> _homeScreen = CupertinoPage<void>(
name: '/',
child: CupertinoPageScaffold(
navigationBar: CupertinoNavigationBar(middle: Text('First')),
child: Center(child: Text('Home Screen')),
),
);
static const CupertinoPage<void> _middleScreen = CupertinoPage<void>(
name: '/middle',
child: CupertinoPageScaffold(
navigationBar: CupertinoNavigationBar(middle: Text('Second')),
child: Center(child: Text('Middle Screen')),
),
);
static const CupertinoPage<void> _lastScreen = CupertinoPage<void>(
name: '/middle/last',
child: CupertinoPageScaffold(
navigationBar: CupertinoNavigationBar(middle: Text('Third')),
child: Center(child: Text('Last Screen')),
),
);
@override
DeepLinkAppState createState() => DeepLinkAppState();
}
class DeepLinkAppState extends State<DeepLinkApp> {
late List<Page<dynamic>> _pages;
@override
void initState() {
super.initState();
_pages = <Page<dynamic>>[DeepLinkApp._homeScreen];
}
void goToDeepScreen() {
setState(() {
_pages = <Page<dynamic>>[
DeepLinkApp._homeScreen,
DeepLinkApp._middleScreen,
DeepLinkApp._lastScreen,
];
});
}
@override
Widget build(BuildContext context) {
return CupertinoApp(
builder: (BuildContext context, Widget? child) {
return Navigator(
pages: _pages,
onDidRemovePage: (Page<Object?> page) {
setState(() {
if (_pages.length > 1) {
_pages = List<Page<dynamic>>.from(_pages)..removeLast();
}
});
},
);
},
);
}
}
Future<void> main() async {
final ui.Image testImage = await createTestImage();
@ -2984,6 +3051,41 @@ Future<void> main() async {
}),
);
// Regression test for https://github.com/flutter/flutter/issues/168267.
testWidgets('Check if previous page is laid out on backswipe gesture before flight', (
WidgetTester tester,
) async {
final GlobalKey<DeepLinkAppState> appKey = GlobalKey();
await tester.pumpWidget(DeepLinkApp(key: appKey));
await tester.pumpAndSettle();
expect(find.text('Home Screen'), findsOneWidget);
expect(find.text('Last Screen'), findsNothing);
appKey.currentState?.goToDeepScreen();
await tester.pumpAndSettle();
expect(find.text('Home Screen'), findsNothing);
expect(find.text('Last Screen'), findsOneWidget);
final TestGesture gesture = await tester.startGesture(const Offset(0.01, 300));
await gesture.moveTo(const Offset(10, 300));
await tester.pump();
// Should not throw an assert here for size and finite space.
await gesture.moveTo(const Offset(500, 300));
await tester.pump();
await gesture.up();
await tester.pumpAndSettle();
expect(find.text('Home Screen'), findsNothing);
expect(find.text('Middle Screen'), findsOneWidget);
expect(find.text('Last Screen'), findsNothing);
});
// Regression test for https://github.com/flutter/flutter/issues/40239.
testWidgets(
'In a pop transition, when fromHero is null, the to hero should eventually become visible',