Rework how messages are marked read.

// FREEBIE

src/Contacts/TSThread.h

@@ -72,7 +72,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (BOOL)hasSafetyNumbers;
 
-- (void)markAllAsRead;
 - (void)markAllAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
 
 /**

src/Contacts/TSThread.m

@@ -228,20 +228,13 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)markAllAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
 {
     for (id<OWSReadTracking> message in [self unseenMessagesWithTransaction:transaction]) {
-        [message markAsReadLocallyWithTransaction:transaction];
+        [message markAsReadWithTransaction:transaction sendReadReceipt:YES];
     }
 
     // Just to be defensive, we'll also check for unread messages.
     OWSAssert([self unseenMessagesWithTransaction:transaction].count < 1);
 }
 
-- (void)markAllAsRead
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
-        [self markAllAsReadWithTransaction:transaction];
-    }];
-}
-
 - (TSInteraction *) lastInteraction {
     __block TSInteraction *last;
     [TSStorageManager.sharedManager.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction){

src/Devices/OWSReadReceiptsProcessor.m

@@ -7,7 +7,10 @@
 #import "OWSReadReceipt.h"
 #import "OWSSignalServiceProtos.pb.h"
 #import "TSContactThread.h"
+#import "TSDatabaseView.h"
 #import "TSIncomingMessage.h"
+#import "TSStorageManager.h"
+#import <YapDatabase/YapDatabaseConnection.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -82,7 +85,36 @@ NSString *const OWSReadReceiptsProcessorMarkedMessageAsReadNotification =
         TSIncomingMessage *message =
             [TSIncomingMessage findMessageWithAuthorId:readReceipt.senderId timestamp:readReceipt.timestamp];
         if (message) {
-            [message markAsReadFromReadReceipt];
+            OWSAssert(message.thread);
+
+            NSMutableArray<id<OWSReadTracking>> *interactionToMarkAsRead = [NSMutableArray new];
+            [self.storageManager.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+                [[transaction ext:TSUnseenDatabaseViewExtensionName]
+                    enumerateRowsInGroup:message.uniqueThreadId
+                              usingBlock:^(NSString *collection,
+                                  NSString *key,
+                                  id object,
+                                  id metadata,
+                                  NSUInteger index,
+                                  BOOL *stop) {
+
+                                  TSInteraction *interaction = object;
+                                  if (interaction.timestampForSorting < message.timestampForSorting) {
+                                      *stop = YES;
+                                      return;
+                                  }
+
+                                  id<OWSReadTracking> possiblyRead = (id<OWSReadTracking>)object;
+                                  OWSAssert(!possiblyRead.read);
+                                  [interactionToMarkAsRead addObject:possiblyRead];
+                              }];
+
+                for (id<OWSReadTracking> possiblyRead in interactionToMarkAsRead) {
+                    // Don't send a read receipt in response to a read receipt.
+                    [possiblyRead markAsReadWithTransaction:transaction sendReadReceipt:NO];
+                }
+            }];
+
             [OWSDisappearingMessagesJob setExpirationForMessage:message expirationStartedAt:readReceipt.timestamp];
             // If it was previously saved, no need to keep it around any longer.
             [readReceipt remove];

src/Messages/Interactions/TSErrorMessage.h

@@ -58,8 +58,6 @@ typedef NS_ENUM(int32_t, TSErrorMessageType) {
 @property (nonatomic, readonly) TSErrorMessageType errorType;
 @property (nullable, nonatomic, readonly) NSString *recipientId;
 
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
-
 @end
 
 NS_ASSUME_NONNULL_END

src/Messages/Interactions/TSErrorMessage.m

@@ -167,19 +167,20 @@ NSUInteger TSErrorMessageSchemaVersion = 1;
     return NO;
 }
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadLocallyWithTransaction:transaction];
-    }];
-}
-
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt
 {
     OWSAssert(transaction);
+
+    if (_read) {
+        return;
+    }
+
     DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
+    [self touchThreadWithTransaction:transaction];
+
+    // Ignore sendReadReceipt; it doesn't apply to error messages.
 }
 
 #pragma mark - Logging

src/Messages/Interactions/TSIncomingMessage.h

@@ -108,20 +108,6 @@ extern NSString *const TSIncomingMessageWasReadOnThisDeviceNotification;
 // This will be 0 for messages created before we were tracking sourceDeviceId
 @property (nonatomic, readonly) UInt32 sourceDeviceId;
 
-/*
- * Marks a message as having been read on this device (as opposed to responding to a remote read receipt).
- *
- */
-- (void)markAsReadLocally;
-// TODO possible to remove?
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
-
-/**
- * Similar to markAsReadWithTransaction, but doesn't send out read receipts.
- * Used for *responding* to a remote read receipt.
- */
-- (void)markAsReadFromReadReceipt;
-
 @end
 
 NS_ASSUME_NONNULL_END

src/Messages/Interactions/TSIncomingMessage.m

@@ -3,6 +3,8 @@
 //
 
 #import "TSIncomingMessage.h"
+#import "OWSDisappearingMessagesConfiguration.h"
+#import "OWSDisappearingMessagesJob.h"
 #import "TSContactThread.h"
 #import "TSDatabaseSecondaryIndexes.h"
 #import "TSGroupThread.h"
@@ -115,37 +117,29 @@ NSString *const TSIncomingMessageWasReadOnThisDeviceNotification = @"TSIncomingM
     return YES;
 }
 
-- (void)markAsReadFromReadReceipt
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt
 {
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadWithoutNotificationWithTransaction:transaction];
-    }];
-}
-
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
-{
-    [self markAsReadWithoutNotificationWithTransaction:transaction];
-    [[NSNotificationCenter defaultCenter] postNotificationName:TSIncomingMessageWasReadOnThisDeviceNotification
-                                                        object:self];
-}
+    OWSAssert(transaction);
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadWithoutNotificationWithTransaction:transaction];
-    }];
-    // Notification must happen outside of the transaction, else we'll likely crash when the notification receiver
-    // tries to do anything with the DB.
-    [[NSNotificationCenter defaultCenter] postNotificationName:TSIncomingMessageWasReadOnThisDeviceNotification
-                                                        object:self];
-}
+    if (_read) {
+        return;
+    }
 
-- (void)markAsReadWithoutNotificationWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
-{
     DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
     [self touchThreadWithTransaction:transaction];
+
+    [OWSDisappearingMessagesJob setExpirationForMessage:self];
+
+    if (sendReadReceipt) {
+        // Notification must happen outside of the transaction, else we'll likely crash when the notification receiver
+        // tries to do anything with the DB.
+        dispatch_async(dispatch_get_main_queue(), ^{
+            [[NSNotificationCenter defaultCenter] postNotificationName:TSIncomingMessageWasReadOnThisDeviceNotification
+                                                                object:self];
+        });
+    }
 }
 
 #pragma mark - Logging

src/Messages/Interactions/TSInfoMessage.h

@@ -44,8 +44,6 @@ typedef NS_ENUM(NSInteger, TSInfoMessageType) {
                  expiresInSeconds:(uint32_t)expiresInSeconds
                   expireStartedAt:(uint64_t)expireStartedAt NS_UNAVAILABLE;
 
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
-
 @end
 
 NS_ASSUME_NONNULL_END

src/Messages/Interactions/TSInfoMessage.m

@@ -109,19 +109,20 @@ NSUInteger TSInfoMessageSchemaVersion = 1;
     return NO;
 }
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadLocallyWithTransaction:transaction];
-    }];
-}
-
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt
 {
     OWSAssert(transaction);
+
+    if (_read) {
+        return;
+    }
+
     DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
+    [self touchThreadWithTransaction:transaction];
+
+    // Ignore sendReadReceipt; it doesn't apply to info messages.
 }
 
 #pragma mark - Logging

src/Messages/OWSIncomingMessageReadObserver.m

@@ -4,8 +4,6 @@
 
 #import "OWSIncomingMessageReadObserver.h"
 #import "NSDate+millisecondTimeStamp.h"
-#import "OWSDisappearingMessagesConfiguration.h"
-#import "OWSDisappearingMessagesJob.h"
 #import "OWSSendReadReceiptsJob.h"
 #import "TSIncomingMessage.h"
 
@@ -13,7 +11,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface OWSIncomingMessageReadObserver ()
 
-@property BOOL isObserving;
+@property (nonatomic) BOOL isObserving;
 @property (nonatomic, readonly) OWSSendReadReceiptsJob *sendReadReceiptsJob;
 
 @end
@@ -41,6 +39,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)startObserving
 {
+    OWSAssert([NSThread isMainThread]);
+
     if (self.isObserving) {
         return;
     }
@@ -60,7 +60,6 @@ NS_ASSUME_NONNULL_BEGIN
     }
 
     TSIncomingMessage *message = (TSIncomingMessage *)notification.object;
-    [OWSDisappearingMessagesJob setExpirationForMessage:message];
     [self.sendReadReceiptsJob runWith:message];
 }
 

src/Messages/OWSReadTracking.h

@@ -20,10 +20,8 @@
 - (BOOL)shouldAffectUnreadCounts;
 
 /**
- * Call when the user viewed the message/call on this device. "locally" as opposed to being notified via a read receipt
- * sync message of a remote read.
+ * Used for *responding* to a remote read receipt or in response to user activity.
  */
-- (void)markAsReadLocally;
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt;
 
 @end

src/Messages/TSCall.m

@@ -86,22 +86,22 @@ NSUInteger TSCallCurrentSchemaVersion = 1;
     return YES;
 }
 
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt
 {
-    DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
+    OWSAssert(transaction);
+
+    if (_read) {
+        return;
+    }
 
+    DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
-
-    // redraw any thread-related unread count UI.
     [self touchThreadWithTransaction:transaction];
-}
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
-        [self markAsReadLocallyWithTransaction:transaction];
-    }];
+    // Ignore sendReadReceipt; it doesn't apply to calls.
+    //
+    // TODO: Should we update expiration of calls?
 }
 
 #pragma mark - Methods

src/Messages/TSMessagesManager.m

@@ -924,7 +924,8 @@ NS_ASSUME_NONNULL_BEGIN
           // automatically marked as read.
           BOOL shouldMarkMessageAsRead = [envelope.source isEqualToString:localNumber];
           if (shouldMarkMessageAsRead) {
-              [incomingMessage markAsReadLocallyWithTransaction:transaction];
+              // Don't send a read receipt for messages sent by ourselves.
+              [incomingMessage markAsReadWithTransaction:transaction sendReadReceipt:NO];
           }
 
           // Other clients allow attachments to be sent along with body, we want the text displayed as a separate