[ASCollectionView] Relayout Nodes as Soon as Bounds Changes (#2121)

* Add failing test case for ASCollectionView rotation

* [ASCollectionView] Relayout nodes immediately on bounds change

AsyncDisplayKit/ASCollectionView.mm

@@ -86,7 +86,7 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 #pragma mark -
 #pragma mark ASCollectionView.
 
-@interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDelegate, ASCollectionDataControllerSource, ASCellNodeInteractionDelegate, ASDelegateProxyInterceptor, ASBatchFetchingScrollView, ASDataControllerEnvironmentDelegate> {
+@interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDelegate, ASCollectionDataControllerSource, ASCellNodeInteractionDelegate, ASDelegateProxyInterceptor, ASBatchFetchingScrollView, ASDataControllerEnvironmentDelegate, ASCALayerExtendedDelegate> {
   ASCollectionViewProxy *_proxyDataSource;
   ASCollectionViewProxy *_proxyDelegate;
   
@@ -106,8 +106,8 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
   
   ASBatchContext *_batchContext;
   
-  CGSize _maxSizeForNodesConstrainedSize;
-  BOOL _ignoreMaxSizeChange;
+  CGSize _lastBoundsSizeUsedForMeasuringNodes;
+  BOOL _ignoreNextBoundsSizeChangeForMeasuringNodes;
   
   NSMutableSet *_registeredSupplementaryKinds;
   
@@ -242,10 +242,10 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
   
   _superIsPendingDataLoad = YES;
   
-  _maxSizeForNodesConstrainedSize = self.bounds.size;
+  _lastBoundsSizeUsedForMeasuringNodes = self.bounds.size;
   // If the initial size is 0, expect a size change very soon which is part of the initial configuration
   // and should not trigger a relayout.
-  _ignoreMaxSizeChange = CGSizeEqualToSize(_maxSizeForNodesConstrainedSize, CGSizeZero);
+  _ignoreNextBoundsSizeChangeForMeasuringNodes = CGSizeEqualToSize(_lastBoundsSizeUsedForMeasuringNodes, CGSizeZero);
   
   _layoutFacilitator = layoutFacilitator;
   
@@ -843,26 +843,6 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
     self.contentInset = UIEdgeInsetsZero;
   }
   
-  if (! CGSizeEqualToSize(_maxSizeForNodesConstrainedSize, self.bounds.size)) {
-    _maxSizeForNodesConstrainedSize = self.bounds.size;
-    
-    // First size change occurs during initial configuration. An expensive relayout pass is unnecessary at that time
-    // and should be avoided, assuming that the initial data loading automatically runs shortly afterward.
-    if (_ignoreMaxSizeChange) {
-      _ignoreMaxSizeChange = NO;
-    } else {
-      // This actually doesn't perform an animation, but prevents the transaction block from being processed in the
-      // data controller's prevent animation block that would interrupt an interrupted relayout happening in an animation block
-      // ie. ASCollectionView bounds change on rotation or multi-tasking split view resize.
-      [self performBatchAnimated:YES updates:^{
-        [_dataController relayoutAllNodes];
-      } completion:nil];
-      // We need to ensure the size requery is done before we update our layout.
-      [self waitUntilAllUpdatesAreCommitted];
-      [self.collectionViewLayout invalidateLayout];
-    }
-  }
-  
   // Flush any pending invalidation action if needed.
   ASCollectionViewInvalidationStyle invalidationStyle = _nextLayoutInvalidationStyle;
   _nextLayoutInvalidationStyle = ASCollectionViewInvalidationStyleNone;
@@ -1309,6 +1289,40 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
   }
 }
 
+#pragma mark ASCALayerExtendedDelegate
+
+/**
+ * UICollectionView inadvertently triggers a -prepareLayout call to its layout object
+ * between [super setFrame:] and [self layoutSubviews] during size changes. So we need
+ * to get in there and re-measure our nodes before that -prepareLayout call.
+ * We can't wait until -layoutSubviews or the end of -setFrame:.
+ *
+ * @see @p testThatNodeCalculatedSizesAreUpdatedBeforeFirstPrepareLayoutAfterRotation
+ */
+- (void)layer:(CALayer *)layer didChangeBoundsWithOldValue:(CGRect)oldBounds newValue:(CGRect)newBounds
+{
+  if (CGSizeEqualToSize(_lastBoundsSizeUsedForMeasuringNodes, newBounds.size)) {
+    return;
+  }
+  _lastBoundsSizeUsedForMeasuringNodes = newBounds.size;
+
+  // First size change occurs during initial configuration. An expensive relayout pass is unnecessary at that time
+  // and should be avoided, assuming that the initial data loading automatically runs shortly afterward.
+  if (_ignoreNextBoundsSizeChangeForMeasuringNodes) {
+    _ignoreNextBoundsSizeChangeForMeasuringNodes = NO;
+  } else {
+    // This actually doesn't perform an animation, but prevents the transaction block from being processed in the
+    // data controller's prevent animation block that would interrupt an interrupted relayout happening in an animation block
+    // ie. ASCollectionView bounds change on rotation or multi-tasking split view resize.
+    [self performBatchAnimated:YES updates:^{
+      [_dataController relayoutAllNodes];
+    } completion:nil];
+    // We need to ensure the size requery is done before we update our layout.
+    [self waitUntilAllUpdatesAreCommitted];
+    [self.collectionViewLayout invalidateLayout];
+  }
+}
+
 #pragma mark - UICollectionView dead-end intercepts
 
 #if ASDISPLAYNODE_ASSERTIONS_ENABLED // Remove implementations entirely for efficiency if not asserting.

AsyncDisplayKit/Details/_ASDisplayLayer.h

@@ -69,6 +69,20 @@ typedef BOOL(^asdisplaynode_iscancelled_block_t)(void);
 
 @end
 
+/**
+ * Optional methods that the view associated with an _ASDisplayLayer can implement. 
+ * This is distinguished from _ASDisplayLayerDelegate in that it points to the _view_
+ * not the node. Unfortunately this is required by ASCollectionView, since we currently
+ * can't guarantee that an ASCollectionNode exists for it.
+ */
+@protocol ASCALayerExtendedDelegate
+
+@optional
+
+- (void)layer:(CALayer *)layer didChangeBoundsWithOldValue:(CGRect)oldBounds newValue:(CGRect)newBounds;
+
+@end
+
 /**
  Implement one of +displayAsyncLayer:parameters:isCancelled: or +drawRect:withParameters:isCancelled: to provide drawing for your node.
  Use -drawParametersForAsyncLayer: to copy any properties that are involved in drawing into an immutable object for use on the display queue.

AsyncDisplayKit/Details/_ASDisplayLayer.mm

@@ -25,6 +25,10 @@
   ASDN::RecursiveMutex _displaySuspendedLock;
   BOOL _displaySuspended;
 
+  struct {
+    BOOL delegateDidChangeBounds:1;
+  } _delegateFlags;
+
   id<_ASDisplayLayerDelegate> __weak _asyncDelegate;
 }
 
@@ -52,6 +56,12 @@
   return _asyncDelegate;
 }
 
+- (void)setDelegate:(id)delegate
+{
+  [super setDelegate:delegate];
+  _delegateFlags.delegateDidChangeBounds = [delegate respondsToSelector:@selector(layer:didChangeBoundsWithOldValue:newValue:)];
+}
+
 - (void)setAsyncDelegate:(id<_ASDisplayLayerDelegate>)asyncDelegate
 {
   ASDisplayNodeAssert(!asyncDelegate || [asyncDelegate isKindOfClass:[ASDisplayNode class]], @"_ASDisplayLayer is inherently coupled to ASDisplayNode and cannot be used with another asyncDelegate.  Please rethink what you are trying to do.");
@@ -82,8 +92,15 @@
 
 - (void)setBounds:(CGRect)bounds
 {
+  if (_delegateFlags.delegateDidChangeBounds) {
+    CGRect oldBounds = self.bounds;
     [super setBounds:bounds];
     self.asyncdisplaykit_node.threadSafeBounds = bounds;
+    [self.delegate layer:self didChangeBoundsWithOldValue:oldBounds newValue:bounds];
+  } else {
+    [super setBounds:bounds];
+    self.asyncdisplaykit_node.threadSafeBounds = bounds;
+  }
 }
 
 #if DEBUG // These override is strictly to help detect application-level threading errors.  Avoid method overhead in release.

AsyncDisplayKitTests/ASCollectionViewTests.mm

@@ -16,6 +16,7 @@
 #import "ASCollectionNode.h"
 #import "ASDisplayNode+Beta.h"
 #import <vector>
+#import <OCMock/OCMock.h>
 
 @interface ASTextCellNodeWithSetSelectedCounter : ASTextCellNode
 
@@ -92,9 +93,10 @@
   if (self) {
     // Populate these immediately so that they're not unexpectedly nil during tests.
     self.asyncDelegate = [[ASCollectionViewTestDelegate alloc] initWithNumberOfSections:10 numberOfItemsInSection:10];
-    
+    id realLayout = [UICollectionViewFlowLayout new];
+    id mockLayout = [OCMockObject partialMockForObject:realLayout];
     self.collectionView = [[ASCollectionView alloc] initWithFrame:self.view.bounds
-                                             collectionViewLayout:[UICollectionViewFlowLayout new]];
+                                             collectionViewLayout:mockLayout];
     self.collectionView.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;
     self.collectionView.asyncDataSource = self.asyncDelegate;
     self.collectionView.asyncDelegate = self.asyncDelegate;
@@ -249,6 +251,7 @@
   __unused ASCollectionViewTestDelegate *del = testController.asyncDelegate;\
   __unused ASCollectionView *cv = testController.collectionView;\
   UIWindow *window = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];\
+  [window makeKeyAndVisible]; \
   window.rootViewController = testController;\
   \
   [testController.collectionView reloadDataImmediately];\
@@ -345,4 +348,36 @@
   } completion:nil]);
 }
 
+
+- (void)testThatNodeCalculatedSizesAreUpdatedBeforeFirstPrepareLayoutAfterRotation
+{
+  updateValidationTestPrologue
+  id layout = cv.collectionViewLayout;
+  CGSize initialItemSize = [cv calculatedSizeForNodeAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]];
+  CGSize initialCVSize = cv.bounds.size;
+
+  // Capture the node size before first call to prepareLayout after frame change.
+  __block CGSize itemSizeAtFirstLayout = CGSizeZero;
+  __block CGSize boundsSizeAtFirstLayout = CGSizeZero;
+  [[[[layout expect] andDo:^(NSInvocation *) {
+    itemSizeAtFirstLayout = [cv calculatedSizeForNodeAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]];
+    boundsSizeAtFirstLayout = [cv bounds].size;
+  }] andForwardToRealObject] prepareLayout];
+
+  // Rotate the device
+  UIDeviceOrientation oldDeviceOrientation = [[UIDevice currentDevice] orientation];
+  [[UIDevice currentDevice] setValue:@(UIDeviceOrientationLandscapeLeft) forKey:@"orientation"];
+
+  CGSize finalItemSize = [cv calculatedSizeForNodeAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]];
+  CGSize finalCVSize = cv.bounds.size;
+  XCTAssertNotEqualObjects(NSStringFromCGSize(initialItemSize),  NSStringFromCGSize(itemSizeAtFirstLayout));
+  XCTAssertNotEqualObjects(NSStringFromCGSize(initialCVSize),  NSStringFromCGSize(boundsSizeAtFirstLayout));
+  XCTAssertEqualObjects(NSStringFromCGSize(itemSizeAtFirstLayout), NSStringFromCGSize(finalItemSize));
+  XCTAssertEqualObjects(NSStringFromCGSize(boundsSizeAtFirstLayout), NSStringFromCGSize(finalCVSize));
+  [layout verify];
+
+  // Teardown
+  [[UIDevice currentDevice] setValue:@(oldDeviceOrientation) forKey:@"orientation"];
+}
+
 @end