Guard against seg fault if replaceChild index is out of bounds

Summary:
We are seeing some seg faults after the new display: contents logic was added. This is either because we are passing in an out of bounds index - in which case we try to read `display_` from protected memory. Or, the `Node *` was deleted at some point without removing it from this array. I think its the out of bounds issue mainly because I am not sure where this deletion would occur

I added an if to revert to the legacy, undefined behavior in this case. This is not ideal and we should find the root cause that is calling into this function improperly but for now it stops apps from crashing on the `replaceChild` call

Changelog: [Internal]

Reviewed By: rozele

Differential Revision: D66038645

tests/YGNodeChildTest.cpp

@@ -33,3 +33,23 @@ TEST(YogaTest, reset_layout_when_child_removed) {
   YGNodeFreeRecursive(root);
   YGNodeFreeRecursive(root_child0);
 }
+
+TEST(YogaTest, replace_child_out_of_bounds) {
+  YGNodeRef root = YGNodeNew();
+
+  YGNodeRef root_child0 = YGNodeNew();
+  YGNodeStyleSetWidth(root_child0, 100);
+  YGNodeStyleSetHeight(root_child0, 100);
+  YGNodeInsertChild(root, root_child0, 0);
+
+  YGNodeRef root_child1 = YGNodeNew();
+  YGNodeStyleSetWidth(root_child1, 100);
+  YGNodeStyleSetHeight(root_child1, 100);
+  YGNodeSwapChild(root, root_child1, 100);
+
+  ASSERT_EQ(1, YGNodeGetChildCount(root));
+  ASSERT_EQ(root_child0, YGNodeGetChild(root, 0));
+
+  YGNodeFreeRecursive(root);
+  YGNodeFreeRecursive(root_child1);
+}

yoga/node/Node.cpp

@@ -117,14 +117,20 @@ void Node::setMeasureFunc(YGMeasureFunc measureFunc) {
 }
 
 void Node::replaceChild(Node* child, size_t index) {
-  auto previousChild = children_[index];
-  if (previousChild->style().display() == Display::Contents &&
-      child->style().display() != Display::Contents) {
-    contentsChildrenCount_--;
-  } else if (
-      previousChild->style().display() != Display::Contents &&
-      child->style().display() == Display::Contents) {
-    contentsChildrenCount_++;
+  // Without this conditional, if the index is out of bounds this will seg
+  // fault, so we are guarding against that here. Writing to this index
+  // afterwards is undefined behavior, and we ideally don't do that, but it is
+  // legacy behavior that we are keeping for now.
+  if (index < children_.size()) {
+    auto previousChild = children_[index];
+    if (previousChild->style().display() == Display::Contents &&
+        child->style().display() != Display::Contents) {
+      contentsChildrenCount_--;
+    } else if (
+        previousChild->style().display() != Display::Contents &&
+        child->style().display() == Display::Contents) {
+      contentsChildrenCount_++;
+    }
   }
 
   children_[index] = child;