Improve locking in ASVideoPlayerNode

TextureGroup · Jul 19, 2018 · 50f37b1 · 50f37b1
1 parent 736e200
commit 50f37b1
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,7 +23,9 @@
 - Split MapKit, Photos, and AssetsLibrary dependent code into separate subspecs to improve binary size and start time when they're not needed. The default subspec includes all three for backwards compatibility, but this **will change in 3.0**. When using non-Cocoapods build environments, define `AS_USE_PHOTOS, AS_USE_MAPKIT, AS_USE_ASSETS_LIBRARY` to 1 respectively to signal their use. [Adlai Holler](https://github.com/Adlai-Holler)
 - Optimization: Removed an NSMutableArray in flattened layouts. [Adlai Holler](https://github.com/Adlai-Holler)
 - Reduced binary size by disabling exception support (which we don't use.) [Adlai Holler](https://github.com/Adlai-Holler)
-- Create and set delegate for clip corner layers within ASDisplayNode [Michael Schneider](https://github.com/maicki) [#1024](https://github.com/TextureGroup/Texture/pull/1029)
+- Create and set delegate for clip corner layers within ASDisplayNode [Michael Schneider](https://github.com/maicki) [#1029](https://github.com/TextureGroup/Texture/pull/1029)
+- Improve locking situation in ASVideoPlayerNode [Michael Schneider](https://github.com/maicki) [#1042](https://github.com/TextureGroup/Texture/pull/1042)
+
 
 ## 2.7
 - Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)

diff --git a/Source/ASVideoPlayerNode.mm b/Source/ASVideoPlayerNode.mm
@@ -209,10 +209,8 @@ - (AVAsset *)asset
 - (void)didLoad
 {
   [super didLoad];
-  {
-    ASLockScopeSelf();
-    [self createControls];
-  }
+
+  [self createControls];
 }
 
 - (void)didEnterPreloadState
@@ -284,15 +282,23 @@ - (void)createControls
 
     if (_delegateFlags.delegateCustomControls && _delegateFlags.delegateLayoutSpecForControls) {
       NSDictionary *customControls = [_delegate videoPlayerNodeCustomControls:self];
+      std::vector<ASDisplayNode *> subnodes;
       for (id key in customControls) {
         id node = customControls[key];
         if (![node isKindOfClass:[ASDisplayNode class]]) {
           continue;
         }
 
-        [self addSubnode:node];
+        subnodes.push_back(node);
         [_cachedControls setObject:node forKey:key];
       }
+
+      {
+        ASUnlockScope(self);
+        for (var subnode : subnodes) {
+          [self addSubnode:subnode];
+        }
+      }
     }
   }
 
@@ -315,14 +321,22 @@ - (NSArray *)createDefaultControlElementArray
 
 - (void)removeControls
 {
-  for (ASDisplayNode *node in [_cachedControls objectEnumerator]) {
-    [node removeFromSupernode];
+  ASLockScope(self);
+
+  // Grab the cached controls for removing it
+  NSMutableDictionary *cachedControls = [_cachedControls copy];
+
+  {
+    ASUnlockScope(self);
+    for (ASDisplayNode *node in [cachedControls objectEnumerator]) {
+      [node removeFromSupernode];
+    }
   }
 
-  [self cleanCachedControls];
+  [self __locked_cleanCachedControls];
 }
 
-- (void)cleanCachedControls
+- (void)__locked_cleanCachedControls
 {
   [_cachedControls removeAllObjects];
 
@@ -355,7 +369,10 @@ - (void)_locked_createPlaybackButton
     [_cachedControls setObject:_playbackButtonNode forKey:@(ASVideoPlayerNodeControlTypePlaybackButton)];
   }
 
-  [self addSubnode:_playbackButtonNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_playbackButtonNode];
+  }
 }
 
 - (void)_locked_createFullScreenButton
@@ -374,7 +391,10 @@ - (void)_locked_createFullScreenButton
     [_cachedControls setObject:_fullScreenButtonNode forKey:@(ASVideoPlayerNodeControlTypeFullScreenButton)];
   }
 
-  [self addSubnode:_fullScreenButtonNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_fullScreenButtonNode];
+  }
 }
 
 - (void)_locked_createElapsedTextField
@@ -389,7 +409,10 @@ - (void)_locked_createElapsedTextField
 
     [_cachedControls setObject:_elapsedTextNode forKey:@(ASVideoPlayerNodeControlTypeElapsedText)];
   }
-  [self addSubnode:_elapsedTextNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_elapsedTextNode];
+  }
 }
 
 - (void)_locked_createDurationTextField
@@ -405,7 +428,10 @@ - (void)_locked_createDurationTextField
     [_cachedControls setObject:_durationTextNode forKey:@(ASVideoPlayerNodeControlTypeDurationText)];
   }
   [self updateDurationTimeLabel];
-  [self addSubnode:_durationTextNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_durationTextNode];
+  }
 }
 
 - (void)_locked_createScrubber
@@ -450,8 +476,10 @@ - (void)_locked_createScrubber
 
     [_cachedControls setObject:_scrubberNode forKey:@(ASVideoPlayerNodeControlTypeScrubber)];
   }
-
-  [self addSubnode:_scrubberNode];
+  {
+    ASUnlockScope(self);
+    [self addSubnode:_scrubberNode];
+  }
 }
 
 - (void)_locked_createControlFlexGrowSpacer
@@ -623,7 +651,6 @@ - (void)showSpinner
   ASLockScopeSelf();
 
   if (!_spinnerNode) {
-
     __weak __typeof__(self) weakSelf = self;
     _spinnerNode = [[ASDisplayNode alloc] initWithViewBlock:^UIView *{
       __typeof__(self) strongSelf = weakSelf;
@@ -644,9 +671,11 @@ - (void)showSpinner
     }];
 
     _spinnerNode.style.preferredSize = CGSizeMake(44.0, 44.0);
-
-    [self addSubnode:_spinnerNode];
-    [self setNeedsLayout];
+    {
+      ASUnlockScope(self);
+      [self addSubnode:_spinnerNode];
+      [self setNeedsLayout];
+    }
   }
   [(UIActivityIndicatorView *)_spinnerNode.view startAnimating];
 }
@@ -658,7 +687,10 @@ - (void)removeSpinner
   if (!_spinnerNode) {
     return;
   }
-  [_spinnerNode removeFromSupernode];
+  {
+    ASUnlockScope(self);
+    [_spinnerNode removeFromSupernode];
+  }
   _spinnerNode = nil;
 }