From bf5e73e46c6e6fd704a3a0d366bcace680296ab2 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 29 Jul 2019 18:08:23 -0700 Subject: [PATCH] Always pass props and state to Create mount item Summary: For Litho interop and to resolve T47926405, always pass props and state to Create mount item so that any ViewManager can create view instances with knowledge of initial props and state. Reviewed By: mdvacca Differential Revision: D16554082 fbshipit-source-id: 3b19a43347b0fa201a054eec60e82fb77cad3625 --- .../react/fabric/FabricUIManager.java | 21 ++++++++++++++++--- .../com/facebook/react/fabric/jni/Binding.cpp | 21 +++++++++++++++++-- .../fabric/mounting/MountingManager.java | 4 +--- .../mounting/mountitems/CreateMountItem.java | 12 ++++++++++- .../facebook/react/uimanager/ViewManager.java | 8 ++++++- 5 files changed, 56 insertions(+), 10 deletions(-) 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 bd001b0863e..68ee00ea153 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -219,7 +219,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { int reactTag, final String componentName, @Nullable ReadableMap props, - Object stateWrapper, + @Nullable Object stateWrapper, boolean isLayoutable) { ThemedReactContext context = mReactContextForRootTag.get(rootTag); String component = getFabricComponentName(componentName); @@ -239,13 +239,25 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @DoNotStrip @SuppressWarnings("unused") private MountItem createMountItem( - String componentName, int reactRootTag, int reactTag, boolean isLayoutable) { + String componentName, + @Nullable ReadableMap props, + @Nullable Object stateWrapper, + int reactRootTag, + int reactTag, + boolean isLayoutable) { String component = getFabricComponentName(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); + return new CreateMountItem( + reactContext, + reactRootTag, + reactTag, + component, + props, + (StateWrapper) stateWrapper, + isLayoutable); } @DoNotStrip @@ -449,6 +461,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { long batchedExecutionStartTime = SystemClock.uptimeMillis(); for (MountItem mountItem : mountItemsToDispatch) { + if (DEBUG) { + FLog.d(TAG, "dispatchMountItems: Executing mountItem: " + mountItem); + } mountItem.execute(mMountingManager); } mBatchedExecutionTime = SystemClock.uptimeMillis() - batchedExecutionStartTime; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index 811a6d90950..42ffb8af28d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -427,13 +427,15 @@ local_ref createDeleteMountItem( return deleteInstruction(javaUIManager, mutation.oldChildShadowView.tag); } +// TODO T48019320: because we pass initial props and state to the Create (and preallocate) mount instruction, +// we technically don't need to pass the first Update to any components. Dedupe? local_ref createCreateMountItem( const jni::global_ref &javaUIManager, const ShadowViewMutation &mutation, const Tag surfaceId) { static auto createJavaInstruction = jni::findClassStatic(UIManagerJavaDescriptor) - ->getMethod(jstring, jint, jint, jboolean)>( + ->getMethod(jstring, ReadableMap::javaobject, jobject, jint, jint, jboolean)>( "createMountItem"); auto newChildShadowView = mutation.newChildShadowView; @@ -444,9 +446,24 @@ local_ref createCreateMountItem( jboolean isLayoutable = newChildShadowView.layoutMetrics != EmptyLayoutMetrics; + local_ref props = castReadableMap( + ReadableNativeMap::newObjectCxxArgs(newChildShadowView.props->rawProps)); + + // Do not hold onto Java object from C + // We DO want to hold onto C object from Java, since we don't know the + // lifetime of the Java object + local_ref javaStateWrapper = nullptr; + if (newChildShadowView.state != nullptr) { + javaStateWrapper = StateWrapperImpl::newObjectJavaArgs(); + StateWrapperImpl *cStateWrapper = cthis(javaStateWrapper); + cStateWrapper->state_ = newChildShadowView.state; + } + return createJavaInstruction( javaUIManager, componentName.get(), + props.get(), + (javaStateWrapper != nullptr ? javaStateWrapper.get() : nullptr), surfaceId, newChildShadowView.tag, isLayoutable); @@ -514,7 +531,7 @@ void Binding::schedulerDidFinishTransaction( } case ShadowViewMutation::Delete: { mountItems[position++] = - createDeleteMountItem(localJavaUIManager, mutation); + createDeleteMountItem(localJavaUIManager, mutation); deletedViewTags.insert(mutation.oldChildShadowView.tag); break; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index 2f3098630b2..e8e3c77b196 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -191,13 +191,11 @@ public class MountingManager { if (isLayoutable) { viewManager = mViewManagerRegistry.get(componentName); + // View Managers are responsible for dealing with initial state and props. view = mViewFactory.getOrCreateView( componentName, propsDiffMap, stateWrapper, themedReactContext); view.setId(reactTag); - if (stateWrapper != null) { - viewManager.updateState(view, propsDiffMap, stateWrapper); - } } ViewState viewState = new ViewState(reactTag, view, viewManager); 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 index 31ac10e53ef..5521992c396 100644 --- 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 @@ -6,7 +6,10 @@ */ package com.facebook.react.fabric.mounting.mountitems; +import androidx.annotation.Nullable; +import com.facebook.react.bridge.ReadableMap; import com.facebook.react.fabric.mounting.MountingManager; +import com.facebook.react.uimanager.StateWrapper; import com.facebook.react.uimanager.ThemedReactContext; public class CreateMountItem implements MountItem { @@ -15,6 +18,8 @@ public class CreateMountItem implements MountItem { private final int mRootTag; private final int mReactTag; private final ThemedReactContext mContext; + private final @Nullable ReadableMap mProps; + private final @Nullable StateWrapper mStateWrapper; private final boolean mIsLayoutable; public CreateMountItem( @@ -22,17 +27,22 @@ public class CreateMountItem implements MountItem { int rootTag, int reactTag, String component, + @Nullable ReadableMap props, + StateWrapper stateWrapper, boolean isLayoutable) { mContext = context; mComponent = component; mRootTag = rootTag; mReactTag = reactTag; + mProps = props; + mStateWrapper = stateWrapper; mIsLayoutable = isLayoutable; } @Override public void execute(MountingManager mountingManager) { - mountingManager.createView(mContext, mComponent, mReactTag, null, null, mIsLayoutable); + mountingManager.createView( + mContext, mComponent, mReactTag, mProps, mStateWrapper, mIsLayoutable); } @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java index 5ee73c48894..084e9d61ab1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java @@ -117,6 +117,12 @@ public abstract class ViewManager if (initialProps != null) { updateProperties(view, initialProps); } + if (stateWrapper != null) { + Object extraData = updateState(view, initialProps, stateWrapper); + if (extraData != null) { + updateExtraData(view, extraData); + } + } return view; } @@ -150,7 +156,7 @@ public abstract class ViewManager * x/y/width/height this is the recommended and thread-safe way of passing extra data from css * node to the native view counterpart. * - *

TODO(7247021): Replace updateExtraData with generic update props mechanism after D2086999 + *

TODO T7247021: Replace updateExtraData with generic update props mechanism after D2086999 */ public abstract void updateExtraData(@NonNull T root, Object extraData);