Skip to content

Commit 58e1b76

Browse files
committed
WorkflowLayout now relies on Lifecycle.
Our 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 add as a new required parameter to `WorkflowLayout.start`. The old overloads are now deprecated, and will be deleted soon.
1 parent 3d79f14 commit 58e1b76

File tree

3 files changed

+33
-4
lines changed

3 files changed

+33
-4
lines changed

samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class HelloWorkflowActivity : AppCompatActivity() {
2121
// succeeding calls.
2222
val model: HelloViewModel by viewModels()
2323
setContentView(
24-
WorkflowLayout(this).apply { start(model.renderings) }
24+
WorkflowLayout(this).also { contentView -> contentView.start(lifecycle, model.renderings) }
2525
)
2626
}
2727
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,11 @@ public abstract interface class com/squareup/workflow1/ui/ViewStarter {
132132
public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/FrameLayout {
133133
public fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;)V
134134
public synthetic fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
135+
public final fun start (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;)V
136+
public final fun start (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewRegistry;)V
135137
public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;)V
136138
public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewRegistry;)V
139+
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
137140
public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V
138141
}
139142

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

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,13 @@ import android.view.View
1010
import android.view.ViewGroup
1111
import android.view.ViewGroup.LayoutParams.MATCH_PARENT
1212
import android.widget.FrameLayout
13+
import androidx.lifecycle.Lifecycle
14+
import androidx.lifecycle.coroutineScope
1315
import kotlinx.coroutines.CoroutineScope
1416
import kotlinx.coroutines.Dispatchers
1517
import kotlinx.coroutines.Job
1618
import kotlinx.coroutines.flow.Flow
19+
import kotlinx.coroutines.flow.collect
1720
import kotlinx.coroutines.flow.launchIn
1821
import kotlinx.coroutines.flow.onEach
1922

@@ -43,28 +46,51 @@ public class WorkflowLayout(
4346

4447
private var restoredChildState: SparseArray<Parcelable>? = null
4548

49+
@Suppress("DeprecatedCallableAddReplaceWith")
50+
@Deprecated("Provide an androidx Lifecyle")
51+
public fun start(
52+
renderings: Flow<Any>,
53+
registry: ViewRegistry
54+
) {
55+
@Suppress("DEPRECATION")
56+
start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry)))
57+
}
58+
4659
/**
4760
* Subscribes to [renderings], and uses [registry] to
4861
* [build a new view][ViewRegistry.buildView] each time a new type of rendering is received,
4962
* making that view the only child of this one.
5063
*/
5164
public fun start(
65+
lifecycle: Lifecycle,
5266
renderings: Flow<Any>,
5367
registry: ViewRegistry
5468
) {
55-
start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry)))
69+
start(lifecycle, renderings, ViewEnvironment(mapOf(ViewRegistry to registry)))
5670
}
5771

72+
@Deprecated("Provide an androidx Lifecyle")
73+
public fun start(
74+
renderings: Flow<Any>,
75+
environment: ViewEnvironment = ViewEnvironment()
76+
) {
77+
takeWhileAttachedIfYouAreAnIdiotNoDoNotUseThis(renderings) { show(it, environment) }
78+
}
79+
80+
5881
/**
5982
* Subscribes to [renderings], and uses the [ViewRegistry] in the given [environment] to
6083
* [build a new view][ViewRegistry.buildView] each time a new type of rendering is received,
6184
* making that view the only child of this one.
6285
*/
6386
public fun start(
87+
lifecycle: Lifecycle,
6488
renderings: Flow<Any>,
6589
environment: ViewEnvironment = ViewEnvironment()
6690
) {
67-
takeWhileAttached(renderings) { show(it, environment) }
91+
lifecycle.coroutineScope.launchWhenStarted {
92+
renderings.collect { show(it, environment) }
93+
}
6894
}
6995

7096
private fun show(
@@ -131,7 +157,7 @@ public class WorkflowLayout(
131157
/**
132158
* Subscribes [update] to [source] only while this [View] is attached to a window.
133159
*/
134-
private fun <S : Any> View.takeWhileAttached(
160+
private fun <S : Any> View.takeWhileAttachedIfYouAreAnIdiotNoDoNotUseThis(
135161
source: Flow<S>,
136162
update: (S) -> Unit
137163
) {

0 commit comments

Comments
 (0)