-
Notifications
You must be signed in to change notification settings - Fork 6k
Wire up channel for restoration data #18042
Changes from all commits
1a06df7
c7ee80b
b207c19
b2a4b80
d7459ca
95563f6
91fe70f
07ac960
9b56d68
09ec445
98dd918
d2c4074
43b10e0
463b7e6
3971edb
3fca33b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.EXTRA_BACKGROUND_MODE; | ||
| import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.EXTRA_CACHED_ENGINE_ID; | ||
| import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.EXTRA_DESTROY_ENGINE_WITH_ACTIVITY; | ||
| import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.EXTRA_ENABLE_STATE_RESTORATION; | ||
| import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.EXTRA_INITIAL_ROUTE; | ||
| import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.INITIAL_ROUTE_META_DATA_KEY; | ||
| import static io.flutter.embedding.android.FlutterActivityLaunchConfigs.NORMAL_THEME_META_DATA_KEY; | ||
|
|
@@ -63,6 +64,7 @@ | |
| * <li>Chooses Flutter's initial route. | ||
| * <li>Renders {@code Activity} transparently, if desired. | ||
| * <li>Offers hooks for subclasses to provide and configure a {@link FlutterEngine}. | ||
| * <li>Save and restore instance state, see {@code #shouldRestoreAndSaveState()}; | ||
| * </ul> | ||
| * | ||
| * <p><strong>Dart entrypoint, initial route, and app bundle path</strong> | ||
|
|
@@ -949,6 +951,18 @@ public void onFlutterUiNoLongerDisplayed() { | |
| // no-op | ||
| } | ||
|
|
||
| @Override | ||
| public boolean shouldRestoreAndSaveState() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please document this in a doc section in the class doc |
||
| if (getIntent().hasExtra(EXTRA_ENABLE_STATE_RESTORATION)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sprinkle some comments here for while some values were chosen |
||
| return getIntent().getBooleanExtra(EXTRA_ENABLE_STATE_RESTORATION, false); | ||
| } | ||
goderbauer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (getCachedEngineId() != null) { | ||
| // Prevent overwriting the existing state in a cached engine with restoration state. | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Registers all plugins that an app lists in its pubspec.yaml. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,8 @@ | |
| */ | ||
goderbauer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /* package */ final class FlutterActivityAndFragmentDelegate { | ||
| private static final String TAG = "FlutterActivityAndFragmentDelegate"; | ||
| private static final String FRAMEWORK_RESTORATION_BUNDLE_KEY = "framework"; | ||
| private static final String PLUGINS_RESTORATION_BUNDLE_KEY = "plugins"; | ||
|
|
||
| // The FlutterActivity or FlutterFragment that is delegating most of its calls | ||
| // to this FlutterActivityAndFragmentDelegate. | ||
|
|
@@ -227,7 +229,8 @@ void onAttach(@NonNull Context context) { | |
| new FlutterEngine( | ||
| host.getContext(), | ||
| host.getFlutterShellArgs().toArray(), | ||
| /*automaticallyRegisterPlugins=*/ false); | ||
| /*automaticallyRegisterPlugins=*/ false, | ||
| /*willProvideRestorationData=*/ host.shouldRestoreAndSaveState()); | ||
| isFlutterEngineFromHost = false; | ||
| } | ||
|
|
||
|
|
@@ -293,11 +296,22 @@ View onCreateView( | |
| } | ||
|
|
||
| void onActivityCreated(@Nullable Bundle bundle) { | ||
| Log.v(TAG, "onActivityCreated. Giving plugins an opportunity to restore state."); | ||
| Log.v(TAG, "onActivityCreated. Giving framework and plugins an opportunity to restore state."); | ||
| ensureAlive(); | ||
|
|
||
| Bundle pluginState = null; | ||
| byte[] frameworkState = null; | ||
| if (bundle != null) { | ||
| pluginState = bundle.getBundle(PLUGINS_RESTORATION_BUNDLE_KEY); | ||
| frameworkState = bundle.getByteArray(FRAMEWORK_RESTORATION_BUNDLE_KEY); | ||
| } | ||
|
|
||
| if (host.shouldRestoreAndSaveState()) { | ||
| flutterEngine.getRestorationChannel().setRestorationData(frameworkState); | ||
| } | ||
|
|
||
| if (host.shouldAttachEngineToActivity()) { | ||
| flutterEngine.getActivityControlSurface().onRestoreInstanceState(bundle); | ||
| flutterEngine.getActivityControlSurface().onRestoreInstanceState(pluginState); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -444,11 +458,19 @@ void onDestroyView() { | |
| } | ||
|
|
||
| void onSaveInstanceState(@Nullable Bundle bundle) { | ||
| Log.v(TAG, "onSaveInstanceState. Giving plugins an opportunity to save state."); | ||
| Log.v(TAG, "onSaveInstanceState. Giving framework and plugins an opportunity to save state."); | ||
| ensureAlive(); | ||
|
|
||
| if (host.shouldRestoreAndSaveState()) { | ||
| bundle.putByteArray( | ||
| FRAMEWORK_RESTORATION_BUNDLE_KEY, | ||
| flutterEngine.getRestorationChannel().getRestorationData()); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really remember. We may have talked about this. Can't we clear the data now that we read this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we delete the data, engine and framework are getting out of sync which may lead to strange behavior and will make it harder to reason about restoration state. In general, framework and engine should always agree on what the current restoration state is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate with a specific example? Perhaps write this down somewhere here for future archeologists too. I get the impression that we'll never run into a case where framework writes, engine reads and deletes, then the framework tries to read again? Is that right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I gave a longer answer to this on the other comment thread. In reality, the sequence of events that you describe will of course not happen. However, in reality, the only moment where we could delete the engine's copy of the restoration state is after we have handed it to the operating system. At that point, our app is moments away from getting killed anyways (thus freeing up all the memory). So why add the extra complexity of allowing framework and engine restoration state to go out of sync in this situation? |
||
|
|
||
| if (host.shouldAttachEngineToActivity()) { | ||
| flutterEngine.getActivityControlSurface().onSaveInstanceState(bundle); | ||
| final Bundle plugins = new Bundle(); | ||
| flutterEngine.getActivityControlSurface().onSaveInstanceState(plugins); | ||
| bundle.putBundle(PLUGINS_RESTORATION_BUNDLE_KEY, plugins); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -804,5 +826,17 @@ PlatformPlugin providePlatformPlugin( | |
|
|
||
| /** Invoked by this delegate when its {@link FlutterView} stops painting pixels. */ | ||
| void onFlutterUiNoLongerDisplayed(); | ||
|
|
||
| /** | ||
| * Whether state restoration is enabled. | ||
| * | ||
| * <p>When this returns true, the instance state provided to {@code onActivityCreated(Bundle)} | ||
| * will be forwarded to the framework via the {@code RestorationChannel} and during {@code | ||
| * onSaveInstanceState(Bundle)} the current framework instance state obtained from {@code | ||
| * RestorationChannel} will be stored in the provided bundle. | ||
| * | ||
| * <p>This defaults to true, unless a cached engine is used. | ||
| */ | ||
goderbauer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| boolean shouldRestoreAndSaveState(); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.