diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 514580837..0da274ab5 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -115,13 +115,6 @@ typedef enum : NSUInteger { kMediaTypeVideo, } kMediaTypes; -typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { - // This mode should only be used when initially configuring the range, - // since we want the range to monotonically grow after that. - MessagesRangeSizeMode_Truncate, - MessagesRangeSizeMode_Normal -}; - #pragma mark - @interface ConversationViewController () 0) { @@ -451,7 +446,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { [self.messageMappings updateWithTransaction:transaction]; }]; - [self updateMessageMappingRangeOptions:MessagesRangeSizeMode_Truncate]; + [self updateMessageMappingRangeOptions]; [self updateShouldObserveDBModifications]; } @@ -1528,7 +1523,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { // the dynamic interactions and re-layout. Here we take a "before" snapshot. CGFloat scrollDistanceToBottom = self.safeContentHeight - self.collectionView.contentOffset.y; - self.page = MIN(self.page + 1, (NSUInteger)kYapDatabaseMaxPageCount - 1); + self.lastRangeLength = MIN(self.lastRangeLength + kYapDatabasePageSize, (NSUInteger) kYapDatabaseRangeMaxLength); [self resetMappings]; @@ -1546,7 +1541,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { - (void)updateShowLoadMoreHeader { - if (self.page == kYapDatabaseMaxPageCount - 1) { + if (self.lastRangeLength == kYapDatabaseRangeMaxLength) { self.showLoadMoreHeader = NO; return; } @@ -1595,20 +1590,19 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { [self updateBarButtonItems]; } -- (void)updateMessageMappingRangeOptions:(MessagesRangeSizeMode)mode +- (void)updateMessageMappingRangeOptions { - // The "old" range length may have been increased by insertions of new messages - // at the bottom of the window. - NSUInteger oldLength = [self.messageMappings numberOfItemsInGroup:self.thread.uniqueId]; - - NSUInteger targetLength = oldLength; - if (mode == MessagesRangeSizeMode_Truncate) { + NSUInteger rangeLength = 0; + + if (self.lastRangeLength == 0) { + // If this is the first time we're configuring the range length, + // try to take into account the position of the unread indicator. OWSAssert(self.dynamicInteractions); - OWSAssert(self.page == 0); if (self.dynamicInteractions.unreadIndicatorPosition) { NSUInteger unreadIndicatorPosition = (NSUInteger)[self.dynamicInteractions.unreadIndicatorPosition longValue]; + // If there is an unread indicator, increase the initial load window // to include it. OWSAssert(unreadIndicatorPosition > 0); @@ -1617,23 +1611,21 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { // We'd like to include at least N seen messages, // to give the user the context of where they left off the conversation. const NSUInteger kPreferredSeenMessageCount = 1; - targetLength = unreadIndicatorPosition + kPreferredSeenMessageCount; - } else { - // Default to a single page of messages. - targetLength = kYapDatabasePageSize; + rangeLength = unreadIndicatorPosition + kPreferredSeenMessageCount; } } - // The "page-based" range length may have been increased by loading "prev" pages at the - // top of the window. - NSUInteger rangeLength; - while (YES) { - rangeLength = kYapDatabasePageSize * (self.page + 1); - if (rangeLength >= targetLength) { - break; - } - self.page = self.page + 1; - } + // Always try to load at least a single page of messages. + rangeLength = MAX(rangeLength, kYapDatabasePageSize); + + // Range size should monotonically increase. + rangeLength = MAX(rangeLength, self.lastRangeLength); + + // Enforce max range size. + rangeLength = MIN(rangeLength, kYapDatabaseRangeMaxLength); + + self.lastRangeLength = rangeLength; + YapDatabaseViewRangeOptions *rangeOptions = [YapDatabaseViewRangeOptions flexibleRangeWithLength:rangeLength offset:0 from:YapDatabaseViewEnd]; @@ -2195,7 +2187,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { { OWSAssertIsOnMainThread(); - const int currentMaxRangeSize = (int)(self.page + 1) * kYapDatabasePageSize; + const int currentMaxRangeSize = (int)self.lastRangeLength; const int maxRangeSize = MAX(kConversationInitialMaxRangeSize, currentMaxRangeSize); // `ensureDynamicInteractionsForThread` should operate on the latest thread contents, so @@ -2828,10 +2820,15 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); - // External database modifications can't be converted into incremental updates, - // so rebuild everything. This is expensive and usually isn't necessary, but - // there's no alternative. - [self resetMappings]; + if (self.shouldObserveDBModifications) { + // External database modifications can't be converted into incremental updates, + // so rebuild everything. This is expensive and usually isn't necessary, but + // there's no alternative. + // + // We don't need to do this if we're not observing db modifications since we'll + // do it when we resume. + [self resetMappings]; + } } - (void)yapDatabaseModified:(NSNotification *)notification @@ -2905,7 +2902,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { // range that are not within the current mapping's contents. We // may need to extend the mapping's contents to reflect the current // range. - [self updateMessageMappingRangeOptions:MessagesRangeSizeMode_Normal]; + [self updateMessageMappingRangeOptions]; // Calling resetContentAndLayout is a bit expensive. // Since by definition this won't affect any cells in the previous // range, it should be sufficient to call invalidateLayout. @@ -4126,6 +4123,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { _shouldObserveDBModifications = shouldObserveDBModifications; if (self.shouldObserveDBModifications) { + DDLogVerbose(@"%@ resume observation of database modifications.", self.logTag); // We need to call resetMappings when we _resume_ observing DB modifications, // since we've been ignore DB modifications so the mappings can be wrong. // @@ -4143,6 +4141,9 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { // // TODO: have a more fine-grained cache expiration based on rows modified. [self.viewItemCache removeAllObjects]; + + // Snapshot the "previousLastTimestamp" value; it will be cleared by resetMappings. + NSNumber *_Nullable previousLastTimestamp = self.previousLastTimestamp; [self resetMappings]; @@ -4154,9 +4155,154 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { CGPoint newContentOffset = CGPointMake(0, MAX(0, newContentHeight - viewTopToContentBottom)); [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". + ConversationViewItem *_Nullable lastViewItem = self.viewItems.lastObject; + BOOL hasAddedNewItems = (lastViewItem && + previousLastTimestamp && + lastViewItem.interaction.timestamp > previousLastTimestamp.unsignedLongLongValue); + + DDLogInfo(@"%@ hasAddedNewItems: %d", self.logTag, 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; + DDLogVerbose(@"%@ resumed observation of database modifications.", self.logTag); + } else { + DDLogVerbose(@"%@ pausing observation of database modifications.", self.logTag); + // 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. + ConversationViewItem *_Nullable lastViewItem = self.viewItems.lastObject; + if (lastViewItem) { + self.previousLastTimestamp = @(lastViewItem.interaction.timestamp); + } else { + self.previousLastTimestamp = 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; + } + DDLogVerbose(@"%@ paused observation of database modifications.", self.logTag); + } +} + +- (nullable NSIndexPath *)firstIndexPathAtViewHorizonTimestamp +{ + OWSAssert(self.shouldObserveDBModifications); + + if (!self.viewHorizonTimestamp) { + return nil; + } + if (self.viewItems.count < 1) { + return nil; + } + uint64_t viewHorizonTimestamp = self.viewHorizonTimestamp.unsignedLongLongValue; + // Binary search for the first view item whose timestamp >= the "view horizon" timestamp. + // We want to move "left" rightward, discarding interactions before this cutoff. + // We want to move "right" leftward, discarding all-but-the-first interaction after this cutoff. + // In the end, if we converge on an item _after_ this cutoff, it's the one we want. + // If we converge on an item _before_ this cutoff, there was no interaction that fit our criteria. + NSUInteger left = 0, right = self.viewItems.count - 1; + while (left != right) { + OWSAssert(left < right); + NSUInteger mid = (left + right) / 2; + OWSAssert(left <= mid); + OWSAssert(mid < right); + ConversationViewItem *viewItem = self.viewItems[mid]; + if (viewItem.interaction.timestamp >= viewHorizonTimestamp) { + right = mid; + } else { + // This is an optimization; it also ensures that we converge. + left = mid + 1; + } + } + OWSAssert(left == right); + ConversationViewItem *viewItem = self.viewItems[left]; + if (viewItem.interaction.timestamp >= viewHorizonTimestamp) { + DDLogInfo(@"%@ firstIndexPathAtViewHorizonTimestamp: %zd / %zd", self.logTag, left, self.viewItems.count); + return [NSIndexPath indexPathForRow:(NSInteger) left inSection:0]; + } else { + DDLogInfo(@"%@ firstIndexPathAtViewHorizonTimestamp: none / %zd", self.logTag, self.viewItems.count); + return nil; } } +// We stop observing database modifications when the app or this view is not visible +// (see: shouldObserveDBModifications). When we resume observing db modifications, +// we want to extend the "range" of this view to include any items added to this +// thread while we were not observing. +- (void)extendRangeToIncludeUnobservedItems +{ + if (!self.shouldObserveDBModifications) { + return; + } + if (!self.previousLastTimestamp) { + return; + } + + uint64_t previousLastTimestamp = self.previousLastTimestamp.unsignedLongLongValue; + __block NSUInteger addedItemCount = 0; + [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + [[transaction ext:TSMessageDatabaseViewExtensionName] + enumerateRowsInGroup:self.thread.uniqueId + withOptions:NSEnumerationReverse + usingBlock:^(NSString *collection, + NSString *key, + id object, + id metadata, + NSUInteger index, + BOOL *stop) { + + if (![object isKindOfClass:[TSInteraction class]]) { + OWSFail(@"Expected a TSInteraction: %@", [object class]); + return; + } + + TSInteraction *interaction = (TSInteraction *)object; + if (interaction.timestamp <= previousLastTimestamp) { + *stop = YES; + return; + } + + addedItemCount++; + }]; + }]; + DDLogInfo(@"%@ extendRangeToIncludeUnobservedItems: %zd", self.logTag, addedItemCount); + self.lastRangeLength += addedItemCount; + // We only want to do this once, so clear the "previous last timestamp". + self.previousLastTimestamp = nil; +} + - (void)resetMappings { // If we're entering "active" mode (e.g. view is visible and app is in foreground), @@ -4168,10 +4314,11 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { // We need to `beginLongLivedReadTransaction` before we update our // mapping in order to jump to the most recent commit. [self.uiDatabaseConnection beginLongLivedReadTransaction]; + [self extendRangeToIncludeUnobservedItems]; [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { [self.messageMappings updateWithTransaction:transaction]; }]; - [self updateMessageMappingRangeOptions:MessagesRangeSizeMode_Normal]; + [self updateMessageMappingRangeOptions]; } [self reloadViewItems]; @@ -4179,6 +4326,15 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { [self ensureDynamicInteractions]; [self updateBackButtonUnreadCount]; [self updateNavigationBarSubtitleLabel]; + + // There appears to be a bug in YapDatabase that sometimes delays modifications + // made in another process (e.g. the SAE) from showing up in other processes. + // There's a simple workaround: a trivial write to the database flushes changes + // made from other processes. + [self.editingDatabaseConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [transaction setObject:[NSUUID UUID].UUIDString forKey:@"conversation_view_noop_mod" inCollection:@"temp"]; + l + }]; } #pragma mark - ConversationCollectionViewDelegate diff --git a/Signal/src/ViewControllers/HomeViewController.m b/Signal/src/ViewControllers/HomeViewController.m index 543df0906..0943158d2 100644 --- a/Signal/src/ViewControllers/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeViewController.m @@ -931,10 +931,15 @@ typedef NS_ENUM(NSInteger, CellState) { kArchiveState, kInboxState }; DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); - // External database modifications can't be converted into incremental updates, - // so rebuild everything. This is expensive and usually isn't necessary, but - // there's no alternative. - [self resetMappings]; + if (self.shouldObserveDBModifications) { + // External database modifications can't be converted into incremental updates, + // so rebuild everything. This is expensive and usually isn't necessary, but + // there's no alternative. + // + // We don't need to do this if we're not observing db modifications since we'll + // do it when we resume. + [self resetMappings]; + } } - (void)yapDatabaseModified:(NSNotification *)notification diff --git a/SignalMessaging/Views/ThreadViewHelper.m b/SignalMessaging/Views/ThreadViewHelper.m index 878f1f6b7..c466b60ea 100644 --- a/SignalMessaging/Views/ThreadViewHelper.m +++ b/SignalMessaging/Views/ThreadViewHelper.m @@ -129,16 +129,21 @@ NS_ASSUME_NONNULL_BEGIN DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); - // External database modifications can't be converted into incremental updates, - // so rebuild everything. This is expensive and usually isn't necessary, but - // there's no alternative. - [self.uiDatabaseConnection beginLongLivedReadTransaction]; - [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - [self.threadMappings updateWithTransaction:transaction]; - }]; - - [self updateThreads]; - [self.delegate threadListDidChange]; + if (self.shouldObserveDBModifications) { + // External database modifications can't be converted into incremental updates, + // so rebuild everything. This is expensive and usually isn't necessary, but + // there's no alternative. + // + // We don't need to do this if we're not observing db modifications since we'll + // do it when we resume. + [self.uiDatabaseConnection beginLongLivedReadTransaction]; + [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + [self.threadMappings updateWithTransaction:transaction]; + }]; + + [self updateThreads]; + [self.delegate threadListDidChange]; + } } - (void)yapDatabaseModified:(NSNotification *)notification diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index b424b2aaa..6687f5e57 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -76,7 +76,10 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); OWSAssert(delegate); OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady); - OWSBackgroundTask *backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; + OWSBackgroundTask *_Nullable backgroundTask = nil; + if (CurrentAppContext().isMainApp) { + backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; + } [super readWriteWithBlock:block]; backgroundTask = nil; } @@ -100,7 +103,10 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); OWSAssert(delegate); OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady); - __block OWSBackgroundTask *backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; + __block OWSBackgroundTask *_Nullable backgroundTask = nil; + if (CurrentAppContext().isMainApp) { + backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; + } [super asyncReadWriteWithBlock:block completionQueue:completionQueue completionBlock:^{ if (completionBlock) { completionBlock();