From a93e5d63c9d26e378c07f3adfd86e16a4ca88143 Mon Sep 17 00:00:00 2001 From: Amir Shalem Date: Mon, 6 Jul 2020 03:41:19 -0700 Subject: [PATCH] Don't call config->setLogger(nullptr) directly to avoid having no logger at all Summary: Changelog: [Internal][Yoga] Don't call config->setLogger(nullptr) directly to avoid having no logger at all Broken in D14151037 (https://github.com/facebook/yoga/commit/05f36a835a3a66b1b8affcdb036ec47117a8d28f) when it started calling ``` config->setLogger(nullptr); ``` instead of going thru ``` YGConfigSetLogger(config, nullptr); ``` which does the right thing by setting the logger to its default value: https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/yoga/yoga/Yoga.cpp?commit=835911317e8b3cf7da1866e40e1c79cda0690136&lines=4320-4330 Also by default YogaConfig always have a logger: https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/yoga/yoga/Yoga.cpp?commit=835911317e8b3cf7da1866e40e1c79cda0690136&lines=335-343 Reviewed By: SidharthGuglani Differential Revision: D22387459 fbshipit-source-id: 4da91da87a696d38cc9d8db2acb5845d29398adb --- java/jni/YGJNIVanilla.cpp | 2 +- .../com/facebook/yoga/YogaLoggerTest.java | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/java/jni/YGJNIVanilla.cpp b/java/jni/YGJNIVanilla.cpp index 698ab561..65343821 100644 --- a/java/jni/YGJNIVanilla.cpp +++ b/java/jni/YGJNIVanilla.cpp @@ -196,7 +196,7 @@ static void jni_YGConfigSetLoggerJNI( delete context; YGConfigSetContext(config, nullptr); } - config->setLogger(nullptr); + YGConfigSetLogger(config, nullptr); } } diff --git a/java/tests/com/facebook/yoga/YogaLoggerTest.java b/java/tests/com/facebook/yoga/YogaLoggerTest.java index 17316f6e..f88409d6 100644 --- a/java/tests/com/facebook/yoga/YogaLoggerTest.java +++ b/java/tests/com/facebook/yoga/YogaLoggerTest.java @@ -9,10 +9,62 @@ package com.facebook.yoga; import org.junit.Test; import java.lang.ref.WeakReference; +import java.util.List; +import java.util.ArrayList; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; public class YogaLoggerTest { + + @Test + public void testRemovingLoggerFromConfig() throws Exception { + final List logs = new ArrayList<>(); + + final YogaConfig config = YogaConfigFactory.create(); + YogaLogger logger = new YogaLogger() { + @Override + public void log(YogaLogLevel level, String message) { + logs.add(message); + } + }; + config.setLogger(logger); + + final YogaNode root = YogaNodeFactory.create(config); + root.setFlexDirection(YogaFlexDirection.ROW); + root.setAlignItems(YogaAlign.BASELINE); + + final YogaNode child1 = YogaNodeFactory.create(config); + root.addChildAt(child1, 0); + + final YogaNode child2 = YogaNodeFactory.create(config); + child2.setBaselineFunction(new YogaBaselineFunction() { + public float baseline(YogaNode node, float width, float height) { + return Float.NaN; + } + }); + root.addChildAt(child2, 1); + + assertEquals(logs.size(), 0); + try { + root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + fail("Expected calculateLayout to throw"); + } catch (IllegalStateException e) { + } + + assertEquals(logs.size(), 1); + + config.setLogger(null); + + try { + root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + fail("Expected calculateLayout to throw again"); + } catch (IllegalStateException e) { + } + + assertEquals(logs.size(), 1); + } + @Test public void testLoggerLeak() throws Exception { final YogaConfig config = YogaConfigFactory.create();