From 2168d680071eb403e9620c098bfe06663c27fdad Mon Sep 17 00:00:00 2001 From: Kazuki Sakamoto Date: Tue, 25 Oct 2016 07:38:06 -0700 Subject: [PATCH] Update CSSNodeFree for C#, Java and Objective-C Summary: - Update CSSNodeFree to unlink parent and children for C#, Java and Objective-C bindings finalizer. - [C#] Fix build (Fix #232) - [C#] Add Clear API for convenience as CSSNodeFreeRecursive. - [C#] Revise and add unit tests Reviewed By: emilsjolander Differential Revision: D4069655 fbshipit-source-id: 1fd764059784d7968af38b6aaf7fb6f70fdee8ee --- CSSLayout/CSSLayout.c | 11 ++ csharp/Facebook.CSSLayout/CSSNode.cs | 11 +- .../tests/Facebook.CSSLayout/CSSNodeTest.cs | 139 ++++++------------ 3 files changed, 67 insertions(+), 94 deletions(-) diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c index 058f390c..809ab4b1 100644 --- a/CSSLayout/CSSLayout.c +++ b/CSSLayout/CSSLayout.c @@ -155,6 +155,17 @@ CSSNodeRef CSSNodeNew(void) { } void CSSNodeFree(const CSSNodeRef node) { + if (node->parent) { + CSSNodeListDelete(node->parent->children, node); + node->parent = NULL; + } + + const uint32_t childCount = CSSNodeChildCount(node); + for (uint32_t i = 0; i < childCount; i++) { + const CSSNodeRef child = CSSNodeGetChild(node, i); + child->parent = NULL; + } + CSSNodeListFree(node->children); free(node); gNodeInstanceCount--; diff --git a/csharp/Facebook.CSSLayout/CSSNode.cs b/csharp/Facebook.CSSLayout/CSSNode.cs index e83204c1..ab17c944 100644 --- a/csharp/Facebook.CSSLayout/CSSNode.cs +++ b/csharp/Facebook.CSSLayout/CSSNode.cs @@ -15,7 +15,7 @@ using System.Text; namespace Facebook.CSSLayout { - public class CSSNode : IDisposable, IEnumerable + public class CSSNode : IEnumerable { private IntPtr _cssNode; @@ -35,7 +35,6 @@ namespace Facebook.CSSLayout { throw new InvalidOperationException("Failed to allocate native memory"); } - } ~CSSNode() @@ -483,6 +482,14 @@ namespace Facebook.CSSLayout Native.CSSNodeRemoveChild(_cssNode, child._cssNode); } + public void Clear() + { + while (_children.Count > 0) + { + RemoveAt(0); + } + } + public int IndexOf(CSSNode node) { return _children.IndexOf(node); diff --git a/csharp/tests/Facebook.CSSLayout/CSSNodeTest.cs b/csharp/tests/Facebook.CSSLayout/CSSNodeTest.cs index a9eef99d..3be61960 100644 --- a/csharp/tests/Facebook.CSSLayout/CSSNodeTest.cs +++ b/csharp/tests/Facebook.CSSLayout/CSSNodeTest.cs @@ -52,117 +52,50 @@ namespace Facebook.CSSLayout } [Test] - [ExpectedException("System.InvalidOperationException")] - public void TestAlreadyInitialize() + public void TestReset() { - CSSNode node = new CSSNode(); - node.Reinitialize(); - } - - [Test] - [ExpectedException("System.InvalidOperationException")] - public void TestNullNativePointer() - { - CSSNode node = new CSSNode(); - node.Free(); - node.CalculateLayout(); - } - - [Test] - [ExpectedException("System.InvalidOperationException")] - public void TestDoubleFree() - { - CSSNode node = new CSSNode(); - node.Free(); - node.Free(); - } - - [Test] - public void TestReinitialize() - { - CSSNode node = new CSSNode(); - node.Free(); - node.Reinitialize(); - } - - [Test] - [ExpectedException("System.ObjectDisposedException")] - public void TestDisposed() - { - CSSNode node = new CSSNode(); - node.Dispose(); - node.CalculateLayout(); - } - - [Test] - public void TestFree() - { - CSSNode node = new CSSNode(); - node.Free(); - } - - [Test] - public void TestDispose() - { - ForceGC(); int instanceCount = CSSNode.GetInstanceCount(); CSSNode node = new CSSNode(); Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); - node.Dispose(); - Assert.AreEqual(instanceCount, CSSNode.GetInstanceCount()); - } - - [Test] - public void TestDisposeWithUsing() - { - ForceGC(); - int instanceCount = CSSNode.GetInstanceCount(); - using (CSSNode node = new CSSNode()) - { - Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); - } - Assert.AreEqual(instanceCount, CSSNode.GetInstanceCount()); + node.Reset(); + Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); } [Test] [ExpectedException("System.InvalidOperationException")] - public void TestFreeParent() + public void TestResetParent() { CSSNode parent = new CSSNode(); CSSNode child = new CSSNode(); parent.Insert(0, child); - parent.Free(); + parent.Reset(); } [Test] [ExpectedException("System.InvalidOperationException")] - public void TestFreeChild() + public void TestResetChild() { CSSNode parent = new CSSNode(); CSSNode child = new CSSNode(); parent.Insert(0, child); - child.Free(); + child.Reset(); } [Test] - public void TestDisposeChild() + public void TestClear() { - ForceGC(); int instanceCount = CSSNode.GetInstanceCount(); CSSNode parent = new CSSNode(); - CSSNode child0 = new CSSNode(); - CSSNode child1 = new CSSNode(); - Assert.AreEqual(instanceCount + 3, CSSNode.GetInstanceCount()); - Assert.AreEqual(0, parent.Count); - parent.Insert(0, child1); - Assert.AreEqual(0, parent.IndexOf(child1)); - parent.Insert(0, child0); - Assert.AreEqual(0, parent.IndexOf(child0)); - Assert.AreEqual(1, parent.IndexOf(child1)); - child0.Dispose(); + Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); + CSSNode child = new CSSNode(); Assert.AreEqual(instanceCount + 2, CSSNode.GetInstanceCount()); + parent.Insert(0, child); Assert.AreEqual(1, parent.Count); - Assert.AreEqual(0, parent.IndexOf(child1)); + Assert.AreEqual(parent, child.Parent); + parent.Clear(); + Assert.AreEqual(0, parent.Count); + Assert.IsNull(child.Parent); + Assert.AreEqual(instanceCount + 2, CSSNode.GetInstanceCount()); } [Test] @@ -257,26 +190,48 @@ namespace Facebook.CSSLayout } [Test] - public void TestDisposeParent() + public void TestParentDestructor() { ForceGC(); int instanceCount = CSSNode.GetInstanceCount(); - CSSNode parent = new CSSNode(); + CSSNode child = new CSSNode(); Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); - TestDisposeParentForGC(parent, instanceCount + 1); + + TestParentDestructorForGC(child, instanceCount + 1); ForceGC(); - Assert.AreEqual(instanceCount + 2, CSSNode.GetInstanceCount()); - parent.Dispose(); - ForceGC(); - Assert.AreEqual(instanceCount, CSSNode.GetInstanceCount()); + + Assert.IsNull(child.Parent); + Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); } - private void TestDisposeParentForGC(CSSNode parent, int instanceCount) + private void TestParentDestructorForGC(CSSNode child, int instanceCount) + { + CSSNode parent = new CSSNode(); + Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); + parent.Insert(0, child); + } + + [Test] + public void TestClearWithChildDestructor() + { + ForceGC(); + int instanceCount = CSSNode.GetInstanceCount(); + CSSNode node = new CSSNode(); + Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); + TestClearWithChildDestructorForGC(node, instanceCount + 1); + ForceGC(); + Assert.AreEqual(instanceCount + 2, CSSNode.GetInstanceCount()); + node.Clear(); + Assert.AreEqual(0, node.Count); + ForceGC(); + Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); + } + + private void TestClearWithChildDestructorForGC(CSSNode parent, int instanceCount) { CSSNode child = new CSSNode(); Assert.AreEqual(instanceCount + 1, CSSNode.GetInstanceCount()); parent.Insert(0, child); - child = null; } #endif }