From a3400ccb6fc0c552b954b3de3b235f27418b3389 Mon Sep 17 00:00:00 2001 From: Andreas Rossbacher Date: Thu, 25 Jan 2024 11:37:48 -0800 Subject: [PATCH] Change the way the Compose interop works to avoid Android 12 bug (#1370) * The way the composeEpoxyModel funtion was written it would create a function that looked like this (in decompiled dex bytecode) ``` public static final ComposeEpoxyModel composeEpoxyModel(String id, Object[] keys, Function2 composeFunction) { ... ComposeEpoxyModel $this$composeEpoxyModel_u24lambda_u240 = new ComposeEpoxyModel(Arrays.copyOf(keys, keys.length), composeFunction); $this$composeEpoxyModel_u24lambda_u240.id(id); return $this$composeEpoxyModel_u24lambda_u240; } ``` this would trigger a ART bug in the (non mainline patched version of Android 12) to optimize the this line ``` $this$composeEpoxyModel_u24lambda_u240.id(id); ``` and lead to crashes on Android 12. More background at: https://github.com/airbnb/epoxy/issues/1199 and https://issuetracker.google.com/issues/197818595 This works around this issue by making sure that the creation, setting the id and adding to the controller of a ComposeEpoxyModelis happening in the same location in compiled bytecode. Note: If you run R8 with optimization enabled, outlining might still outline some of that inlined code again and thus still cause this probblem but that is a different issue that can be dealed with by disabling outlining or all optimization. * Fix lint error --------- Co-authored-by: Andreas Rossbacher --- .../java/com/airbnb/epoxy/ComposeInterop.kt | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/epoxy-compose/src/main/java/com/airbnb/epoxy/ComposeInterop.kt b/epoxy-compose/src/main/java/com/airbnb/epoxy/ComposeInterop.kt index 25877fbfc9..5ee7db3eef 100644 --- a/epoxy-compose/src/main/java/com/airbnb/epoxy/ComposeInterop.kt +++ b/epoxy-compose/src/main/java/com/airbnb/epoxy/ComposeInterop.kt @@ -69,13 +69,28 @@ fun ModelCollector.composableInterop( vararg keys: Any, composeFunction: @Composable () -> Unit ) { - add(composeEpoxyModel(id, *keys, composeFunction = composeFunction)) + // Note this is done to avoid ART bug in Android 12 (background https://issuetracker.google.com/issues/197818595 and + // https://github.com/airbnb/epoxy/issues/1199). + // The main objective is to have the creation of the ComposeEpoxyModel the setting of the id and the adding to the + // controller in the same method. + // Note that even this manual inlining might be spoiled by R8 outlining which, if enabled might outline the creation + // of the ComposeEpoxyModel and setting of the id to a separate method. + val composeEpoxyModel = ComposeEpoxyModel(*keys, composeFunction = composeFunction) + composeEpoxyModel.id(id) + add(composeEpoxyModel) } /** * [composeEpoxyModel] can be used directly in cases where more control over the epoxy model * is needed. Eg. When the epoxy model needs to be modified before it's added. */ +@Deprecated( + message = "Use composeEpoxyModel with modelAction lambda instead to avoid crash on Android 12", + replaceWith = ReplaceWith( + expression = "composeEpoxyModel(id, *keys, modelAction = modelAction, composeFunction = composeFunction)", + imports = ["com.airbnb.epoxy.composeEpoxyModel"] + ) +) fun composeEpoxyModel( id: String, vararg keys: Any, @@ -86,6 +101,31 @@ fun composeEpoxyModel( } } +/** + * [composeEpoxyModel] can be used directly in cases where more control over the epoxy model + * is needed. Eg. When the epoxy model needs to be modified before it's added. + * + * Note: This does not return the model and instead takes a modelAction lambda that can be used to + * add the model to the controller, or modify it before adding. + * + * This is done to avoid ART bug in Android 12 (background https://issuetracker.google.com/issues/197818595 and + * https://github.com/airbnb/epoxy/issues/1199). + * The main objective is to have the creation of the ComposeEpoxyModel the setting of the id and + * the adding to the controller in the same method. + * Note that even with this construct this might be spoiled by R8 outlining which, if enabled might + * outline the creation of the ComposeEpoxyModel and setting of the id to a separate method. + */ +inline fun composeEpoxyModel( + id: String, + vararg keys: Any, + noinline composeFunction: @Composable () -> Unit, + modelAction: (ComposeEpoxyModel) -> Unit +) { + val composeEpoxyModel = ComposeEpoxyModel(*keys, composeFunction = composeFunction) + composeEpoxyModel.id(id) + modelAction.invoke(composeEpoxyModel) +} + @Composable inline fun > EpoxyInterop( modifier: Modifier = Modifier,