Refactor cloning of YogaNode

Reviewed By: priteshrnandgaonkar

Differential Revision: D7339832

fbshipit-source-id: 2de6f47ae7601ac083d3b9fbe10ffaf6307ae760

ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java

@@ -10,6 +10,7 @@ import static java.lang.System.arraycopy;
 
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.uimanager.annotations.ReactPropertyHolder;
+import com.facebook.yoga.YogaNodeCloneFunction;
 import com.facebook.yoga.YogaAlign;
 import com.facebook.yoga.YogaBaselineFunction;
 import com.facebook.yoga.YogaConfig;
@@ -21,7 +22,6 @@ import com.facebook.yoga.YogaFlexDirection;
 import com.facebook.yoga.YogaJustify;
 import com.facebook.yoga.YogaMeasureFunction;
 import com.facebook.yoga.YogaNode;
-import com.facebook.yoga.YogaNodeClonedFunction;
 import com.facebook.yoga.YogaOverflow;
 import com.facebook.yoga.YogaPositionType;
 import com.facebook.yoga.YogaValue;
@@ -61,17 +61,19 @@ public class ReactShadowNodeImpl implements ReactShadowNode<ReactShadowNodeImpl>
   private static final YogaConfig sYogaConfig;
   static {
     sYogaConfig = ReactYogaConfigProvider.get();
-    sYogaConfig.setOnNodeCloned(new YogaNodeClonedFunction() {
+    sYogaConfig.setOnCloneNode(new YogaNodeCloneFunction() {
       @Override
-      public void onNodeCloned(YogaNode oldYogaNode,
-          YogaNode newYogaNode,
+      public YogaNode cloneNode(YogaNode oldYogaNode,
           YogaNode parent,
           int childIndex) {
-        ReactShadowNode parentReactShadowNode = (ReactShadowNode) parent.getData();
+        ReactShadowNodeImpl parentReactShadowNode = (ReactShadowNodeImpl) parent.getData();
         Assertions.assertNotNull(parentReactShadowNode);
-
-        ReactShadowNode newReactShadowNode = (ReactShadowNode) newYogaNode.getData();
+        ReactShadowNodeImpl newReactShadowNode = (ReactShadowNodeImpl) oldYogaNode.getData();
         Assertions.assertNotNull(newReactShadowNode);
+
+        ReactShadowNodeImpl newNode = newReactShadowNode.mutableCopy();
+        parentReactShadowNode.replaceChild(newNode, childIndex);
+        return newNode.mYogaNode;
       }
     });
   }
@@ -133,6 +135,11 @@ public class ReactShadowNodeImpl implements ReactShadowNode<ReactShadowNodeImpl>
     mOriginalReactShadowNode = original;
   }
 
+  private void replaceChild(ReactShadowNodeImpl newNode, int childIndex) {
+    mChildren.remove(childIndex);
+    mChildren.add(childIndex, newNode);
+  }
+
   /**
    * @return a copy of this object (no including copy of its children or the underlying yogaNode).
    */
@@ -145,7 +152,7 @@ public class ReactShadowNodeImpl implements ReactShadowNode<ReactShadowNodeImpl>
     ReactShadowNodeImpl copy = copy();
     copy.mYogaNode = mYogaNode;
     // TODO: T26729293 clone YogaNode instead of reusing the same instance
-    //mYogaNode = original.mYogaNode.clone();
+    //copy.mYogaNode = mYogaNode.clone();
     copy.mNativeChildren = mNativeChildren == null ? null : new ArrayList<>(mNativeChildren);
     copy.mTotalNativeChildren = mTotalNativeChildren;
     copy.mChildren = mChildren == null ? null : new ArrayList<>(mChildren);
@@ -165,7 +172,7 @@ public class ReactShadowNodeImpl implements ReactShadowNode<ReactShadowNodeImpl>
     ReactShadowNodeImpl copy = copy();
     copy.mYogaNode = mYogaNode;
     // TODO: T26729293 clone YogaNode instead of reusing the same instance
-    //mYogaNode = original.mYogaNode.cloneWithNewChildren();
+    //copy.mYogaNode = mYogaNode.clone();
     copy.mNativeChildren = null;
     copy.mChildren = null;
     copy.mTotalNativeChildren = 0;

ReactAndroid/src/main/java/com/facebook/yoga/YogaConfig.java

@@ -25,7 +25,7 @@ public class YogaConfig {
 
   long mNativePointer;
   private YogaLogger mLogger;
-  private YogaNodeClonedFunction mNodeClonedFunction;
+  private YogaNodeCloneFunction mYogaNodeCloneFunction;
 
   private native long jni_YGConfigNew();
   public YogaConfig() {
@@ -97,16 +97,15 @@ public class YogaConfig {
     return mLogger;
   }
 
-  private native void jni_YGConfigSetHasNodeClonedFunc(long nativePointer, boolean hasClonedFunc);
+  private native void jni_YGConfigSetHasCloneNodeFunc(long nativePointer, boolean hasClonedFunc);
 
-  public void setOnNodeCloned(YogaNodeClonedFunction nodeClonedFunction) {
-    mNodeClonedFunction = nodeClonedFunction;
-    jni_YGConfigSetHasNodeClonedFunc(mNativePointer, nodeClonedFunction != null);
+  public void setOnCloneNode(YogaNodeCloneFunction cloneYogaNodeFunction) {
+    mYogaNodeCloneFunction = cloneYogaNodeFunction;
+    jni_YGConfigSetHasCloneNodeFunc(mNativePointer, cloneYogaNodeFunction != null);
   }
 
   @DoNotStrip
-  public final void onNodeCloned(
-      YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex) {
-    mNodeClonedFunction.onNodeCloned(oldNode, newNode, parent, childIndex);
+  private final YogaNode cloneNode(YogaNode oldNode, YogaNode parent, int childIndex) {
+    return mYogaNodeCloneFunction.cloneNode(oldNode, parent, childIndex);
   }
 }

ReactAndroid/src/main/java/com/facebook/yoga/YogaNode.java

@@ -697,4 +697,18 @@ public class YogaNode implements Cloneable {
   public void print() {
     jni_YGNodePrint(mNativePointer);
   }
+
+  /**
+   * This method replaces the child at childIndex position with the newNode received by parameter.
+   * This is different than calling removeChildAt and addChildAt because this method ONLY replaces
+   * the child in the mChildren datastructure. @DoNotStrip: called from JNI
+   *
+   * @return the nativePointer of the newNode {@linl YogaNode}
+   */
+  @DoNotStrip
+  private final long replaceChild(YogaNode newNode, int childIndex) {
+    mChildren.remove(childIndex);
+    mChildren.add(childIndex, newNode);
+    return newNode.mNativePointer;
+  }
 }

ReactAndroid/src/main/java/com/facebook/yoga/YogaNodeCloneFunction.java

@@ -10,8 +10,8 @@ package com.facebook.yoga;
 import com.facebook.proguard.annotations.DoNotStrip;
 
 @DoNotStrip
-public interface YogaNodeClonedFunction {
+public interface YogaNodeCloneFunction {
 
   @DoNotStrip
-  void onNodeCloned(YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex);
+  YogaNode cloneNode(YogaNode oldNode, YogaNode parent, int childIndex);
 }

ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp

@@ -142,31 +142,49 @@ static float YGJNIBaselineFunc(YGNodeRef node, float width, float height) {
   }
 }
 
-static void YGJNIOnNodeClonedFunc(
+static inline YGNodeRef _jlong2YGNodeRef(jlong addr) {
+  return reinterpret_cast<YGNodeRef>(static_cast<intptr_t>(addr));
+}
+
+static inline YGConfigRef _jlong2YGConfigRef(jlong addr) {
+  return reinterpret_cast<YGConfigRef>(static_cast<intptr_t>(addr));
+}
+
+static YGNodeRef YGJNIOnNodeClonedFunc(
     YGNodeRef oldNode,
-    YGNodeRef newNode,
     YGNodeRef parent,
     int childIndex) {
   auto config = oldNode->getConfig();
   if (!config) {
-    return;
+    return nullptr;
   }
+
   static auto onNodeClonedFunc = findClassStatic("com/facebook/yoga/YogaConfig")
-                                     ->getMethod<void(
+                                     ->getMethod<alias_ref<JYogaNode>(
                                          local_ref<JYogaNode>,
                                          local_ref<JYogaNode>,
-                                         local_ref<JYogaNode>,
-                                         jint)>("onNodeCloned");
+                                         jint)>("cloneNode");
 
   auto context = reinterpret_cast<YGConfigContext*>(YGConfigGetContext(config));
   auto javaConfig = context->config;
 
-  onNodeClonedFunc(
+  auto newNode = onNodeClonedFunc(
       javaConfig->get(),
       YGNodeJobject(oldNode)->lockLocal(),
-      YGNodeJobject(newNode)->lockLocal(),
       YGNodeJobject(parent)->lockLocal(),
       childIndex);
+
+  static auto replaceChild = findClassStatic("com/facebook/yoga/YogaNode")
+                                     ->getMethod<jlong(
+                                         local_ref<JYogaNode>,
+                                         jint)>("replaceChild");
+
+  jlong newNodeNativePointer = replaceChild(
+      YGNodeJobject(parent)->lockLocal(),
+      newNode,
+      childIndex);
+
+  return _jlong2YGNodeRef(newNodeNativePointer);
 }
 
 static YGSize YGJNIMeasureFunc(
@@ -234,14 +252,6 @@ static int YGJNILogFunc(const YGConfigRef config,
   return result;
 }
 
-static inline YGNodeRef _jlong2YGNodeRef(jlong addr) {
-  return reinterpret_cast<YGNodeRef>(static_cast<intptr_t>(addr));
-}
-
-static inline YGConfigRef _jlong2YGConfigRef(jlong addr) {
-  return reinterpret_cast<YGConfigRef>(static_cast<intptr_t>(addr));
-}
-
 jlong jni_YGNodeNew(alias_ref<jobject> thiz) {
   const YGNodeRef node = YGNodeNew();
   node->setContext(new weak_ref<jobject>(make_weak(thiz)));
@@ -506,10 +516,10 @@ void jni_YGConfigSetUseLegacyStretchBehaviour(alias_ref<jobject>,
   YGConfigSetUseLegacyStretchBehaviour(config, useLegacyStretchBehaviour);
 }
 
-void jni_YGConfigSetHasNodeClonedFunc(
+void jni_YGConfigSetHasCloneNodeFunc(
     alias_ref<jobject> thiz,
     jlong nativePointer,
-    jboolean hasNodeClonedFunc) {
+    jboolean hasCloneNodeFunc) {
   const YGConfigRef config = _jlong2YGConfigRef(nativePointer);
   auto context = reinterpret_cast<YGConfigContext*>(YGConfigGetContext(config));
   if (context && context->config) {
@@ -517,15 +527,15 @@ void jni_YGConfigSetHasNodeClonedFunc(
     context->config = nullptr;
   }
 
-  if (hasNodeClonedFunc) {
+  if (hasCloneNodeFunc) {
     if (!context) {
       context = new YGConfigContext();
       YGConfigSetContext(config, context);
     }
     context->config = new global_ref<jobject>(make_global(thiz));
-    YGConfigSetNodeClonedFunc(config, YGJNIOnNodeClonedFunc);
+    YGConfigSetCloneNodeFunc(config, YGJNIOnNodeClonedFunc);
   } else {
-    YGConfigSetNodeClonedFunc(config, nullptr);
+    YGConfigSetCloneNodeFunc(config, nullptr);
   }
 }
 
@@ -652,7 +662,7 @@ jint JNI_OnLoad(JavaVM *vm, void *) {
             YGMakeNativeMethod(jni_YGConfigSetPointScaleFactor),
             YGMakeNativeMethod(jni_YGConfigSetUseLegacyStretchBehaviour),
             YGMakeNativeMethod(jni_YGConfigSetLogger),
-            YGMakeNativeMethod(jni_YGConfigSetHasNodeClonedFunc),
+            YGMakeNativeMethod(jni_YGConfigSetHasCloneNodeFunc),
             YGMakeNativeMethod(
                 jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviour),
         });

ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp

@@ -24,7 +24,7 @@ SharedYogaConfig YogaLayoutableShadowNode::suitableYogaConfig() {
 
   if (!sharedYogaConfig) {
     sharedYogaConfig = std::make_shared<YGConfig>(YGConfig({
-      .cloneNodeCallback = YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector
+      // .cloneNodeCallback = YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector
     }));
   }
 

ReactCommon/yoga/yoga/YGNode.cpp

@@ -558,15 +558,18 @@ void YGNode::cloneChildrenIfNeeded() {
     return;
   }
 
-  const YGNodeClonedFunc cloneNodeCallback = config_->cloneNodeCallback;
+  const YGCloneNodeFunc cloneNodeCallback = config_->cloneNodeCallback;
   for (uint32_t i = 0; i < childCount; ++i) {
     const YGNodeRef oldChild = children_[i];
-    const YGNodeRef newChild = YGNodeClone(oldChild);
+    YGNodeRef newChild = nullptr;
+    if (cloneNodeCallback) {
+      newChild = cloneNodeCallback(oldChild, this, i);
+    }
+    if (newChild == nullptr) {
+      newChild = YGNodeClone(oldChild);
+    }
     replaceChild(newChild, i);
     newChild->setParent(this);
-    if (cloneNodeCallback) {
-      cloneNodeCallback(oldChild, newChild, this, i);
-    }
   }
 }
 

ReactCommon/yoga/yoga/Yoga-internal.h

@@ -94,7 +94,7 @@ struct YGConfig {
   bool shouldDiffLayoutWithoutLegacyStretchBehaviour;
   float pointScaleFactor;
   YGLogger logger;
-  YGNodeClonedFunc cloneNodeCallback;
+  YGCloneNodeFunc cloneNodeCallback;
   void* context;
 };
 

ReactCommon/yoga/yoga/Yoga.cpp

@@ -428,7 +428,7 @@ void YGNodeRemoveChild(const YGNodeRef parent, const YGNodeRef excludedChild) {
   // Otherwise we have to clone the node list except for the child we're trying to delete.
   // We don't want to simply clone all children, because then the host will need to free
   // the clone of the child that was just deleted.
-  const YGNodeClonedFunc cloneNodeCallback =
+  const YGCloneNodeFunc cloneNodeCallback =
       parent->getConfig()->cloneNodeCallback;
   uint32_t nextInsertIndex = 0;
   for (uint32_t i = 0; i < childCount; i++) {
@@ -440,12 +440,16 @@ void YGNodeRemoveChild(const YGNodeRef parent, const YGNodeRef excludedChild) {
       parent->markDirtyAndPropogate();
       continue;
     }
-    const YGNodeRef newChild = YGNodeClone(oldChild);
+    YGNodeRef newChild = nullptr;
+    if (cloneNodeCallback) {
+      newChild = cloneNodeCallback(oldChild, parent, nextInsertIndex);
+    }
+    if (newChild == nullptr) {
+      newChild = YGNodeClone(oldChild);
+    }
     parent->replaceChild(newChild, nextInsertIndex);
     newChild->setParent(parent);
-    if (cloneNodeCallback) {
-      cloneNodeCallback(oldChild, newChild, parent, nextInsertIndex);
-    }
+
     nextInsertIndex++;
   }
   while (nextInsertIndex < childCount) {
@@ -3964,7 +3968,7 @@ void *YGConfigGetContext(const YGConfigRef config) {
   return config->context;
 }
 
-void YGConfigSetNodeClonedFunc(const YGConfigRef config, const YGNodeClonedFunc callback) {
+void YGConfigSetCloneNodeFunc(const YGConfigRef config, const YGCloneNodeFunc callback) {
   config->cloneNodeCallback = callback;
 }
 

ReactCommon/yoga/yoga/Yoga.h

@@ -62,8 +62,7 @@ typedef int (*YGLogger)(const YGConfigRef config,
                         YGLogLevel level,
                         const char *format,
                         va_list args);
-typedef void (*YGNodeClonedFunc)(YGNodeRef oldNode,
-                                 YGNodeRef newNode,
+typedef YGNodeRef (*YGCloneNodeFunc)(YGNodeRef oldNode,
                                  YGNodeRef parent,
                                  int childIndex);
 
@@ -283,8 +282,8 @@ WIN_EXPORT bool YGConfigIsExperimentalFeatureEnabled(const YGConfigRef config,
 WIN_EXPORT void YGConfigSetUseWebDefaults(const YGConfigRef config, const bool enabled);
 WIN_EXPORT bool YGConfigGetUseWebDefaults(const YGConfigRef config);
 
-WIN_EXPORT void YGConfigSetNodeClonedFunc(const YGConfigRef config,
-                                          const YGNodeClonedFunc callback);
+WIN_EXPORT void YGConfigSetCloneNodeFunc(const YGConfigRef config,
+                                          const YGCloneNodeFunc callback);
 
 // Export only for C#
 WIN_EXPORT YGConfigRef YGConfigGetDefault(void);