[ASVideoNode] Change superclass to ASNetworkImageNode so that it can be its own placeholder (#1710)

* [ASVideoNode] Change superclass to ASNetworkImageNode so that ASVideoNode can be its own placeholder
- remove _placeholderImageNode property of ASVideoNode (use self.image now instead)
- move layoutSpecThatFits: code to calculateSizeThatFits: & layout: methods as ASImageNode uses calculateSizeThatFits:

* [ASVideoNode] Tweaks to the definition of the delegate protocols to integrate with ASNetworkImageNode (superclass)

Author: hannahmbanana

AsyncDisplayKit/ASNetworkImageNode.h

@@ -45,7 +45,7 @@ NS_ASSUME_NONNULL_BEGIN
 /**
  * The delegate, which must conform to the <ASNetworkImageNodeDelegate> protocol.
  */
-@property (atomic, weak, readwrite) id<ASNetworkImageNodeDelegate> delegate;
+@property (nullable, atomic, weak, readwrite) id<ASNetworkImageNodeDelegate> delegate;
 
 /**
  * A placeholder image to display while the URL is loading.
@@ -100,6 +100,7 @@ NS_ASSUME_NONNULL_BEGIN
  * notifications such as finished decoding and downloading an image.
  */
 @protocol ASNetworkImageNodeDelegate <NSObject>
+@optional
 
 /**
  * Notification that the image node finished downloading an image.
@@ -111,8 +112,6 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)imageNode:(ASNetworkImageNode *)imageNode didLoadImage:(UIImage *)image;
 
-@optional
-
 /**
  * Notification that the image node started to load
  *

AsyncDisplayKit/ASVideoNode.h

@@ -8,6 +8,7 @@
 
 #if TARGET_OS_IOS
 #import <AsyncDisplayKit/ASButtonNode.h>
+#import <AsyncDisplayKit/ASNetworkImageNode.h>
 
 @class AVAsset, AVPlayer, AVPlayerItem;
 @protocol ASVideoNodeDelegate;
@@ -31,7 +32,7 @@ NS_ASSUME_NONNULL_BEGIN
 //    there is room for further expansion and optimization.  Please report any issues or requests
 //    in an issue on GitHub: https://github.com/facebook/AsyncDisplayKit/issues
 
-@interface ASVideoNode : ASControlNode
+@interface ASVideoNode : ASNetworkImageNode
 
 - (void)play;
 - (void)pause;
@@ -60,11 +61,11 @@ NS_ASSUME_NONNULL_BEGIN
 //! Defaults to AVLayerVideoGravityResizeAspect
 @property (atomic) NSString *gravity;
 
-@property (nullable, atomic, weak, readwrite) id<ASVideoNodeDelegate> delegate;
+@property (nullable, atomic, weak, readwrite) id<ASVideoNodeDelegate, ASNetworkImageNodeDelegate> delegate;
 
 @end
 
-@protocol ASVideoNodeDelegate <NSObject>
+@protocol ASVideoNodeDelegate <ASNetworkImageNodeDelegate>
 @optional
 /**
  * @abstract Delegate method invoked when the node's video has played to its end time.

AsyncDisplayKit/ASVideoNode.mm

@@ -71,8 +71,6 @@ @interface ASVideoNode ()
   int32_t _periodicTimeObserverTimescale;
   CMTime _timeObserverInterval;
 
-  ASImageNode *_placeholderImageNode; // TODO: Make ASVideoNode an ASImageNode subclass; remove this.
-
   ASDisplayNode *_playerNode;
   NSString *_gravity;
 }
@@ -151,7 +149,7 @@ - (void)prepareToPlayAsset:(AVAsset *)asset withKeys:(NSArray<NSString *> *)requ
     self.player = [AVPlayer playerWithPlayerItem:playerItem];
   }
 
-  if (_placeholderImageNode.image == nil) {
+  if (self.image == nil) {
     [self generatePlaceholderImage];
   }
 
@@ -193,33 +191,34 @@ - (void)removePlayerItemObservers:(AVPlayerItem *)playerItem
   [notificationCenter removeObserver:self name:AVPlayerItemNewErrorLogEntryNotification object:playerItem];
 }
 
-- (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize
+- (void)layout
 {
-    // All subnodes should taking the whole node frame
-    CGSize maxSize = constrainedSize.max;
-    if (!CGSizeEqualToSize(self.preferredFrameSize, CGSizeZero)) {
-        maxSize = self.preferredFrameSize;
-    }
-    
-    // Prevent crashes through if infinite width or height
-    if (isinf(maxSize.width) || isinf(maxSize.height)) {
-        ASDisplayNodeAssert(NO, @"Infinite width or height in ASVideoNode");
-        maxSize = CGSizeZero;
-    }
-    
-    // Stretch out play button, placeholder image player node to the max size
-    NSMutableArray *children = [NSMutableArray array];
+  [super layout];
+  // The _playerNode wraps AVPlayerLayer, and therefore should extend across the entire bounds.
+  _playerNode.frame = self.bounds;
+}
 
-    if (_placeholderImageNode) {
-        _placeholderImageNode.preferredFrameSize = maxSize;
-        [children addObject:_placeholderImageNode];
-    }
-    if (_playerNode) {
-        _playerNode.preferredFrameSize = maxSize;
-        [children addObject:_playerNode];
-    }
-    
-    return [ASStaticLayoutSpec staticLayoutSpecWithChildren:children];
+- (CGSize)calculateSizeThatFits:(CGSize)constrainedSize
+{
+  ASDN::MutexLocker l(_videoLock);
+  CGSize calculatedSize = constrainedSize;
+  
+  // if a preferredFrameSize is set, call the superclass to return that instead of using the image size.
+  if (CGSizeEqualToSize(self.preferredFrameSize, CGSizeZero) == NO)
+    calculatedSize = self.preferredFrameSize;
+ 
+  // Prevent crashes through if infinite width or height
+  if (isinf(calculatedSize.width) || isinf(calculatedSize.height)) {
+    ASDisplayNodeAssert(NO, @"Infinite width or height in ASVideoNode");
+    calculatedSize = CGSizeZero;
+  }
+  
+  if (_playerNode) {
+    _playerNode.preferredFrameSize = calculatedSize;
+    [_playerNode measure:calculatedSize];
+  }
+  
+  return calculatedSize;
 }
 
 - (void)generatePlaceholderImage
@@ -265,23 +264,10 @@ - (void)imageAtTime:(CMTime)imageTime completionHandler:(void(^)(UIImage *image)
 - (void)setVideoPlaceholderImage:(UIImage *)image
 {
   ASDN::MutexLocker l(_videoLock);
-
-  if (_placeholderImageNode == nil && image != nil) {
-    _placeholderImageNode = [[ASImageNode alloc] init];
-    _placeholderImageNode.layerBacked = YES;
-    _placeholderImageNode.contentMode = ASContentModeFromVideoGravity(_gravity);
+  if (image != nil) {
+    self.contentMode = ASContentModeFromVideoGravity(_gravity);
   }
-
-  _placeholderImageNode.image = image;
-
-  ASPerformBlockOnMainThread(^{
-    ASDN::MutexLocker l(_videoLock);
-
-    if (_placeholderImageNode != nil) {
-      [self insertSubnode:_placeholderImageNode atIndex:0];
-      [self setNeedsLayout];
-    }
-  });
+  self.image = image;
 }
 
 - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
@@ -296,7 +282,7 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
     if ([change[NSKeyValueChangeNewKey] integerValue] == AVPlayerItemStatusReadyToPlay) {
       self.playerState = ASVideoNodePlayerStateReadyToPlay;
       // If we don't yet have a placeholder image update it now that we should have data available for it
-      if (_placeholderImageNode.image == nil) {
+      if (self.image == nil) {
         [self generatePlaceholderImage];
       }
     }
@@ -393,7 +379,6 @@ - (void)clearFetchedData
 
     self.player = nil;
     self.currentItem = nil;
-    _placeholderImageNode.image = nil;
   }
 }
 
@@ -496,7 +481,7 @@ - (void)setGravity:(NSString *)gravity
   if (_playerNode.isNodeLoaded) {
     ((AVPlayerLayer *)_playerNode.layer).videoGravity = gravity;
   }
-  _placeholderImageNode.contentMode = ASContentModeFromVideoGravity(gravity);
+  self.contentMode = ASContentModeFromVideoGravity(gravity);
   _gravity = gravity;
 }
 
@@ -637,11 +622,6 @@ - (void)errorWhilePlaying:(NSNotification *)notification
 }
 
 #pragma mark - Internal Properties
-- (ASImageNode *)placeholderImageNode
-{
-  ASDN::MutexLocker l(_videoLock);
-  return _placeholderImageNode;
-}
 
 - (AVPlayerItem *)currentItem
 {

AsyncDisplayKitTests/ASVideoNodeTests.m

@@ -29,7 +29,6 @@ @interface ASVideoNode () {
 }
 @property (atomic, readwrite) ASInterfaceState interfaceState;
 @property (atomic, readonly) ASDisplayNode *spinner;
-@property (atomic, readonly) ASImageNode *placeholderImageNode;
 @property (atomic, readwrite) ASDisplayNode *playerNode;
 @property (atomic, readwrite) AVPlayer *player;
 @property (atomic, readwrite) BOOL shouldBePlaying;
@@ -355,16 +354,16 @@ - (void)testVideoResumedWhenBufferIsLikelyToKeepUp
 - (void)testSettingVideoGravityChangesPlaceholderContentMode
 {
   [_videoNode setVideoPlaceholderImage:[[UIImage alloc] init]];
-  XCTAssertEqual(UIViewContentModeScaleAspectFit, _videoNode.placeholderImageNode.contentMode);
+  XCTAssertEqual(UIViewContentModeScaleAspectFit, _videoNode.contentMode);
 
   _videoNode.gravity = AVLayerVideoGravityResize;
-  XCTAssertEqual(UIViewContentModeScaleToFill, _videoNode.placeholderImageNode.contentMode);
+  XCTAssertEqual(UIViewContentModeScaleToFill, _videoNode.contentMode);
 
   _videoNode.gravity = AVLayerVideoGravityResizeAspect;
-  XCTAssertEqual(UIViewContentModeScaleAspectFit, _videoNode.placeholderImageNode.contentMode);
+  XCTAssertEqual(UIViewContentModeScaleAspectFit, _videoNode.contentMode);
 
   _videoNode.gravity = AVLayerVideoGravityResizeAspectFill;
-  XCTAssertEqual(UIViewContentModeScaleAspectFill, _videoNode.placeholderImageNode.contentMode);
+  XCTAssertEqual(UIViewContentModeScaleAspectFill, _videoNode.contentMode);
 }
 
 - (void)testChangingAssetsChangesPlaceholderImage
@@ -373,10 +372,10 @@ - (void)testChangingAssetsChangesPlaceholderImage
 
   _videoNode.asset = _firstAsset;
   [_videoNode setVideoPlaceholderImage:firstImage];
-  XCTAssertEqual(firstImage, _videoNode.placeholderImageNode.image);
+  XCTAssertEqual(firstImage, _videoNode.image);
 
   _videoNode.asset = _secondAsset;
-  XCTAssertNotEqual(firstImage, _videoNode.placeholderImageNode.image);
+  XCTAssertNotEqual(firstImage, _videoNode.image);
 }
 
 - (void)testClearingFetchedContentShouldClearAssetData
@@ -397,12 +396,12 @@ - (void)testClearingFetchedContentShouldClearAssetData
 
   XCTAssertNotNil(_videoNode.player);
   XCTAssertNotNil(_videoNode.currentItem);
-  XCTAssertNotNil(_videoNode.placeholderImageNode.image);
+  XCTAssertNotNil(_videoNode.image);
 
   [_videoNode clearFetchedData];
   XCTAssertNil(_videoNode.player);
   XCTAssertNil(_videoNode.currentItem);
-  XCTAssertNil(_videoNode.placeholderImageNode.image);
+  XCTAssertNil(_videoNode.image);
 }
 
 @end