Skip to content

Commit 4902a03

Browse files
Emil SjolanderFacebook Github Bot
authored andcommitted
Fix usage of weak references to check for null
Reviewed By: lexs Differential Revision: D4251133 fbshipit-source-id: 2d8949252b31447ce54bc16a35cb25fabe72230b
1 parent be559d9 commit 4902a03

File tree

1 file changed

+47
-35
lines changed
  • ReactAndroid/src/main/jni/first-party/csslayoutjni/jni

1 file changed

+47
-35
lines changed

ReactAndroid/src/main/jni/first-party/csslayoutjni/jni/CSSJNI.cpp

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,58 +14,70 @@
1414
using namespace facebook::jni;
1515
using namespace std;
1616

17+
static inline weak_ref<jobject> *jobjectContext(CSSNodeRef node) {
18+
return reinterpret_cast<weak_ref<jobject> *>(CSSNodeGetContext(node));
19+
}
20+
1721
static void _jniTransferLayoutDirection(CSSNodeRef node, alias_ref<jobject> javaNode) {
1822
static auto layoutDirectionField = javaNode->getClass()->getField<jint>("mLayoutDirection");
1923
javaNode->setFieldValue(layoutDirectionField, static_cast<jint>(CSSNodeLayoutGetDirection(node)));
2024
}
2125

2226
static void _jniTransferLayoutOutputsRecursive(CSSNodeRef root) {
23-
auto javaNode = adopt_local(
24-
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(root))));
25-
26-
static auto widthField = javaNode->getClass()->getField<jfloat>("mWidth");
27-
static auto heightField = javaNode->getClass()->getField<jfloat>("mHeight");
28-
static auto leftField = javaNode->getClass()->getField<jfloat>("mLeft");
29-
static auto topField = javaNode->getClass()->getField<jfloat>("mTop");
30-
31-
javaNode->setFieldValue(widthField, CSSNodeLayoutGetWidth(root));
32-
javaNode->setFieldValue(heightField, CSSNodeLayoutGetHeight(root));
33-
javaNode->setFieldValue(leftField, CSSNodeLayoutGetLeft(root));
34-
javaNode->setFieldValue(topField, CSSNodeLayoutGetTop(root));
35-
_jniTransferLayoutDirection(root, javaNode);
36-
37-
for (uint32_t i = 0; i < CSSNodeChildCount(root); i++) {
38-
_jniTransferLayoutOutputsRecursive(CSSNodeGetChild(root, i));
27+
if (auto obj = jobjectContext(root)->lockLocal()) {
28+
static auto widthField = obj->getClass()->getField<jfloat>("mWidth");
29+
static auto heightField = obj->getClass()->getField<jfloat>("mHeight");
30+
static auto leftField = obj->getClass()->getField<jfloat>("mLeft");
31+
static auto topField = obj->getClass()->getField<jfloat>("mTop");
32+
33+
obj->setFieldValue(widthField, CSSNodeLayoutGetWidth(root));
34+
obj->setFieldValue(heightField, CSSNodeLayoutGetHeight(root));
35+
obj->setFieldValue(leftField, CSSNodeLayoutGetLeft(root));
36+
obj->setFieldValue(topField, CSSNodeLayoutGetTop(root));
37+
_jniTransferLayoutDirection(root, obj);
38+
39+
for (uint32_t i = 0; i < CSSNodeChildCount(root); i++) {
40+
_jniTransferLayoutOutputsRecursive(CSSNodeGetChild(root, i));
41+
}
42+
} else {
43+
CSSLog(CSSLogLevelError, "Java CSSNode was GCed during layout calculation\n");
3944
}
4045
}
4146

4247
static void _jniPrint(CSSNodeRef node) {
43-
auto obj = adopt_local(
44-
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
45-
cout << obj->toString() << endl;
48+
if (auto obj = jobjectContext(node)->lockLocal()) {
49+
cout << obj->toString() << endl;
50+
} else {
51+
CSSLog(CSSLogLevelError, "Java CSSNode was GCed during layout calculation\n");
52+
}
4653
}
4754

4855
static CSSSize _jniMeasureFunc(CSSNodeRef node,
4956
float width,
5057
CSSMeasureMode widthMode,
5158
float height,
5259
CSSMeasureMode heightMode) {
53-
auto obj = adopt_local(
54-
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
60+
if (auto obj = jobjectContext(node)->lockLocal()) {
61+
static auto measureFunc = findClassLocal("com/facebook/csslayout/CSSNode")
62+
->getMethod<jlong(jfloat, jint, jfloat, jint)>("measure");
5563

56-
static auto measureFunc = findClassLocal("com/facebook/csslayout/CSSNode")
57-
->getMethod<jlong(jfloat, jint, jfloat, jint)>("measure");
64+
_jniTransferLayoutDirection(node, obj);
65+
const auto measureResult = measureFunc(obj, width, widthMode, height, heightMode);
5866

59-
_jniTransferLayoutDirection(node, obj);
60-
const auto measureResult = measureFunc(obj, width, widthMode, height, heightMode);
67+
static_assert(sizeof(measureResult) == 8,
68+
"Expected measureResult to be 8 bytes, or two 32 bit ints");
6169

62-
static_assert(sizeof(measureResult) == 8,
63-
"Expected measureResult to be 8 bytes, or two 32 bit ints");
70+
const float measuredWidth = static_cast<float>(0xFFFFFFFF & (measureResult >> 32));
71+
const float measuredHeight = static_cast<float>(0xFFFFFFFF & measureResult);
6472

65-
const float measuredWidth = static_cast<float>(0xFFFFFFFF & (measureResult >> 32));
66-
const float measuredHeight = static_cast<float>(0xFFFFFFFF & measureResult);
67-
68-
return CSSSize{measuredWidth, measuredHeight};
73+
return CSSSize{measuredWidth, measuredHeight};
74+
} else {
75+
CSSLog(CSSLogLevelError, "Java CSSNode was GCed during layout calculation\n");
76+
return CSSSize{
77+
widthMode == CSSMeasureModeUndefined ? 0 : width,
78+
heightMode == CSSMeasureModeUndefined ? 0 : height,
79+
};
80+
}
6981
}
7082

7183
struct JCSSLogLevel : public JavaClass<JCSSLogLevel> {
@@ -131,15 +143,15 @@ jint jni_CSSNodeGetInstanceCount(alias_ref<jclass> clazz) {
131143

132144
jlong jni_CSSNodeNew(alias_ref<jobject> thiz) {
133145
const CSSNodeRef node = CSSNodeNew();
134-
CSSNodeSetContext(node, Environment::current()->NewWeakGlobalRef(thiz.get()));
146+
CSSNodeSetContext(node, new weak_ref<jobject>(make_weak(thiz)));
135147
CSSNodeSetPrintFunc(node, _jniPrint);
136148
return reinterpret_cast<jlong>(node);
137149
}
138150

139151
void jni_CSSNodeFree(alias_ref<jobject> thiz, jlong nativePointer) {
140-
Environment::current()->DeleteWeakGlobalRef(
141-
reinterpret_cast<jweak>(CSSNodeGetContext(_jlong2CSSNodeRef(nativePointer))));
142-
CSSNodeFree(_jlong2CSSNodeRef(nativePointer));
152+
const CSSNodeRef node = _jlong2CSSNodeRef(nativePointer);
153+
delete jobjectContext(node);
154+
CSSNodeFree(node);
143155
}
144156

145157
void jni_CSSNodeReset(alias_ref<jobject> thiz, jlong nativePointer) {

0 commit comments

Comments
 (0)