Replaces *.initializeView with *.viewStarter

Fixes #597.

It used to be the case that every `DecorativeViewFactory` called
`showRendering()` twice (#397). We fixed that (we thought) by introducing the
`initializeView` lambda to `ViewRegistry.buildView` and
`DecorativeViewFactory` (#408).

Unfortunately, that fix botched recursion. Individual `DecorativeViewFactory`
instances work fine, but if you wrap them you still get one `showRendering`
call from each. Worse, upstream `initializeView` lambdas are clobbered by
immediately downstream ones. e.g., when a `WorkflowViewStub` shows a
`DecorativeViewFactory`, the `WorkflowLifecycleRunner.installOn` call in the
former is clobbered.

The fix is to completely decouple building a view from from this kind of
initialization. `ViewRegistry.buildView` and its wrappers no longer try to
call `showRendering` at all.

Instead the caller of `buildView` (mostly `WorkflowViewStub`) is reponsible
for immediately calling `View.start` on the new `View`. `View.start` makes
the initial `showRendering` call that formerly was the job of
`ViewFactory.buildView` -- the factory builds the view, and the container
turns the key. Since `View.start` is called only after all wrapped
`ViewFactory.buildView` functions have executed, we're certain it will only
happen once.

Of course we still need the ability to customize view initialization via
wrapping, especially to invoke `WorkflowLifecycleOwner.installOn`. To
accomodate that, the function that `View.start` executes can be wrapped via
the new `viewStarter` argument to `ViewRegistry.buildView` and
`DecorativeViewFactory`, which replaces `initializeView`.

This required a pretty thorough overhaul of `ViewShowRendering.kt`, which is
renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class
that hangs off of a view tag, with two implementations (`New` and `Started`)
to help us enforce the order of the `ViewRegistry.buildView`,
`View.bindShowRendering`, `View.start` and `View.showRendering` calls.

Author: rjrjr

workflow-ui/compose/src/main/java/com/squareup/workflow1/ui/compose/WorkflowRendering.kt

@@ -27,6 +27,7 @@ import com.squareup.workflow1.ui.WorkflowViewStub
 import com.squareup.workflow1.ui.getFactoryForRendering
 import com.squareup.workflow1.ui.getShowRendering
 import com.squareup.workflow1.ui.showRendering
+import com.squareup.workflow1.ui.start
 import kotlin.reflect.KClass
 
 /**
@@ -184,6 +185,8 @@ private fun <R : Any> ViewFactory<R>.asComposeViewFactory() =
           // we don't have access to that.
           originalFactory.buildView(rendering, viewEnvironment, context, container = null)
             .also { view ->
+              view.start()
+
               // Mirrors the check done in ViewRegistry.buildView.
               checkNotNull(view.getShowRendering<Any>()) {
                 "View.bindShowRendering should have been called for $view, typically by the " +

workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt

@@ -32,8 +32,8 @@ import com.squareup.workflow1.ui.canShowRendering
 import com.squareup.workflow1.ui.compatible
 import com.squareup.workflow1.ui.container.R
 import com.squareup.workflow1.ui.getRendering
-import com.squareup.workflow1.ui.showFirstRendering
 import com.squareup.workflow1.ui.showRendering
+import com.squareup.workflow1.ui.start
 
 /**
  * A container view that can display a stream of [BackStackScreen] instances.
@@ -103,11 +103,12 @@ public open class BackStackContainer @JvmOverloads constructor(
       initialViewEnvironment = environment,
       contextForNewView = this.context,
       container = this,
-      initializeView = {
-        WorkflowLifecycleOwner.installOn(this)
-        showFirstRendering()
+      viewStarter = { view, doStart ->
+        WorkflowLifecycleOwner.installOn(view)
+        doStart()
       }
     )
+    newView.start()
     viewStateCache.update(named.backStack, oldViewMaybe, newView)
 
     val popped = currentRendering?.backStack?.any { compatible(it, named.top) } == true

workflow-ui/core-android/api/core-android.api

@@ -24,10 +24,10 @@ public final class com/squareup/workflow1/ui/BuilderViewFactory : com/squareup/w
 }
 
 public final class com/squareup/workflow1/ui/DecorativeViewFactory : com/squareup/workflow1/ui/ViewFactory {
-	public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;)V
-	public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
-	public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;)V
-	public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;)V
+	public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;)V
+	public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun buildView (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View;
 	public fun getType ()Lkotlin/reflect/KClass;
 }
@@ -51,21 +51,6 @@ public final class com/squareup/workflow1/ui/LayoutRunnerViewFactory : com/squar
 	public fun getType ()Lkotlin/reflect/KClass;
 }
 
-public final class com/squareup/workflow1/ui/ShowRenderingTag {
-	public fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
-	public final fun component1 ()Ljava/lang/Object;
-	public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment;
-	public final fun component3 ()Lkotlin/jvm/functions/Function2;
-	public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/ShowRenderingTag;
-	public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/ShowRenderingTag;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/ShowRenderingTag;
-	public fun equals (Ljava/lang/Object;)Z
-	public final fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
-	public final fun getShowRendering ()Lkotlin/jvm/functions/Function2;
-	public final fun getShowing ()Ljava/lang/Object;
-	public fun hashCode ()I
-	public fun toString ()Ljava/lang/String;
-}
-
 public final class com/squareup/workflow1/ui/SnapshotParcelsKt {
 	public static final fun toSnapshot (Landroid/os/Parcelable;)Lcom/squareup/workflow1/Snapshot;
 }
@@ -124,21 +109,15 @@ public final class com/squareup/workflow1/ui/ViewRegistry$Companion : com/square
 public final class com/squareup/workflow1/ui/ViewRegistryKt {
 	public static final fun ViewRegistry ()Lcom/squareup/workflow1/ui/ViewRegistry;
 	public static final fun ViewRegistry ([Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry;
-	public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lkotlin/jvm/functions/Function1;)Landroid/view/View;
-	public static synthetic fun buildView$default (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Landroid/view/View;
+	public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lcom/squareup/workflow1/ui/ViewStarter;)Landroid/view/View;
+	public static synthetic fun buildView$default (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lcom/squareup/workflow1/ui/ViewStarter;ILjava/lang/Object;)Landroid/view/View;
 	public static final fun getFactoryForRendering (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;)Lcom/squareup/workflow1/ui/ViewFactory;
 	public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry;
 	public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewRegistry;)Lcom/squareup/workflow1/ui/ViewRegistry;
-	public static final fun showFirstRendering (Landroid/view/View;)V
 }
 
-public final class com/squareup/workflow1/ui/ViewShowRenderingKt {
-	public static final fun bindShowRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
-	public static final fun canShowRendering (Landroid/view/View;Ljava/lang/Object;)Z
-	public static final fun getEnvironment (Landroid/view/View;)Lcom/squareup/workflow1/ui/ViewEnvironment;
-	public static final fun getRendering (Landroid/view/View;)Ljava/lang/Object;
-	public static final fun getShowRendering (Landroid/view/View;)Lkotlin/jvm/functions/Function2;
-	public static final fun showRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V
+public abstract interface class com/squareup/workflow1/ui/ViewStarter {
+	public abstract fun startView (Landroid/view/View;Lkotlin/jvm/functions/Function0;)V
 }
 
 public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/FrameLayout {
@@ -149,6 +128,51 @@ public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/Fra
 	public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V
 }
 
+public abstract class com/squareup/workflow1/ui/WorkflowUiViewState {
+	public abstract fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
+	public abstract fun getShowRendering ()Lkotlin/jvm/functions/Function2;
+}
+
+public final class com/squareup/workflow1/ui/WorkflowUiViewState$New : com/squareup/workflow1/ui/WorkflowUiViewState {
+	public fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;)V
+	public synthetic fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment;
+	public final fun component3 ()Lkotlin/jvm/functions/Function2;
+	public final fun component4 ()Lkotlin/jvm/functions/Function1;
+	public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/WorkflowUiViewState$New;
+	public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/WorkflowUiViewState$New;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/WorkflowUiViewState$New;
+	public fun equals (Ljava/lang/Object;)Z
+	public fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
+	public fun getShowRendering ()Lkotlin/jvm/functions/Function2;
+	public synthetic fun getShowing ()Ljava/lang/Object;
+	public final fun getStarter ()Lkotlin/jvm/functions/Function1;
+	public fun hashCode ()I
+	public fun toString ()Ljava/lang/String;
+}
+
+public final class com/squareup/workflow1/ui/WorkflowUiViewState$Started : com/squareup/workflow1/ui/WorkflowUiViewState {
+	public fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
+	public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment;
+	public final fun component3 ()Lkotlin/jvm/functions/Function2;
+	public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/WorkflowUiViewState$Started;
+	public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/WorkflowUiViewState$Started;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/WorkflowUiViewState$Started;
+	public fun equals (Ljava/lang/Object;)Z
+	public fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
+	public fun getShowRendering ()Lkotlin/jvm/functions/Function2;
+	public synthetic fun getShowing ()Ljava/lang/Object;
+	public fun hashCode ()I
+	public fun toString ()Ljava/lang/String;
+}
+
+public final class com/squareup/workflow1/ui/WorkflowUiViewStateKt {
+	public static final fun bindShowRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
+	public static final fun canShowRendering (Landroid/view/View;Ljava/lang/Object;)Z
+	public static final fun getEnvironment (Landroid/view/View;)Lcom/squareup/workflow1/ui/ViewEnvironment;
+	public static final fun getShowRendering (Landroid/view/View;)Lkotlin/jvm/functions/Function2;
+	public static final fun showRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V
+	public static final fun start (Landroid/view/View;)V
+}
+
 public final class com/squareup/workflow1/ui/WorkflowViewStub : android/view/View {
 	public fun <init> (Landroid/content/Context;)V
 	public fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;)V

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

@@ -11,7 +11,7 @@ import org.junit.Test
 internal class DecorativeViewFactoryTest {
   private val instrumentation = InstrumentationRegistry.getInstrumentation()
 
-  @Test fun initializeView_is_only_call_to_showRendering() {
+  @Test fun onStartWrapper_is_only_call_to_showRendering() {
     val events = mutableListOf<String>()
 
     val innerViewFactory = object : ViewFactory<InnerRendering> {
@@ -38,27 +38,26 @@ internal class DecorativeViewFactoryTest {
         val enhancedEnv = env + (envString to "Updated environment")
         Pair(outer.wrapped, enhancedEnv)
       },
-      initializeView = {
-        val outerRendering = getRendering<OuterRendering>()
-        events += "initializeView $outerRendering ${environment!![envString]}"
-        showFirstRendering()
-        events += "exit initializeView"
+      viewStarter = { view, doStart ->
+        events += "onStartWrapper ${view.getRendering<Any>()} ${view.environment!![envString]}"
+        doStart()
+        events += "exit onStartWrapper"
       }
     )
-    val viewRegistry = ViewRegistry(innerViewFactory)
+    val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
     val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
 
-    outerViewFactory.buildView(
+    viewRegistry.buildView(
       OuterRendering("outer", InnerRendering("inner")),
       viewEnvironment,
       instrumentation.context
-    )
+    ).start()
 
     assertThat(events).containsExactly(
-      "initializeView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " +
+      "onStartWrapper OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " +
         "Updated environment",
       "inner showRendering InnerRendering(innerData=inner)",
-      "exit initializeView"
+      "exit onStartWrapper"
     )
   }
 
@@ -86,14 +85,14 @@ internal class DecorativeViewFactoryTest {
         innerShowRendering(outerRendering.wrapped, env)
       }
     )
-    val viewRegistry = ViewRegistry(innerViewFactory)
+    val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
     val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
 
-    outerViewFactory.buildView(
+    viewRegistry.buildView(
       OuterRendering("outer", InnerRendering("inner")),
       viewEnvironment,
       instrumentation.context
-    )
+    ).start()
 
     assertThat(events).containsExactly(
       "doShowRendering OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))",
@@ -126,28 +125,27 @@ internal class DecorativeViewFactoryTest {
     val outerViewFactory = DecorativeViewFactory(
       type = OuterRendering::class,
       map = { outer, env ->
-        val enhancedEnv = env + (envString to "Outer Updated environment")
+        val enhancedEnv = env + (envString to "Outer Updated environment" +
+          " SHOULD NOT SEE THIS! It will be clobbered by WayOutRendering")
         Pair(outer.wrapped, enhancedEnv)
       },
-      initializeView = {
-        val outerRendering = getRendering<OuterRendering>()
-        events += "outer initializeView $outerRendering ${environment!![envString]}"
-        showFirstRendering()
-        events += "exit outer initializeView"
+      viewStarter = { view, doStart ->
+        events += "outer onStartWrapper ${view.getRendering<Any>()} ${view.environment!![envString]}"
+        doStart()
+        events += "exit outer onStartWrapper"
       }
     )
 
     val wayOutViewFactory = DecorativeViewFactory(
       type = WayOutRendering::class,
       map = { wayOut, env ->
-        val enhancedEnv = env + (envString to "Way Out Updated environment")
+        val enhancedEnv = env + (envString to "Way Out Updated environment triumphs over all")
         Pair(wayOut.wrapped, enhancedEnv)
       },
-      initializeView = {
-        val wayOutRendering = getRendering<WayOutRendering>()
-        events += "way out initializeView $wayOutRendering ${environment!![envString]}"
-        showFirstRendering()
-        events += "exit way out initializeView"
+      viewStarter = { view, doStart ->
+        events += "way out onStartWrapper ${view.getRendering<Any>()} ${view.environment!![envString]}"
+        doStart()
+        events += "exit way out onStartWrapper"
       }
     )
     val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory, wayOutViewFactory)
@@ -157,21 +155,26 @@ internal class DecorativeViewFactoryTest {
       WayOutRendering("way out", OuterRendering("outer", InnerRendering("inner"))),
       viewEnvironment,
       instrumentation.context
-    )
+    ).start()
 
     assertThat(events).containsExactly(
-      "way out initializeView " +
+      "way out onStartWrapper " +
         "WayOutRendering(wayOutData=way out, wrapped=" +
         "OuterRendering(outerData=outer, wrapped=" +
         "InnerRendering(innerData=inner))) " +
-        "Way Out Updated environment",
-      "outer initializeView " +
+        "Way Out Updated environment triumphs over all",
+      "outer onStartWrapper " +
+        // Notice that both the initial rendering and the ViewEnvironment are stomped by
+        // the outermost wrapper before inners are invoked. Could try to give
+        // the inner wrapper access to the rendering it expected, but there are no
+        // use cases and it trashes the API.
+        "WayOutRendering(wayOutData=way out, wrapped=" +
         "OuterRendering(outerData=outer, wrapped=" +
-        "InnerRendering(innerData=inner)) " +
-        "Outer Updated environment",
+        "InnerRendering(innerData=inner))) " +
+        "Way Out Updated environment triumphs over all",
       "inner showRendering InnerRendering(innerData=inner)",
-      "exit outer initializeView",
-      "exit way out initializeView"
+      "exit outer onStartWrapper",
+      "exit way out onStartWrapper"
     )
   }
 
@@ -199,14 +202,14 @@ internal class DecorativeViewFactoryTest {
         innerShowRendering(outerRendering.wrapped, env)
       }
     )
-    val viewRegistry = ViewRegistry(innerViewFactory)
+    val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
     val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
 
-    val view = outerViewFactory.buildView(
+    val view = viewRegistry.buildView(
       OuterRendering("out1", InnerRendering("in1")),
       viewEnvironment,
       instrumentation.context
-    )
+    ).apply { start() }
     events.clear()
 
     view.showRendering(OuterRendering("out2", InnerRendering("in2")), viewEnvironment)
@@ -222,6 +225,7 @@ internal class DecorativeViewFactoryTest {
     val outerData: String,
     val wrapped: InnerRendering
   )
+
   private data class WayOutRendering(
     val wayOutData: String,
     val wrapped: OuterRendering