diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index 0632693c4d..60fecdaef6 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -269,6 +269,7 @@ public final class com/squareup/workflow1/ui/WorkflowViewStub : android/view/Vie public final class com/squareup/workflow1/ui/androidx/WorkflowAndroidXSupport { public static final field INSTANCE Lcom/squareup/workflow1/ui/androidx/WorkflowAndroidXSupport; + public final fun lifecycleOwnerFromContext (Landroid/content/Context;)Landroidx/lifecycle/LifecycleOwner; public final fun lifecycleOwnerFromViewTreeOrContextOrNull (Landroid/view/View;)Landroidx/lifecycle/LifecycleOwner; public final fun stateRegistryOwnerFromViewTreeOrContext (Landroid/view/View;)Landroidx/savedstate/SavedStateRegistryOwner; } diff --git a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt index 8bef7adfae..d23a304b4a 100644 --- a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt +++ b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt @@ -4,6 +4,7 @@ import android.app.Dialog import android.view.View import android.widget.EditText import androidx.activity.ComponentActivity +import androidx.lifecycle.Lifecycle.State.DESTROYED import androidx.test.ext.junit.rules.ActivityScenarioRule import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat @@ -133,4 +134,19 @@ internal class DialogIntegrationTest { assertThat(latestDialog).isSameInstanceAs(originalDialogOne) } } + + @Test fun finishingActivityEarlyDismissesDialogs() { + val screen = BodyAndOverlaysScreen( + ContentRendering("body"), + DialogRendering("dialog", ContentRendering("content")) + ) + + scenario.onActivity { activity -> + val root = WorkflowLayout(activity) + root.show(screen) + } + + scenario.moveToState(DESTROYED) + assertThat(latestDialog?.isShowing).isFalse() + } } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowAndroidXSupport.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowAndroidXSupport.kt index c0a33c74ff..a0c25d73b9 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowAndroidXSupport.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowAndroidXSupport.kt @@ -15,6 +15,17 @@ import kotlin.reflect.cast * Namespace for some helper functions for interacting with the AndroidX libraries. */ public object WorkflowAndroidXSupport { + /** + * Returns the [LifecycleOwner] managing [context]. + * + * @throws IllegalArgumentException if [context] is unmanaged + */ + @WorkflowUiExperimentalApi + public fun lifecycleOwnerFromContext(context: Context): LifecycleOwner = + requireNotNull(context.ownerOrNull(LifecycleOwner::class)) { + "Expected $context to lead to a LifecycleOwner" + } + /** * Tries to get the parent lifecycle from the current view via [ViewTreeLifecycleOwner], if that * fails it looks up the context chain for a [LifecycleOwner], and if that fails it just returns diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt index 76a837ed4f..5bfe27cc69 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt @@ -9,12 +9,15 @@ import android.view.MotionEvent import android.view.View import android.view.View.OnAttachStateChangeListener import android.view.ViewTreeObserver.OnGlobalLayoutListener +import androidx.core.view.doOnAttach +import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ViewTreeLifecycleOwner import com.squareup.workflow1.ui.Compatible import com.squareup.workflow1.ui.ViewEnvironment import com.squareup.workflow1.ui.WorkflowUiExperimentalApi import com.squareup.workflow1.ui.androidx.WorkflowAndroidXSupport +import com.squareup.workflow1.ui.androidx.WorkflowAndroidXSupport.lifecycleOwnerFromContext import com.squareup.workflow1.ui.androidx.WorkflowLifecycleOwner import com.squareup.workflow1.ui.androidx.WorkflowSavedStateRegistryAggregator import com.squareup.workflow1.ui.container.DialogSession.KeyAndBundle @@ -277,11 +280,11 @@ public class LayeredDialogSessions private constructor( ): LayeredDialogSessions { val boundsRect = Rect() if (view.isAttachedToWindow) view.getGlobalVisibleRect(boundsRect) - val bounds = MutableStateFlow(Rect(boundsRect)) + val boundsStateFlow = MutableStateFlow(Rect(boundsRect)) return LayeredDialogSessions( context = view.context, - bounds = bounds, + bounds = boundsStateFlow, cancelEvents = { // Note similar code in DialogSession. @@ -298,16 +301,36 @@ public class LayeredDialogSessions private constructor( "Expected a ViewTreeLifecycleOwner on $view" } }.also { dialogs -> + fun closeAll() { + dialogs.update(emptyList(), ViewEnvironment.EMPTY) {} + } - val boundsListener = OnGlobalLayoutListener { - if (view.getGlobalVisibleRect(boundsRect) && boundsRect != bounds.value) { - bounds.value = Rect(boundsRect) - } - // Should we close the dialogs if getGlobalVisibleRect returns false? - // https://github.com/square/workflow-kotlin/issues/599 + // We rely on the hosting View's WorkflowLifecycleOwner to tell us to tear things down. + // WorkflowLifecycleOwner gets hooked up when the View is attached to its window. + // But the Activity might finish before the hosting view is ever attached. And we have + // lots of time to show Dialogs before then. They will leak. + // + // To guard against that we hang a default observer directly off of the Activity that + // will close all Dialogs when it is destroyed; and we remove it as soon as the hosting + // view is attached for the first time. + val failsafe = object : DefaultLifecycleObserver { + override fun onDestroy(owner: LifecycleOwner) = closeAll() + } + lifecycleOwnerFromContext(view.context).lifecycle.addObserver(failsafe) + view.doOnAttach { + lifecycleOwnerFromContext(it.context).lifecycle.removeObserver(failsafe) } + // While the hosting view is attached, monitor its bounds and report them + // through boundsStateFlow so that managed Dialogs can constrain themselves + // accordingly. val attachStateChangeListener = object : OnAttachStateChangeListener { + val boundsListener = OnGlobalLayoutListener { + if (view.getGlobalVisibleRect(boundsRect) && boundsRect != boundsStateFlow.value) { + boundsStateFlow.value = Rect(boundsRect) + } + } + override fun onViewAttachedToWindow(v: View) { boundsListener.onGlobalLayout() v.viewTreeObserver.addOnGlobalLayoutListener(boundsListener) @@ -316,9 +339,9 @@ public class LayeredDialogSessions private constructor( override fun onViewDetachedFromWindow(v: View) { // Don't leak the dialogs if we're suddenly yanked out of view. // https://github.com/square/workflow-kotlin/issues/314 - dialogs.update(emptyList(), ViewEnvironment.EMPTY) {} + closeAll() v.viewTreeObserver.removeOnGlobalLayoutListener(boundsListener) - bounds.value = Rect() + boundsStateFlow.value = Rect() } }