Breaking remove YogaNode#clone

Summary:
@public

The cloning features of YogaNode don’t seem to be used. Let’s remove them.

Reviewed By: SidharthGuglani

Differential Revision: D14165624

fbshipit-source-id: 5b710964a4abf1b35f3bcc25b143ffc719a03cec

java/com/facebook/yoga/YogaConfig.java

@@ -98,16 +98,4 @@ public class YogaConfig {
   public YogaLogger getLogger() {
     return mLogger;
   }
-
-  private native void jni_YGConfigSetHasCloneNodeFunc(long nativePointer, boolean hasClonedFunc);
-
-  public void setOnCloneNode(YogaNodeCloneFunction cloneYogaNodeFunction) {
-    mYogaNodeCloneFunction = cloneYogaNodeFunction;
-    jni_YGConfigSetHasCloneNodeFunc(mNativePointer, cloneYogaNodeFunction != null);
-  }
-
-  @DoNotStrip
-  private final YogaNodeJNI cloneNode(YogaNodeJNI oldNode, YogaNodeJNI parent, int childIndex) {
-    return (YogaNodeJNI) mYogaNodeCloneFunction.cloneNode(oldNode, parent, childIndex);
-  }
 }

java/com/facebook/yoga/YogaNodeJNI.java

@@ -13,7 +13,7 @@ import java.util.List;
 import javax.annotation.Nullable;
 
 @DoNotStrip
-public class YogaNodeJNI extends YogaNode implements Cloneable {
+public class YogaNodeJNI extends YogaNode {
 
   static {
     SoLoader.loadLibrary("yoga");
@@ -189,53 +189,6 @@ public class YogaNodeJNI extends YogaNode implements Cloneable {
     return jni_YGNodeIsReferenceBaseline(mNativePointer);
   }
 
-  private static native void jni_YGNodeSetOwner(long nativePointer, long newOwnerNativePointer);
-
-  private native long jni_YGNodeClone(long nativePointer, Object newNode, boolean avoidGlobalJNIRefs);
-
-  @Override
-  public YogaNodeJNI clone() {
-    try {
-      YogaNodeJNI clonedYogaNode = (YogaNodeJNI) super.clone();
-      long clonedNativePointer = jni_YGNodeClone(mNativePointer, clonedYogaNode, mAvoidGlobalJNIRefs);
-
-      if (mChildren != null) {
-        for (YogaNodeJNI child : mChildren) {
-          jni_YGNodeSetOwner(child.mNativePointer, 0);
-          child.mOwner = null;
-        }
-      }
-
-      clonedYogaNode.mNativePointer = clonedNativePointer;
-      clonedYogaNode.mOwner = null;
-      clonedYogaNode.mChildren =
-          mChildren != null ? (List<YogaNodeJNI>) ((ArrayList) mChildren).clone() : null;
-      if (clonedYogaNode.mChildren != null) {
-        for (YogaNodeJNI child : clonedYogaNode.mChildren) {
-          child.mOwner = null;
-        }
-      }
-      return clonedYogaNode;
-    } catch (CloneNotSupportedException ex) {
-      // This class implements Cloneable, this should not happen
-      throw new RuntimeException(ex);
-    }
-  }
-
-  public YogaNodeJNI cloneWithNewChildren() {
-    try {
-      YogaNodeJNI clonedYogaNode = (YogaNodeJNI) super.clone();
-      long clonedNativePointer = jni_YGNodeClone(mNativePointer, clonedYogaNode, mAvoidGlobalJNIRefs);
-      clonedYogaNode.mOwner = null;
-      clonedYogaNode.mNativePointer = clonedNativePointer;
-      clonedYogaNode.clearChildren();
-      return clonedYogaNode;
-    } catch (CloneNotSupportedException ex) {
-      // This class implements Cloneable, this should not happen
-      throw new RuntimeException(ex);
-    }
-  }
-
   private static native void jni_YGNodeClearChildren(long nativePointer);
 
   private void clearChildren() {

java/jni/YGJNI.cpp

@@ -226,40 +226,6 @@ static inline YGConfigRef _jlong2YGConfigRef(jlong addr) {
   return reinterpret_cast<YGConfigRef>(static_cast<intptr_t>(addr));
 }
 
-static YGNodeRef YGJNIOnNodeClonedFunc(
-    YGNodeRef oldNode,
-    YGNodeRef owner,
-    int childIndex,
-    void* layoutContext) {
-  auto config = oldNode->getConfig();
-  if (!config) {
-    return nullptr;
-  }
-
-  static auto onNodeClonedFunc =
-      findClassStatic("com/facebook/yoga/YogaConfig")
-          ->getMethod<alias_ref<JYogaNode>(
-              local_ref<JYogaNode>, local_ref<JYogaNode>, jint)>("cloneNode");
-
-  auto context = reinterpret_cast<YGConfigContext*>(YGConfigGetContext(config));
-  auto javaConfig = context->config;
-
-  auto newNode = onNodeClonedFunc(
-      javaConfig->get(),
-      YGNodeJobject(oldNode, layoutContext),
-      YGNodeJobject(owner, layoutContext),
-      childIndex);
-
-  static auto replaceChild =
-      findClassStatic("com/facebook/yoga/YogaNodeJNI")
-          ->getMethod<jlong(local_ref<JYogaNode>, jint)>("replaceChild");
-
-  jlong newNodeNativePointer =
-      replaceChild(YGNodeJobject(owner, layoutContext), newNode, childIndex);
-
-  return _jlong2YGNodeRef(newNodeNativePointer);
-}
-
 static YGSize YGJNIMeasureFunc(
     YGNodeRef node,
     float width,
@@ -356,26 +322,6 @@ jlong jni_YGNodeNewWithConfig(
   return reinterpret_cast<jlong>(node);
 }
 
-void jni_YGNodeSetOwner(jlong nativePointer, jlong newOwnerNativePointer) {
-  const YGNodeRef node = _jlong2YGNodeRef(nativePointer);
-  const YGNodeRef newOwnerNode = _jlong2YGNodeRef(newOwnerNativePointer);
-
-  node->setOwner(newOwnerNode);
-}
-
-jlong jni_YGNodeClone(
-    alias_ref<jobject> thiz,
-    jlong nativePointer,
-    alias_ref<jobject> clonedJavaObject,
-    jboolean avoidGlobalJNIRefs) {
-  const YGNodeRef clonedYogaNode = YGNodeClone(_jlong2YGNodeRef(nativePointer));
-  if (!avoidGlobalJNIRefs) {
-    clonedYogaNode->setContext(
-        new weak_ref<jobject>(make_weak(clonedJavaObject)));
-  }
-  return reinterpret_cast<jlong>(clonedYogaNode);
-}
-
 void jni_YGNodeFree(alias_ref<jclass>, jlong nativePointer) {
   if (nativePointer == 0) {
     return;
@@ -668,29 +614,6 @@ void jni_YGConfigSetUseLegacyStretchBehaviour(
   YGConfigSetUseLegacyStretchBehaviour(config, useLegacyStretchBehaviour);
 }
 
-void jni_YGConfigSetHasCloneNodeFunc(
-    alias_ref<jobject> thiz,
-    jlong nativePointer,
-    jboolean hasCloneNodeFunc) {
-  const YGConfigRef config = _jlong2YGConfigRef(nativePointer);
-  auto context = reinterpret_cast<YGConfigContext*>(YGConfigGetContext(config));
-  if (context && context->config) {
-    delete context->config;
-    context->config = nullptr;
-  }
-
-  if (hasCloneNodeFunc) {
-    if (!context) {
-      context = new YGConfigContext();
-      YGConfigSetContext(config, context);
-    }
-    context->config = new global_ref<jobject>(make_global(thiz));
-    config->setCloneNodeCallback(YGJNIOnNodeClonedFunc);
-  } else {
-    config->setCloneNodeCallback(nullptr);
-  }
-}
-
 void jni_YGConfigSetLogger(
     alias_ref<jobject>,
     jlong nativePointer,
@@ -719,7 +642,8 @@ jint jni_YGNodeGetInstanceCount() {
 }
 
 #define YGMakeNativeMethod(name) makeNativeMethod(#name, name)
-#define YGMakeCriticalNativeMethod(name) makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED(#name, name)
+#define YGMakeCriticalNativeMethod(name) \
+  makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED(#name, name)
 
 jint JNI_OnLoad(JavaVM* vm, void*) {
   return initialize(vm, [] {
@@ -807,8 +731,6 @@ jint JNI_OnLoad(JavaVM* vm, void*) {
             YGMakeCriticalNativeMethod(jni_YGNodeStyleSetAspectRatio),
             YGMakeCriticalNativeMethod(jni_YGNodeGetInstanceCount),
             YGMakeCriticalNativeMethod(jni_YGNodePrint),
-            YGMakeNativeMethod(jni_YGNodeClone),
-            YGMakeCriticalNativeMethod(jni_YGNodeSetOwner),
         });
     registerNatives(
         "com/facebook/yoga/YogaConfig",
@@ -821,7 +743,6 @@ jint JNI_OnLoad(JavaVM* vm, void*) {
             YGMakeNativeMethod(jni_YGConfigSetPointScaleFactor),
             YGMakeNativeMethod(jni_YGConfigSetUseLegacyStretchBehaviour),
             YGMakeNativeMethod(jni_YGConfigSetLogger),
-            YGMakeNativeMethod(jni_YGConfigSetHasCloneNodeFunc),
             YGMakeNativeMethod(
                 jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviour),
         });

java/tests/com/facebook/yoga/YogaNodeTest.java

@@ -233,112 +233,6 @@ public class YogaNodeTest {
     }
   }
 
-  @Test
-  public void testCloneNode() throws Exception {
-    YogaConfig config = new YogaConfig();
-    YogaNode root = createNode(config);
-    YogaNode child = createNode(config);
-    YogaNode grandChild = createNode(config);
-    root.addChildAt(child, 0);
-    child.addChildAt(grandChild, 0);
-    child.setFlexDirection(YogaFlexDirection.ROW);
-
-    YogaNode clonedChild = ((YogaNodeJNI) child).clone();
-
-    assertNotSame(clonedChild, child);
-
-    assertEquals(YogaFlexDirection.ROW, child.getFlexDirection());
-    assertEquals(child.getFlexDirection(), clonedChild.getFlexDirection());
-
-    // Verify the cloning is shallow on the List of children
-    assertEquals(1, child.getChildCount());
-    assertEquals(child.getChildCount(), clonedChild.getChildCount());
-    assertEquals(child.getChildAt(0), clonedChild.getChildAt(0));
-
-    child.removeChildAt(0);
-    assertEquals(0, child.getChildCount());
-    assertEquals(1, clonedChild.getChildCount());
-  }
-
-  @Test
-  public void testCloneWithNewChildren() throws Exception {
-    YogaConfig config = new YogaConfig();
-    YogaNode root = createNode(config);
-    YogaNode child = createNode(config);
-    YogaNode grandChild = createNode(config);
-    root.addChildAt(child, 0);
-    child.addChildAt(grandChild, 0);
-    child.setFlexDirection(YogaFlexDirection.ROW);
-
-    YogaNode clonedChild = ((YogaNodeJNI) child).cloneWithNewChildren();
-
-    assertNotSame(clonedChild, child);
-    assertEquals(YogaFlexDirection.ROW, clonedChild.getFlexDirection());
-    assertEquals(child.getFlexDirection(), clonedChild.getFlexDirection());
-    assertEquals(0, clonedChild.getChildCount());
-    assertEquals(1, child.getChildCount());
-  }
-
-  @Test
-  public void testCloneNodeListener() throws Exception {
-    final AtomicBoolean onNodeClonedExecuted = new AtomicBoolean(false);
-    YogaConfig config = new YogaConfig();
-    config.setOnCloneNode(
-        new YogaNodeCloneFunction() {
-          @Override
-          public YogaNode cloneNode(YogaNode oldNode, YogaNode owner, int childIndex) {
-            onNodeClonedExecuted.set(true);
-            return ((YogaNodeJNI) oldNode).clone();
-          }
-        });
-    YogaNode root = createNode(config);
-    root.setWidth(100f);
-    root.setHeight(100f);
-    YogaNode child0 = createNode(config);
-    root.addChildAt(child0, 0);
-    child0.setWidth(50f);
-    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
-
-    // Force a clone to happen.
-    final YogaNode root2 = ((YogaNodeJNI) root).clone();
-    root2.setWidth(200f);
-    root2.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
-
-    assertTrue(onNodeClonedExecuted.get());
-    assertEquals(1, root2.getChildCount());
-    YogaNode clonedNode = root2.getChildAt(0);
-    assertNotSame(child0, clonedNode);
-    assertEquals(child0.getWidth(), clonedNode.getWidth());
-    assertEquals(50f, clonedNode.getWidth().value, 0.01f);
-  }
-
-  @Test
-  public void testOnNodeClonedLeak() throws Exception {
-    YogaConfig config = new YogaConfig();
-    config.setOnCloneNode(
-        new YogaNodeCloneFunction() {
-          @Override
-          public YogaNode cloneNode(YogaNode oldNode, YogaNode owner, int childIndex) {
-            return ((YogaNodeJNI) oldNode).clone();
-          }
-        });
-    config.setOnCloneNode(null);
-    WeakReference<Object> ref = new WeakReference<Object>(config);
-    // noinspection UnusedAssignment
-    config = null;
-    // try and free for the next 5 seconds, usually it works after the
-    // first GC attempt.
-    for (int i = 0; i < 50; i++) {
-      System.gc();
-      if (ref.get() == null) {
-        // free successfully
-        return;
-      }
-      Thread.sleep(100);
-    }
-    fail("YogaConfig leaked");
-  }
-
   @Test
   public void testFlagShouldDiffLayoutWithoutLegacyStretchBehaviour() throws Exception {
     YogaConfig config = new YogaConfig();