diff --git a/litho-core/src/main/java/com/facebook/litho/DynamicPropsExtension.java b/litho-core/src/main/java/com/facebook/litho/DynamicPropsExtension.java index 17d4585b346..509134ebe68 100644 --- a/litho-core/src/main/java/com/facebook/litho/DynamicPropsExtension.java +++ b/litho-core/src/main/java/com/facebook/litho/DynamicPropsExtension.java @@ -16,18 +16,22 @@ package com.facebook.litho; +import android.graphics.Rect; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.facebook.infer.annotation.Nullsafe; +import com.facebook.litho.config.ComponentsConfiguration; import com.facebook.rendercore.RenderTreeNode; import com.facebook.rendercore.RenderUnit; import com.facebook.rendercore.extensions.ExtensionState; import com.facebook.rendercore.extensions.MountExtension; import com.facebook.rendercore.extensions.OnItemCallbacks; +import java.util.Map; @Nullsafe(Nullsafe.Mode.LOCAL) public class DynamicPropsExtension - extends MountExtension + extends MountExtension< + DynamicPropsExtensionInput, DynamicPropsExtension.DynamicPropsExtensionState> implements OnItemCallbacks { private static final DynamicPropsExtension sInstance = new DynamicPropsExtension(); @@ -43,21 +47,61 @@ public static DynamicPropsExtension getInstance() { return sInstance; } + @Override + public void beforeMount( + ExtensionState extensionState, + @Nullable DynamicPropsExtensionInput dynamicPropsExtensionInput, + Rect localVisibleRect) { + extensionState.getState().mInput = + dynamicPropsExtensionInput != null + ? dynamicPropsExtensionInput.getDynamicValueOutputs() + : null; + } + + @Override + public void onUnmount(ExtensionState extensionState) { + extensionState.releaseAllAcquiredReferences(); + final DynamicPropsExtensionState state = extensionState.getState(); + state.mInput = null; + state.mPreviousInput = null; + } + + @Override + public void afterMount(ExtensionState extensionState) { + final DynamicPropsExtensionState state = extensionState.getState(); + state.mPreviousInput = state.mInput; + state.mInput = null; + } + @Override public void onBindItem( final ExtensionState extensionState, final RenderUnit renderUnit, final Object content, final @Nullable Object layoutData) { - if (renderUnit instanceof LithoRenderUnit) { - final LithoRenderUnit lithoRenderUnit = (LithoRenderUnit) renderUnit; - final DynamicPropsExtensionState state = extensionState.getState(); - - state.mDynamicPropsManager.onBindComponentToContent( - lithoRenderUnit.getComponent(), - lithoRenderUnit.componentContext, - lithoRenderUnit.commonDynamicProps, - content); + final DynamicPropsExtensionState state = extensionState.getState(); + + if (ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix) { + final DynamicValueOutput dynamicValueOutput = + state.mInput != null ? state.mInput.get(renderUnit.getId()) : null; + + if (dynamicValueOutput != null) { + state.mDynamicPropsManager.onBindComponentToContent( + dynamicValueOutput.getComponent(), + dynamicValueOutput.getScopedContext(), + dynamicValueOutput.getCommonDynamicProps(), + content); + } + } else { + if (renderUnit instanceof LithoRenderUnit) { + final LithoRenderUnit lithoRenderUnit = (LithoRenderUnit) renderUnit; + + state.mDynamicPropsManager.onBindComponentToContent( + lithoRenderUnit.getComponent(), + lithoRenderUnit.componentContext, + lithoRenderUnit.commonDynamicProps, + content); + } } } @@ -67,12 +111,23 @@ public void onUnbindItem( final RenderUnit renderUnit, final Object content, final @Nullable Object layoutData) { - if (renderUnit instanceof LithoRenderUnit) { - final LithoRenderUnit lithoRenderUnit = (LithoRenderUnit) renderUnit; - final DynamicPropsExtensionState state = extensionState.getState(); - - state.mDynamicPropsManager.onUnbindComponent( - lithoRenderUnit.getComponent(), lithoRenderUnit.commonDynamicProps, content); + final DynamicPropsExtensionState state = extensionState.getState(); + + if (ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix) { + final DynamicValueOutput dynamicValueOutput = + state.mPreviousInput != null ? state.mPreviousInput.get(renderUnit.getId()) : null; + + if (dynamicValueOutput != null) { + state.mDynamicPropsManager.onUnbindComponent( + dynamicValueOutput.getComponent(), dynamicValueOutput.getCommonDynamicProps(), content); + } + } else { + if (renderUnit instanceof LithoRenderUnit) { + final LithoRenderUnit lithoRenderUnit = (LithoRenderUnit) renderUnit; + + state.mDynamicPropsManager.onUnbindComponent( + lithoRenderUnit.getComponent(), lithoRenderUnit.commonDynamicProps, content); + } } } @@ -116,6 +171,10 @@ public void onBoundsAppliedToItem( static class DynamicPropsExtensionState { private final DynamicPropsManager mDynamicPropsManager = new DynamicPropsManager(); + @Nullable private Map mInput; + + @Nullable private Map mPreviousInput; + @VisibleForTesting public DynamicPropsManager getDynamicPropsManager() { return mDynamicPropsManager; diff --git a/litho-core/src/main/java/com/facebook/litho/DynamicPropsExtensionInput.kt b/litho-core/src/main/java/com/facebook/litho/DynamicPropsExtensionInput.kt new file mode 100644 index 00000000000..20d66f98a7d --- /dev/null +++ b/litho-core/src/main/java/com/facebook/litho/DynamicPropsExtensionInput.kt @@ -0,0 +1,21 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.facebook.litho + +interface DynamicPropsExtensionInput { + val dynamicValueOutputs: Map +} diff --git a/litho-core/src/main/java/com/facebook/litho/DynamicValueOutput.kt b/litho-core/src/main/java/com/facebook/litho/DynamicValueOutput.kt new file mode 100644 index 00000000000..836b8847025 --- /dev/null +++ b/litho-core/src/main/java/com/facebook/litho/DynamicValueOutput.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.facebook.litho + +import android.util.SparseArray + +class DynamicValueOutput( + val component: Component, + val scopedContext: ComponentContext?, + val commonDynamicProps: SparseArray> +) diff --git a/litho-core/src/main/java/com/facebook/litho/LayoutState.java b/litho-core/src/main/java/com/facebook/litho/LayoutState.java index a947bf3f215..a20b31f8862 100644 --- a/litho-core/src/main/java/com/facebook/litho/LayoutState.java +++ b/litho-core/src/main/java/com/facebook/litho/LayoutState.java @@ -72,7 +72,8 @@ public class LayoutState TransitionsExtensionInput, EndToEndTestingExtensionInput, PotentiallyPartialResult, - ViewAttributesInput { + ViewAttributesInput, + DynamicPropsExtensionInput { @Nullable private Transition.RootBoundsTransition mRootWidthAnimation; @Nullable private Transition.RootBoundsTransition mRootHeightAnimation; @@ -110,6 +111,7 @@ public static boolean isFromSyncLayout(@RenderSource int source) { final LongSparseArray mOutputsIdToPositionMap = new LongSparseArray<>(8); final Map mRenderUnitsWithViewAttributes = new HashMap<>(8); final Map mIncrementalMountOutputs = new LinkedHashMap<>(8); + final Map mDynamicValueOutputs = new LinkedHashMap<>(8); final ArrayList mMountableOutputTops = new ArrayList<>(); final ArrayList mMountableOutputBottoms = new ArrayList<>(); final LongSparseArray mAnimatableItems = new LongSparseArray<>(8); @@ -787,4 +789,9 @@ public Transition.RootBoundsTransition getRootWidthAnimation() { public VisibilityBoundsTransformer getVisibilityBoundsTransformer() { return getComponentContext().getVisibilityBoundsTransformer(); } + + @Override + public Map getDynamicValueOutputs() { + return mDynamicValueOutputs; + } } diff --git a/litho-core/src/main/java/com/facebook/litho/LithoReducer.java b/litho-core/src/main/java/com/facebook/litho/LithoReducer.java index 3c13192e3ba..4e73d8ea5e1 100644 --- a/litho-core/src/main/java/com/facebook/litho/LithoReducer.java +++ b/litho-core/src/main/java/com/facebook/litho/LithoReducer.java @@ -1086,6 +1086,18 @@ private static void addRenderTreeNode( layoutState.mRenderUnitsWithViewAttributes.put(id, attrs); } + if (node.getRenderUnit() instanceof LithoRenderUnit) { + final LithoRenderUnit lithoRenderUnit = (LithoRenderUnit) node.getRenderUnit(); + if (lithoRenderUnit.commonDynamicProps != null) { + layoutState.mDynamicValueOutputs.put( + lithoRenderUnit.getId(), + new DynamicValueOutput( + lithoRenderUnit.getComponent(), + lithoRenderUnit.componentContext, + lithoRenderUnit.commonDynamicProps)); + } + } + final AnimatableItem animatableItem = createAnimatableItem(unit, absoluteBounds, type, transitionId); diff --git a/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java b/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java index 600c0d7c6ec..1eb8b02455b 100644 --- a/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java +++ b/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java @@ -234,6 +234,8 @@ public static boolean isRenderInfoDebuggingEnabled() { public static int useCachedLruCacheSize = 0; + public static boolean enablePrimitiveDynamicPropsExtensionFix = false; + public static boolean shouldOverrideHasTransientState = false; public static boolean shouldUseLruCacheForUseCached() { diff --git a/litho-it/src/test/java/com/facebook/litho/DynamicPropsTest.kt b/litho-it/src/test/java/com/facebook/litho/DynamicPropsTest.kt index d9d2e960be2..c5db0285f72 100644 --- a/litho-it/src/test/java/com/facebook/litho/DynamicPropsTest.kt +++ b/litho-it/src/test/java/com/facebook/litho/DynamicPropsTest.kt @@ -434,6 +434,120 @@ class DynamicPropsTest { alphaDV.set(1f) assertThat(hostView.alpha).isEqualTo(1f) } + + @Test + fun testPrimitiveWithDynamicValueIsCorrectlyUnsubscribed() { + val alphaDV1 = DynamicValue(1f) + var lithoView = + legacyLithoViewRule + .attachToWindow() + .setRoot( + SimpleTestPrimitiveComponent( + style = Style.width(80.px).height(80.px).alpha(alphaDV1))) + .measure() + .layout() + .lithoView + legacyLithoViewRule.idle() + + assertThat(alphaDV1.numberOfListeners).isEqualTo(1) + + // simulate update + val alphaDV2 = DynamicValue(1f) + lithoView = + legacyLithoViewRule + .attachToWindow() + .setRoot( + SimpleTestPrimitiveComponent( + style = Style.width(80.px).height(80.px).alpha(alphaDV2))) + .measure() + .layout() + .lithoView + legacyLithoViewRule.idle() + + // should unsubscribe from the old DV and subscribe to the new DV + if (ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix) { + // works correctly when fix is applied + assertThat(alphaDV1.numberOfListeners).isEqualTo(0) + assertThat(alphaDV2.numberOfListeners).isEqualTo(1) + } else { + // doesn't work without the fix + assertThat(alphaDV1.numberOfListeners).isEqualTo(1) + assertThat(alphaDV2.numberOfListeners).isEqualTo(0) + } + + lithoView.unmountAllItems() + legacyLithoViewRule.idle() + + // should unsubscribe from all DVs + if (ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix) { + // works correctly when fix is applied + assertThat(alphaDV1.numberOfListeners).isEqualTo(0) + } else { + // doesn't work without the fix + assertThat(alphaDV1.numberOfListeners).isEqualTo(1) + } + assertThat(alphaDV2.numberOfListeners).isEqualTo(0) + } + + @Test + fun testWrappedPrimitiveWithDynamicValueIsCorrectlyUnsubscribed() { + val alphaDV1 = DynamicValue(1f) + var lithoView = + legacyLithoViewRule + .attachToWindow() + .setRoot( + Wrapper.create(context) + .delegate( + SimpleTestPrimitiveComponent(style = Style.width(80.px).height(80.px))) + .kotlinStyle(Style.alpha(alphaDV1)) + .build()) + .measure() + .layout() + .lithoView + legacyLithoViewRule.idle() + + assertThat(alphaDV1.numberOfListeners).isEqualTo(1) + + // simulate update + val alphaDV2 = DynamicValue(1f) + lithoView = + legacyLithoViewRule + .attachToWindow() + .setRoot( + Wrapper.create(context) + .delegate( + SimpleTestPrimitiveComponent(style = Style.width(80.px).height(80.px))) + .kotlinStyle(Style.alpha(alphaDV2)) + .build()) + .measure() + .layout() + .lithoView + legacyLithoViewRule.idle() + + // should unsubscribe from the old DV and subscribe to the new DV + if (ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix) { + // works correctly when fix is applied + assertThat(alphaDV1.numberOfListeners).isEqualTo(0) + assertThat(alphaDV2.numberOfListeners).isEqualTo(1) + } else { + // doesn't work without the fix + assertThat(alphaDV1.numberOfListeners).isEqualTo(1) + assertThat(alphaDV2.numberOfListeners).isEqualTo(0) + } + + lithoView.unmountAllItems() + legacyLithoViewRule.idle() + + // should unsubscribe from all DVs + if (ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix) { + // works correctly when fix is applied + assertThat(alphaDV1.numberOfListeners).isEqualTo(0) + } else { + // doesn't work without the fix + assertThat(alphaDV1.numberOfListeners).isEqualTo(1) + } + assertThat(alphaDV2.numberOfListeners).isEqualTo(0) + } } private class SimpleTestPrimitiveComponent( diff --git a/litho-testing/src/main/java/com/facebook/litho/testing/testrunner/DynamicValuesTestRunConfiguration.java b/litho-testing/src/main/java/com/facebook/litho/testing/testrunner/DynamicValuesTestRunConfiguration.java new file mode 100644 index 00000000000..7bdb0126117 --- /dev/null +++ b/litho-testing/src/main/java/com/facebook/litho/testing/testrunner/DynamicValuesTestRunConfiguration.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.facebook.litho.testing.testrunner; + +import com.facebook.litho.config.ComponentsConfiguration; +import org.junit.runners.model.FrameworkMethod; + +public class DynamicValuesTestRunConfiguration implements LithoTestRunConfiguration { + + private final boolean enablePrimitiveDynamicPropsExtensionFix = + ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix; + private final ComponentsConfiguration.Builder mDefaultComponentsConfiguration = + ComponentsConfiguration.getDefaultComponentsConfigurationBuilder(); + + @Override + public void beforeTest(FrameworkMethod method) { + ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix = true; + ComponentsConfiguration.setDefaultComponentsConfigurationBuilder( + ComponentsConfiguration.create().shouldReuseOutputs(true)); + } + + @Override + public void afterTest(FrameworkMethod method) { + ComponentsConfiguration.enablePrimitiveDynamicPropsExtensionFix = + enablePrimitiveDynamicPropsExtensionFix; + ComponentsConfiguration.setDefaultComponentsConfigurationBuilder( + mDefaultComponentsConfiguration); + } +} diff --git a/litho-testing/src/main/java/com/facebook/litho/testing/testrunner/LithoTestRunner.java b/litho-testing/src/main/java/com/facebook/litho/testing/testrunner/LithoTestRunner.java index ee9f2485afd..09920088521 100644 --- a/litho-testing/src/main/java/com/facebook/litho/testing/testrunner/LithoTestRunner.java +++ b/litho-testing/src/main/java/com/facebook/litho/testing/testrunner/LithoTestRunner.java @@ -76,7 +76,8 @@ private List> getGlobalConf return Arrays.asList( LayoutCachingTestRunConfiguration.class, SkipRootCheckingTestRunConfiguration.class, - UseCachedUseLruCacheTestRunConfiguration.class); + UseCachedUseLruCacheTestRunConfiguration.class, + DynamicValuesTestRunConfiguration.class); } @Override