clean up config context

Summary: `YGConfigContext` was using `global_ref` instances for the config, leading to the config object never being freed. Since we no longer need it after getting rid of cloning, we can make the context a pointer to a `global_ref` to the logger.

Reviewed By: SidharthGuglani

Differential Revision: D14258571

fbshipit-source-id: cce632499839a680eef00a3854f61ab74ae2a87a
This commit is contained in:
David Aurelio
2019-03-01 05:42:10 -08:00
committed by Facebook Github Bot
parent a935a222b5
commit 6368416178

View File

@@ -94,18 +94,6 @@ public:
} }
}; };
struct YGConfigContext {
global_ref<jobject>* logger;
global_ref<jobject>* config;
YGConfigContext() : logger(nullptr), config(nullptr) {}
~YGConfigContext() {
delete config;
config = nullptr;
delete logger;
logger = nullptr;
}
};
struct YGNodeContext { struct YGNodeContext {
weak_ref<jobject>* ygNodeJObjectRef{nullptr}; weak_ref<jobject>* ygNodeJObjectRef{nullptr};
int edgeSetFlag = 0; int edgeSetFlag = 0;
@@ -234,6 +222,7 @@ static void YGTransferLayoutOutputsRecursive(
obj->setFieldValue( obj->setFieldValue(
borderBottomField, YGNodeLayoutGetBorder(root, YGEdgeBottom)); borderBottomField, YGNodeLayoutGetBorder(root, YGEdgeBottom));
} }
obj->setFieldValue<jboolean>(hasNewLayoutField, true); obj->setFieldValue<jboolean>(hasNewLayoutField, true);
YGTransferLayoutDirection(root, obj); YGTransferLayoutDirection(root, obj);
root->setHasNewLayout(false); root->setHasNewLayout(false);
@@ -342,16 +331,18 @@ static int YGJNILogFunc(
JYogaLogLevel::javaClassStatic() JYogaLogLevel::javaClassStatic()
->getStaticMethod<JYogaLogLevel::javaobject(jint)>("fromInt"); ->getStaticMethod<JYogaLogLevel::javaobject(jint)>("fromInt");
auto jloggerPtr =
static_cast<global_ref<jobject>*>(YGConfigGetContext(config));
if (jloggerPtr != nullptr) {
if (auto obj = YGNodeJobject(node, layoutContext)) { if (auto obj = YGNodeJobject(node, layoutContext)) {
auto jlogger =
reinterpret_cast<global_ref<jobject>*>(YGConfigGetContext(config));
logFunc( logFunc(
jlogger->get(), *jloggerPtr,
obj, obj,
logLevelFromInt( logLevelFromInt(
JYogaLogLevel::javaClassStatic(), static_cast<jint>(level)), JYogaLogLevel::javaClassStatic(), static_cast<jint>(level)),
Environment::current()->NewStringUTF(buffer.data())); Environment::current()->NewStringUTF(buffer.data()));
} }
}
return result; return result;
} }
@@ -613,10 +604,9 @@ jlong jni_YGConfigNew(alias_ref<jobject>) {
void jni_YGConfigFree(alias_ref<jobject>, jlong nativePointer) { void jni_YGConfigFree(alias_ref<jobject>, jlong nativePointer) {
const YGConfigRef config = _jlong2YGConfigRef(nativePointer); const YGConfigRef config = _jlong2YGConfigRef(nativePointer);
auto context = reinterpret_cast<YGConfigContext*>(YGConfigGetContext(config)); // unique_ptr will destruct the underlying global_ref, if present.
if (context) { auto context = std::unique_ptr<global_ref<jobject>>{
delete context; static_cast<global_ref<jobject>*>(YGConfigGetContext(config))};
}
YGConfigFree(config); YGConfigFree(config);
} }
@@ -675,20 +665,21 @@ void jni_YGConfigSetLogger(
jlong nativePointer, jlong nativePointer,
alias_ref<jobject> logger) { alias_ref<jobject> logger) {
const YGConfigRef config = _jlong2YGConfigRef(nativePointer); const YGConfigRef config = _jlong2YGConfigRef(nativePointer);
auto context = reinterpret_cast<YGConfigContext*>(YGConfigGetContext(config)); auto context =
if (context && context->logger) { reinterpret_cast<global_ref<jobject>*>(YGConfigGetContext(config));
delete context->logger;
context->logger = nullptr;
}
if (logger) { if (logger) {
if (!context) { if (context == nullptr) {
context = new YGConfigContext(); context = new global_ref<jobject>{};
YGConfigSetContext(config, context); YGConfigSetContext(config, context);
} }
context->logger = new global_ref<jobject>(make_global(logger)); *context = make_global(logger);
config->setLogger(YGJNILogFunc); config->setLogger(YGJNILogFunc);
} else { } else {
if (context != nullptr) {
delete context;
YGConfigSetContext(config, nullptr);
}
config->setLogger(nullptr); config->setLogger(nullptr);
} }
} }