From b1222bf83e23042cb488ba6858682aeb598f3a05 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Wed, 14 Feb 2018 09:31:10 -0800 Subject: [PATCH] Expose methods of persistent yoga for Java Summary: This diff extends the JNI version of yoga in order to allow Java instances of the YogaConfig class to receive a callback when a Yoga node is cloned. Reviewed By: priteshrnandgaonkar Differential Revision: D6918605 fbshipit-source-id: e424c78680c04e21154ebe21405671c4e90f6529 --- java/com/facebook/yoga/YogaConfig.java | 14 +++ .../facebook/yoga/YogaNodeClonedFunction.java | 19 ++++ java/jni/YGJNI.cpp | 100 ++++++++++++++++-- .../tests/com/facebook/yoga/YogaNodeTest.java | 33 +++++- 4 files changed, 153 insertions(+), 13 deletions(-) create mode 100644 java/com/facebook/yoga/YogaNodeClonedFunction.java diff --git a/java/com/facebook/yoga/YogaConfig.java b/java/com/facebook/yoga/YogaConfig.java index dfb559f2..5e82fe22 100644 --- a/java/com/facebook/yoga/YogaConfig.java +++ b/java/com/facebook/yoga/YogaConfig.java @@ -23,6 +23,7 @@ public class YogaConfig { long mNativePointer; private YogaLogger mLogger; + private YogaNodeClonedFunction mNodeClonedFunction; private native long jni_YGConfigNew(); public YogaConfig() { @@ -80,4 +81,17 @@ public class YogaConfig { public YogaLogger getLogger() { return mLogger; } + + private native void jni_YGConfigSetHasNodeClonedFunc(long nativePointer, boolean hasClonedFunc); + + public void setOnNodeCloned(YogaNodeClonedFunction nodeClonedFunction) { + mNodeClonedFunction = nodeClonedFunction; + jni_YGConfigSetHasNodeClonedFunc(mNativePointer, nodeClonedFunction != null); + } + + @DoNotStrip + public final void onNodeCloned( + YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex) { + mNodeClonedFunction.onNodeCloned(oldNode, newNode, parent, childIndex); + } } diff --git a/java/com/facebook/yoga/YogaNodeClonedFunction.java b/java/com/facebook/yoga/YogaNodeClonedFunction.java new file mode 100644 index 00000000..8c27d16b --- /dev/null +++ b/java/com/facebook/yoga/YogaNodeClonedFunction.java @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2017-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package com.facebook.yoga; + +import com.facebook.proguard.annotations.DoNotStrip; + +@DoNotStrip +public interface YogaNodeClonedFunction { + + @DoNotStrip + void onNodeCloned(YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex); +} diff --git a/java/jni/YGJNI.cpp b/java/jni/YGJNI.cpp index a54c062f..67042729 100644 --- a/java/jni/YGJNI.cpp +++ b/java/jni/YGJNI.cpp @@ -19,6 +19,22 @@ struct JYogaNode : public JavaClass { static constexpr auto kJavaDescriptor = "Lcom/facebook/yoga/YogaNode;"; }; +struct JYogaConfig : public JavaClass { + static constexpr auto kJavaDescriptor = "Lcom/facebook/yoga/YogaConfig;"; +}; + +struct YGConfigContext { + global_ref* logger; + global_ref* config; + YGConfigContext() : logger(nullptr), config(nullptr) {} + ~YGConfigContext() { + delete config; + config = nullptr; + delete logger; + logger = nullptr; + } +}; + static inline weak_ref *YGNodeJobject(YGNodeRef node) { return reinterpret_cast*>(node->getContext()); } @@ -118,11 +134,39 @@ static float YGJNIBaselineFunc(YGNodeRef node, float width, float height) { } } -static YGSize YGJNIMeasureFunc(YGNodeRef node, - float width, - YGMeasureMode widthMode, - float height, - YGMeasureMode heightMode) { +static void YGJNIOnNodeClonedFunc( + YGNodeRef oldNode, + YGNodeRef newNode, + YGNodeRef parent, + int childIndex) { + auto config = oldNode->getConfig(); + if (!config) { + return; + } + static auto onNodeClonedFunc = findClassStatic("com/facebook/yoga/YogaConfig") + ->getMethod, + local_ref, + local_ref, + jint)>("onNodeCloned"); + + auto context = reinterpret_cast(YGConfigGetContext(config)); + auto javaConfig = context->config; + + onNodeClonedFunc( + javaConfig->get(), + YGNodeJobject(oldNode)->lockLocal(), + YGNodeJobject(newNode)->lockLocal(), + YGNodeJobject(parent)->lockLocal(), + childIndex); +} + +static YGSize YGJNIMeasureFunc( + YGNodeRef node, + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { if (auto obj = YGNodeJobject(node)->lockLocal()) { static auto measureFunc = findClassStatic("com/facebook/yoga/YogaNode") ->getMethod("measure"); @@ -396,6 +440,10 @@ jlong jni_YGConfigNew(alias_ref) { void jni_YGConfigFree(alias_ref, jlong nativePointer) { const YGConfigRef config = _jlong2YGConfigRef(nativePointer); + auto context = reinterpret_cast(YGConfigGetContext(config)); + if (context) { + delete context; + } YGConfigFree(config); } @@ -430,19 +478,48 @@ void jni_YGConfigSetUseLegacyStretchBehaviour(alias_ref, YGConfigSetUseLegacyStretchBehaviour(config, useLegacyStretchBehaviour); } -void jni_YGConfigSetLogger(alias_ref, jlong nativePointer, alias_ref logger) { +void jni_YGConfigSetHasNodeClonedFunc( + alias_ref thiz, + jlong nativePointer, + jboolean hasNodeClonedFunc) { const YGConfigRef config = _jlong2YGConfigRef(nativePointer); + auto context = reinterpret_cast(YGConfigGetContext(config)); + if (context && context->config) { + delete context->config; + context->config = nullptr; + } - auto context = YGConfigGetContext(config); - if (context) { - delete reinterpret_cast *>(context); + if (hasNodeClonedFunc) { + if (!context) { + context = new YGConfigContext(); + YGConfigSetContext(config, context); + } + context->config = new global_ref(make_global(thiz)); + YGConfigSetNodeClonedFunc(config, YGJNIOnNodeClonedFunc); + } else { + YGConfigSetNodeClonedFunc(config, nullptr); + } +} + +void jni_YGConfigSetLogger( + alias_ref, + jlong nativePointer, + alias_ref logger) { + const YGConfigRef config = _jlong2YGConfigRef(nativePointer); + auto context = reinterpret_cast(YGConfigGetContext(config)); + if (context && context->logger) { + delete context->logger; + context->logger = nullptr; } if (logger) { - YGConfigSetContext(config, new global_ref(make_global(logger))); + if (!context) { + context = new YGConfigContext(); + YGConfigSetContext(config, context); + } + context->logger = new global_ref(make_global(logger)); YGConfigSetLogger(config, YGJNILogFunc); } else { - YGConfigSetContext(config, NULL); YGConfigSetLogger(config, NULL); } } @@ -545,6 +622,7 @@ jint JNI_OnLoad(JavaVM *vm, void *) { YGMakeNativeMethod(jni_YGConfigSetPointScaleFactor), YGMakeNativeMethod(jni_YGConfigSetUseLegacyStretchBehaviour), YGMakeNativeMethod(jni_YGConfigSetLogger), + YGMakeNativeMethod(jni_YGConfigSetHasNodeClonedFunc), }); }); } diff --git a/java/tests/com/facebook/yoga/YogaNodeTest.java b/java/tests/com/facebook/yoga/YogaNodeTest.java index 0e0d37df..c655f081 100644 --- a/java/tests/com/facebook/yoga/YogaNodeTest.java +++ b/java/tests/com/facebook/yoga/YogaNodeTest.java @@ -9,10 +9,11 @@ package com.facebook.yoga; -import org.junit.Test; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import org.junit.Test; public class YogaNodeTest { @@ -218,4 +219,32 @@ public class YogaNodeTest { assertTrue(YogaConstants.isUndefined(node.getBorder(edge))); } } + + @Test + public void testNodeClonedLeak() throws Exception { + YogaConfig config = new YogaConfig(); + config.setOnNodeCloned( + new YogaNodeClonedFunction() { + @Override + public void onNodeCloned( + YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex) { + // Do nothing + } + }); + config.setOnNodeCloned(null); + java.lang.ref.WeakReference ref = new java.lang.ref.WeakReference(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"); + } }