[State Restoration] Shrine #389

shihaohong · 2020-12-16T03:37:03Z

Description

Adds State Restoration for Shrine mini app.

Bumps along the way (Documenting here)

Routes issue

/cc @goderbauer I'm not sure what's the problem here and I was debugging this for quite a while, but every time I try to restore the Shrine mini app, I get the following error message:

(had to use a screenshot because VSCode is giving me trouble with highlighting/copy-pasting the error)

Some of the lines may be a little off because I put some print statements in to try and debug and am quite lost. The setup of this app is similar to the Reply app, so I'm surprised that this one is acting up in this way. Any ideas?

To reproduce:

Open the Gallery app on an Android device
Swipe the carousel to the Shrine app card and tap it.
On the Shrine app home page, kill the activity and reopen the app.

Hot reload issue

The app seems to be state restorable now. However, I somehow broke hot reload with this PR... Any change in the code for gallery/studies/shrine/app.dart while on any screen in the gallery results in the following error message:

Unhandled exception:
root::package:gallery/studies/shrine/app.dart::_RestorableAppStateModel::@methods::package:flutter/src/widgets/restoration.dart::_debugAssertNotDisposed is already bound
#0      CanonicalName.bindTo (package:kernel/canonical_name.dart:192:7)
#1      Class.computeCanonicalNames (package:kernel/ast.dart:1201:51)
#2      Library.computeCanonicalNames (package:kernel/ast.dart:559:14)
#3      Component.computeCanonicalNamesForLibrary (package:kernel/ast.dart:10821:13)
#4      Component.computeCanonicalNames (package:kernel/ast.dart:10783:7)
#5      sortComponent (package:vm/kernel_front_end.dart:679:13)
#6      FrontendCompiler.writeDillFile (package:frontend_server/frontend_server.dart:656:5)
#7      FrontendCompiler.recompileDelta (package:frontend_server/frontend_server.dart:766:13)
<asynchronous suspension>

the Dart compiler exited unexpectedly.
Exited (1)

I'm not really sure what's going on. The app works fine otherwise.

shihaohong · 2020-12-16T03:38:16Z

lib/studies/shrine/app.dart

+        child: WillPopScope(
+          onWillPop: _onWillPop,
+          child: MaterialApp(
+            restorationScopeId: 'shrineApp',


The error is shows up with the addition of this line.

goderbauer · 2020-12-16T18:20:12Z

lib/studies/shrine/app.dart

+            onGenerateInitialRoutes: (_) {
+              return [
+                MaterialPageRoute<void>(
+                  builder: (context) => const LoginPage(),


The problem here is that the route(s) returned by onGenerateInitialRoutes do not have Route.settings.name set. This means, they cannot be restored and you end up with an empty set of routes to restore. We should provide a better error here. I'll file an issue for it.

In this case, the fix is really simple: Just remove the onGenerateInitialRoutes argument altogether. The app is already specifying an initialRoute: ShrineApp.loginRoute, which when looked up in the routes table below produces a route with a LoginPage - that's exactly the same thing that onGenerateInitialRoutes is doing. So instead of specifying it, we can just rely on the defaults.

Filed flutter/flutter#72453.

…able-shrine

shihaohong · 2020-12-17T07:31:04Z

The app seems to be state restorable now. However, I somehow broke hot reload with this PR... Any change in the code for gallery/studies/shrine/app.dart while on any screen in the gallery results in the following error message:

Unhandled exception:
root::package:gallery/studies/shrine/app.dart::_RestorableAppStateModel::@methods::package:flutter/src/widgets/restoration.dart::_debugAssertNotDisposed is already bound
#0      CanonicalName.bindTo (package:kernel/canonical_name.dart:192:7)
#1      Class.computeCanonicalNames (package:kernel/ast.dart:1201:51)
#2      Library.computeCanonicalNames (package:kernel/ast.dart:559:14)
#3      Component.computeCanonicalNamesForLibrary (package:kernel/ast.dart:10821:13)
#4      Component.computeCanonicalNames (package:kernel/ast.dart:10783:7)
#5      sortComponent (package:vm/kernel_front_end.dart:679:13)
#6      FrontendCompiler.writeDillFile (package:frontend_server/frontend_server.dart:656:5)
#7      FrontendCompiler.recompileDelta (package:frontend_server/frontend_server.dart:766:13)
<asynchronous suspension>

the Dart compiler exited unexpectedly.
Exited (1)

I'm not really sure what's going on. The app works fine otherwise.

Edit: I also just realized that the same thing happens for the Reply mini app.

shihaohong · 2020-12-21T04:45:17Z

I made a small reproducible test app with the hot reload crash: https://github.com/shihaohong/state_restoration_provider_crash

guidezpl

  _    ___ _____ __  __ 
 | |  / __|_   _|  \/  |
 | |_| (_ | | | | |\/| |
 |____\___| |_| |_|  |_|

Merging should be held pending the hot reload crash fix

guidezpl · 2020-12-21T12:42:49Z

lib/studies/shrine/app.dart


  @override
  Widget build(BuildContext context) {
+    if (_isExpandingControllerCompleted.value !=


Using a statusListener for this feels a bit clunky. Could there be a cleaner way to do this?

I've been thinking about this too... Let me see if I can come up with a better API for this, here and everywhere else tab controller is being used.

I refactored this out into its own class here: https://github.com/flutter/gallery/pull/389/files#diff-4576115e6281d08372fae69eebb7d35cff0bc534481112f35d3825370064b6abR32

Let me know what you think or if you have any improvement suggestions!

Nice! For clarity, I wonder if setAnimationController would be better named as registerStatusListener or something similar

shihaohong · 2020-12-21T13:34:31Z

Filed an issue at dart-lang/sdk#44523 and flutter/flutter#72700 about the hot reload crash. We'll see what the Dart folks say and see if they can identify the root cause.

…ry into state-restorable-shrine

shihaohong · 2020-12-22T07:39:51Z

According to flutter/flutter#72700 (comment), this seems like a Dart issue unrelated to state restoration. Should we still hold off until that issue is fixed before moving forward with this PR?

…ry into state-restorable-shrine

guidezpl · 2020-12-23T10:54:22Z

If unrelated, this seems fine to merge

WIP - state restoration for shrine app

888c76c

shihaohong commented Dec 16, 2020

View reviewed changes

shihaohong mentioned this pull request Dec 16, 2020

☂️ State Restoration Remaining Work flutter/flutter#62916

Closed

24 tasks

goderbauer reviewed Dec 16, 2020

View reviewed changes

shihaohong added 7 commits December 17, 2020 11:25

Remove onGenerateInitialRoutes

59e9423

Restorable login page

2946130

WIP - app store

102885b

Restorable app state

247fc24

Flutter format

0b683a3

Merge branch 'master' of github.com:flutter/gallery into state-restor…

d1c3a82

…able-shrine

Restorable menu and cart pages

df95c3d

Remove unnecessary restoration scopes

0611d58

shihaohong changed the title ~~WIP - state restoration for shrine app~~ State Restoration for Shrine Dec 17, 2020

shihaohong requested a review from guidezpl December 21, 2020 09:31

shihaohong changed the title ~~State Restoration for Shrine~~ [State Restoration] Shrine Dec 21, 2020

Dispose

5ddd435

guidezpl approved these changes Dec 21, 2020

View reviewed changes

Shi-Hao Hong and others added 3 commits December 21, 2020 21:52

Merge branch 'master' into state-restorable-shrine

39a343b

Refactor to use a _RestorableAnimationValue

f2c1db8

Merge branch 'state-restorable-shrine' of github.com:shihaohong/galle…

350fc5b

…ry into state-restorable-shrine

Shi-Hao Hong and others added 3 commits December 22, 2020 17:31

Merge branch 'master' into state-restorable-shrine

8d0dec6

remember to dispose, no need for setState usage

107545b

Merge branch 'state-restorable-shrine' of github.com:shihaohong/galle…

39203e6

…ry into state-restorable-shrine

Merge branch 'master' into state-restorable-shrine

f213190

Merge branch 'master' into state-restorable-shrine

83bdf2b

shihaohong merged commit 82a50e7 into flutter:master Dec 24, 2020

shihaohong deleted the state-restorable-shrine branch December 24, 2020 10:11

[State Restoration] Shrine #389

[State Restoration] Shrine #389

Uh oh!

Conversation

shihaohong commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Bumps along the way (Documenting here)

Routes issue

Hot reload issue

Uh oh!

shihaohong Dec 16, 2020

Choose a reason for hiding this comment

Uh oh!

goderbauer Dec 16, 2020

Choose a reason for hiding this comment

Uh oh!

goderbauer Dec 16, 2020

Choose a reason for hiding this comment

Uh oh!

shihaohong commented Dec 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shihaohong commented Dec 21, 2020

Uh oh!

guidezpl left a comment

Choose a reason for hiding this comment

Uh oh!

guidezpl Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

shihaohong Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

shihaohong Dec 22, 2020

Choose a reason for hiding this comment

Uh oh!

guidezpl Dec 22, 2020

Choose a reason for hiding this comment

Uh oh!

shihaohong commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shihaohong commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guidezpl commented Dec 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shihaohong commented Dec 16, 2020 •

edited

Loading

shihaohong commented Dec 17, 2020 •

edited

Loading

shihaohong commented Dec 21, 2020 •

edited

Loading

shihaohong commented Dec 22, 2020 •

edited

Loading