Adds BackPressedHandlerTest

workflow-ui/core-android/src/androidTest/AndroidManifest.xml

@@ -7,6 +7,6 @@
     <activity
         android:name="com.squareup.workflow1.ui.container.fixtures.BackStackContainerLifecycleActivity"
         android:theme="@style/Theme.AppCompat.NoActionBar"/>
-<activity android:name="androidx.activity.ComponentActivity"/>
+    <activity android:name="androidx.activity.ComponentActivity"/>
   </application>
 </manifest>

workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/BackPressedHandlerTest.kt

@@ -0,0 +1,117 @@
+package com.squareup.workflow1.ui
+
+import android.view.View
+import android.view.ViewGroup
+import androidx.activity.ComponentActivity
+import androidx.activity.OnBackPressedCallback
+import androidx.activity.OnBackPressedDispatcherSpy
+import androidx.lifecycle.Lifecycle.State.DESTROYED
+import androidx.lifecycle.Lifecycle.State.RESUMED
+import androidx.lifecycle.LifecycleRegistry
+import androidx.lifecycle.ViewTreeLifecycleOwner
+import androidx.test.ext.junit.rules.ActivityScenarioRule
+import com.google.common.truth.Truth.assertThat
+import com.squareup.workflow1.ui.internal.test.DetectLeaksAfterTestSuccess
+import com.squareup.workflow1.ui.internal.test.IdlingDispatcherRule
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.RuleChain
+
+@OptIn(WorkflowUiExperimentalApi::class)
+internal class BackPressedHandlerTest {
+  private val scenarioRule = ActivityScenarioRule(ComponentActivity::class.java)
+  private val scenario get() = scenarioRule.scenario
+
+  @get:Rule val rules: RuleChain = RuleChain.outerRule(DetectLeaksAfterTestSuccess())
+    .around(scenarioRule)
+    .around(IdlingDispatcherRule)
+
+  private var viewHandlerCount = 0
+  private val viewBackHandler: BackPressHandler = {
+    viewHandlerCount++
+  }
+
+  @Test fun itWorksWhenHandlerIsAddedBeforeAttach() {
+    scenario.onActivity { activity ->
+      val view = View(activity)
+      view.backPressedHandler = viewBackHandler
+
+      activity.setContentView(view)
+      assertThat(viewHandlerCount).isEqualTo(0)
+
+      activity.onBackPressed()
+      assertThat(viewHandlerCount).isEqualTo(1)
+    }
+  }
+
+  @Test fun itWorksWhenHandlerIsAddedAfterAttach() {
+    scenario.onActivity { activity ->
+      val view = View(activity)
+      view.backPressedHandler = viewBackHandler
+
+      activity.setContentView(view)
+      assertThat(viewHandlerCount).isEqualTo(0)
+
+      activity.onBackPressed()
+      assertThat(viewHandlerCount).isEqualTo(1)
+    }
+  }
+
+  @Test fun onlyActiveWhileViewIsAttached() {
+    var fallbackCallCount = 0
+    val defaultBackHandler = object : OnBackPressedCallback(true) {
+      override fun handleOnBackPressed() {
+        fallbackCallCount++
+      }
+    }
+
+    scenario.onActivity { activity ->
+      activity.onBackPressedDispatcher.addCallback(defaultBackHandler)
+
+      val view = View(activity)
+      view.backPressedHandler = viewBackHandler
+
+      activity.onBackPressed()
+      assertThat(fallbackCallCount).isEqualTo(1)
+      assertThat(viewHandlerCount).isEqualTo(0)
+
+      activity.setContentView(view)
+      activity.onBackPressed()
+      assertThat(fallbackCallCount).isEqualTo(1)
+      assertThat(viewHandlerCount).isEqualTo(1)
+
+      (view.parent as ViewGroup).removeView(view)
+      activity.onBackPressed()
+      assertThat(fallbackCallCount).isEqualTo(2)
+      assertThat(viewHandlerCount).isEqualTo(1)
+
+      activity.setContentView(view)
+      activity.onBackPressed()
+      assertThat(fallbackCallCount).isEqualTo(2)
+      assertThat(viewHandlerCount).isEqualTo(2)
+    }
+  }
+
+  @Test fun callbackIsRemoved() {
+    scenario.onActivity { activity ->
+      val spy = OnBackPressedDispatcherSpy(activity.onBackPressedDispatcher)
+      assertThat(spy.callbacks()).isEmpty()
+
+      val lifecycle = LifecycleRegistry(activity)
+      lifecycle.currentState = RESUMED
+
+      val view = View(activity)
+      view.backPressedHandler = viewBackHandler
+      assertThat(spy.callbacks()).hasSize(1)
+
+      ViewTreeLifecycleOwner.set(view) { lifecycle }
+      activity.setContentView(view)
+
+      (view.parent as ViewGroup).removeView(view)
+      assertThat(spy.callbacks()).hasSize(1)
+
+      lifecycle.currentState = DESTROYED
+      assertThat(spy.callbacks()).isEmpty()
+    }
+  }
+}

workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/OnBackPressedDispatcherSpy.java

@@ -0,0 +1,15 @@
+package androidx.activity;
+
+import java.util.ArrayDeque;
+
+public class OnBackPressedDispatcherSpy {
+  private final OnBackPressedDispatcher dispatcher;
+
+  public OnBackPressedDispatcherSpy(OnBackPressedDispatcher dispatcher) {
+    this.dispatcher = dispatcher;
+  }
+
+  public ArrayDeque<OnBackPressedCallback> callbacks() {
+    return dispatcher.mOnBackPressedCallbacks;
+  }
+}

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackPressHandler.kt

@@ -64,9 +64,19 @@ private var View.observerOrNull: AttachStateAndLifecycleObserver?
  * so that we can know when it's time to remove the [onBackPressedCallback] from
  * the dispatch stack
  * ([no memory leaks please](https://github.com/square/workflow-kotlin/issues/889)).
- * As a belt-and-suspenders guard against leaking, we also take care to null out the
- * pointer from the [onBackPressedCallback] to the actual [handler] while the [view]
- * is detached.
+ *
+ * Why is it okay to wait for the [ViewTreeLifecycleOwner] to be destroyed before we
+ * remove [onBackPressedCallback] from the dispatcher? In normal apps that's
+ * the `Activity` or a `Fragment`, which will live a very long time, but Workflow UI
+ * is more controlling than that. `WorkflowViewStub` and the rest of the stock container
+ * classes use `WorkflowLifecycleOwner` to provide a short lived [ViewTreeLifecycleOwner]
+ * for each [View] they create, and tear it down before moving to the next one.
+ *
+ * None the less, as a belt-and-suspenders guard against leaking,
+ * we also take care to null out the pointer from the [onBackPressedCallback] to the
+ * actual [handler] while the [view] is detached. We can't be confident that the
+ * [ViewTreeLifecycleOwner] we find will be a well behaved one that was put in place
+ * by `WorkflowLifecycleOwner`. Who knows what adventures our clients will get up to.
  */
 @WorkflowUiExperimentalApi
 private class AttachStateAndLifecycleObserver(