From 579230a09582567a08e2d69fa78b572959ba7fb2 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Tue, 18 Jan 2022 09:25:09 -0800 Subject: [PATCH] WorkflowLayout.start now relies on Lifecycle. Our sketchy homegrown `View.takeWhileAttached` somehow causes a strange issue on API 30 devices where `View.onAttached` can be called twice, at least since the recent change to introduce `View.start` landed in #602. We've seen crashes on a variety of Samsung devices out in the wild, and can reproduce the issue on API 30 AVDs via Android Studio's _Apply Changes and Restart Activity_. The fix is to be mainstream and use `Lifecycle`, which is added as a new required parameter to `WorkflowLayout.start`. The old overloads are now deprecated, and will be deleted soon. We also promote `WorkflowLayout.show` to be publicly visible, to give apps the option of taking control over how they collect their renderings. --- .../helloworkflow/HelloWorkflowActivity.kt | 2 +- workflow-ui/core-android/api/core-android.api | 4 ++ .../squareup/workflow1/ui/WorkflowLayout.kt | 50 +++++++++++++++++-- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt b/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt index 99318b4eb0..6eafa61063 100644 --- a/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt +++ b/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt @@ -21,7 +21,7 @@ class HelloWorkflowActivity : AppCompatActivity() { // succeeding calls. val model: HelloViewModel by viewModels() setContentView( - WorkflowLayout(this).apply { start(model.renderings) } + WorkflowLayout(this).also { contentView -> contentView.start(lifecycle, model.renderings) } ) } } diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index 55a76c083a..d2442ca0b9 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -203,8 +203,12 @@ public abstract interface class com/squareup/workflow1/ui/ViewStarter { public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/FrameLayout { public fun (Landroid/content/Context;Landroid/util/AttributeSet;)V public synthetic fun (Landroid/content/Context;Landroid/util/AttributeSet;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun show (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V + public final fun start (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;)V + public final fun start (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewRegistry;)V public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;)V public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewRegistry;)V + public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt index 071a0a105f..f4a0f24f9c 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt @@ -10,12 +10,17 @@ import android.view.View import android.view.ViewGroup import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.widget.FrameLayout +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.coroutineScope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch +import androidx.lifecycle.repeatOnLifecycle /** * A view that can be driven by a stream of renderings (and an optional [ViewRegistry]) @@ -43,31 +48,68 @@ public class WorkflowLayout( private var restoredChildState: SparseArray? = null + @Suppress("DeprecatedCallableAddReplaceWith") + @Deprecated("Provide an androidx Lifecyle") + public fun start( + renderings: Flow, + registry: ViewRegistry + ) { + @Suppress("DEPRECATION") + start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry))) + } + /** * Subscribes to [renderings], and uses [registry] to * [build a new view][ViewRegistry.buildView] each time a new type of rendering is received, * making that view the only child of this one. + * + * @see [show] */ public fun start( + lifecycle: Lifecycle, renderings: Flow, registry: ViewRegistry ) { - start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry))) + start(lifecycle, renderings, ViewEnvironment(mapOf(ViewRegistry to registry))) + } + + @Deprecated("Provide an androidx Lifecyle") + public fun start( + renderings: Flow, + environment: ViewEnvironment = ViewEnvironment() + ) { + takeWhileAttachedIfYouAreAnIdiotNoDoNotUseThis(renderings) { show(it, environment) } } /** * Subscribes to [renderings], and uses the [ViewRegistry] in the given [environment] to * [build a new view][ViewRegistry.buildView] each time a new type of rendering is received, * making that view the only child of this one. + * + * @see [show] */ public fun start( + lifecycle: Lifecycle, renderings: Flow, environment: ViewEnvironment = ViewEnvironment() ) { - takeWhileAttached(renderings) { show(it, environment) } + lifecycle.coroutineScope.launch { + lifecycle.repeatOnLifecycle(Lifecycle.State.RESUMED) { + renderings.collect { show(it, environment) } + } + } } - private fun show( + /** + * Called from [start] as each rendering is collected from its `renderings: Flow` + * parameter. Updates the current child view from [newRendering] and [environment] + * if [newRendering] is [compatible] with the current one, or else replaces the + * current view with a new one. + * + * This method is exposed as an alternative to [start], to offer clients complete control + * over collecting their renderings. + */ + public fun show( newRendering: Any, environment: ViewEnvironment ) { @@ -131,7 +173,7 @@ public class WorkflowLayout( /** * Subscribes [update] to [source] only while this [View] is attached to a window. */ - private fun View.takeWhileAttached( + private fun View.takeWhileAttachedIfYouAreAnIdiotNoDoNotUseThis( source: Flow, update: (S) -> Unit ) {