From f00116c3a67dfe5eb6fc7f9b0e34a4aab65085e3 Mon Sep 17 00:00:00 2001
From: Uts Sikder <uts@fb.com>
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
---
 java/com/facebook/yoga/YogaLogger.java           |  2 +-
 java/jni/YGJNI.cpp                               | 11 ++++-------
 java/jni/YGJTypes.cpp                            |  8 +++-----
 java/jni/YGJTypes.h                              |  5 +----
 java/tests/com/facebook/yoga/YogaLoggerTest.java |  2 +-
 5 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/java/com/facebook/yoga/YogaLogger.java b/java/com/facebook/yoga/YogaLogger.java
index 2fd4d821..d9411061 100644
--- a/java/com/facebook/yoga/YogaLogger.java
+++ b/java/com/facebook/yoga/YogaLogger.java
@@ -15,5 +15,5 @@ import com.facebook.proguard.annotations.DoNotStrip;
 @DoNotStrip
 public interface YogaLogger {
   @DoNotStrip
-  void log(YogaNode node, YogaLogLevel level, String message);
+  void log(YogaLogLevel level, String message);
 }
diff --git a/java/jni/YGJNI.cpp b/java/jni/YGJNI.cpp
index 6a1a790a..f23d0f2b 100644
--- a/java/jni/YGJNI.cpp
+++ b/java/jni/YGJNI.cpp
@@ -311,13 +311,10 @@ static int YGJNILogFunc(
   auto jloggerPtr =
       static_cast<global_ref<JYogaLogger>*>(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/java/jni/YGJTypes.cpp b/java/jni/YGJTypes.cpp
index 032be5c5..012a2837 100644
--- a/java/jni/YGJTypes.cpp
+++ b/java/jni/YGJTypes.cpp
@@ -33,12 +33,10 @@ facebook::jni::local_ref<JYogaLogLevel> JYogaLogLevel::fromInt(jint logLevel) {
 }
 
 void JYogaLogger::log(
-    facebook::jni::alias_ref<JYogaNode> node,
     facebook::jni::alias_ref<JYogaLogLevel> logLevel,
     jstring message) {
   static auto javaMethod =
-      javaClassLocal()
-          ->getMethod<void(
-              alias_ref<JYogaNode>, alias_ref<JYogaLogLevel>, jstring)>("log");
-  javaMethod(self(), node, logLevel, message);
+      javaClassLocal()->getMethod<void(alias_ref<JYogaLogLevel>, jstring)>(
+          "log");
+  javaMethod(self(), logLevel, message);
 }
diff --git a/java/jni/YGJTypes.h b/java/jni/YGJTypes.h
index 0c4bc9a8..6561ce2d 100644
--- a/java/jni/YGJTypes.h
+++ b/java/jni/YGJTypes.h
@@ -28,10 +28,7 @@ struct JYogaLogLevel : public facebook::jni::JavaClass<JYogaLogLevel> {
 struct JYogaLogger : public facebook::jni::JavaClass<JYogaLogger> {
   static constexpr auto kJavaDescriptor = "Lcom/facebook/yoga/YogaLogger;";
 
-  void log(
-      facebook::jni::alias_ref<JYogaNode>,
-      facebook::jni::alias_ref<JYogaLogLevel>,
-      jstring);
+  void log(facebook::jni::alias_ref<JYogaLogLevel>, jstring);
 };
 
 class PtrJNodeMap {
diff --git a/java/tests/com/facebook/yoga/YogaLoggerTest.java b/java/tests/com/facebook/yoga/YogaLoggerTest.java
index b111f872..1f56a923 100644
--- a/java/tests/com/facebook/yoga/YogaLoggerTest.java
+++ b/java/tests/com/facebook/yoga/YogaLoggerTest.java
@@ -17,7 +17,7 @@ public class YogaLoggerTest {
     final YogaConfig config = YogaConfigFactory.create();
     YogaLogger logger = new YogaLogger() {
       @Override
-      public void log(YogaNode yogaNode, YogaLogLevel level, String message) {
+      public void log(YogaLogLevel level, String message) {
       }
     };
     config.setLogger(logger);