Merge pull request #941 from square/ray/dialog-failsafe

Close dialogs even if the managing view is never attached.

Author: rjrjr

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;
 }

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()
+  }
 }

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

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()
           }
         }