Respond to CR.

// FREEBIE

Signal/src/ViewControllers/ConversationView/Cells/ConversationViewCell.h

@@ -43,6 +43,10 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (NSAttributedString *)attributedContactOrProfileNameForPhoneIdentifier:(NSString *)recipientId;
 
+#pragma mark - Caching
+
+- (NSCache *)cellMediaCache;
+
 @end
 
 #pragma mark -

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m

@@ -269,15 +269,22 @@ NS_ASSUME_NONNULL_BEGIN
 // but lazy-load any expensive media (photo, gif, etc.) used in those views. Note that
 // this lazy-load can fail, in which case we modify the view hierarchy to use an "error"
 // state. The didCellMediaFailToLoad reflects media load fails.
-- (nullable id)tryToLoadCellMedia:(nullable id (^)())loadCellMediaBlock mediaView:(UIView *)mediaView
+- (nullable id)tryToLoadCellMedia:(nullable id (^)())loadCellMediaBlock
+                        mediaView:(UIView *)mediaView
+                         cacheKey:(NSString *)cacheKey
 {
     OWSAssert(self.attachmentStream);
     OWSAssert(mediaView);
+    OWSAssert(cacheKey);
 
     if (self.viewItem.didCellMediaFailToLoad) {
         return nil;
     }
-    id _Nullable cellMedia = self.viewItem.cachedCellMedia;
+
+    NSCache *cellMediaCache = self.delegate.cellMediaCache;
+    OWSAssert(cellMediaCache);
+
+    id _Nullable cellMedia = [cellMediaCache objectForKey:cacheKey];
     if (cellMedia) {
         DDLogVerbose(@"%@ cell media cache hit", self.logTag);
         return cellMedia;
@@ -285,7 +292,7 @@ NS_ASSUME_NONNULL_BEGIN
     cellMedia = loadCellMediaBlock();
     if (cellMedia) {
         DDLogVerbose(@"%@ cell media cache miss", self.logTag);
-        self.viewItem.cachedCellMedia = cellMedia;
+        [cellMediaCache setObject:cellMedia forKey:cacheKey];
     } else {
         DDLogError(@"%@ Failed to load cell media: %@", [self logTag], [self.attachmentStream mediaURL]);
         self.viewItem.didCellMediaFailToLoad = YES;
@@ -316,17 +323,14 @@ NS_ASSUME_NONNULL_BEGIN
                 OWSAssert([self.attachmentStream isImage]);
                 return self.attachmentStream.image;
             }
-                                                       mediaView:self.stillImageView];
+                                                       mediaView:self.stillImageView
+                                                        cacheKey:self.attachmentStream.uniqueId];
             break;
         }
         case OWSMessageCellType_AnimatedImage: {
             if (self.animatedImageView.image) {
                 return;
             }
-            DDLogError(@"%@ ---- ensureViewMediaState loading[%zd]: %@",
-                self.logTag,
-                self.viewItem.row,
-                self.viewItem.interaction.description);
             self.animatedImageView.image = [self tryToLoadCellMedia:^{
                 OWSAssert([self.attachmentStream isAnimated]);
 
@@ -337,23 +341,21 @@ NS_ASSUME_NONNULL_BEGIN
                 }
                 return animatedImage;
             }
-                                                          mediaView:self.animatedImageView];
+                                                          mediaView:self.animatedImageView
+                                                           cacheKey:self.attachmentStream.uniqueId];
             break;
         }
         case OWSMessageCellType_Video: {
             if (self.stillImageView.image) {
                 return;
             }
-            DDLogError(@"%@ ---- ensureViewMediaState loading[%zd]: %@",
-                self.logTag,
-                self.viewItem.row,
-                self.viewItem.interaction.description);
             self.stillImageView.image = [self tryToLoadCellMedia:^{
                 OWSAssert([self.attachmentStream isVideo]);
 
                 return self.attachmentStream.image;
             }
-                                                       mediaView:self.stillImageView];
+                                                       mediaView:self.stillImageView
+                                                        cacheKey:self.attachmentStream.uniqueId];
             break;
         }
         case OWSMessageCellType_TextMessage:

Signal/src/ViewControllers/ConversationView/ConversationCollectionView.m

@@ -57,6 +57,36 @@ NS_ASSUME_NONNULL_BEGIN
     [super setContentOffset:contentOffset];
 }
 
+- (void)reloadData
+{
+    DDLogVerbose(@"%@ reloadData", self.logTag);
+    [super reloadData];
+}
+
+- (void)reloadSections:(NSIndexSet *)sections
+{
+    DDLogVerbose(@"%@ reloadSections", self.logTag);
+    [super reloadSections:sections];
+}
+
+- (void)reloadItemsAtIndexPaths:(NSArray<NSIndexPath *> *)indexPaths
+{
+    DDLogVerbose(@"%@ reloadItemsAtIndexPaths", self.logTag);
+    [super reloadItemsAtIndexPaths:indexPaths];
+}
+
+#pragma mark - Logging
+
++ (NSString *)logTag
+{
+    return [NSString stringWithFormat:@"[%@]", self.class];
+}
+
+- (NSString *)logTag
+{
+    return self.class.logTag;
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

Signal/src/ViewControllers/ConversationView/ConversationViewController.m

@@ -167,7 +167,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) {
 @property (nonatomic, nullable) NSUUID *voiceMessageUUID;
 
 @property (nonatomic, nullable) NSTimer *readTimer;
-@property (nonatomic, nullable) NSTimer *cellMediaCachesTimer;
+@property (nonatomic) NSCache *cellMediaCache;
 @property (nonatomic) ConversationHeaderView *navigationBarTitleView;
 @property (nonatomic) UILabel *navigationBarTitleLabel;
 @property (nonatomic) UILabel *navigationBarSubtitleLabel;
@@ -394,6 +394,9 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) {
     _isGroupConversation = [self.thread isKindOfClass:[TSGroupThread class]];
     _composeOnOpen = keyboardOnViewAppearing;
     _callOnOpen = callOnViewAppearing;
+    _cellMediaCache = [NSCache new];
+    // Cache the cell media for ~24 cells.
+    self.cellMediaCache.countLimit = 24;
 
     [self.uiDatabaseConnection beginLongLivedReadTransaction];
 
@@ -528,7 +531,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) {
         self.hasClearedUnreadMessagesIndicator = NO;
         [self.dynamicInteractions clearUnreadIndicatorState];
     }
-    [self clearCellMediaCaches];
+    [self.cellMediaCache removeAllObjects];
 }
 
 - (void)applicationWillResignActive:(NSNotification *)notification
@@ -949,68 +952,6 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) {
     self.readTimer = nil;
 }
 
-- (void)startCellMediaCachesTimerIfNecessary
-{
-    if (self.cellMediaCachesTimer) {
-        return;
-    }
-    self.cellMediaCachesTimer = [NSTimer weakScheduledTimerWithTimeInterval:3.f
-                                                                     target:self
-                                                                   selector:@selector(auditCellMediaCaches)
-                                                                   userInfo:nil
-                                                                    repeats:NO];
-}
-
-- (void)auditCellMediaCaches
-{
-    [self.cellMediaCachesTimer invalidate];
-    self.cellMediaCachesTimer = nil;
-
-    NSIndexPath *_Nullable firstVisibleIndexPath = nil;
-    NSIndexPath *_Nullable lastVisibleIndexPath = nil;
-
-    for (NSIndexPath *indexPath in self.collectionView.indexPathsForVisibleItems) {
-        if (!firstVisibleIndexPath || firstVisibleIndexPath.row > indexPath.row) {
-            firstVisibleIndexPath = indexPath;
-        }
-        if (!lastVisibleIndexPath || lastVisibleIndexPath.row < indexPath.row) {
-            lastVisibleIndexPath = indexPath;
-        }
-    }
-
-    if (!firstVisibleIndexPath || !lastVisibleIndexPath) {
-        [self clearCellMediaCaches];
-        return;
-    }
-
-    // Only retain the cached cell media for N cells at a time.
-    const NSInteger kCellMediaCacheWindowSize = 24;
-    NSInteger centerRow = (firstVisibleIndexPath.row + lastVisibleIndexPath.row) / 2;
-    // Determine the first and last rows (inclusive) of the cached cell media load window.
-    // Note these row values may not correspond to actual rows in the collection view.
-    //
-    // Always retain the cached cell media for any visible cells.
-    NSInteger firstLoadWindowRow = MIN(firstVisibleIndexPath.row, centerRow - kCellMediaCacheWindowSize / 2);
-    NSInteger lastLoadWindowRow = MAX(lastVisibleIndexPath.row, centerRow + kCellMediaCacheWindowSize / 2);
-
-    // Cull cached cell media outside the load window.
-    for (ConversationViewItem *viewItem in self.viewItems) {
-        if (viewItem.row < firstLoadWindowRow || viewItem.row > lastLoadWindowRow) {
-            viewItem.cachedCellMedia = nil;
-        }
-    }
-}
-
-- (void)clearCellMediaCaches
-{
-    [self.cellMediaCachesTimer invalidate];
-    self.cellMediaCachesTimer = nil;
-
-    for (ConversationViewItem *viewItem in self.viewItems) {
-        viewItem.cachedCellMedia = nil;
-    }
-}
-
 - (void)viewDidAppear:(BOOL)animated
 {
     [super viewDidAppear:animated];
@@ -1052,7 +993,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) {
     [self saveDraft];
     [self markVisibleMessagesAsRead];
     [self cancelVoiceMemo];
-    [self clearCellMediaCaches];
+    [self.cellMediaCache removeAllObjects];
 
     self.isUserScrolling = NO;
 }
@@ -3598,8 +3539,6 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) {
 - (void)scrollViewDidScroll:(UIScrollView *)scrollView
 {
     [self updateLastVisibleTimestamp];
-
-    [self startCellMediaCachesTimerIfNecessary];
 }
 
 - (void)scrollViewWillBeginDragging:(UIScrollView *)scrollView

Signal/src/ViewControllers/ConversationView/ConversationViewItem.h

@@ -86,11 +86,8 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType);
 - (nullable TSAttachmentPointer *)attachmentPointer;
 - (CGSize)contentSize;
 
-// A generic property that cells can use to cache their loaded
+// We don't want to try to load the media for this item (if any)
-// media.  This cache is volatile and will get evacuated based
+// if a load has previously failed.
-// on scroll state, so that we only retain state for a sliding
-// window of cells that are almost on-screen.
-@property (nonatomic, nullable) id cachedCellMedia;
 @property (nonatomic) BOOL didCellMediaFailToLoad;
 
 // TODO: