From 10a076f6e220488ac3fcd7aea4b1333a5aad8d27 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Thu, 24 Aug 2023 12:48:56 -0700 Subject: [PATCH] Ship the fix for local reference overflow in Yoga (#39132) Summary: X-link: https://github.com/facebook/litho/pull/954 Pull Request resolved: https://github.com/facebook/react-native/pull/39132 X-link: https://github.com/facebook/yoga/pull/1347 # Context Reviewed By: NickGerleman, astreet Differential Revision: D48607502 fbshipit-source-id: 79552bc76879d1fc15341423ae6fbadeab2fb7af --- .../yoga/YogaExperimentalFeature.java | 4 +- .../first-party/yogajni/jni/YGJNIVanilla.cpp | 37 ++++++++----------- .../ReactCommon/yoga/yoga/YGEnums.cpp | 2 - .../ReactCommon/yoga/yoga/YGEnums.h | 3 +- 4 files changed, 17 insertions(+), 29 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java index daa87bf0f83..a9e621ef5a7 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java @@ -11,8 +11,7 @@ package com.facebook.yoga; public enum YogaExperimentalFeature { WEB_FLEX_BASIS(0), - ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE(1), - FIX_JNILOCAL_REF_OVERFLOWS(2); + ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE(1); private final int mIntValue; @@ -28,7 +27,6 @@ public enum YogaExperimentalFeature { switch (value) { case 0: return WEB_FLEX_BASIS; case 1: return ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE; - case 2: return FIX_JNILOCAL_REF_OVERFLOWS; default: throw new IllegalArgumentException("Unknown enum value: " + value); } } diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp index 26511ca0787..bc108b5dafd 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp @@ -282,8 +282,7 @@ static void YGTransferLayoutOutputsRecursive( JNIEnv* env, jobject thiz, YGNodeRef root, - void* layoutContext, - bool shouldCleanLocalRef) { + void* layoutContext) { if (!YGNodeGetHasNewLayout(root)) { return; } @@ -337,28 +336,26 @@ static void YGTransferLayoutOutputsRecursive( arr[borderStartIndex + 3] = YGNodeLayoutGetBorder(root, YGEdgeBottom); } - // Don't change this field name without changing the name of the field in - // Database.java - auto objectClass = facebook::yoga::vanillajni::make_local_ref( - env, env->GetObjectClass(obj.get())); - static const jfieldID arrField = facebook::yoga::vanillajni::getFieldId( - env, objectClass.get(), "arr", "[F"); + // Create scope to make sure to release any local refs created here + { + // Don't change this field name without changing the name of the field in + // Database.java + auto objectClass = facebook::yoga::vanillajni::make_local_ref( + env, env->GetObjectClass(obj.get())); + static const jfieldID arrField = facebook::yoga::vanillajni::getFieldId( + env, objectClass.get(), "arr", "[F"); - ScopedLocalRef arrFinal = - make_local_ref(env, env->NewFloatArray(arrSize)); - env->SetFloatArrayRegion(arrFinal.get(), 0, arrSize, arr); - env->SetObjectField(obj.get(), arrField, arrFinal.get()); - - if (shouldCleanLocalRef) { - objectClass.reset(); - arrFinal.reset(); + ScopedLocalRef arrFinal = + make_local_ref(env, env->NewFloatArray(arrSize)); + env->SetFloatArrayRegion(arrFinal.get(), 0, arrSize, arr); + env->SetObjectField(obj.get(), arrField, arrFinal.get()); } YGNodeSetHasNewLayout(root, false); for (uint32_t i = 0; i < YGNodeGetChildCount(root); i++) { YGTransferLayoutOutputsRecursive( - env, thiz, YGNodeGetChild(root, i), layoutContext, shouldCleanLocalRef); + env, thiz, YGNodeGetChild(root, i), layoutContext); } } @@ -380,17 +377,13 @@ static void jni_YGNodeCalculateLayoutJNI( } const YGNodeRef root = _jlong2YGNodeRef(nativePointer); - const bool shouldCleanLocalRef = - root->getConfig()->isExperimentalFeatureEnabled( - YGExperimentalFeatureFixJNILocalRefOverflows); YGNodeCalculateLayoutWithContext( root, static_cast(width), static_cast(height), YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer)), layoutContext); - YGTransferLayoutOutputsRecursive( - env, obj, root, layoutContext, shouldCleanLocalRef); + YGTransferLayoutOutputsRecursive(env, obj, root, layoutContext); } catch (const YogaJniException& jniException) { ScopedLocalRef throwable = jniException.getThrowable(); if (throwable.get()) { diff --git a/packages/react-native/ReactCommon/yoga/yoga/YGEnums.cpp b/packages/react-native/ReactCommon/yoga/yoga/YGEnums.cpp index e8ace4b38ef..f7220eff720 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/YGEnums.cpp +++ b/packages/react-native/ReactCommon/yoga/yoga/YGEnums.cpp @@ -107,8 +107,6 @@ const char* YGExperimentalFeatureToString(const YGExperimentalFeature value) { return "web-flex-basis"; case YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge: return "absolute-percentage-against-padding-edge"; - case YGExperimentalFeatureFixJNILocalRefOverflows: - return "fix-jnilocal-ref-overflows"; } return "unknown"; } diff --git a/packages/react-native/ReactCommon/yoga/yoga/YGEnums.h b/packages/react-native/ReactCommon/yoga/yoga/YGEnums.h index a502d39b161..7abe5d92ecb 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/YGEnums.h +++ b/packages/react-native/ReactCommon/yoga/yoga/YGEnums.h @@ -65,8 +65,7 @@ YG_DEFINE_ENUM_FLAG_OPERATORS(YGErrata) YG_ENUM_SEQ_DECL( YGExperimentalFeature, YGExperimentalFeatureWebFlexBasis, - YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge, - YGExperimentalFeatureFixJNILocalRefOverflows) + YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge) YG_ENUM_SEQ_DECL( YGFlexDirection,