Ship the fix for local reference overflow in Yoga (#1347)

Summary:
X-link: https://github.com/facebook/litho/pull/954

X-link: https://github.com/facebook/react-native/pull/39132

Pull Request resolved: https://github.com/facebook/yoga/pull/1347

# Context

Reviewed By: NickGerleman, astreet

Differential Revision: D48607502

fbshipit-source-id: 79552bc76879d1fc15341423ae6fbadeab2fb7af
This commit is contained in:
Andrew Wang
2023-08-24 12:48:56 -07:00
committed by Facebook GitHub Bot
parent 38ad93c87b
commit 49fbd406b6
6 changed files with 17 additions and 33 deletions

View File

@@ -52,8 +52,6 @@ ENUMS = {
"WebFlexBasis", "WebFlexBasis",
# Conformance fix: https://github.com/facebook/yoga/pull/1028 # Conformance fix: https://github.com/facebook/yoga/pull/1028
"AbsolutePercentageAgainstPaddingEdge", "AbsolutePercentageAgainstPaddingEdge",
# fix JNI local ref overflows
"FixJNILocalRefOverflows",
], ],
"PrintOptions": [ "PrintOptions": [
("Layout", 1 << 0), ("Layout", 1 << 0),

View File

@@ -11,8 +11,7 @@ package com.facebook.yoga;
public enum YogaExperimentalFeature { public enum YogaExperimentalFeature {
WEB_FLEX_BASIS(0), WEB_FLEX_BASIS(0),
ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE(1), ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE(1);
FIX_JNILOCAL_REF_OVERFLOWS(2);
private final int mIntValue; private final int mIntValue;
@@ -28,7 +27,6 @@ public enum YogaExperimentalFeature {
switch (value) { switch (value) {
case 0: return WEB_FLEX_BASIS; case 0: return WEB_FLEX_BASIS;
case 1: return ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE; case 1: return ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE;
case 2: return FIX_JNILOCAL_REF_OVERFLOWS;
default: throw new IllegalArgumentException("Unknown enum value: " + value); default: throw new IllegalArgumentException("Unknown enum value: " + value);
} }
} }

View File

@@ -282,8 +282,7 @@ static void YGTransferLayoutOutputsRecursive(
JNIEnv* env, JNIEnv* env,
jobject thiz, jobject thiz,
YGNodeRef root, YGNodeRef root,
void* layoutContext, void* layoutContext) {
bool shouldCleanLocalRef) {
if (!YGNodeGetHasNewLayout(root)) { if (!YGNodeGetHasNewLayout(root)) {
return; return;
} }
@@ -337,6 +336,8 @@ static void YGTransferLayoutOutputsRecursive(
arr[borderStartIndex + 3] = YGNodeLayoutGetBorder(root, YGEdgeBottom); arr[borderStartIndex + 3] = YGNodeLayoutGetBorder(root, YGEdgeBottom);
} }
// 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 // Don't change this field name without changing the name of the field in
// Database.java // Database.java
auto objectClass = facebook::yoga::vanillajni::make_local_ref( auto objectClass = facebook::yoga::vanillajni::make_local_ref(
@@ -348,17 +349,13 @@ static void YGTransferLayoutOutputsRecursive(
make_local_ref(env, env->NewFloatArray(arrSize)); make_local_ref(env, env->NewFloatArray(arrSize));
env->SetFloatArrayRegion(arrFinal.get(), 0, arrSize, arr); env->SetFloatArrayRegion(arrFinal.get(), 0, arrSize, arr);
env->SetObjectField(obj.get(), arrField, arrFinal.get()); env->SetObjectField(obj.get(), arrField, arrFinal.get());
if (shouldCleanLocalRef) {
objectClass.reset();
arrFinal.reset();
} }
YGNodeSetHasNewLayout(root, false); YGNodeSetHasNewLayout(root, false);
for (uint32_t i = 0; i < YGNodeGetChildCount(root); i++) { for (uint32_t i = 0; i < YGNodeGetChildCount(root); i++) {
YGTransferLayoutOutputsRecursive( 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 YGNodeRef root = _jlong2YGNodeRef(nativePointer);
const bool shouldCleanLocalRef =
root->getConfig()->isExperimentalFeatureEnabled(
YGExperimentalFeatureFixJNILocalRefOverflows);
YGNodeCalculateLayoutWithContext( YGNodeCalculateLayoutWithContext(
root, root,
static_cast<float>(width), static_cast<float>(width),
static_cast<float>(height), static_cast<float>(height),
YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer)), YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer)),
layoutContext); layoutContext);
YGTransferLayoutOutputsRecursive( YGTransferLayoutOutputsRecursive(env, obj, root, layoutContext);
env, obj, root, layoutContext, shouldCleanLocalRef);
} catch (const YogaJniException& jniException) { } catch (const YogaJniException& jniException) {
ScopedLocalRef<jthrowable> throwable = jniException.getThrowable(); ScopedLocalRef<jthrowable> throwable = jniException.getThrowable();
if (throwable.get()) { if (throwable.get()) {

View File

@@ -56,7 +56,6 @@ export enum Errata {
export enum ExperimentalFeature { export enum ExperimentalFeature {
WebFlexBasis = 0, WebFlexBasis = 0,
AbsolutePercentageAgainstPaddingEdge = 1, AbsolutePercentageAgainstPaddingEdge = 1,
FixJNILocalRefOverflows = 2,
} }
export enum FlexDirection { export enum FlexDirection {
@@ -163,7 +162,6 @@ const constants = {
ERRATA_CLASSIC: Errata.Classic, ERRATA_CLASSIC: Errata.Classic,
EXPERIMENTAL_FEATURE_WEB_FLEX_BASIS: ExperimentalFeature.WebFlexBasis, EXPERIMENTAL_FEATURE_WEB_FLEX_BASIS: ExperimentalFeature.WebFlexBasis,
EXPERIMENTAL_FEATURE_ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE: ExperimentalFeature.AbsolutePercentageAgainstPaddingEdge, EXPERIMENTAL_FEATURE_ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE: ExperimentalFeature.AbsolutePercentageAgainstPaddingEdge,
EXPERIMENTAL_FEATURE_FIX_JNILOCAL_REF_OVERFLOWS: ExperimentalFeature.FixJNILocalRefOverflows,
FLEX_DIRECTION_COLUMN: FlexDirection.Column, FLEX_DIRECTION_COLUMN: FlexDirection.Column,
FLEX_DIRECTION_COLUMN_REVERSE: FlexDirection.ColumnReverse, FLEX_DIRECTION_COLUMN_REVERSE: FlexDirection.ColumnReverse,
FLEX_DIRECTION_ROW: FlexDirection.Row, FLEX_DIRECTION_ROW: FlexDirection.Row,

View File

@@ -107,8 +107,6 @@ const char* YGExperimentalFeatureToString(const YGExperimentalFeature value) {
return "web-flex-basis"; return "web-flex-basis";
case YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge: case YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge:
return "absolute-percentage-against-padding-edge"; return "absolute-percentage-against-padding-edge";
case YGExperimentalFeatureFixJNILocalRefOverflows:
return "fix-jnilocal-ref-overflows";
} }
return "unknown"; return "unknown";
} }

View File

@@ -65,8 +65,7 @@ YG_DEFINE_ENUM_FLAG_OPERATORS(YGErrata)
YG_ENUM_SEQ_DECL( YG_ENUM_SEQ_DECL(
YGExperimentalFeature, YGExperimentalFeature,
YGExperimentalFeatureWebFlexBasis, YGExperimentalFeatureWebFlexBasis,
YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge, YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge)
YGExperimentalFeatureFixJNILocalRefOverflows)
YG_ENUM_SEQ_DECL( YG_ENUM_SEQ_DECL(
YGFlexDirection, YGFlexDirection,