Merge branch 'charlesmchen/redundantSyncSends'

SignalServiceKit/src/Devices/OWSDevice.h

@@ -17,6 +17,7 @@ extern uint32_t const OWSDevicePrimaryDeviceId;
 
 - (BOOL)mayHaveLinkedDevices:(YapDatabaseConnection *)dbConnection;
 - (void)setMayHaveLinkedDevices;
+- (void)clearMayHaveLinkedDevicesIfNotSet;
 
 - (BOOL)hasReceivedSyncMessageInLastSeconds:(NSTimeInterval)intervalSeconds;
 - (void)setHasReceivedSyncMessage;

SignalServiceKit/src/Devices/OWSDevice.m

@@ -19,9 +19,6 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May
 
 @interface OWSDeviceManager ()
 
-// This property should only be accessed while synchronized on self.
-@property (atomic, nullable) NSNumber *mayHaveLinkedDevicesCached;
-
 @property (atomic) NSDate *lastReceivedSyncMessage;
 
 @end
@@ -49,30 +46,34 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May
 {
     OWSAssert(dbConnection);
 
-    @synchronized(self)
-    {
-        if (!self.mayHaveLinkedDevicesCached) {
-            self.mayHaveLinkedDevicesCached = @([dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices
-                                                            inCollection:kTSStorageManager_OWSDeviceCollection
-                                                            defaultValue:YES]);
-        }
+    return [dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices
+                       inCollection:kTSStorageManager_OWSDeviceCollection
+                       defaultValue:YES];
+}
 
-        return [self.mayHaveLinkedDevicesCached boolValue];
-    }
+// In order to avoid skipping necessary sync messages, the default value
+// for mayHaveLinkedDevices is YES.  Once we've successfully sent a
+// sync message with no device messages (e.g. the service has confirmed
+// that we have no linked devices), we can set mayHaveLinkedDevices to NO
+// to avoid unnecessary message sends for sync messages until we learn
+// of a linked device (e.g. through the device linking UI or by receiving
+// a sync message, etc.).
+- (void)clearMayHaveLinkedDevicesIfNotSet
+{
+    // Note that we write async to avoid opening transactions within transactions.
+    [TSStorageManager.sharedManager.newDatabaseConnection
+        asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
+            if (![transaction objectForKey:kTSStorageManager_MayHaveLinkedDevices
+                              inCollection:kTSStorageManager_OWSDeviceCollection]) {
+                [transaction setObject:@(NO)
+                                forKey:kTSStorageManager_MayHaveLinkedDevices
+                          inCollection:kTSStorageManager_OWSDeviceCollection];
+            }
+        }];
 }
 
 - (void)setMayHaveLinkedDevices
 {
-    @synchronized(self)
-    {
-        if (self.mayHaveLinkedDevicesCached != nil && self.mayHaveLinkedDevicesCached.boolValue) {
-            // Skip redundant writes.
-            return;
-        }
-
-        self.mayHaveLinkedDevicesCached = @(YES);
-    }
-
     // Note that we write async to avoid opening transactions within transactions.
     [TSStorageManager.sharedManager.newDatabaseConnection
         asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {

SignalServiceKit/src/Messages/OWSMessageSender.m

@@ -934,9 +934,10 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
     }
 
     NSString *localNumber = [TSAccountManager localNumber];
-    if ([localNumber isEqualToString:recipient.uniqueId]) {
+    BOOL isLocalNumber = [localNumber isEqualToString:recipient.uniqueId];
+    if (isLocalNumber) {
         OWSAssert([message isKindOfClass:[OWSOutgoingSyncMessage class]]);
-        // Messages send to the "local number" should be sync messages.
+        // Messages sent to the "local number" should be sync messages.
         //
         // We can skip sending sync messages if we know that we have no linked
         // devices. However, we need to be sure to handle the case where the
@@ -982,13 +983,38 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
         OWSAssert(deviceMessages.count > 0);
     }
 
+    if (deviceMessages.count == 0) {
+        // This might happen:
+        //
+        // * The first (after upgrading?) time we send a sync message to our linked devices.
+        // * After unlinking all linked devices.
+        // * After trying and failing to link a device.
+        //
+        // When we're not sure if we have linked devices, we need to try
+        // to send self-sync messages even if they have no device messages
+        // so that we can learn from the service whether or not there are
+        // linked devices that we don't know about.
+        DDLogWarn(@"%@ Sending a message with no device messages.", self.logTag);
+    }
+
     TSSubmitMessageRequest *request = [[TSSubmitMessageRequest alloc] initWithRecipient:recipient.uniqueId
                                                                                messages:deviceMessages
                                                                                   relay:recipient.relay
                                                                               timeStamp:message.timestamp];
-
     [self.networkManager makeRequest:request
         success:^(NSURLSessionDataTask *task, id responseObject) {
+            if (isLocalNumber && deviceMessages.count == 0) {
+                DDLogInfo(@"%@ Sent a message with no device messages; clearing 'mayHaveLinkedDevices'.", self.logTag);
+                // In order to avoid skipping necessary sync messages, the default value
+                // for mayHaveLinkedDevices is YES.  Once we've successfully sent a
+                // sync message with no device messages (e.g. the service has confirmed
+                // that we have no linked devices), we can set mayHaveLinkedDevices to NO
+                // to avoid unnecessary message sends for sync messages until we learn
+                // of a linked device (e.g. through the device linking UI or by receiving
+                // a sync message, etc.).
+                [OWSDeviceManager.sharedManager clearMayHaveLinkedDevicesIfNotSet];
+            }
+
             dispatch_async([OWSDispatch sendingQueue], ^{
                 [recipient save];
                 [self handleMessageSentLocally:message];