Reverted commit D4147298

Summary: If there is a single child which is flex grow and flex shrink then instead of measuring and then shrinking we can just set the flex basis to zero as we know the final result will be that the child take up all remaining space.

Reviewed By: gkassabli

Differential Revision: D4147298

fbshipit-source-id: 8416508a31e8d317bf59d956abdbe5760b55bb6d

CSSLayout/CSSLayout.c

@@ -259,8 +259,7 @@ static void _CSSNodeMarkDirty(const CSSNodeRef node) {
 }
 
 void CSSNodeSetMeasureFunc(const CSSNodeRef node, CSSMeasureFunc measureFunc) {
-  CSS_ASSERT(CSSNodeChildCount(node) == 0,
+  CSS_ASSERT(CSSNodeChildCount(node) == 0, "Cannot set measure function: Nodes with measure functions cannot have children.");
-             "Cannot set measure function: Nodes with measure functions cannot have children.");
   node->measure = measureFunc;
 }
 
@@ -270,8 +269,7 @@ CSSMeasureFunc CSSNodeGetMeasureFunc(const CSSNodeRef node) {
 
 void CSSNodeInsertChild(const CSSNodeRef node, const CSSNodeRef child, const uint32_t index) {
   CSS_ASSERT(child->parent == NULL, "Child already has a parent, it must be removed first.");
-  CSS_ASSERT(node->measure == NULL,
+  CSS_ASSERT(node->measure == NULL, "Cannot add child: Nodes with measure functions cannot have children.");
-             "Cannot add child: Nodes with measure functions cannot have children.");
   CSSNodeListInsert(&node->children, child, index);
   child->parent = node;
   _CSSNodeMarkDirty(node);
@@ -1369,26 +1367,6 @@ static void layoutNodeImpl(const CSSNodeRef node,
   const float availableInnerMainDim = isMainAxisRow ? availableInnerWidth : availableInnerHeight;
   const float availableInnerCrossDim = isMainAxisRow ? availableInnerHeight : availableInnerWidth;
 
-  // If there is only one child with flexGrow + flexShrink it means we can set the
-  // computedFlexBasis to 0 instead of measuring and shrinking / flexing the child to exactly
-  // match the remaining space
-  CSSNodeRef singleFlexChild = NULL;
-  if ((isMainAxisRow && widthMeasureMode != CSSMeasureModeUndefined) ||
-      (!isMainAxisRow && heightMeasureMode != CSSMeasureModeUndefined)) {
-    for (uint32_t i = 0; i < childCount; i++) {
-      const CSSNodeRef child = CSSNodeGetChild(node, i);
-      if (singleFlexChild) {
-        if (isFlex(child)) {
-          // There is already a flexible child, abort.
-          singleFlexChild = NULL;
-          break;
-        }
-      } else if (CSSNodeStyleGetFlexGrow(child) > 0 && CSSNodeStyleGetFlexShrink(child) > 0) {
-        singleFlexChild = child;
-      }
-    }
-  }
-
   // STEP 3: DETERMINE FLEX BASIS FOR EACH ITEM
   for (uint32_t i = 0; i < childCount; i++) {
     const CSSNodeRef child = CSSNodeListGet(node->children, i);
@@ -1412,9 +1390,6 @@ static void layoutNodeImpl(const CSSNodeRef node,
       }
       currentAbsoluteChild = child;
       child->nextChild = NULL;
-    } else {
-      if (child == singleFlexChild) {
-        child->layout.computedFlexBasis = 0;
     } else {
       computeChildFlexBasis(node,
                             child,
@@ -1425,7 +1400,6 @@ static void layoutNodeImpl(const CSSNodeRef node,
                             direction);
     }
   }
-  }
 
   // STEP 4: COLLECT FLEX ITEMS INTO FLEX LINES
 
@@ -2187,16 +2161,19 @@ bool CSSNodeCanUseCachedMeasurement(const CSSMeasureMode widthMode,
       newMeasureSizeIsStricterAndStillValid(
           widthMode, width - marginRow, lastWidthMode, lastWidth, lastComputedWidth);
 
-  const bool heightIsCompatible =
+  const bool heightIsCompatible = hasSameHeightSpec ||
-      hasSameHeightSpec || newSizeIsExactAndMatchesOldMeasuredSize(heightMode,
+                                  newSizeIsExactAndMatchesOldMeasuredSize(heightMode,
                                                                           height - marginColumn,
                                                                           lastComputedHeight) ||
                                   oldSizeIsUnspecifiedAndStillFits(heightMode,
                                                                    height - marginColumn,
                                                                    lastHeightMode,
                                                                    lastComputedHeight) ||
-      newMeasureSizeIsStricterAndStillValid(
+                                  newMeasureSizeIsStricterAndStillValid(heightMode,
-          heightMode, height - marginColumn, lastHeightMode, lastHeight, lastComputedHeight);
+                                                                        height - marginColumn,
+                                                                        lastHeightMode,
+                                                                        lastHeight,
+                                                                        lastComputedHeight);
 
   return widthIsCompatible && heightIsCompatible;
 }

java/jni/CSSJNI.cpp

@@ -40,8 +40,7 @@ static void _jniTransferLayoutOutputsRecursive(CSSNodeRef root) {
 }
 
 static void _jniPrint(CSSNodeRef node) {
-  auto obj = adopt_local(
+  auto obj = adopt_local(Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
-      Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
   cout << obj->toString() << endl;
 }
 
@@ -50,8 +49,7 @@ static CSSSize _jniMeasureFunc(CSSNodeRef node,
                                CSSMeasureMode widthMode,
                                float height,
                                CSSMeasureMode heightMode) {
-  auto obj = adopt_local(
+  auto obj = adopt_local(Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
-      Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
 
   static auto measureFunc = findClassLocal("com/facebook/csslayout/CSSNode")
       ->getMethod<jlong(jfloat, jint, jfloat, jint)>("measure");

tests/CSSLayoutMeasureCacheTest.cpp

@@ -17,8 +17,7 @@ static CSSSize _measureMax(CSSNodeRef node,
                         CSSMeasureMode heightMode) {
 
   int *measureCount = (int *)CSSNodeGetContext(node);
-  (*measureCount)++;
+  *measureCount = *measureCount + 1;
-
   return CSSSize {
       .width = widthMode == CSSMeasureModeUndefined ? 10 : width,
       .height = heightMode == CSSMeasureModeUndefined ? 10 : height,

tests/CSSLayoutMeasureTest.cpp

@@ -10,42 +10,18 @@
 #include <CSSLayout/CSSLayout.h>
 #include <gtest/gtest.h>
 
+#if GTEST_HAS_DEATH_TEST
 static CSSSize _measure(CSSNodeRef node,
                         float width,
                         CSSMeasureMode widthMode,
                         float height,
                         CSSMeasureMode heightMode) {
-  int *measureCount = (int*) CSSNodeGetContext(node);
-  if (measureCount) {
-    (*measureCount)++;
-  }
-
   return CSSSize {
-      .width = 10,
+      .width = 0,
-      .height = 10,
+      .height = 0,
   };
 }
 
-TEST(CSSLayoutTest, dont_measure_single_grow_shrink_child) {
-  const CSSNodeRef root = CSSNodeNew();
-  CSSNodeStyleSetWidth(root, 100);
-  CSSNodeStyleSetHeight(root, 100);
-
-  int measureCount = 0;
-
-  const CSSNodeRef root_child0 = CSSNodeNew();
-  CSSNodeSetContext(root_child0, &measureCount);
-  CSSNodeSetMeasureFunc(root_child0, _measure);
-  CSSNodeStyleSetFlexGrow(root_child0, 1);
-  CSSNodeStyleSetFlexShrink(root_child0, 1);
-  CSSNodeInsertChild(root, root_child0, 0);
-
-  CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR);
-
-  ASSERT_EQ(0, measureCount);
-}
-
-#if GTEST_HAS_DEATH_TEST
 TEST(CSSLayoutTest, cannot_add_child_to_node_with_measure_func) {
   const CSSNodeRef root = CSSNodeNew();
   CSSNodeSetMeasureFunc(root, _measure);