From 586b362b890d7561949bc5cd19e418d429fda450 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 19 Feb 2019 17:20:07 -0500 Subject: [PATCH 1/4] Introduce ConversationSnapshot. --- .../ConversationViewController.m | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 788a68e65..26828fa18 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 @@ -1697,7 +1719,7 @@ typedef enum : NSUInteger { { OWSAssertDebug(self.conversationViewModel); - self.showLoadMoreHeader = self.conversationViewModel.canLoadMoreItems; + self.showLoadMoreHeader = self.conversationSnapshot.canLoadMoreItems; } - (void)setShowLoadMoreHeader:(BOOL)showLoadMoreHeader @@ -4138,6 +4160,7 @@ typedef enum : NSUInteger { if (self.shouldObserveVMUpdates) { OWSLogVerbose(@"resume observation of view model."); + [self updateConversationSnapshot]; [self resetContentAndLayout]; [self updateBackButtonUnreadCount]; [self updateNavigationBarSubtitleLabel]; @@ -4612,6 +4635,7 @@ typedef enum : NSUInteger { return; } + [self updateConversationSnapshot]; [self updateBackButtonUnreadCount]; [self updateNavigationBarSubtitleLabel]; @@ -4909,6 +4933,17 @@ typedef enum : NSUInteger { [self.inputToolbar updateLayoutWithSafeAreaInsets:safeAreaInsets]; } +#pragma mark - Conversation Snapshot + +- (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 From 56e5feca46f1299bb838ddd13097e4d2743f576d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 19 Feb 2019 17:49:40 -0500 Subject: [PATCH 2/4] Introduce ConversationSnapshot. --- .../ConversationViewController.m | 26 +++++++++---- .../ConversationView/ConversationViewModel.h | 2 +- .../ConversationView/ConversationViewModel.m | 39 ++++++++++++------- SignalMessaging/utils/ThreadUtil.m | 15 +++++++ 4 files changed, 59 insertions(+), 23 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 26828fa18..8c2f534f7 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -451,11 +451,11 @@ typedef enum : NSUInteger { NSString *_Nullable recipientId = notification.userInfo[kNSNotificationKey_ProfileRecipientId]; NSData *_Nullable groupId = notification.userInfo[kNSNotificationKey_ProfileGroupId]; if (recipientId.length > 0 && [self.thread.recipientIdentifiers containsObject:recipientId]) { - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } else if (groupId.length > 0 && self.thread.isGroupThread) { TSGroupThread *groupThread = (TSGroupThread *)self.thread; if ([groupThread.groupModel.groupId isEqualToData:groupId]) { - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; [self ensureBannerState]; } } @@ -869,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]; @@ -2446,7 +2447,7 @@ typedef enum : NSUInteger { - (void)contactsViewHelperDidUpdateContacts { - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } - (void)createConversationScrollButtons @@ -2484,7 +2485,7 @@ typedef enum : NSUInteger { _hasUnreadMessages = hasUnreadMessages; self.scrollDownButton.hasUnreadMessages = hasUnreadMessages; - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } - (void)scrollDownButtonTapped @@ -2629,7 +2630,7 @@ typedef enum : NSUInteger { [self showApprovalDialogForAttachment:attachment]; [ThreadUtil addThreadToProfileWhitelistIfEmptyContactThread:self.thread]; - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } - (void)messageWasSent:(TSOutgoingMessage *)message @@ -2989,7 +2990,7 @@ typedef enum : NSUInteger { [self messageWasSent:message]; if (didAddToProfileWhitelist) { - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } }]; } @@ -3641,7 +3642,7 @@ typedef enum : NSUInteger { [self messageWasSent:message]; if (didAddToProfileWhitelist) { - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } }); } @@ -4031,7 +4032,7 @@ typedef enum : NSUInteger { [self clearDraft]; if (didAddToProfileWhitelist) { - [self.conversationViewModel ensureDynamicInteractions]; + [self.conversationViewModel ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } } @@ -4935,6 +4936,15 @@ typedef enum : NSUInteger { #pragma mark - Conversation Snapshot +- (void)tryToUpdateConversationSnapshot +{ + if (!self.isObservingVMUpdates) { + return; + } + + [self updateConversationSnapshot]; +} + - (void)updateConversationSnapshot { ConversationSnapshot *conversationSnapshot = [ConversationSnapshot new]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h index a855c6f4c..d13a38c36 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h @@ -96,7 +96,7 @@ typedef NS_ENUM(NSUInteger, ConversationUpdateItemType) { focusMessageIdOnOpen:(nullable NSString *)focusMessageIdOnOpen delegate:(id)delegate NS_DESIGNATED_INITIALIZER; -- (void)ensureDynamicInteractions; +- (void)ensureDynamicInteractionsAndUpdateIfNecessary:(BOOL)updateIfNecessary; - (void)clearUnreadMessagesIndicator; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m index 602a4fc0d..75428952d 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m @@ -273,7 +273,7 @@ static const int kYapDatabaseRangeMaxLength = 25000; { OWSAssertIsOnMainThread(); - [self ensureDynamicInteractions]; + [self ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } - (void)profileWhitelistDidChange:(NSNotification *)notification @@ -308,7 +308,7 @@ static const int kYapDatabaseRangeMaxLength = 25000; self.typingIndicatorsSender = [self.typingIndicators typingRecipientIdForThread:self.thread]; self.collapseCutoffDate = [NSDate new]; - [self ensureDynamicInteractions]; + [self ensureDynamicInteractionsAndUpdateIfNecessary:NO]; [self.primaryStorage updateUIDatabaseConnectionToLatest]; [self createNewMessageMapping]; @@ -464,21 +464,32 @@ static const int kYapDatabaseRangeMaxLength = 25000; self.collapseCutoffDate = [NSDate new]; } -- (void)ensureDynamicInteractions +- (void)ensureDynamicInteractionsAndUpdateIfNecessary:(BOOL)updateIfNecessary { OWSAssertIsOnMainThread(); const int currentMaxRangeSize = (int)self.messageMapping.desiredLength; const int maxRangeSize = MAX(kConversationInitialMaxRangeSize, currentMaxRangeSize); - self.dynamicInteractions = [ThreadUtil ensureDynamicInteractionsForThread:self.thread - contactsManager:self.contactsManager - blockingManager:self.blockingManager - dbConnection:self.editingDatabaseConnection - hideUnreadMessagesIndicator:self.hasClearedUnreadMessagesIndicator - lastUnreadIndicator:self.dynamicInteractions.unreadIndicator - focusMessageId:self.focusMessageIdOnOpen - maxRangeSize:maxRangeSize]; + ThreadDynamicInteractions *dynamicInteractions = + [ThreadUtil ensureDynamicInteractionsForThread:self.thread + contactsManager:self.contactsManager + blockingManager:self.blockingManager + dbConnection:self.editingDatabaseConnection + hideUnreadMessagesIndicator:self.hasClearedUnreadMessagesIndicator + lastUnreadIndicator:self.dynamicInteractions.unreadIndicator + focusMessageId:self.focusMessageIdOnOpen + maxRangeSize:maxRangeSize]; + BOOL didChange = ![NSObject isNullableObject:self.dynamicInteractions equalTo:dynamicInteractions]; + self.dynamicInteractions = dynamicInteractions; + + if (didChange && updateIfNecessary) { + if (![self reloadViewItems]) { + OWSFailDebug(@"Failed to reload view items."); + } + + [self.delegate conversationViewModelDidUpdate:ConversationUpdate.reloadUpdate]; + } } - (nullable id)viewItemForUnreadMessagesIndicator @@ -519,7 +530,7 @@ static const int kYapDatabaseRangeMaxLength = 25000; if (self.dynamicInteractions.unreadIndicator) { // If we've just cleared the "unread messages" indicator, // update the dynamic interactions. - [self ensureDynamicInteractions]; + [self ensureDynamicInteractionsAndUpdateIfNecessary:YES]; } } @@ -968,7 +979,7 @@ static const int kYapDatabaseRangeMaxLength = 25000; self.collapseCutoffDate = [NSDate new]; - [self ensureDynamicInteractions]; + [self ensureDynamicInteractionsAndUpdateIfNecessary:NO]; if (![self reloadViewItems]) { OWSFailDebug(@"failed to reload view items in resetMapping."); @@ -1590,7 +1601,7 @@ static const int kYapDatabaseRangeMaxLength = 25000; self.collapseCutoffDate = [NSDate new]; - [self ensureDynamicInteractions]; + [self ensureDynamicInteractionsAndUpdateIfNecessary:NO]; if (![self reloadViewItems]) { OWSFailDebug(@"failed to reload view items in resetMapping."); diff --git a/SignalMessaging/utils/ThreadUtil.m b/SignalMessaging/utils/ThreadUtil.m index 099e56448..c7c0dcf24 100644 --- a/SignalMessaging/utils/ThreadUtil.m +++ b/SignalMessaging/utils/ThreadUtil.m @@ -46,6 +46,21 @@ NS_ASSUME_NONNULL_BEGIN self.unreadIndicator = nil; } +- (BOOL)isEqual:(id)object +{ + if (self == object) { + return YES; + } + + if (![object isKindOfClass:[ThreadDynamicInteractions class]]) { + return NO; + } + + ThreadDynamicInteractions *other = (ThreadDynamicInteractions *)object; + return ([NSObject isNullableObject:self.focusMessagePosition equalTo:other.focusMessagePosition] && + [NSObject isNullableObject:self.unreadIndicator equalTo:other.unreadIndicator]); +} + @end #pragma mark - From 6ed4045fbe664a061f2a25f69029556aa61388a0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Feb 2019 13:01:57 -0500 Subject: [PATCH 3/4] Conversation view always observes view model. --- .../ConversationViewController.m | 203 +----------------- .../ConversationView/ConversationViewModel.h | 2 - .../ConversationView/ConversationViewModel.m | 14 -- 3 files changed, 6 insertions(+), 213 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 8c2f534f7..053870d6d 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -108,25 +108,6 @@ 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.conversationSnapshot.viewItems; + return self.conversationViewModel.viewItems; } - (ThreadDynamicInteractions *)dynamicInteractions { - return self.conversationSnapshot.dynamicInteractions; + return self.conversationViewModel.dynamicInteractions; } - (NSIndexPath *_Nullable)indexPathOfUnreadMessagesIndicator @@ -869,7 +840,6 @@ 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]; @@ -1720,7 +1690,7 @@ typedef enum : NSUInteger { { OWSAssertDebug(self.conversationViewModel); - self.showLoadMoreHeader = self.conversationSnapshot.canLoadMoreItems; + self.showLoadMoreHeader = self.conversationViewModel.canLoadMoreItems; } - (void)setShowLoadMoreHeader:(BOOL)showLoadMoreHeader @@ -1961,8 +1931,6 @@ typedef enum : NSUInteger { - (void)menuActionsDidHide:(MenuActionsViewController *)menuActionsViewController { [[OWSWindowManager sharedManager] hideMenuActionsWindow]; - - [self updateShouldObserveVMUpdates]; } - (void)menuActions:(MenuActionsViewController *)menuActionsViewController @@ -2069,8 +2037,6 @@ typedef enum : NSUInteger { self.conversationViewModel.mostRecentMenuActionsViewItem = cell.viewItem; [[OWSWindowManager sharedManager] showMenuActionsWindow:menuActionsViewController]; - - [self updateShouldObserveVMUpdates]; } - (NSAttributedString *)attributedContactOrProfileNameForPhoneIdentifier:(NSString *)recipientId @@ -4117,7 +4083,6 @@ typedef enum : NSUInteger { { _isViewVisible = isViewVisible; - [self updateShouldObserveVMUpdates]; [self updateCellsVisible]; } @@ -4130,134 +4095,8 @@ 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; } @@ -4604,11 +4443,6 @@ typedef enum : NSUInteger { #pragma mark - ConversationViewModelDelegate -- (BOOL)isObservingVMUpdates -{ - return self.shouldObserveVMUpdates; -} - - (void)conversationViewModelWillUpdate { OWSAssertIsOnMainThread(); @@ -4632,11 +4466,6 @@ typedef enum : NSUInteger { OWSAssertDebug(conversationUpdate); OWSAssertDebug(self.conversationViewModel); - if (!self.shouldObserveVMUpdates) { - return; - } - - [self updateConversationSnapshot]; [self updateBackButtonUnreadCount]; [self updateNavigationBarSubtitleLabel]; @@ -4934,26 +4763,6 @@ 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 d13a38c36..fd708309b 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.h @@ -76,8 +76,6 @@ 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 75428952d..ea386b597 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m @@ -542,19 +542,12 @@ 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]; } @@ -705,13 +698,6 @@ 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]; From 7711ee92a7d5491ba70c35175a0049fc6aa23594 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Feb 2019 13:03:00 -0500 Subject: [PATCH 4/4] 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];