Skip to content

Commit

Permalink
add config for cloning state list animators in LithoViewAttributesExt…
Browse files Browse the repository at this point in the history
…ension

Summary:
This is another attempt to address the SLA (state list animator) JNI crashes we keep running into iterations of the nested tree experiment. The current hypothesis is that the SLA is shared across views such that one view attempts to update its SLA drawable state while another has unmounted causing the underlying animators to lose their view references.

To verify this hypothesis, we clone any SLAs we're given during application of the `LithoViewAttributesExtension` to ensure that it has a fresh state.

Reviewed By: adityasharat

Differential Revision: D58471256

fbshipit-source-id: fb44f01a4858958a20eb79d7541ca8801ab05814
  • Loading branch information
Daniel Famakin authored and facebook-github-bot committed Jun 12, 2024
1 parent aaeba5e commit f726c2f
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,8 @@ protected void setupMountExtensions() {
mMountState,
configuration != null
&& (configuration.useFineGrainedViewAttributesExtension
|| configuration.componentHostRecyclingEnabled));
|| configuration.componentHostRecyclingEnabled),
configuration != null && configuration.cloneStateListAnimators);

mLithoHostListenerCoordinator.enableNestedLithoViewsExtension();
mLithoHostListenerCoordinator.enableTransitions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,15 @@ public class LithoHostListenerCoordinator {
private @Nullable ExtensionState<Void> mEndToEndTestingExtensionState;
private boolean mUseFineGrainedLithoViewAttributesState;

private boolean mCloneStateListAnimators;

public LithoHostListenerCoordinator(
MountDelegateTarget mountDelegateTarget, boolean useFineGrainedLithoViewAttributesState) {
MountDelegateTarget mountDelegateTarget,
boolean useFineGrainedLithoViewAttributesState,
boolean cloneStateListAnimators) {
mMountDelegateTarget = mountDelegateTarget;
mUseFineGrainedLithoViewAttributesState = useFineGrainedLithoViewAttributesState;
mCloneStateListAnimators = cloneStateListAnimators;
}

public void setCollectNotifyVisibleBoundsChangedCalls(boolean value) {
Expand Down Expand Up @@ -200,7 +205,8 @@ void enableViewAttributes() {

mViewAttributesExtensionState =
mMountDelegateTarget.registerMountExtension(
LithoViewAttributesExtension.getInstance(mUseFineGrainedLithoViewAttributesState));
LithoViewAttributesExtension.getInstance(
mUseFineGrainedLithoViewAttributesState, mCloneStateListAnimators));
}

void enableNestedLithoViewsExtension() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ import com.facebook.rendercore.extensions.OnItemCallbacks
import com.facebook.rendercore.utils.equals

class LithoViewAttributesExtension
private constructor(private val useFineGrainedAttributesState: Boolean) :
private constructor(
private val useFineGrainedAttributesState: Boolean,
private val cloneStateListAnimators: Boolean
) :
MountExtension<ViewAttributesInput, LithoViewAttributesState>(),
OnItemCallbacks<LithoViewAttributesState> {

Expand All @@ -51,10 +54,12 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :
}

override fun createState(): LithoViewAttributesState =
if (useFineGrainedAttributesState) FineGrainedLithoViewAttributesState()
else DefaultLithoViewAttributesState()
if (useFineGrainedAttributesState)
FineGrainedLithoViewAttributesState(cloneStateListAnimators)
else DefaultLithoViewAttributesState(cloneStateListAnimators)

interface LithoViewAttributesState {
val shouldCloneStateListAnimators: Boolean

fun onUnitMounted(id: Long)

Expand All @@ -78,7 +83,9 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :
fun reset()
}

private class DefaultLithoViewAttributesState : LithoViewAttributesState {
private class DefaultLithoViewAttributesState(
override val shouldCloneStateListAnimators: Boolean
) : LithoViewAttributesState {
private val _defaultViewAttributes: MutableMap<Long, Int> = HashMap()
private var currentUnits: Map<Long, ViewAttributes>? = null
private var newUnits: Map<Long, ViewAttributes>? = null
Expand Down Expand Up @@ -118,7 +125,9 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :
override fun onUnitUnmounted(id: Long) = Unit
}

private class FineGrainedLithoViewAttributesState : LithoViewAttributesState {
private class FineGrainedLithoViewAttributesState(
override val shouldCloneStateListAnimators: Boolean
) : LithoViewAttributesState {
private val _defaultViewAttributes: MutableMap<Long, Int> = HashMap()
private val currentUnits: MutableScatterMap<Long, ViewAttributes> = MutableScatterMap()
private var toBeCommittedUnits: Map<Long, ViewAttributes>? = null
Expand Down Expand Up @@ -195,7 +204,7 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :
}
state.setDefaultViewAttributes(id, flags)
}
setViewAttributes(content, viewAttributes, renderUnit)
setViewAttributes(content, viewAttributes, renderUnit, state.shouldCloneStateListAnimators)
state.onUnitMounted(id)
}
}
Expand Down Expand Up @@ -285,11 +294,19 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :

companion object {
@JvmStatic
fun getInstance(useFineGrainedAttributesState: Boolean): LithoViewAttributesExtension =
LithoViewAttributesExtension(useFineGrainedAttributesState)
fun getInstance(
useFineGrainedAttributesState: Boolean,
cloneStateListAnimators: Boolean = false,
): LithoViewAttributesExtension =
LithoViewAttributesExtension(useFineGrainedAttributesState, cloneStateListAnimators)

@JvmStatic
fun setViewAttributes(content: Any?, attributes: ViewAttributes, unit: RenderUnit<*>?) {
fun setViewAttributes(
content: Any?,
attributes: ViewAttributes,
unit: RenderUnit<*>?,
cloneStateListAnimators: Boolean = false
) {
if (content !is View) {
return
}
Expand Down Expand Up @@ -334,7 +351,7 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :
setImportantForAccessibility(content, attributes.importantForAccessibility)
val isHostSpec = attributes.isHostSpec
setViewLayerType(content, attributes)
setViewStateListAnimator(content, attributes)
setViewStateListAnimator(content, attributes, cloneStateListAnimators)
if (attributes.disableDrawableOutputs) {
setViewBackground(content, attributes)
setViewForeground(content, attributes.foreground)
Expand Down Expand Up @@ -981,7 +998,11 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :
view.layoutDirection = View.LAYOUT_DIRECTION_INHERIT
}

private fun setViewStateListAnimator(view: View, attributes: ViewAttributes) {
private fun setViewStateListAnimator(
view: View,
attributes: ViewAttributes,
cloneStateListAnimators: Boolean
) {
var stateListAnimator = attributes.stateListAnimator
val stateListAnimatorRes = attributes.stateListAnimatorRes
if (stateListAnimator == null && stateListAnimatorRes == 0) {
Expand All @@ -994,6 +1015,15 @@ private constructor(private val useFineGrainedAttributesState: Boolean) :
stateListAnimator =
AnimatorInflater.loadStateListAnimator(view.context, stateListAnimatorRes)
}
if (cloneStateListAnimators) {
stateListAnimator =
try {
stateListAnimator?.clone()
} catch (e: CloneNotSupportedException) {
// If we fail to clone, just fallback to the original animator
stateListAnimator
}
}
view.stateListAnimator = stateListAnimator
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ internal constructor(
* performance regressions.
*/
@JvmField val useFineGrainedViewAttributesExtension: Boolean = false,
/**
* This is a temporary config param for debugging state list animator crashes during layout of a
* [ComponentHost]
*/
@JvmField val cloneStateListAnimators: Boolean = false,
@JvmField val enableFacadeStateUpdater: Boolean = false,
@JvmField val skipSecondIsInWorkingRangeCheck: Boolean = false,
@JvmField val enableVisibilityFixForNestedLithoView: Boolean = false,
Expand Down Expand Up @@ -343,6 +348,7 @@ internal constructor(
private var skipHostAlphaReset = baseConfig.skipHostAlphaReset
private var useFineGrainedViewAttributesExtension =
baseConfig.useFineGrainedViewAttributesExtension
private var cloneStateListAnimators = baseConfig.cloneStateListAnimators
private var enableFacadeStateUpdater = baseConfig.enableFacadeStateUpdater
private var skipSecondIsInWorkingRangeCheck = baseConfig.skipSecondIsInWorkingRangeCheck
private var enableVisibilityFixForNestedLithoView =
Expand Down Expand Up @@ -442,6 +448,10 @@ internal constructor(
useFineGrainedViewAttributesExtension = enabled
}

fun cloneStateListAnimators(enabled: Boolean): Builder = also {
cloneStateListAnimators = enabled
}

fun enableFacadeStateUpdater(enabled: Boolean): Builder = also {
enableFacadeStateUpdater = enabled
}
Expand Down Expand Up @@ -507,6 +517,7 @@ internal constructor(
enableSetLifecycleOwnerTreePropViaDefaultLifecycleOwner,
skipHostAlphaReset = skipHostAlphaReset,
useFineGrainedViewAttributesExtension = useFineGrainedViewAttributesExtension,
cloneStateListAnimators = cloneStateListAnimators,
enableFacadeStateUpdater = enableFacadeStateUpdater,
skipSecondIsInWorkingRangeCheck = skipSecondIsInWorkingRangeCheck,
enableVisibilityFixForNestedLithoView = enableVisibilityFixForNestedLithoView,
Expand Down

0 comments on commit f726c2f

Please sign in to comment.