Copy yogaChildren in accessor method. Avoid using accessor method int…

…ernally (TextureGroup#1283) * Copy yogaChildren in accessor method. Avoid using accessor method internally. * Further improvements
mikezucc · Dec 20, 2018 · ab0a00c · ab0a00c
1 parent 4982c84
commit ab0a00c
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 10 deletions.
diff --git 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
@@ -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,14 +107,16 @@ - (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];
 
   // YGNodeRef insertion is done in setParent:
   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
@@ -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);
   }
 }