[ASDisplayNode] Proper handling of constrainedSize (#2505)

* Check in ASLayout if size is valid for sizing instead of valid for layout

* Return constrainedSize from calculateSizeThatFits:

Remove invalid constrainedSize check within ASNetworkImageNode. Furthermore as ASDisplayNode does not return CGSizeZero anymore we have to give the display nodes we use in tests and are involving a stack spec an intrinsic content size.

* Remove extra constrainedSize handling in ASNetworkImageNode handling

* Change test to use FLT_MAX

AsyncDisplayKit/ASDisplayNode.mm

@@ -2491,7 +2491,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
 {
   __ASDisplayNodeCheckForLayoutMethodOverrides;
 
-  return CGSizeZero;
+  return constrainedSize;
 }
 
 - (id<ASLayoutElement>)_layoutElementThatFits:(ASSizeRange)constrainedSize

AsyncDisplayKit/ASImageNode.mm

@@ -190,7 +190,7 @@ struct ASImageNodeDrawParameters {
   ASDN::MutexLocker l(__instanceLock__);
 
   if (_image == nil) {
-    return constrainedSize;
+    return [super calculateSizeThatFits:constrainedSize];
   }
 
   return _image.size;

AsyncDisplayKit/ASNetworkImageNode.mm

@@ -331,23 +331,6 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0};
   }
 }
 
-#pragma mark - Layout and Sizing
-
-- (CGSize)calculateSizeThatFits:(CGSize)constrainedSize
-{
-  ASDN::MutexLocker l(__instanceLock__);
-
-  // If the image node is currently in the loading process and no valid size is applied return CGSizeZero for the time
-  // being.
-  // TODO: After 2.0 is stable we should remove this behavior as a ASNetworkImageNode is a replaced element and the
-  // client code should set the size of an image or it's container it's embedded in
-  if (ASIsCGSizeValidForSize(constrainedSize) == NO && _URL != nil && self.image == nil) {
-    return CGSizeZero;
-  }
-    
-  return [super calculateSizeThatFits:constrainedSize];
-}
-
 #pragma mark - Private methods, safe to call without lock
 
 - (void)handleProgressImage:(UIImage *)progressImage progress:(CGFloat)progress downloadIdentifier:(nullable id)downloadIdentifier

AsyncDisplayKit/Layout/ASLayout.mm

@@ -72,7 +72,7 @@ static inline NSString * descriptionIndents(NSUInteger indents)
     // Read this now to avoid @c weak overhead later.
     _layoutElementType = layoutElement.layoutElementType;
     
-    if (!ASIsCGSizeValidForLayout(size)) {
+    if (!ASIsCGSizeValidForSize(size)) {
       ASDisplayNodeAssert(NO, @"layoutSize is invalid and unsafe to provide to Core Animation! Release configurations will force to 0, 0.  Size = %@, node = %@", NSStringFromCGSize(size), layoutElement);
       size = CGSizeZero;
     } else {

AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m

@@ -59,11 +59,21 @@
 
 - (void)testInitialNodeInsertionWithOrdering
 {
+  static CGSize kSize = {100, 100};
+  
   ASDisplayNode *node1 = [[ASDisplayNode alloc] init];
   ASDisplayNode *node2 = [[ASDisplayNode alloc] init];
   ASDisplayNode *node3 = [[ASDisplayNode alloc] init];
   ASDisplayNode *node4 = [[ASDisplayNode alloc] init];
   ASDisplayNode *node5 = [[ASDisplayNode alloc] init];
+  
+  
+  // As we will involve a stack spec we have to give the nodes an intrinsic content size
+  node1.style.preferredSize = kSize;
+  node2.style.preferredSize = kSize;
+  node3.style.preferredSize = kSize;
+  node4.style.preferredSize = kSize;
+  node5.style.preferredSize = kSize;
 
   ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init];
   node.automaticallyManagesSubnodes = YES;
@@ -88,10 +98,17 @@
 
 - (void)testCalculatedLayoutHierarchyTransitions
 {
+  static CGSize kSize = {100, 100};
+  
   ASDisplayNode *node1 = [[ASDisplayNode alloc] init];
   ASDisplayNode *node2 = [[ASDisplayNode alloc] init];
   ASDisplayNode *node3 = [[ASDisplayNode alloc] init];
   
+  // As we will involve a stack spec we have to give the nodes an intrinsic content size
+  node1.style.preferredSize = kSize;
+  node2.style.preferredSize = kSize;
+  node3.style.preferredSize = kSize;
+  
   ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init];
   node.automaticallyManagesSubnodes = YES;
   node.layoutSpecBlock = ^(ASDisplayNode *weakNode, ASSizeRange constrainedSize){

AsyncDisplayKitTests/ASDisplayNodeTests.m

@@ -2151,4 +2151,21 @@ static bool stringContainsPointer(NSString *description, id p) {
   XCTAssertThrowsSpecificNamed([node calculateLayoutThatFits:ASSizeRangeMake(CGSizeMake(100, 100))], NSException, NSInternalInconsistencyException);
 }
 
+- (void)testThatLayoutWithInvalidSizeCausesException
+{
+  ASDisplayNode *displayNode = [[ASDisplayNode alloc] init];
+  ASDisplayNode *node = [[ASDisplayNode alloc] init];
+  node.layoutSpecBlock = ^ASLayoutSpec *(ASDisplayNode *node, ASSizeRange constrainedSize) {
+    return [ASWrapperLayoutSpec wrapperWithLayoutElement:displayNode];
+  };
+  
+  XCTAssertThrows([node layoutThatFits:ASSizeRangeMake(CGSizeMake(0, FLT_MAX))]);
+  
+  // This dance is necessary as we would assert in case we create an ASDimension that is not real numbers
+  ASDimension width = displayNode.style.width;
+  width.value = INFINITY;
+  displayNode.style.width = width;
+  XCTAssertThrows([node layoutThatFits:ASSizeRangeMake(CGSizeMake(0, 0), CGSizeMake(INFINITY, INFINITY))]);
+}
+
 @end