From 16a1dcfb77938d5f4e5799f69dfbc0cc94f21a0f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 27 Jun 2018 09:53:45 -0400 Subject: [PATCH] Respond to CR. --- .../ConversationViewController.m | 65 +++++-------------- .../ConversationView/ConversationViewItem.h | 6 -- .../ConversationView/ConversationViewItem.m | 1 - .../src/Network/WebSockets/TSSocketManager.m | 2 +- 4 files changed, 19 insertions(+), 55 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 8c9b20864..6fcbb6bb6 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -465,6 +465,12 @@ typedef enum : NSUInteger { return; } + [self.messageMappings setCellDrawingDependencyOffsets:[NSSet setWithArray:@[ + @(-1), + @(+1), + ]] + forGroup:self.thread.uniqueId]; + // We need to impose the range restrictions on the mappings immediately to avoid // doing a great deal of unnecessary work and causing a perf hotspot. [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { @@ -3303,8 +3309,13 @@ typedef enum : NSUInteger { case YapDatabaseViewChangeUpdate: { YapCollectionKey *collectionKey = rowChange.collectionKey; if (collectionKey.key) { - ConversationViewItem *viewItem = self.viewItemCache[collectionKey.key]; - [self reloadInteractionForViewItem:viewItem]; + ConversationViewItem *_Nullable viewItem = self.viewItemCache[collectionKey.key]; + if (viewItem) { + [self reloadInteractionForViewItem:viewItem]; + } + } else if (rowChange.indexPath && rowChange.originalIndex < self.viewItems.count) { + // Do nothing, this is a pseudo-update generated due to + // setCellDrawingDependencyOffsets. } else { hasMalformedRowChange = YES; } @@ -3332,7 +3343,8 @@ typedef enum : NSUInteger { if (hasMalformedRowChange) { // These errors seems to be very rare; they can only be reproduced // using the more extreme actions in the debug UI. - DDLogError(@"%@ hasMalformedRowChange", self.logTag); + OWSProdLogAndFail(@"%@ hasMalformedRowChange", self.logTag); + [self reloadViewItems]; [self.collectionView reloadData]; [self updateLastVisibleTimestamp]; [self cleanUpUnreadIndicatorIfNecessary]; @@ -3340,7 +3352,7 @@ typedef enum : NSUInteger { } NSUInteger oldViewItemCount = self.viewItems.count; - NSMutableSet *rowsThatChangedSize = [[self reloadViewItems] mutableCopy]; + [self reloadViewItems]; BOOL wasAtBottom = [self isScrolledToBottom]; // We want sending messages to feel snappy. So, if the only @@ -3372,8 +3384,6 @@ typedef enum : NSUInteger { rowChange.newIndexPath, rowChange.finalIndex); [self.collectionView insertItemsAtIndexPaths:@[ rowChange.newIndexPath ]]; - // We don't want to reload a row that we just inserted. - [rowsThatChangedSize removeObject:@(rowChange.originalIndex)]; ConversationViewItem *_Nullable viewItem = [self viewItemForIndex:(NSInteger)rowChange.finalIndex]; if ([viewItem.interaction isKindOfClass:[TSOutgoingMessage class]]) { @@ -3392,8 +3402,6 @@ typedef enum : NSUInteger { rowChange.newIndexPath, rowChange.finalIndex); [self.collectionView moveItemAtIndexPath:rowChange.indexPath toIndexPath:rowChange.newIndexPath]; - // We don't want to reload a row that we just moved. - [rowsThatChangedSize removeObject:@(rowChange.originalIndex)]; break; } case YapDatabaseViewChangeUpdate: { @@ -3402,23 +3410,10 @@ typedef enum : NSUInteger { rowChange.indexPath, rowChange.finalIndex); [self.collectionView reloadItemsAtIndexPaths:@[ rowChange.indexPath ]]; - // We don't want to reload a row that we've already reloaded. - [rowsThatChangedSize removeObject:@(rowChange.originalIndex)]; break; } } } - - // The changes performed above may affect the size of neighboring cells, - // as they may affect which cells show "date" headers or "status" footers. - NSMutableArray *rowsToReload = [NSMutableArray new]; - for (NSNumber *row in rowsThatChangedSize) { - DDLogVerbose(@"rowsToReload: %@", row); - [rowsToReload addObject:[NSIndexPath indexPathForRow:row.integerValue inSection:0]]; - } - if (rowsToReload.count > 0) { - [self.collectionView reloadItemsAtIndexPaths:rowsToReload]; - } }; DDLogVerbose(@"self.viewItems.count: %zd -> %zd", oldViewItemCount, self.viewItems.count); @@ -4775,11 +4770,7 @@ typedef enum : NSUInteger { // This is a key method. It builds or rebuilds the list of // cell view models. -// -// Returns a list of the rows which may have changed size and -// need to be reloaded if we're doing an incremental update -// of the view. -- (NSSet *)reloadViewItems +- (void)reloadViewItems { NSMutableArray *viewItems = [NSMutableArray new]; NSMutableDictionary *viewItemCache = [NSMutableDictionary new]; @@ -4809,9 +4800,7 @@ typedef enum : NSUInteger { } ConversationViewItem *_Nullable viewItem = self.viewItemCache[interaction.uniqueId]; - if (viewItem) { - viewItem.previousRow = viewItem.row; - } else { + if (!viewItem) { viewItem = [[ConversationViewItem alloc] initWithInteraction:interaction isGroupThread:isGroupThread transaction:transaction @@ -4824,8 +4813,6 @@ typedef enum : NSUInteger { } }]; - NSMutableSet *rowsThatChangedSize = [NSMutableSet new]; - // Update the "shouldShowDate" property of the view items. BOOL shouldShowDateOnNextViewItem = YES; uint64_t previousViewItemTimestamp = 0; @@ -4865,12 +4852,6 @@ typedef enum : NSUInteger { shouldShowDateOnNextViewItem = NO; } - // If this is an existing view item and it has changed size, - // note that so that we can reload this cell while doing - // incremental updates. - if (viewItem.shouldShowDate != shouldShowDate && viewItem.previousRow != NSNotFound) { - [rowsThatChangedSize addObject:@(viewItem.previousRow)]; - } viewItem.shouldShowDate = shouldShowDate; previousViewItemTimestamp = viewItem.interaction.timestampForSorting; @@ -4910,22 +4891,12 @@ typedef enum : NSUInteger { } lastInteractionType = interactionType; - // When `shouldHideRecipientStatus` changes, reload the cell if necessary. - if (viewItem.shouldHideRecipientStatus != shouldHideRecipientStatus && viewItem.previousRow != NSNotFound) { - [rowsThatChangedSize addObject:@(viewItem.previousRow)]; - } viewItem.shouldHideRecipientStatus = shouldHideRecipientStatus; - // When `shouldHideAvatar` changes, reload the cell if necessary. - if (viewItem.shouldHideAvatar != shouldHideAvatar && viewItem.previousRow != NSNotFound) { - [rowsThatChangedSize addObject:@(viewItem.previousRow)]; - } viewItem.shouldHideAvatar = shouldHideAvatar; } self.viewItems = viewItems; self.viewItemCache = viewItemCache; - - return [rowsThatChangedSize copy]; } // Whenever an interaction is modified, we need to reload it from the DB diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h index 4c6694124..1e245e210 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h @@ -61,12 +61,6 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType); @property (nonatomic) BOOL shouldHideAvatar; @property (nonatomic) NSInteger row; -// During updates, we sometimes need the previous row index -// (before this update) of this item. -// -// If NSNotFound, this view item was just created in the -// previous update. -@property (nonatomic) NSInteger previousRow; @property (nonatomic, readonly) ConversationStyle *conversationStyle; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m index 1356b66dc..0bc3e8d38 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m @@ -93,7 +93,6 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) _isGroupThread = isGroupThread; _conversationStyle = conversationStyle; self.row = NSNotFound; - self.previousRow = NSNotFound; [self ensureViewState:transaction]; diff --git a/SignalServiceKit/src/Network/WebSockets/TSSocketManager.m b/SignalServiceKit/src/Network/WebSockets/TSSocketManager.m index d30251745..a187f8275 100644 --- a/SignalServiceKit/src/Network/WebSockets/TSSocketManager.m +++ b/SignalServiceKit/src/Network/WebSockets/TSSocketManager.m @@ -526,7 +526,7 @@ NSString *const kNSNotification_SocketManagerStateDidChange = @"kNSNotification_ NSError *error; BOOL wasScheduled = [self.websocket sendDataNoCopy:messageData error:&error]; if (!wasScheduled || error) { - OWSProdLogAndFail(@"%@ could not serialize request JSON: %@", self.logTag, error); + OWSProdLogAndFail(@"%@ could not send socket request: %@", self.logTag, error); [socketMessage didFailBeforeSending]; return; }