Only skip updating computed flex basis within the same generation

Emil Sjolander · Facebook Github Bot · commit b16c22a8f38e · 2016-11-20T05:07:33.000-08:00
Summary:
computed flex basis was incorrectly being reused in some circumstances between calls to caluclateLayout

also run format script.

Reviewed By: dshahidehpour

Differential Revision: D4207106

fbshipit-source-id: fc1063ca79ecf75f6101aadded53bca96cb0585d
diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c
@@ -49,6 +49,7 @@ typedef struct CSSLayout {
   float dimensions[2];
   CSSDirection direction;
 
+  uint32_t computedFlexBasisGeneration;
   float computedFlexBasis;
 
   // Instead of recomputing the entire layout every single time, we
@@ -967,7 +968,8 @@ static void computeChildFlexBasis(const CSSNodeRef node,
 
   if (!CSSValueIsUndefined(CSSNodeStyleGetFlexBasis(child)) &&
       !CSSValueIsUndefined(isMainAxisRow ? width : height)) {
-    if (CSSValueIsUndefined(child->layout.computedFlexBasis)) {
+    if (CSSValueIsUndefined(child->layout.computedFlexBasis) ||
+        child->layout.computedFlexBasisGeneration != gCurrentGenerationCount) {
       child->layout.computedFlexBasis =
           fmaxf(CSSNodeStyleGetFlexBasis(child), getPaddingAndBorderAxis(child, mainAxis));
     }
@@ -1052,6 +1054,8 @@ static void computeChildFlexBasis(const CSSNodeRef node,
                             : child->layout.measuredDimensions[CSSDimensionHeight],
               getPaddingAndBorderAxis(child, mainAxis));
   }
+
+  child->layout.computedFlexBasisGeneration = gCurrentGenerationCount;
 }
 
 static void absoluteLayoutChild(const CSSNodeRef node,
@@ -1477,6 +1481,7 @@ static void layoutNodeImpl(const CSSNodeRef node,
       child->nextChild = NULL;
     } else {
       if (child == singleFlexChild) {
+        child->layout.computedFlexBasisGeneration = gCurrentGenerationCount;
         child->layout.computedFlexBasis = 0;
       } else {
         computeChildFlexBasis(node,
@@ -1813,8 +1818,8 @@ static void layoutNodeImpl(const CSSNodeRef node,
       if (!CSSValueIsUndefined(node->style.minDimensions[dim[mainAxis]]) &&
           node->style.minDimensions[dim[mainAxis]] >= 0) {
         remainingFreeSpace = fmaxf(0,
-                                  node->style.minDimensions[dim[mainAxis]] -
-                                      (availableInnerMainDim - remainingFreeSpace));
+                                   node->style.minDimensions[dim[mainAxis]] -
+                                       (availableInnerMainDim - remainingFreeSpace));
       } else {
         remainingFreeSpace = 0;
       }
@@ -2542,10 +2547,9 @@ void CSSLayoutSetMemoryFuncs(CSSMalloc cssMalloc,
                              CSSCalloc cssCalloc,
                              CSSRealloc cssRealloc,
                              CSSFree cssFree) {
-  CSS_ASSERT(gNodeInstanceCount == 0,
-             "Cannot set memory functions: all node must be freed first");
+  CSS_ASSERT(gNodeInstanceCount == 0, "Cannot set memory functions: all node must be freed first");
   CSS_ASSERT((cssMalloc == NULL && cssCalloc == NULL && cssRealloc == NULL && cssFree == NULL) ||
-             (cssMalloc != NULL && cssCalloc != NULL && cssRealloc != NULL && cssFree != NULL),
+                 (cssMalloc != NULL && cssCalloc != NULL && cssRealloc != NULL && cssFree != NULL),
              "Cannot set memory functions: functions must be all NULL or Non-NULL");
 
   if (cssMalloc == NULL || cssCalloc == NULL || cssRealloc == NULL || cssFree == NULL) {
diff --git a/CSSLayout/CSSLayout.h b/CSSLayout/CSSLayout.h
@@ -28,8 +28,8 @@ static const unsigned long __nan[2] = {0xffffffff, 0x7fffffff};
 
 #define CSSUndefined NAN
 
-#include "CSSMacros.h"
 #include "CSSEnums.h"
+#include "CSSMacros.h"
 
 CSS_EXTERN_C_BEGIN
 
@@ -160,7 +160,8 @@ CSS_NODE_LAYOUT_PROPERTY(CSSDirection, Direction);
 WIN_EXPORT void CSSLayoutSetLogger(CSSLogger logger);
 WIN_EXPORT void CSSLog(CSSLogLevel level, const char *message, ...);
 
-WIN_EXPORT void CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeature feature, bool enabled);
+WIN_EXPORT void CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeature feature,
+                                                       bool enabled);
 WIN_EXPORT bool CSSLayoutIsExperimentalFeatureEnabled(CSSExperimentalFeature feature);
 
 WIN_EXPORT void CSSLayoutSetMemoryFuncs(CSSMalloc cssMalloc,
diff --git a/java/jni/CSSJNI.cpp b/java/jni/CSSJNI.cpp
@@ -77,12 +77,15 @@ static int _jniLog(CSSLogLevel level, const char *format, va_list args) {
   char buffer[256];
   int result = vsnprintf(buffer, sizeof(buffer), format, args);
 
-  static auto logFunc =
-      findClassLocal("com/facebook/csslayout/CSSLogger")->getMethod<void(local_ref<JCSSLogLevel>, jstring)>("log");
+  static auto logFunc = findClassLocal("com/facebook/csslayout/CSSLogger")
+                            ->getMethod<void(local_ref<JCSSLogLevel>, jstring)>("log");
 
-  static auto logLevelFromInt = JCSSLogLevel::javaClassStatic()->getStaticMethod<JCSSLogLevel::javaobject(jint)>("fromInt");
+  static auto logLevelFromInt =
+      JCSSLogLevel::javaClassStatic()->getStaticMethod<JCSSLogLevel::javaobject(jint)>("fromInt");
 
-  logFunc(jLogger->get(), logLevelFromInt(JCSSLogLevel::javaClassStatic(), static_cast<jint>(level)), Environment::current()->NewStringUTF(buffer));
+  logFunc(jLogger->get(),
+          logLevelFromInt(JCSSLogLevel::javaClassStatic(), static_cast<jint>(level)),
+          Environment::current()->NewStringUTF(buffer));
 
   return result;
 }
@@ -112,7 +115,9 @@ void jni_CSSLog(alias_ref<jclass> clazz, jint level, jstring message) {
   Environment::current()->ReleaseStringUTFChars(message, nMessage);
 }
 
-void jni_CSSLayoutSetExperimentalFeatureEnabled(alias_ref<jclass> clazz, jint feature, jboolean enabled) {
+void jni_CSSLayoutSetExperimentalFeatureEnabled(alias_ref<jclass> clazz,
+                                                jint feature,
+                                                jboolean enabled) {
   CSSLayoutSetExperimentalFeatureEnabled(static_cast<CSSExperimentalFeature>(feature), enabled);
 }
 
diff --git a/tests/CSSLayoutRelayoutTest.cpp b/tests/CSSLayoutRelayoutTest.cpp
@@ -0,0 +1,27 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+#include <CSSLayout/CSSLayout.h>
+#include <gtest/gtest.h>
+
+TEST(CSSLayoutTest, dont_cache_computed_flex_basis_between_layouts) {
+  const CSSNodeRef root = CSSNodeNew();
+
+  const CSSNodeRef root_child0 = CSSNodeNew();
+  CSSNodeStyleSetHeight(root_child0, 10);
+  CSSNodeStyleSetFlexBasis(root_child0, 20);
+  CSSNodeInsertChild(root, root_child0, 0);
+
+  CSSNodeCalculateLayout(root, 100, CSSUndefined, CSSDirectionLTR);
+  CSSNodeCalculateLayout(root, 100, 100, CSSDirectionLTR);
+
+  ASSERT_EQ(20, CSSNodeLayoutGetHeight(root_child0));
+
+  CSSNodeFreeRecursive(root);
+}
