From 7711ee92a7d5491ba70c35175a0049fc6aa23594 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Feb 2019 13:03:00 -0500 Subject: [PATCH] Revert "Conversation view always observes view model." This reverts commit 9d39e829a44f28f324f79e0b74a6c8692678d788. --- .../ConversationViewController.m | 203 +++++++++++++++++- .../ConversationView/ConversationViewModel.h | 2 + .../ConversationView/ConversationViewModel.m | 14 ++ 3 files changed, 213 insertions(+), 6 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 053870d6d..8c2f534f7 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -108,6 +108,25 @@ typedef enum : NSUInteger { #pragma mark - +// We use snapshots to ensure that the view has a consistent +// representation of view model state which is not updated +// when the view is not observing view model changes. +@interface ConversationSnapshot : NSObject + +@property (nonatomic) NSArray> *viewItems; +@property (nonatomic) ThreadDynamicInteractions *dynamicInteractions; +@property (nonatomic) BOOL canLoadMoreItems; + +@end + +#pragma mark - + +@implementation ConversationSnapshot + +@end + +#pragma mark - + @interface ConversationViewController () > *)viewItems { - return self.conversationViewModel.viewItems; + return self.conversationSnapshot.viewItems; } - (ThreadDynamicInteractions *)dynamicInteractions { - return self.conversationViewModel.dynamicInteractions; + return self.conversationSnapshot.dynamicInteractions; } - (NSIndexPath *_Nullable)indexPathOfUnreadMessagesIndicator @@ -840,6 +869,7 @@ typedef enum : NSUInteger { // Avoid layout corrupt issues and out-of-date message subtitles. self.lastReloadDate = [NSDate new]; [self.conversationViewModel viewDidResetContentAndLayout]; + [self tryToUpdateConversationSnapshot]; [self.collectionView.collectionViewLayout invalidateLayout]; [self.collectionView reloadData]; @@ -1690,7 +1720,7 @@ typedef enum : NSUInteger { { OWSAssertDebug(self.conversationViewModel); - self.showLoadMoreHeader = self.conversationViewModel.canLoadMoreItems; + self.showLoadMoreHeader = self.conversationSnapshot.canLoadMoreItems; } - (void)setShowLoadMoreHeader:(BOOL)showLoadMoreHeader @@ -1931,6 +1961,8 @@ typedef enum : NSUInteger { - (void)menuActionsDidHide:(MenuActionsViewController *)menuActionsViewController { [[OWSWindowManager sharedManager] hideMenuActionsWindow]; + + [self updateShouldObserveVMUpdates]; } - (void)menuActions:(MenuActionsViewController *)menuActionsViewController @@ -2037,6 +2069,8 @@ typedef enum : NSUInteger { self.conversationViewModel.mostRecentMenuActionsViewItem = cell.viewItem; [[OWSWindowManager sharedManager] showMenuActionsWindow:menuActionsViewController]; + + [self updateShouldObserveVMUpdates]; } - (NSAttributedString *)attributedContactOrProfileNameForPhoneIdentifier:(NSString *)recipientId @@ -4083,6 +4117,7 @@ typedef enum : NSUInteger { { _isViewVisible = isViewVisible; + [self updateShouldObserveVMUpdates]; [self updateCellsVisible]; } @@ -4095,8 +4130,134 @@ typedef enum : NSUInteger { } } +- (void)updateShouldObserveVMUpdates +{ + if (!CurrentAppContext().isAppForegroundAndActive) { + self.shouldObserveVMUpdates = NO; + return; + } + + if (!self.isViewVisible) { + self.shouldObserveVMUpdates = NO; + return; + } + + if (OWSWindowManager.sharedManager.isPresentingMenuActions) { + self.shouldObserveVMUpdates = NO; + return; + } + + self.shouldObserveVMUpdates = YES; +} + +- (void)setShouldObserveVMUpdates:(BOOL)shouldObserveVMUpdates +{ + if (_shouldObserveVMUpdates == shouldObserveVMUpdates) { + return; + } + + _shouldObserveVMUpdates = shouldObserveVMUpdates; + + if (self.shouldObserveVMUpdates) { + OWSLogVerbose(@"resume observation of view model."); + + [self updateConversationSnapshot]; + [self resetContentAndLayout]; + [self updateBackButtonUnreadCount]; + [self updateNavigationBarSubtitleLabel]; + [self updateDisappearingMessagesConfiguration]; + + // Detect changes in the mapping's "window" size. + if (self.previousViewTopToContentBottom && self.previousViewItemCount + && self.previousViewItemCount.unsignedIntegerValue != self.viewItems.count) { + CGFloat newContentHeight = self.safeContentHeight; + CGPoint newContentOffset + = CGPointMake(0, MAX(0, newContentHeight - self.previousViewTopToContentBottom.floatValue)); + [self.collectionView setContentOffset:newContentOffset animated:NO]; + } + + // When we resume observing database changes, we want to scroll to show the user + // any new items inserted while we were not observing. We therefore find the + // first item at or after the "view horizon". See the comments below which explain + // the "view horizon". + id _Nullable lastViewItem = self.viewItems.lastObject; + BOOL hasAddedNewItems = (lastViewItem && self.previousLastTimestamp + && lastViewItem.interaction.timestamp > self.previousLastTimestamp.unsignedLongLongValue); + + OWSLogInfo(@"hasAddedNewItems: %d", hasAddedNewItems); + if (hasAddedNewItems) { + NSIndexPath *_Nullable indexPathToShow = [self firstIndexPathAtViewHorizonTimestamp]; + if (indexPathToShow) { + // The goal is to show _both_ the last item before the "view horizon" and the + // first item after the "view horizon". We can't do "top on first item after" + // or "bottom on last item before" or we won't see the other. Unfortunately, + // this gets tricky if either is huge. The largest cells are oversize text, + // which should be rare. Other cells are considerably smaller than a screenful. + [self.collectionView scrollToItemAtIndexPath:indexPathToShow + atScrollPosition:UICollectionViewScrollPositionCenteredVertically + animated:NO]; + } + } + self.viewHorizonTimestamp = nil; + OWSLogVerbose(@"resumed observation of view model."); + } else { + OWSLogVerbose(@"pausing observation of view model."); + // When stopping observation, try to record the timestamp of the "view horizon". + // The "view horizon" is where we'll want to focus the users when we resume + // observation if any changes have happened while we weren't observing. + // Ideally, we'll focus on those changes. But we can't skip over unread + // interactions, so we prioritize those, if any. + // + // We'll use this later to update the view to reflect any changes made while + // we were not observing the database. See extendRangeToIncludeUnobservedItems + // and the logic above. + id _Nullable lastViewItem = self.viewItems.lastObject; + if (lastViewItem) { + self.previousLastTimestamp = @(lastViewItem.interaction.timestamp); + self.previousViewItemCount = @(self.viewItems.count); + } else { + self.previousLastTimestamp = nil; + self.previousViewItemCount = nil; + } + + __block TSInteraction *_Nullable firstUnseenInteraction = nil; + [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + firstUnseenInteraction = + [[TSDatabaseView unseenDatabaseViewExtension:transaction] firstObjectInGroup:self.thread.uniqueId]; + }]; + if (firstUnseenInteraction) { + // If there are any unread interactions, focus on the first one. + self.viewHorizonTimestamp = @(firstUnseenInteraction.timestamp); + } else if (lastViewItem) { + // Otherwise, focus _just after_ the last interaction. + self.viewHorizonTimestamp = @(lastViewItem.interaction.timestamp + 1); + } else { + self.viewHorizonTimestamp = nil; + } + + // Snapshot the scroll state by measuring the "distance from top of view to + // bottom of content"; if the mapping's "window" size grows, it will grow + // _upward_. + OWSAssertDebug([self.collectionView.collectionViewLayout isKindOfClass:[ConversationViewLayout class]]); + ConversationViewLayout *conversationViewLayout + = (ConversationViewLayout *)self.collectionView.collectionViewLayout; + // To avoid laying out the collection view during initial view + // presentation, don't trigger layout here (via safeContentHeight) + // until layout has been done at least once. + if (conversationViewLayout.hasEverHadLayout) { + self.previousViewTopToContentBottom = @(self.safeContentHeight - self.collectionView.contentOffset.y); + } else { + self.previousViewTopToContentBottom = nil; + } + + OWSLogVerbose(@"paused observation of view model."); + } +} + - (nullable NSIndexPath *)firstIndexPathAtViewHorizonTimestamp { + OWSAssertDebug(self.shouldObserveVMUpdates); + if (!self.viewHorizonTimestamp) { return nil; } @@ -4443,6 +4604,11 @@ typedef enum : NSUInteger { #pragma mark - ConversationViewModelDelegate +- (BOOL)isObservingVMUpdates +{ + return self.shouldObserveVMUpdates; +} + - (void)conversationViewModelWillUpdate { OWSAssertIsOnMainThread(); @@ -4466,6 +4632,11 @@ typedef enum : NSUInteger { OWSAssertDebug(conversationUpdate); OWSAssertDebug(self.conversationViewModel); + if (!self.shouldObserveVMUpdates) { + return; + } + + [self updateConversationSnapshot]; [self updateBackButtonUnreadCount]; [self updateNavigationBarSubtitleLabel]; @@ -4763,6 +4934,26 @@ typedef enum : NSUInteger { [self.inputToolbar updateLayoutWithSafeAreaInsets:safeAreaInsets]; } +#pragma mark - Conversation Snapshot + +- (void)tryToUpdateConversationSnapshot +{ + if (!self.isObservingVMUpdates) { + return; + } + + [self updateConversationSnapshot]; +} + +- (void)updateConversationSnapshot +{ + ConversationSnapshot *conversationSnapshot = [ConversationSnapshot new]; + conversationSnapshot.viewItems = self.conversationViewModel.viewItems; + conversationSnapshot.dynamicInteractions = self.conversationViewModel.dynamicInteractions; + conversationSnapshot.canLoadMoreItems = self.conversationViewModel.canLoadMoreItems; + _conversationSnapshot = conversationSnapshot; +} + @end NS_ASSUME_NONNULL_END diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h index fd708309b..d13a38c36 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h @@ -76,6 +76,8 @@ typedef NS_ENUM(NSUInteger, ConversationUpdateItemType) { - (void)conversationViewModelDidDeleteMostRecentMenuActionsViewItem; +- (BOOL)isObservingVMUpdates; + - (ConversationStyle *)conversationStyle; @end diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m index ea386b597..75428952d 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m @@ -542,12 +542,19 @@ static const int kYapDatabaseRangeMaxLength = 25000; OWSLogVerbose(@""); + if (!self.delegate.isObservingVMUpdates) { + return; + } + // External database modifications (e.g. changes from another process such as the SAE) // are "flushed" using touchDbAsync when the app re-enters the foreground. } - (void)uiDatabaseWillUpdate:(NSNotification *)notification { + if (!self.delegate.isObservingVMUpdates) { + return; + } [self.delegate conversationViewModelWillUpdate]; } @@ -698,6 +705,13 @@ static const int kYapDatabaseRangeMaxLength = 25000; OWSAssertDebug(oldItemIdList); OWSAssertDebug(updatedItemSetParam); + if (!self.delegate.isObservingVMUpdates) { + OWSLogVerbose(@"Skipping VM update."); + // We fire this event, but it will be ignored. + [self.delegate conversationViewModelDidUpdate:ConversationUpdate.minorUpdate]; + return; + } + if (oldItemIdList.count != [NSSet setWithArray:oldItemIdList].count) { OWSFailDebug(@"Old view item list has duplicates."); [self.delegate conversationViewModelDidUpdate:ConversationUpdate.reloadUpdate];