only attempt to restore View saved state while INITIALIZED #635

RBusarow · 2022-01-19T15:58:36Z

fixes #570

CLAassistant · 2022-01-19T15:58:42Z

All committers have signed the CLA.

steve-the-edwards

Is this just pulling this fix into main from the ui-update branch?

steve-the-edwards · 2022-01-19T16:40:02Z

...low-ui/container-android/src/main/java/com/squareup/workflow1/ui/backstack/ViewStateCache.kt

+        // We're only allowed to restore from an INITIALIZED state, but this callback can also be
+        // invoked while the owner is already CREATED.
+        // https://github.com/square/workflow-kotlin/issues/570
+        ?.takeIf { it.lifecycle.currentState == Lifecycle.State.INITIALIZED }


I think @rjrjr originally tried this solution but decided against it? I can't remember now.

I believe it was just a matter of being able to reproduce the bug first.

If this line is removed, everything still passes except for the new test, even if you call scenario.recreate(), or change orientation or something.

Yup, I feel fine about the fix, just didn't want to do it w/o proof.

rjrjr · 2022-01-19T18:51:38Z

...roid/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/BackstackContainerTest.kt

+      it.nestedTestView().viewState = "some state"
+    }
+
+    // Recreating the activity sends one ONE_CREATE event,


fixes #570

rjrjr · 2022-01-19T23:02:02Z

@RBusarow what changed in those last couple of force pushes?

RBusarow · 2022-01-19T23:03:33Z

@RBusarow what changed in those last couple of force pushes?

I fixed this typo:

And I added that same .api file change.

rjrjr · 2022-02-08T19:30:03Z

Debugging a problem where a ComposeRendering in a dialog is crashing in a pretty similar way:

    java.lang.IllegalStateException: You can consumeRestoredStateForKey only after super.onCreate of corresponding component
        at androidx.savedstate.SavedStateRegistry.consumeRestoredStateForKey(SavedStateRegistry.java:77)
        at androidx.compose.ui.platform.DisposableSaveableStateRegistry_androidKt.DisposableSaveableStateRegistry(DisposableSaveableStateRegistry.android.kt:75)
        at androidx.compose.ui.platform.DisposableSaveableStateRegistry_androidKt.DisposableSaveableStateRegistry(DisposableSaveableStateRegistry.android.kt:54)
        at androidx.compose.ui.platform.AndroidCompositionLocals_androidKt.ProvideAndroidCompositionLocals(AndroidCompositionLocals.android.kt:105)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1$1$3.invoke(Wrapper.android.kt:157)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1$1$3.invoke(Wrapper.android.kt:156)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
        at androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1$1.invoke(Wrapper.android.kt:156)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1$1.invoke(Wrapper.android.kt:140)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
        at androidx.compose.runtime.ComposerKt.invokeComposable(Composer.kt:3337)
        at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:2582)
        at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:2571)
        at androidx.compose.runtime.SnapshotStateKt__DerivedStateKt.observeDerivedStateRecalculations(DerivedState.kt:247)
        at androidx.compose.runtime.SnapshotStateKt.observeDerivedStateRecalculations(Unknown Source:1)
        at androidx.compose.runtime.ComposerImpl.doCompose(Composer.kt:2571)
        at androidx.compose.runtime.ComposerImpl.composeContent$runtime_release(Composer.kt:2522)
        at androidx.compose.runtime.CompositionImpl.composeContent(Composition.kt:478)
        at androidx.compose.runtime.Recomposer.composeInitial$runtime_release(Recomposer.kt:748)
        at androidx.compose.runtime.CompositionImpl.setContent(Composition.kt:433)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:140)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:131)
        at androidx.compose.ui.platform.AndroidComposeView.setOnViewTreeOwnersAvailable(AndroidComposeView.android.kt:907)
        at androidx.compose.ui.platform.WrappedComposition.setContent(Wrapper.android.kt:131)
        at androidx.compose.ui.platform.WrappedComposition.onStateChanged(Wrapper.android.kt:182)
        at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:354)
        at androidx.lifecycle.LifecycleRegistry.addObserver(LifecycleRegistry.java:196)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:138)
        at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:131)
        at androidx.compose.ui.platform.AndroidComposeView.onAttachedToWindow(AndroidComposeView.android.kt:994)
        at android.view.View.dispatchAttachedToWindow(View.java:20479)
        at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3489)
        at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3496)
        at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3496)
        at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3496)
        at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3496)
        at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3496)
        at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3496)
E/AndroidRuntime:     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3496)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2417)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1952)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8171)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:972)
        at android.view.Choreographer.doCallbacks(Choreographer.java:796)
        at android.view.Choreographer.doFrame(Choreographer.java:731)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:957)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

I'm starting to think that the real problem here is that RealWorkflowLifecycleOwner is too eager to reflect its parent's state. It gets to RESUMED long before restore happens, and so we crash.

Wondering if we should link it and StateRegistryAggregator, and stay in CREATED until a restore happens.

Update: not so much "similar" as maybe related. Like b/c we're squelching the restore call, downstream we puke b/c of trying to consume something that never was restored.

rjrjr · 2022-02-08T22:14:40Z

@RBusarow When I revert everything but the test from this, the test still passes. Do you remember if there was any particular AVD or API level you were using to reproduce the failure?

RBusarow · 2022-02-08T22:44:12Z

@rjrjr I just tested against a 31 "Pixel 5" AVD and with just the INITIALIZED check commented out it failed. I then ran it against API 29 and it passed.

rjrjr · 2022-02-08T22:44:47Z

Perfect, thanks.

I don't know that these two issues are actually related, still debugging.

rjrjr · 2022-02-09T00:38:20Z

Seems unrelated, just a missing call to restoreRegistryControllerIfReady in ModalContainer. Phew!

RBusarow requested review from a team and zach-klippenstein as code owners January 19, 2022 15:58

steve-the-edwards approved these changes Jan 19, 2022

View reviewed changes

rjrjr reviewed Jan 19, 2022

View reviewed changes

rjrjr approved these changes Jan 19, 2022

View reviewed changes

only attempt to restore view saved state while INITIALIZED

6f9c45b

fixes #570

RBusarow merged commit 1882f1d into square:main Jan 20, 2022

only attempt to restore View saved state while INITIALIZED #635

only attempt to restore View saved state while INITIALIZED #635

Uh oh!

Conversation

RBusarow commented Jan 19, 2022

Uh oh!

CLAassistant commented Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steve-the-edwards left a comment

Choose a reason for hiding this comment

Uh oh!

steve-the-edwards Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

RBusarow Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

rjrjr Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

rjrjr Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

rjrjr commented Jan 19, 2022

Uh oh!

RBusarow commented Jan 19, 2022

Uh oh!

rjrjr commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rjrjr commented Feb 8, 2022

Uh oh!

RBusarow commented Feb 8, 2022

Uh oh!

rjrjr commented Feb 8, 2022

Uh oh!

rjrjr commented Feb 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Jan 19, 2022 •

edited

Loading

rjrjr commented Feb 8, 2022 •

edited

Loading