From ab0a00c21c83fa46c3cc748af76e981f97c9c4d0 Mon Sep 17 00:00:00 2001
From: Michael Schneider <maicki@users.noreply.github.com>
Date: Thu, 20 Dec 2018 17:45:52 +0100
Subject: [PATCH] Copy yogaChildren in accessor method. Avoid using accessor
 method internally (#1283)

* Copy yogaChildren in accessor method. Avoid using accessor method internally.

* Further improvements
---
 Source/ASDisplayNode+Beta.h      |  4 ++--
 Source/ASDisplayNode+Yoga.mm     | 33 +++++++++++++++++++++++++-------
 Source/Layout/ASYogaUtilities.mm |  4 +++-
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Source/ASDisplayNode+Beta.h b/Source/ASDisplayNode+Beta.h
index 7adaf5e16..8bd825216 100644
--- a/Source/ASDisplayNode+Beta.h
+++ b/Source/ASDisplayNode+Beta.h
@@ -188,8 +188,7 @@ AS_EXTERN void ASDisplayNodePerformBlockOnEveryYogaChild(ASDisplayNode * _Nullab
 
 @interface ASDisplayNode (Yoga)
 
-// TODO: Make this and yogaCalculatedLayout atomic (lock).
-@property (nullable, nonatomic) NSArray *yogaChildren;
+@property (copy) NSArray *yogaChildren;
 
 - (void)addYogaChild:(ASDisplayNode *)child;
 - (void)removeYogaChild:(ASDisplayNode *)child;
@@ -198,6 +197,7 @@ AS_EXTERN void ASDisplayNodePerformBlockOnEveryYogaChild(ASDisplayNode * _Nullab
 - (void)semanticContentAttributeDidChange:(UISemanticContentAttribute)attribute;
 
 @property BOOL yogaLayoutInProgress;
+// TODO: Make this atomic (lock).
 @property (nullable, nonatomic) ASLayout *yogaCalculatedLayout;
 
 // Will walk up the Yoga tree and returns the root node
diff --git a/Source/ASDisplayNode+Yoga.mm b/Source/ASDisplayNode+Yoga.mm
index 6dc1e8a8e..c770cbd3a 100644
--- a/Source/ASDisplayNode+Yoga.mm
+++ b/Source/ASDisplayNode+Yoga.mm
@@ -48,32 +48,43 @@ - (void)setYogaChildren:(NSArray *)yogaChildren
   for (ASDisplayNode *child in [_yogaChildren copy]) {
     // Make sure to un-associate the YGNodeRef tree before replacing _yogaChildren
     // If this becomes a performance bottleneck, it can be optimized by not doing the NSArray removals here.
-    [self removeYogaChild:child];
+    [self _locked_removeYogaChild:child];
   }
   _yogaChildren = nil;
   for (ASDisplayNode *child in yogaChildren) {
-    [self addYogaChild:child];
+    [self _locked_addYogaChild:child];
   }
 }
 
 - (NSArray *)yogaChildren
 {
-  return _yogaChildren;
+  ASLockScope(self.yogaRoot);
+  return [_yogaChildren copy] ?: @[];
 }
 
 - (void)addYogaChild:(ASDisplayNode *)child
 {
   ASLockScope(self.yogaRoot);
+  [self _locked_addYogaChild:child];
+}
+
+- (void)_locked_addYogaChild:(ASDisplayNode *)child
+{
   [self insertYogaChild:child atIndex:_yogaChildren.count];
 }
 
 - (void)removeYogaChild:(ASDisplayNode *)child
 {
   ASLockScope(self.yogaRoot);
+  [self _locked_removeYogaChild:child];
+}
+
+- (void)_locked_removeYogaChild:(ASDisplayNode *)child
+{
   if (child == nil) {
     return;
   }
-  
+
   [_yogaChildren removeObjectIdenticalTo:child];
 
   // YGNodeRef removal is done in setParent:
@@ -83,6 +94,11 @@ - (void)removeYogaChild:(ASDisplayNode *)child
 - (void)insertYogaChild:(ASDisplayNode *)child atIndex:(NSUInteger)index
 {
   ASLockScope(self.yogaRoot);
+  [self _locked_insertYogaChild:child atIndex:index];
+}
+
+- (void)_locked_insertYogaChild:(ASDisplayNode *)child atIndex:(NSUInteger)index
+{
   if (child == nil) {
     return;
   }
@@ -91,7 +107,7 @@ - (void)insertYogaChild:(ASDisplayNode *)child atIndex:(NSUInteger)index
   }
 
   // Clean up state in case this child had another parent.
-  [self removeYogaChild:child];
+  [self _locked_removeYogaChild:child];
 
   [_yogaChildren insertObject:child atIndex:index];
 
@@ -99,6 +115,8 @@ - (void)insertYogaChild:(ASDisplayNode *)child atIndex:(NSUInteger)index
   child.yogaParent = self;
 }
 
+#pragma mark - Subclass Hooks
+
 - (void)semanticContentAttributeDidChange:(UISemanticContentAttribute)attribute
 {
   UIUserInterfaceLayoutDirection layoutDirection =
@@ -168,8 +186,9 @@ - (void)setupYogaCalculatedLayout
 
   YGNodeRef yogaNode = self.style.yogaNode;
   uint32_t childCount = YGNodeGetChildCount(yogaNode);
-  ASDisplayNodeAssert(childCount == self.yogaChildren.count,
-                      @"Yoga tree should always be in sync with .yogaNodes array! %@", self.yogaChildren);
+  ASDisplayNodeAssert(childCount == _yogaChildren.count,
+                      @"Yoga tree should always be in sync with .yogaNodes array! %@",
+                      _yogaChildren);
 
   ASLayout *rawSublayouts[childCount];
   int i = 0;
diff --git a/Source/Layout/ASYogaUtilities.mm b/Source/Layout/ASYogaUtilities.mm
index 161a5baef..f0602ed4b 100644
--- a/Source/Layout/ASYogaUtilities.mm
+++ b/Source/Layout/ASYogaUtilities.mm
@@ -49,7 +49,9 @@ void ASDisplayNodePerformBlockOnEveryYogaChild(ASDisplayNode *node, void(^block)
     return;
   }
   block(node);
-  for (ASDisplayNode *child in [node yogaChildren]) {
+  // We use the accessor here despite the copy, because the block may modify the yoga tree e.g.
+  // replacing a node.
+  for (ASDisplayNode *child in node.yogaChildren) {
     ASDisplayNodePerformBlockOnEveryYogaChild(child, block);
   }
 }