From 90844d62c5d5cd7f1cfe07961cf3256e5e5f52e8 Mon Sep 17 00:00:00 2001
From: Kazuki Sakamoto <kaz@fb.com>
Date: Fri, 7 Oct 2016 11:07:51 -0700
Subject: [PATCH] WeakReference for parent

Summary: - Use WeakReference for parent to avoid circular reference although GC will treat it

Reviewed By: emilsjolander

Differential Revision: D3982520

fbshipit-source-id: b0f6764aa4df3da53be51f6cb4fe2994d989afdf
---
 csharp/Facebook.CSSLayout/CSSNode.cs          |  6 +--
 .../csharp/Facebook.CSSLayout/CSSNodeTest.cs  | 43 ++++++++++++++++++-
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/csharp/Facebook.CSSLayout/CSSNode.cs b/csharp/Facebook.CSSLayout/CSSNode.cs
index 63f029f0..1f941fe5 100644
--- a/csharp/Facebook.CSSLayout/CSSNode.cs
+++ b/csharp/Facebook.CSSLayout/CSSNode.cs
@@ -19,7 +19,7 @@ namespace Facebook.CSSLayout
         private bool _isDisposed;
         private IntPtr _cssNode;
 
-        private CSSNode _parent;
+        private WeakReference _parent;
         private List<CSSNode> _children;
         private MeasureFunction _measureFunction;
         private CSSMeasureFunc _measureFunc;
@@ -155,7 +155,7 @@ namespace Facebook.CSSLayout
             get
             {
                 AssertNativeInstance();
-                return _parent;
+                return _parent != null ? _parent.Target as CSSNode : null;
             }
         }
 
@@ -632,7 +632,7 @@ namespace Facebook.CSSLayout
         {
             AssertNativeInstance();
             _children.Insert(index, node);
-            node._parent = this;
+            node._parent = new WeakReference(this);
             Native.CSSNodeInsertChild(_cssNode, node._cssNode, (uint)index);
         }
 
diff --git a/tests/csharp/Facebook.CSSLayout/CSSNodeTest.cs b/tests/csharp/Facebook.CSSLayout/CSSNodeTest.cs
index 87a5e97d..0edc1904 100644
--- a/tests/csharp/Facebook.CSSLayout/CSSNodeTest.cs
+++ b/tests/csharp/Facebook.CSSLayout/CSSNodeTest.cs
@@ -117,12 +117,12 @@ namespace Facebook.CSSLayout
         {
             ForceGC();
             Assert.AreEqual(0, CSSNode.GetInstanceCount());
-            TestDestructorFunc();
+            TestDestructorForGC();
             ForceGC();
             Assert.AreEqual(0, CSSNode.GetInstanceCount());
         }
 
-        private void TestDestructorFunc()
+        private void TestDestructorForGC()
         {
             CSSNode node = new CSSNode();
             Assert.AreEqual(0, CSSNode.GetInstanceCount());
@@ -131,6 +131,45 @@ namespace Facebook.CSSLayout
             node = null;
         }
 
+        [Test]
+        public void TestDestructorWithChildren()
+        {
+            ForceGC();
+            Assert.AreEqual(0, CSSNode.GetInstanceCount());
+            TestDestructorWithChildrenForGC1();
+            ForceGC();
+            Assert.AreEqual(0, CSSNode.GetInstanceCount());
+        }
+
+        private void TestDestructorWithChildrenForGC1()
+        {
+            CSSNode node = new CSSNode();
+            Assert.AreEqual(0, CSSNode.GetInstanceCount());
+            node.Initialize();
+            Assert.AreEqual(1, CSSNode.GetInstanceCount());
+
+            TestDestructorWithChildrenForGC2(node, 1);
+            ForceGC();
+            Assert.AreEqual(2, CSSNode.GetInstanceCount());
+
+            TestDestructorWithChildrenForGC2(node, 2);
+            ForceGC();
+            Assert.AreEqual(3, CSSNode.GetInstanceCount());
+
+            node = null;
+        }
+
+        private void TestDestructorWithChildrenForGC2(CSSNode parent, int count)
+        {
+            CSSNode child = new CSSNode();
+            Assert.AreEqual(count, CSSNode.GetInstanceCount());
+            child.Initialize();
+            Assert.AreEqual(count + 1, CSSNode.GetInstanceCount());
+
+            parent.Insert(0, child);
+            child = null;
+        }
+
         private void ForceGC()
         {
             GC.Collect(GC.MaxGeneration);