From e00e30ca150b3212457bac3e4efd9e22383001ad Mon Sep 17 00:00:00 2001
From: Dustin Shahidehpour <dust@instagram.com>
Date: Thu, 3 Nov 2016 13:30:34 -0700
Subject: [PATCH] Only mark Nodes dirty if an actual node is removed.

Summary: Currently, when we try to remove a child from a node, that node is mark dirty //regardless of whether or not anything was actually removed//. This fixes it.

Reviewed By: gkassabli

Differential Revision: D4125453

fbshipit-source-id: 745cfc55269415fea106a80c72401eb3074f2d31
---
 CSSLayout/CSSLayout.c               |  7 ++++---
 tests/CSSLayoutDirtyMarkingTest.cpp | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c
index dadaab50..dd63fb25 100644
--- a/CSSLayout/CSSLayout.c
+++ b/CSSLayout/CSSLayout.c
@@ -263,9 +263,10 @@ void CSSNodeInsertChild(const CSSNodeRef node, const CSSNodeRef child, const uin
 }
 
 void CSSNodeRemoveChild(const CSSNodeRef node, const CSSNodeRef child) {
-  CSSNodeListDelete(node->children, child);
-  child->parent = NULL;
-  _CSSNodeMarkDirty(node);
+  if (CSSNodeListDelete(node->children, child) != NULL) {
+    child->parent = NULL;
+    _CSSNodeMarkDirty(node);
+  }
 }
 
 CSSNodeRef CSSNodeGetChild(const CSSNodeRef node, const uint32_t index) {
diff --git a/tests/CSSLayoutDirtyMarkingTest.cpp b/tests/CSSLayoutDirtyMarkingTest.cpp
index 50d86c7c..7bafe352 100644
--- a/tests/CSSLayoutDirtyMarkingTest.cpp
+++ b/tests/CSSLayoutDirtyMarkingTest.cpp
@@ -69,3 +69,25 @@ TEST(CSSLayoutTest, dirty_propagation_only_if_prop_changed) {
 
   CSSNodeFreeRecursive(root);
 }
+
+TEST(CSSLayoutTest, dirty_node_only_if_children_are_actually_removed) {
+  const CSSNodeRef root = CSSNodeNew();
+  CSSNodeStyleSetAlignItems(root, CSSAlignFlexStart);
+  CSSNodeStyleSetWidth(root, 50);
+  CSSNodeStyleSetHeight(root, 50);
+
+  const CSSNodeRef child0 = CSSNodeNew();
+  CSSNodeStyleSetWidth(child0, 50);
+  CSSNodeStyleSetHeight(child0, 25);
+  CSSNodeInsertChild(root, child0, 0);
+
+  CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR);
+
+  CSSNodeRemoveChild(root, CSSNodeNew());
+  EXPECT_FALSE(CSSNodeIsDirty(root));
+
+  CSSNodeRemoveChild(root, child0);
+  EXPECT_TRUE(CSSNodeIsDirty(root));
+
+  CSSNodeFreeRecursive(root);
+}