Ignore Relayout Requests for Deleted Cell Nodes (#279)

* Data Controller: Ignore relayout requests for elements that have been deleted in the mean-time. * Bolster our logchange * Add sanity check
TextureGroup · May 16, 2017 · 432018c · 432018c
1 parent e6accc7
commit 432018c
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,3 +22,4 @@
 - [ASVideoNode] Added error reporing to ASVideoNode and it's delegate [#260](https://github.com/TextureGroup/Texture/pull/260)
 - [ASCollectionNode] Fixed conversion of item index paths between node & view. [Adlai Holler](https://github.com/Adlai-Holler) [#262](https://github.com/TextureGroup/Texture/pull/262)
 - [Layout] Extract layout implementation code into it's own subcategories [Michael Schneider] (https://github.com/maicki)[#272](https://github.com/TextureGroup/Texture/pull/272)
+- [Fix] Fix a potential crash when cell nodes that need layout are deleted during the same runloop.  [Adlai Holler](https://github.com/Adlai-Holler) [#279](https://github.com/TextureGroup/Texture/pull/279)
diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm
@@ -1664,9 +1664,12 @@ - (NSUInteger)numberOfSectionsInDataController:(ASDataController *)dataControlle
 - (BOOL)dataController:(ASDataController *)dataController presentedSizeForElement:(ASCollectionElement *)element matchesSize:(CGSize)size
 {
   NSIndexPath *indexPath = [self indexPathForNode:element.node];
+  if (indexPath == nil) {
+    ASDisplayNodeFailAssert(@"Data controller should not ask for presented size for element that is not presented.");
+    return YES;
+  }
   UICollectionViewLayoutAttributes *attributes = [self layoutAttributesForItemAtIndexPath:indexPath];
-  CGRect rect = attributes.frame;
-  return CGSizeEqualToSizeWithIn(rect.size, size, FLT_EPSILON);
+  return CGSizeEqualToSizeWithIn(attributes.size, size, FLT_EPSILON);
 
 }
 

diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm
@@ -1750,6 +1750,10 @@ - (NSUInteger)numberOfSectionsInDataController:(ASDataController *)dataControlle
 - (BOOL)dataController:(ASDataController *)dataController presentedSizeForElement:(ASCollectionElement *)element matchesSize:(CGSize)size
 {
   NSIndexPath *indexPath = [self indexPathForNode:element.node];
+  if (indexPath == nil) {
+    ASDisplayNodeFailAssert(@"Data controller should not ask for presented size for element that is not presented.");
+    return YES;
+  }
   CGRect rect = [self rectForRowAtIndexPath:indexPath];
 
   /**

diff --git a/Source/Details/ASDataController.h b/Source/Details/ASDataController.h
@@ -71,7 +71,8 @@ extern NSString * const ASCollectionInvalidUpdateException;
 - (NSUInteger)numberOfSectionsInDataController:(ASDataController *)dataController;
 
 /**
- Returns if the collection element size matches a given size
+ Returns if the collection element size matches a given size.
+ @precondition The element is present in the data controller's visible map.
  */
 - (BOOL)dataController:(ASDataController *)dataController presentedSizeForElement:(ASCollectionElement *)element matchesSize:(CGSize)size;
 

diff --git a/Source/Details/ASDataController.mm b/Source/Details/ASDataController.mm
@@ -717,10 +717,20 @@ - (void)relayoutNodes:(id<NSFastEnumeration>)nodes nodesSizeChanged:(NSMutableAr
   }
 
   id<ASDataControllerSource> dataSource = self.dataSource;
+  auto visibleMap = self.visibleMap;
+  auto pendingMap = self.pendingMap;
   for (ASCellNode *node in nodes) {
-    ASSizeRange constrainedSize = [self constrainedSizeForElement:node.collectionElement inElementMap:_pendingMap];
+    auto element = node.collectionElement;
+    // Ensure the element is present in both maps or skip it. If it's not in the visible map,
+    // then we can't check the presented size. If it's not in the pending map, we can't get the constrained size.
+    // This will only happen if the element has been deleted, so the specifics of this behavior aren't important.
+    if ([visibleMap indexPathForElement:element] == nil || [pendingMap indexPathForElement:element] == nil) {
+      continue;
+    }
+
+    ASSizeRange constrainedSize = [self constrainedSizeForElement:element inElementMap:pendingMap];
     [self _layoutNode:node withConstrainedSize:constrainedSize];
-    BOOL matchesSize = [dataSource dataController:self presentedSizeForElement:node.collectionElement matchesSize:node.frame.size];
+    BOOL matchesSize = [dataSource dataController:self presentedSizeForElement:element matchesSize:node.frame.size];
     if (! matchesSize) {
       [nodesSizesChanged addObject:node];
     }