From a0d560a24bc5d4b895352dcea9a194c68aabe4b2 Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Wed, 23 Nov 2016 05:25:50 -0800 Subject: [PATCH] Fixup recent fix to flex basis and put it behind an experimental flag Summary: D4207106 previously fixed an issue where the computed flex basis was cached in between layout calculations, potentially caching wrong values. After landing we noticed that some product were implicitly relying on this behavior so this diff instead puts that behind a feature flag. Reviewed By: gkassabli Differential Revision: D4222910 fbshipit-source-id: d693482441fcc4d37a288e2e3529057a04f60541 --- CSSLayout/CSSEnums.h | 1 + CSSLayout/CSSLayout.c | 3 ++- CSSLayoutKit/UIView+CSSLayout.m | 6 ++++++ .../Facebook.CSSLayout/CSSExperimentalFeature.cs | 1 + enums.py | 2 ++ .../facebook/csslayout/CSSExperimentalFeature.java | 4 +++- tests/CSSLayoutRelayoutTest.cpp | 14 +++++++++++++- 7 files changed, 28 insertions(+), 3 deletions(-) diff --git a/CSSLayout/CSSEnums.h b/CSSLayout/CSSEnums.h index dbcb58e2..94a6ba1b 100644 --- a/CSSLayout/CSSEnums.h +++ b/CSSLayout/CSSEnums.h @@ -68,6 +68,7 @@ typedef enum CSSDirection { typedef enum CSSExperimentalFeature { CSSExperimentalFeatureRounding, + CSSExperimentalFeatureWebFlexBasis, CSSExperimentalFeatureCount, } CSSExperimentalFeature; diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c index eeb99195..5b102509 100644 --- a/CSSLayout/CSSLayout.c +++ b/CSSLayout/CSSLayout.c @@ -977,7 +977,8 @@ static void computeChildFlexBasis(const CSSNodeRef node, if (!CSSValueIsUndefined(CSSNodeStyleGetFlexBasis(child)) && !CSSValueIsUndefined(isMainAxisRow ? width : height)) { if (CSSValueIsUndefined(child->layout.computedFlexBasis) || - child->layout.computedFlexBasisGeneration != gCurrentGenerationCount) { + (CSSLayoutIsExperimentalFeatureEnabled(CSSExperimentalFeatureWebFlexBasis) && + child->layout.computedFlexBasisGeneration != gCurrentGenerationCount)) { child->layout.computedFlexBasis = fmaxf(CSSNodeStyleGetFlexBasis(child), getPaddingAndBorderAxis(child, mainAxis)); } diff --git a/CSSLayoutKit/UIView+CSSLayout.m b/CSSLayoutKit/UIView+CSSLayout.m index 56ddfcf7..664f50fb 100644 --- a/CSSLayoutKit/UIView+CSSLayout.m +++ b/CSSLayoutKit/UIView+CSSLayout.m @@ -16,6 +16,12 @@ @end @implementation CSSNodeBridge + ++ (void)initialize +{ + CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeatureWebFlexBasis, true); +} + - (instancetype)init { if ([super init]) { diff --git a/csharp/Facebook.CSSLayout/CSSExperimentalFeature.cs b/csharp/Facebook.CSSLayout/CSSExperimentalFeature.cs index 9eb74309..160fc724 100644 --- a/csharp/Facebook.CSSLayout/CSSExperimentalFeature.cs +++ b/csharp/Facebook.CSSLayout/CSSExperimentalFeature.cs @@ -12,5 +12,6 @@ namespace Facebook.CSSLayout public enum CSSExperimentalFeature { Rounding, + WebFlexBasis, } } diff --git a/enums.py b/enums.py index 76261b7f..5e361b9e 100644 --- a/enums.py +++ b/enums.py @@ -79,6 +79,8 @@ ENUMS = { ], 'CSSExperimentalFeature': [ 'Rounding', + # Mimic web flex-basis behavior. + 'WebFlexBasis', ], 'CSSPrintOptions': [ ('Layout', 1), diff --git a/java/com/facebook/csslayout/CSSExperimentalFeature.java b/java/com/facebook/csslayout/CSSExperimentalFeature.java index c2b8b7af..d9476e81 100644 --- a/java/com/facebook/csslayout/CSSExperimentalFeature.java +++ b/java/com/facebook/csslayout/CSSExperimentalFeature.java @@ -10,7 +10,8 @@ package com.facebook.csslayout; public enum CSSExperimentalFeature { - ROUNDING(0); + ROUNDING(0), + WEB_FLEX_BASIS(1); private int mIntValue; @@ -25,6 +26,7 @@ public enum CSSExperimentalFeature { public static CSSExperimentalFeature fromInt(int value) { switch (value) { case 0: return ROUNDING; + case 1: return WEB_FLEX_BASIS; default: throw new IllegalArgumentException("Unkown enum value: " + value); } } diff --git a/tests/CSSLayoutRelayoutTest.cpp b/tests/CSSLayoutRelayoutTest.cpp index e653f049..fa5226b4 100644 --- a/tests/CSSLayoutRelayoutTest.cpp +++ b/tests/CSSLayoutRelayoutTest.cpp @@ -10,7 +10,19 @@ #include #include -TEST(CSSLayoutTest, dont_cache_computed_flex_basis_between_layouts) { +class CSSLayoutRelayoutTest : public ::testing::Test { + +protected: + virtual void SetUp() { + CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeatureWebFlexBasis, true); + } + + virtual void TearDown() { + CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeatureWebFlexBasis, false); + } +}; + +TEST_F(CSSLayoutRelayoutTest, dont_cache_computed_flex_basis_between_layouts) { const CSSNodeRef root = CSSNodeNew(); const CSSNodeRef root_child0 = CSSNodeNew();