From 94e8ccf9c02318eb407d3ce58e5e14a273967375 Mon Sep 17 00:00:00 2001 From: Uts Sikder Date: Wed, 25 Sep 2019 09:10:51 -0700 Subject: [PATCH] BREAKING: rm YogaNode parameter from YogaLogger#log Summary: In D17439957, I noted that YogaLogger#log throws a NoMethodFoundException when called from C++ b/c C++ and Java's signatures of that method don't match. C++ uses YogaNodeJNIBase for the first param, Java uses YogaNode. Both my attempts to fix this failed. Attempt #1 - Make Java use YogaNodeJNIBase. This doesn't work because the :java-interface target includes YogaLogger but not YogaNodeJNIBase. Moving YogaLogger to the impl target doesn't work either b/c other files in :java-interface reference YogaLogger. Attempt #2 - Make C++ use YogaNode. This doesn't work b/c we try to call the log method with objects of fbjni type YogaNodeJNIBase. This would be fine in Java since YogaNodeJNIBase extends YogaNode. But fbjni's typing isn't advanced enough to know this, so the Yoga C++ fails to compile. At this point, I was wondering what the value of having this param in the log function at all was. None of the implementations in our codebase use it today. It might be easier to just remove it all together. This also removes a bug with YGNodePrint where we pass a null layout context that eventually causes a SIG_ABRT when we use it to try to find a YogaNode to pass to this function. (https://fburl.com/diffusion/ssw9h8lv). Reviewed By: amir-shalem Differential Revision: D17470379 fbshipit-source-id: 8fc2d95505971a52af2399a9fbb60b63f27f0ec2 --- .../src/main/java/com/facebook/yoga/YogaLogger.java | 2 +- .../src/main/jni/first-party/yogajni/jni/YGJNI.cpp | 11 ++++------- .../src/main/jni/first-party/yogajni/jni/YGJTypes.cpp | 8 +++----- .../src/main/jni/first-party/yogajni/jni/YGJTypes.h | 5 +---- 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/yoga/YogaLogger.java b/ReactAndroid/src/main/java/com/facebook/yoga/YogaLogger.java index 2fd4d8217d9e72..d9411061e4cd0a 100644 --- a/ReactAndroid/src/main/java/com/facebook/yoga/YogaLogger.java +++ b/ReactAndroid/src/main/java/com/facebook/yoga/YogaLogger.java @@ -15,5 +15,5 @@ @DoNotStrip public interface YogaLogger { @DoNotStrip - void log(YogaNode node, YogaLogLevel level, String message); + void log(YogaLogLevel level, String message); } diff --git a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp index 6a1a790a2565a3..f23d0f2b95206a 100644 --- a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp +++ b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp @@ -311,13 +311,10 @@ static int YGJNILogFunc( auto jloggerPtr = static_cast*>(YGConfigGetContext(config)); if (jloggerPtr != nullptr) { - if (auto obj = YGNodeJobject(node, layoutContext)) { - (*jloggerPtr) - ->log( - obj, - JYogaLogLevel::fromInt(level), - Environment::current()->NewStringUTF(buffer.data())); - } + (*jloggerPtr) + ->log( + JYogaLogLevel::fromInt(level), + Environment::current()->NewStringUTF(buffer.data())); } return result; diff --git a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.cpp b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.cpp index 032be5c5e499aa..012a2837c841ec 100644 --- a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.cpp +++ b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.cpp @@ -33,12 +33,10 @@ facebook::jni::local_ref JYogaLogLevel::fromInt(jint logLevel) { } void JYogaLogger::log( - facebook::jni::alias_ref node, facebook::jni::alias_ref logLevel, jstring message) { static auto javaMethod = - javaClassLocal() - ->getMethod, alias_ref, jstring)>("log"); - javaMethod(self(), node, logLevel, message); + javaClassLocal()->getMethod, jstring)>( + "log"); + javaMethod(self(), logLevel, message); } diff --git a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.h b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.h index 0c4bc9a84f0e4e..6561ce2d7ca7a4 100644 --- a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.h +++ b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.h @@ -28,10 +28,7 @@ struct JYogaLogLevel : public facebook::jni::JavaClass { struct JYogaLogger : public facebook::jni::JavaClass { static constexpr auto kJavaDescriptor = "Lcom/facebook/yoga/YogaLogger;"; - void log( - facebook::jni::alias_ref, - facebook::jni::alias_ref, - jstring); + void log(facebook::jni::alias_ref, jstring); }; class PtrJNodeMap {