From f98880b1a3f8438b051c954b406901224220814e Mon Sep 17 00:00:00 2001 From: David Vacca Date: Tue, 30 Apr 2019 01:45:08 -0700 Subject: [PATCH] - Fix mounting of views when a re-order happen caused by changes in viewflattening Summary: This diff fixes a bug that is reproducible when a view is reordered in a different level of hierarchy in the react tree. Even if this is not supported by react, this can still happen because of viewFlattening. Reviewed By: shergin Differential Revision: D14817452 fbshipit-source-id: 13425b0e6a280affe681e80b4a6daa17ee56251a --- .../react/fabric/FabricUIManager.java | 17 +++++++- .../facebook/react/fabric/jsi/jni/Binding.cpp | 40 ++++++++++++++++- .../mounting/mountitems/CreateMountItem.java | 43 +++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index d9a5f289b7d..4765f51e34a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -39,6 +39,7 @@ import com.facebook.react.fabric.jsi.EventBeatManager; import com.facebook.react.fabric.jsi.EventEmitterWrapper; import com.facebook.react.fabric.jsi.FabricSoLoader; import com.facebook.react.fabric.jsi.StateWrapperImpl; +import com.facebook.react.fabric.mounting.mountitems.CreateMountItem; import com.facebook.react.fabric.mounting.MountingManager; import com.facebook.react.fabric.mounting.mountitems.BatchMountItem; import com.facebook.react.fabric.mounting.mountitems.DeleteMountItem; @@ -73,7 +74,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { public static final String TAG = FabricUIManager.class.getSimpleName(); public static final boolean DEBUG = PrinterHolder.getPrinter().shouldDisplayLogMessage(ReactDebugOverlayTags.FABRIC_UI_MANAGER); - private static final Map sComponentNames = new HashMap<>(); private static final int FRAME_TIME_MS = 16; private static final int MAX_TIME_IN_FRAME_FOR_NON_BATCHED_OPERATIONS_MS = 8; @@ -203,6 +203,21 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } } + @DoNotStrip + @SuppressWarnings("unused") + private MountItem createMountItem( + String componentName, int reactRootTag, int reactTag, boolean isLayoutable) { + String component = sComponentNames.get(componentName); + if (component == null) { + throw new IllegalArgumentException("Unable to find component with name " + componentName); + } + ThemedReactContext reactContext = mReactContextForRootTag.get(reactRootTag); + if (reactContext == null) { + throw new IllegalArgumentException("Unable to find ReactContext for root: " + reactRootTag); + } + return new CreateMountItem(reactContext, reactRootTag, reactTag, component, isLayoutable); + } + @DoNotStrip @SuppressWarnings("unused") private MountItem removeMountItem(int reactTag, int parentReactTag, int index) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp index 0705cd93625..dbf79b8928c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp @@ -328,12 +328,37 @@ local_ref createDeleteMountItem( return deleteInstruction(javaUIManager, mutation.oldChildShadowView.tag); } +local_ref createCreateMountItem( + const jni::global_ref& javaUIManager, + const ShadowViewMutation& mutation, + const Tag rootTag) { + static auto createJavaInstruction = + jni::findClassStatic(UIManagerJavaDescriptor) + ->getMethod(jstring, jint, jint, jboolean)>( + "createMountItem"); + + auto newChildShadowView = mutation.newChildShadowView; + + local_ref componentName = + getPlatformComponentName(newChildShadowView); + + jboolean isLayoutable = newChildShadowView.layoutMetrics != EmptyLayoutMetrics; + + return createJavaInstruction( + javaUIManager, + componentName.get(), + rootTag, + newChildShadowView.tag, + isLayoutable); +} + void Binding::schedulerDidFinishTransaction( MountingTransaction &&mountingTransaction) { SystraceSection s("FabricUIManager::schedulerDidFinishTransaction"); auto telemetry = mountingTransaction.getTelemetry(); auto mutations = mountingTransaction.getMutations(); + auto surfaceId = mountingTransaction.getSurfaceId(); std::vector> queue; // Upper bound estimation of mount items to be delivered to Java side. @@ -345,6 +370,7 @@ void Binding::schedulerDidFinishTransaction( JArrayClass::newArray(size); auto mountItems = *(mountItemsArray); + std::unordered_set deletedViews; int position = 0; for (const auto& mutation : mutations) { @@ -355,6 +381,14 @@ void Binding::schedulerDidFinishTransaction( oldChildShadowView.layoutMetrics == EmptyLayoutMetrics; switch (mutation.type) { + case ShadowViewMutation::Create: { + if (mutation.newChildShadowView.props->revision > 1 + || deletedViews.find(mutation.newChildShadowView.tag) != deletedViews.end()) { + mountItems[position++] = + createCreateMountItem(javaUIManager_, mutation, surfaceId); + } + break; + } case ShadowViewMutation::Remove: { if (!isVirtual) { mountItems[position++] = @@ -365,6 +399,8 @@ void Binding::schedulerDidFinishTransaction( case ShadowViewMutation::Delete: { mountItems[position++] = createDeleteMountItem(javaUIManager_, mutation); + + deletedViews.insert(mutation.oldChildShadowView.tag); break; } case ShadowViewMutation::Update: { @@ -407,8 +443,8 @@ void Binding::schedulerDidFinishTransaction( // Insert item mountItems[position++] = createInsertMountItem(javaUIManager_, mutation); - // Props - if (mutation.newChildShadowView.props->revision > 1) { + if (mutation.newChildShadowView.props->revision > 1 || + deletedViews.find(mutation.newChildShadowView.tag) != deletedViews.end()) { mountItems[position++] = createUpdatePropsMountItem(javaUIManager_, mutation); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java new file mode 100644 index 00000000000..af518f1c471 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/CreateMountItem.java @@ -0,0 +1,43 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +package com.facebook.react.fabric.mounting.mountitems; + +import com.facebook.react.fabric.mounting.MountingManager; +import com.facebook.react.uimanager.ThemedReactContext; + +public class CreateMountItem implements MountItem { + + private final String mComponent; + private final int mRootTag; + private final int mReactTag; + private final ThemedReactContext mContext; + private final boolean mIsLayoutable; + + public CreateMountItem( + ThemedReactContext context, + int rootTag, + int reactTag, + String component, + boolean isLayoutable) { + mContext = context; + mComponent = component; + mRootTag = rootTag; + mReactTag = reactTag; + mIsLayoutable = isLayoutable; + } + + @Override + public void execute(MountingManager mountingManager) { + mountingManager.createViewWithProps(mContext, mComponent, mReactTag, null, mIsLayoutable); + } + + @Override + public String toString() { + return "CreateMountItem [" + mReactTag + "]"; + } +} +