From e3b91eab697ff5f9bdf56e59991f5a7485607b2e Mon Sep 17 00:00:00 2001
From: Adlai Holler <guy.on.internet@icloud.com>
Date: Mon, 19 Jun 2017 11:19:06 -0700
Subject: [PATCH 1/4] Fix double-reload issue with ASCollectionNode

---
 Source/ASCollectionNode.mm                    | 23 ++++++---
 Source/ASCollectionView.h                     |  6 +--
 Source/ASCollectionView.mm                    | 51 ++++++-------------
 Source/Details/ASCollectionInternal.h         |  5 ++
 .../Private/ASCollectionView+Undeprecated.h   | 24 ---------
 Tests/ASCollectionModernDataSourceTests.m     | 32 +++++++-----
 Tests/ASCollectionViewTests.mm                | 13 +++--
 7 files changed, 66 insertions(+), 88 deletions(-)

diff --git a/Source/ASCollectionNode.mm b/Source/ASCollectionNode.mm
index 3412dd67e..69912ab5e 100644
--- a/Source/ASCollectionNode.mm
+++ b/Source/ASCollectionNode.mm
@@ -28,6 +28,7 @@
 #import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
 #import <AsyncDisplayKit/ASInternalHelpers.h>
 #import <AsyncDisplayKit/ASCellNode+Internal.h>
+#import <AsyncDisplayKit/_ASHierarchyChangeSet.h>
 #import <AsyncDisplayKit/AsyncDisplayKit+Debug.h>
 #import <AsyncDisplayKit/ASSectionContext.h>
 #import <AsyncDisplayKit/ASDataController.h>
@@ -682,9 +683,17 @@ - (void)waitUntilAllUpdatesAreCommitted
 - (void)reloadDataWithCompletion:(void (^)())completion
 {
   ASDisplayNodeAssertMainThread();
-  if (self.nodeLoaded) {
-    [self.view reloadDataWithCompletion:completion];
+  if (!self.nodeLoaded) {
+    return;
   }
+  
+  [self performBatchUpdates:^{
+    [self.view.changeSet reloadData];
+  } completion:^(BOOL finished){
+    if (completion) {
+      completion();
+    }
+  }];
 }
 
 - (void)reloadData
@@ -692,14 +701,16 @@ - (void)reloadData
   [self reloadDataWithCompletion:nil];
 }
 
-- (void)relayoutItems
+- (void)reloadDataImmediately
 {
-  [self.view relayoutItems];
+  ASDisplayNodeAssertMainThread();
+  [self reloadData];
+  [self waitUntilAllUpdatesAreCommitted];
 }
 
-- (void)reloadDataImmediately
+- (void)relayoutItems
 {
-  [self.view reloadDataImmediately];
+  [self.view relayoutItems];
 }
 
 - (void)beginUpdates
diff --git a/Source/ASCollectionView.h b/Source/ASCollectionView.h
index 2972ced49..b2f015cf5 100644
--- a/Source/ASCollectionView.h
+++ b/Source/ASCollectionView.h
@@ -266,14 +266,14 @@ NS_ASSUME_NONNULL_BEGIN
  * the main thread.
  * @warning This method is substantially more expensive than UICollectionView's version.
  */
-- (void)reloadDataWithCompletion:(nullable void (^)())completion ASDISPLAYNODE_DEPRECATED_MSG("Use ASCollectionNode method instead.");
+- (void)reloadDataWithCompletion:(nullable void (^)())completion AS_UNAVAILABLE("Use ASCollectionNode method instead.");
 
 /**
  * Reload everything from scratch, destroying the working range and all cached nodes.
  *
  * @warning This method is substantially more expensive than UICollectionView's version.
  */
-- (void)reloadData ASDISPLAYNODE_DEPRECATED_MSG("Use ASCollectionNode method instead.");
+- (void)reloadData AS_UNAVAILABLE("Use ASCollectionNode method instead.");
 
 /**
  * Reload everything from scratch entirely on the main thread, destroying the working range and all cached nodes.
@@ -281,7 +281,7 @@ NS_ASSUME_NONNULL_BEGIN
  * @warning This method is substantially more expensive than UICollectionView's version and will block the main thread
  * while all the cells load.
  */
-- (void)reloadDataImmediately ASDISPLAYNODE_DEPRECATED_MSG("Use ASCollectionNode's -reloadDataWithCompletion: followed by -waitUntilAllUpdatesAreCommitted instead.");
+- (void)reloadDataImmediately AS_UNAVAILABLE("Use ASCollectionNode method instead.");
 
 /**
  * Triggers a relayout of all nodes.
diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm
index 68b4c3468..906268349 100644
--- a/Source/ASCollectionView.mm
+++ b/Source/ASCollectionView.mm
@@ -144,11 +144,6 @@ @interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDe
    * (0 sections) we always check at least once after each update (initial reload is the first update.)
    */
   BOOL _hasEverCheckedForBatchFetchingDueToUpdate;
-
-  /**
-   * The change set that we're currently building, if any.
-   */
-  _ASHierarchyChangeSet *_changeSet;
   
   /**
    * Counter used to keep track of nested batch updates.
@@ -333,31 +328,23 @@ - (void)dealloc
 #pragma mark -
 #pragma mark Overrides.
 
-- (void)reloadDataWithCompletion:(void (^)())completion
-{
-  ASDisplayNodeAssertMainThread();
-  
-  if (! _dataController.initialReloadDataHasBeenCalled) {
-    // If this is the first reload, forward to super immediately to prevent it from triggering more "initial" loads while our data controller is working.
-    _superIsPendingDataLoad = YES;
-    [super reloadData];
-  }
-  
-  void (^batchUpdatesCompletion)(BOOL);
-  if (completion) {
-    batchUpdatesCompletion = ^(BOOL) {
-      completion();
-    };
-  }
-  
-  [self performBatchUpdates:^{
-    [_changeSet reloadData];
-  } completion:batchUpdatesCompletion];
-}
-
+/**
+ * This method is not available to be called by the public i.e.
+ * it should only be called by UICollectionView itself. UICollectionView
+ * does this e.g. during the first layout pass, or if you call -numberOfSections
+ * before its content is loaded.
+ */
 - (void)reloadData
 {
-  [self reloadDataWithCompletion:nil];
+  [super reloadData];
+
+  // UICollectionView calls -reloadData during first layoutSubviews and when the data source changes.
+  // This fires off the first load of cell nodes.
+  if (_asyncDataSource != nil && !self.dataController.initialReloadDataHasBeenCalled) {
+    [self performBatchUpdates:^{
+      [_changeSet reloadData];
+    } completion:nil];
+  }
 }
 
 - (void)scrollToItemAtIndexPath:(NSIndexPath *)indexPath atScrollPosition:(UICollectionViewScrollPosition)scrollPosition animated:(BOOL)animated
@@ -367,13 +354,6 @@ - (void)scrollToItemAtIndexPath:(NSIndexPath *)indexPath atScrollPosition:(UICol
   }
 }
 
-- (void)reloadDataImmediately
-{
-  ASDisplayNodeAssertMainThread();
-  [self reloadData];
-  [self waitUntilAllUpdatesAreCommitted];
-}
-
 - (void)relayoutItems
 {
   [_dataController relayoutAllNodes];
@@ -1721,7 +1701,6 @@ - (BOOL)dataController:(ASDataController *)dataController presentedSizeForElemen
   }
   UICollectionViewLayoutAttributes *attributes = [self layoutAttributesForItemAtIndexPath:indexPath];
   return CGSizeEqualToSizeWithIn(attributes.size, size, FLT_EPSILON);
-  
 }
 
 #pragma mark - ASDataControllerSource optional methods
diff --git a/Source/Details/ASCollectionInternal.h b/Source/Details/ASCollectionInternal.h
index bfab0bf94..28e62f827 100644
--- a/Source/Details/ASCollectionInternal.h
+++ b/Source/Details/ASCollectionInternal.h
@@ -31,6 +31,11 @@ NS_ASSUME_NONNULL_BEGIN
 @property (nonatomic, strong, readonly) ASDataController *dataController;
 @property (nonatomic, strong, readonly) ASRangeController *rangeController;
 
+/**
+ * The change set that we're currently building, if any.
+ */
+@property (nonatomic, strong, nullable, readonly) _ASHierarchyChangeSet *changeSet;
+
 /**
  * @see ASCollectionNode+Beta.h for full documentation.
  */
diff --git a/Source/Private/ASCollectionView+Undeprecated.h b/Source/Private/ASCollectionView+Undeprecated.h
index 2764d479a..be3a3f1a0 100644
--- a/Source/Private/ASCollectionView+Undeprecated.h
+++ b/Source/Private/ASCollectionView+Undeprecated.h
@@ -163,30 +163,6 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)performBatchUpdates:(nullable AS_NOESCAPE void (^)())updates completion:(nullable void (^)(BOOL finished))completion;
 
-/**
- * Reload everything from scratch, destroying the working range and all cached nodes.
- *
- * @param completion block to run on completion of asynchronous loading or nil. If supplied, the block is run on
- * the main thread.
- * @warning This method is substantially more expensive than UICollectionView's version.
- */
-- (void)reloadDataWithCompletion:(nullable void (^)())completion;
-
-/**
- * Reload everything from scratch, destroying the working range and all cached nodes.
- *
- * @warning This method is substantially more expensive than UICollectionView's version.
- */
-- (void)reloadData;
-
-/**
- * Reload everything from scratch entirely on the main thread, destroying the working range and all cached nodes.
- *
- * @warning This method is substantially more expensive than UICollectionView's version and will block the main thread
- * while all the cells load.
- */
-- (void)reloadDataImmediately;
-
 /**
  * Triggers a relayout of all nodes.
  *
diff --git a/Tests/ASCollectionModernDataSourceTests.m b/Tests/ASCollectionModernDataSourceTests.m
index 4a53bda1a..ec97e8c64 100644
--- a/Tests/ASCollectionModernDataSourceTests.m
+++ b/Tests/ASCollectionModernDataSourceTests.m
@@ -151,26 +151,30 @@ - (void)testReloadingASection
 
 - (void)loadInitialData
 {
-  /// BUG: these methods are called twice in a row i.e. this for-loop shouldn't be here. https://github.com/TextureGroup/Texture/issues/351
+  // Count methods are called twice in a row for first data load.
+  // Since -reloadData is routed through our batch update system,
+  // the batch update latches the "old data source counts" if needed at -beginUpdates time
+  // and then verifies them against the "new data source counts" after the updates.
+  // This isn't ideal, but the cost is very small and the system works well.
   for (int i = 0; i < 2; i++) {
     // It reads all the counts
     [self expectDataSourceCountMethods];
+  }
 
-    // It reads each section object.
-    for (NSInteger section = 0; section < sections.count; section++) {
-      [self expectContextMethodForSection:section];
-    }
+  // It reads each section object.
+  for (NSInteger section = 0; section < sections.count; section++) {
+    [self expectContextMethodForSection:section];
+  }
 
-    // It reads the contents for each item.
-    for (NSInteger section = 0; section < sections.count; section++) {
-      NSArray *viewModels = sections[section].viewModels;
+  // It reads the contents for each item.
+  for (NSInteger section = 0; section < sections.count; section++) {
+    NSArray *viewModels = sections[section].viewModels;
 
-      // For each item:
-      for (NSInteger i = 0; i < viewModels.count; i++) {
-        NSIndexPath *indexPath = [NSIndexPath indexPathForItem:i inSection:section];
-        [self expectViewModelMethodForItemAtIndexPath:indexPath viewModel:viewModels[i]];
-        [self expectNodeBlockMethodForItemAtIndexPath:indexPath];
-      }
+    // For each item:
+    for (NSInteger i = 0; i < viewModels.count; i++) {
+      NSIndexPath *indexPath = [NSIndexPath indexPathForItem:i inSection:section];
+      [self expectViewModelMethodForItemAtIndexPath:indexPath viewModel:viewModels[i]];
+      [self expectNodeBlockMethodForItemAtIndexPath:indexPath];
     }
   }
 
diff --git a/Tests/ASCollectionViewTests.mm b/Tests/ASCollectionViewTests.mm
index b201f63a0..ca2b45cb0 100644
--- a/Tests/ASCollectionViewTests.mm
+++ b/Tests/ASCollectionViewTests.mm
@@ -259,7 +259,8 @@ - (void)testSelection
   [window setRootViewController:testController];
   [window makeKeyAndVisible];
   
-  [testController.collectionView reloadDataImmediately];
+  [testController.collectionNode reloadData];
+  [testController.collectionNode waitUntilAllUpdatesAreCommitted];
   [testController.collectionView layoutIfNeeded];
   
   NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:0];
@@ -390,12 +391,13 @@ - (void)testThatCollectionNodeConformsToExpectedProtocols
   ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil];\
   __unused ASCollectionViewTestDelegate *del = testController.asyncDelegate;\
   __unused ASCollectionView *cv = testController.collectionView;\
-  __unused ASCollectionNode *cn = testController.collectionNode;\
+  ASCollectionNode *cn = testController.collectionNode;\
   UIWindow *window = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];\
   [window makeKeyAndVisible]; \
   window.rootViewController = testController;\
   \
-  [testController.collectionView reloadDataImmediately];\
+  [cn reloadData];\
+  [cn waitUntilAllUpdatesAreCommitted]; \
   [testController.collectionView layoutIfNeeded];
 
 - (void)testThatSubmittingAValidInsertDoesNotThrowAnException
@@ -620,7 +622,7 @@ - (void)testThatDisappearingSupplementariesWithLayerBackedNodesDontFailAssert
   for (NSInteger i = 0; i < 2; i++) {
     // NOTE: waitUntilAllUpdatesAreCommitted or reloadDataImmediately is not sufficient here!!
     XCTestExpectation *done = [self expectationWithDescription:[NSString stringWithFormat:@"Reload #%td complete", i]];
-    [cv reloadDataWithCompletion:^{
+    [cn reloadDataWithCompletion:^{
       [done fulfill];
     }];
     [self waitForExpectationsWithTimeout:1 handler:nil];
@@ -752,7 +754,8 @@ - (void)testThatSectionContextsAreCorrectAfterReloadData
   updateValidationTestPrologue
   
   del.sectionGeneration++;
-  [cv reloadDataImmediately];
+  [cn reloadData];
+  [cn waitUntilAllUpdatesAreCommitted];
   
   NSInteger sectionCount = del->_itemCounts.size();
   for (NSInteger section = 0; section < sectionCount; section++) {

From 12404b3d90be5ec6aa3e4c7ea977274d89322f17 Mon Sep 17 00:00:00 2001
From: Adlai Holler <guy.on.internet@icloud.com>
Date: Mon, 19 Jun 2017 11:24:00 -0700
Subject: [PATCH 2/4] Add a message to the change pile

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e26c62f89..fb5adc85a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@
 - [Yoga] Implement ASYogaLayoutSpec, a simplified integration strategy for Yoga-powered layout calculation. [Scott Goodson](https://github.com/appleguy)
 - Fixed an issue where calls to setNeedsDisplay and setNeedsLayout would stop working on loaded nodes. [Garrett Moon](https://github.com/garrettmoon)
 - Migrated unit tests to OCMock 3.4 (from 2.2) and improved the multiplex image node tests. [Adlai Holler](https://github.com/Adlai-Holler)
+- Fix CollectionNode double-load issue. This should significantly improve performance in cases where a collection node has content immediately available on first layout i.e. not fetched from the network. [Adlai Holler](https://github.com/Adlai-Holler)
 
 ## 2.3.3
 - [ASTextKitFontSizeAdjuster] Replace use of NSAttributedString's boundingRectWithSize:options:context: with NSLayoutManager's boundingRectForGlyphRange:inTextContainer: [Ricky Cancro](https://github.com/rcancro)

From b71d643a00feaf205e3a55bdf1213abbf743b6b6 Mon Sep 17 00:00:00 2001
From: Adlai Holler <guy.on.internet@icloud.com>
Date: Mon, 19 Jun 2017 11:29:29 -0700
Subject: [PATCH 3/4] Fix some license headers

---
 Tests/ASCollectionViewTests.mm    | 2 +-
 Tests/ASDisplayNodeLayoutTests.mm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Tests/ASCollectionViewTests.mm b/Tests/ASCollectionViewTests.mm
index ca2b45cb0..00cf58d2b 100644
--- a/Tests/ASCollectionViewTests.mm
+++ b/Tests/ASCollectionViewTests.mm
@@ -1,5 +1,5 @@
 //
-//  ASCollectionViewTests.m
+//  ASCollectionViewTests.mm
 //  Texture
 //
 //  Copyright (c) 2014-present, Facebook, Inc.  All rights reserved.
diff --git a/Tests/ASDisplayNodeLayoutTests.mm b/Tests/ASDisplayNodeLayoutTests.mm
index 202b3b4fa..8688e5cec 100644
--- a/Tests/ASDisplayNodeLayoutTests.mm
+++ b/Tests/ASDisplayNodeLayoutTests.mm
@@ -1,5 +1,5 @@
 //
-//  ASDisplayNodeLayoutTests.m
+//  ASDisplayNodeLayoutTests.mm
 //  Texture
 //
 //  Copyright (c) 2014-present, Facebook, Inc.  All rights reserved.

From 65cf7a9a45b3c9df32755ae5503eb2dd4d277967 Mon Sep 17 00:00:00 2001
From: Adlai Holler <guy.on.internet@icloud.com>
Date: Mon, 19 Jun 2017 13:52:52 -0700
Subject: [PATCH 4/4] Address feedback

---
 Source/ASCollectionNode.mm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Source/ASCollectionNode.mm b/Source/ASCollectionNode.mm
index 69912ab5e..6c7e3799c 100644
--- a/Source/ASCollectionNode.mm
+++ b/Source/ASCollectionNode.mm
@@ -710,7 +710,10 @@ - (void)reloadDataImmediately
 
 - (void)relayoutItems
 {
-  [self.view relayoutItems];
+  ASDisplayNodeAssertMainThread();
+  if (self.nodeLoaded) {
+  	[self.view relayoutItems];
+  }
 }
 
 - (void)beginUpdates