From 49a21e657bfb4a99a5f73e9426930dd68024bcae Mon Sep 17 00:00:00 2001
From: Dustin Shahidehpour <dust@instagram.com>
Date: Mon, 21 Nov 2016 18:06:37 -0800
Subject: [PATCH] Fix bug where we insert nodes at wrong index in view
 hierarchy.

Summary:
We currently have a bug in `UIView+CSSLayout.m` that you can repro in a scenario like this:

```
UIView *container = [[UIView alloc] initWithFrame:CGRectZero];
  [container css_setUsesFlexbox:YES];
  [container css_setFlexDirection:CSSFlexDirectionRow];

  UIView *subview1 = [[UIView alloc] initWithFrame:CGRectZero];
  [subview1 css_setUsesFlexbox:YES];
  [subview1 css_setIncludeInLayout:NO];
  [container addSubview:subview1];

  UIView *subview2 = [[UIView alloc] initWithFrame:CGRectZero];
  [subview2 css_setUsesFlexbox:YES];
  [subview2 css_setIncludeInLayout:NO];
  [container addSubview:subview2];

  UIView *subview3 = [[UIView alloc] initWithFrame:CGRectZero];
  [subview3 css_setUsesFlexbox:YES];
  [subview3 css_setIncludeInLayout:YES];
  [container addSubview:subview3];

  [container css_applyLayout];
```

`subview3` (which is the only view whose is going to be included in layout calculation) is inserted at child index 2, instead of 0. This eventually can cause crash in CSSLayout.c because it will attempt access a child in the list which is null.

Reviewed By: emilsjolander

Differential Revision: D4215659

fbshipit-source-id: a615f50e51f85b15d3bdb437e55958865898b183
---
 CSSLayoutKit/Tests/CSSLayoutKitTests.m | 29 ++++++++++++++++++++++++++
 CSSLayoutKit/UIView+CSSLayout.h        |  5 +++++
 CSSLayoutKit/UIView+CSSLayout.m        | 19 ++++++++++-------
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/CSSLayoutKit/Tests/CSSLayoutKitTests.m b/CSSLayoutKit/Tests/CSSLayoutKitTests.m
index 720e0cb2..c6773895 100644
--- a/CSSLayoutKit/Tests/CSSLayoutKitTests.m
+++ b/CSSLayoutKit/Tests/CSSLayoutKitTests.m
@@ -164,6 +164,35 @@
   XCTAssertTrue(CGSizeEqualToSize(CGSizeMake(100, 50), subview3.bounds.size), @"Actual size is %@", NSStringFromCGSize(subview3.bounds.size));
 }
 
+- (void)testThatNumberOfChildrenIsCorrectWhenWeIgnoreSubviews
+{
+  UIView *container = [[UIView alloc] initWithFrame:CGRectZero];
+  [container css_setUsesFlexbox:YES];
+  [container css_setFlexDirection:CSSFlexDirectionRow];
+
+  UIView *subview1 = [[UIView alloc] initWithFrame:CGRectZero];
+  [subview1 css_setUsesFlexbox:YES];
+  [subview1 css_setIncludeInLayout:NO];
+  [container addSubview:subview1];
+
+  UIView *subview2 = [[UIView alloc] initWithFrame:CGRectZero];
+  [subview2 css_setUsesFlexbox:YES];
+  [subview2 css_setIncludeInLayout:NO];
+  [container addSubview:subview2];
+
+  UIView *subview3 = [[UIView alloc] initWithFrame:CGRectZero];
+  [subview3 css_setUsesFlexbox:YES];
+  [subview3 css_setIncludeInLayout:YES];
+  [container addSubview:subview3];
+
+  [container css_applyLayout];
+  XCTAssertEqual(1, [container css_numberOfChildren]);
+
+  [subview2 css_setIncludeInLayout:YES];
+  [container css_applyLayout];
+  XCTAssertEqual(2, [container css_numberOfChildren]);
+}
+
 - (void)testThatViewNotIncludedInFirstLayoutPassAreIncludedInSecond
 {
   UIView *container = [[UIView alloc] initWithFrame:CGRectMake(0, 0, 300, 50)];
diff --git a/CSSLayoutKit/UIView+CSSLayout.h b/CSSLayoutKit/UIView+CSSLayout.h
index d8d5ef77..6938fd95 100644
--- a/CSSLayoutKit/UIView+CSSLayout.h
+++ b/CSSLayoutKit/UIView+CSSLayout.h
@@ -64,4 +64,9 @@
  */
 - (CGSize)css_intrinsicSize;
 
+/**
+ Returns the number of children that are using Flexbox.
+ */
+- (NSUInteger)css_numberOfChildren;
+
 @end
diff --git a/CSSLayoutKit/UIView+CSSLayout.m b/CSSLayoutKit/UIView+CSSLayout.m
index db5e1b98..57c9e305 100644
--- a/CSSLayoutKit/UIView+CSSLayout.m
+++ b/CSSLayoutKit/UIView+CSSLayout.m
@@ -45,6 +45,11 @@
   return (includeInLayout != nil) ? [includeInLayout boolValue] : YES;
 }
 
+- (NSUInteger)css_numberOfChildren
+{
+  return CSSNodeChildCount([self cssNode]);
+}
+
 #pragma mark - Setters
 
 - (void)css_setIncludeInLayout:(BOOL)includeInLayout
@@ -281,24 +286,24 @@ static void CSSAttachNodesFromViewHierachy(UIView *view) {
   } else {
     CSSNodeSetMeasureFunc(node, NULL);
 
-    NSUInteger numSubviewsInLayout = 0;
+    NSUInteger subviewIndex = 0;
     // Add any children which were added since the last call to css_applyLayout
-    for (NSUInteger i = 0; i < view.subviews.count; i++) {
-      UIView *const subview = view.subviews[i];
+    for (UIView *subview in view.subviews) {
       if (![subview css_includeInLayout]) {
         continue;
       }
-      numSubviewsInLayout++;
 
       CSSNodeRef childNode = [subview cssNode];
-      if (CSSNodeChildCount(node) < i + 1 || CSSNodeGetChild(node, i) != childNode) {
-        CSSNodeInsertChild(node, childNode, i);
+      if (CSSNodeGetChild(node, subviewIndex) != childNode) {
+        CSSNodeInsertChild(node, childNode, subviewIndex);
       }
+
       CSSAttachNodesFromViewHierachy(subview);
+      subviewIndex++;
     }
 
     // Remove any children which were removed since the last call to css_applyLayout
-    while (numSubviewsInLayout < CSSNodeChildCount(node)) {
+    while (subviewIndex < CSSNodeChildCount(node)) {
       CSSNodeRemoveChild(node, CSSNodeGetChild(node, CSSNodeChildCount(node) - 1));
     }
   }