Simplify expireTimer change handling, queue for contact sync

ts/background.ts

@@ -317,7 +317,7 @@ export async function startApp(): Promise<void> {
     );
     messageReceiver.addEventListener(
       'contact',
-      queuedEventListener(onContactReceived)
+      queuedEventListener(onContactReceived, false)
     );
     messageReceiver.addEventListener(
       'contactSync',
@@ -2718,7 +2718,9 @@ export async function startApp(): Promise<void> {
     window.Whisper.events.trigger('contactSync:complete');
   }
 
-  async function onContactReceived(ev: ContactEvent) {
+  // Note: Like the handling for incoming/outgoing messages, this method is synchronous,
+  //   deferring its async logic to the function passed to conversation.queueJob().
+  function onContactReceived(ev: ContactEvent) {
     const details = ev.contactDetails;
 
     const c = new window.Whisper.Conversation({
@@ -2735,60 +2737,66 @@ export async function startApp(): Promise<void> {
       return;
     }
 
-    try {
+    const detailsId = window.ConversationController.ensureContactIds({
-      const detailsId = window.ConversationController.ensureContactIds({
+      e164: details.number,
-        e164: details.number,
+      uuid: details.uuid,
-        uuid: details.uuid,
+      highTrust: true,
-        highTrust: true,
+      reason: 'onContactReceived',
-        reason: 'onContactReceived',
+    });
-      });
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    const conversation = window.ConversationController.get(detailsId)!;
-      const conversation = window.ConversationController.get(detailsId)!;
 
-      conversation.set({
+    // It's important to use queueJob here because we might update the expiration timer
-        name: details.name,
+    //   and we don't want conflicts with incoming message processing happening on the
-        inbox_position: details.inboxPosition,
+    //   conversation queue.
-      });
+    conversation.queueJob('onContactReceived', async () => {
-
+      try {
-      // Update the conversation avatar only if new avatar exists and hash differs
+        conversation.set({
-      const { avatar } = details;
+          name: details.name,
-      if (avatar && avatar.data) {
+          inbox_position: details.inboxPosition,
-        const newAttributes = await Conversation.maybeUpdateAvatar(
-          conversation.attributes,
-          avatar.data,
-          {
-            writeNewAttachmentData,
-            deleteAttachmentData,
-            doesAttachmentExist,
-          }
-        );
-        conversation.set(newAttributes);
-      } else {
-        const { attributes } = conversation;
-        if (attributes.avatar && attributes.avatar.path) {
-          await deleteAttachmentData(attributes.avatar.path);
-        }
-        conversation.set({ avatar: null });
-      }
-
-      window.Signal.Data.updateConversation(conversation.attributes);
-
-      // expireTimer isn't stored in Storage Service so we have to rely on the
-      // contact sync.
-      const { expireTimer } = details;
-      const isValidExpireTimer = typeof expireTimer === 'number';
-      if (isValidExpireTimer) {
-        await conversation.updateExpirationTimer(expireTimer, {
-          source: window.ConversationController.getOurConversationId(),
-          receivedAt: ev.receivedAtCounter,
-          fromSync: true,
-          isInitialSync,
-          reason: 'contact sync',
         });
+
+        // Update the conversation avatar only if new avatar exists and hash differs
+        const { avatar } = details;
+        if (avatar && avatar.data) {
+          const newAttributes = await Conversation.maybeUpdateAvatar(
+            conversation.attributes,
+            avatar.data,
+            {
+              writeNewAttachmentData,
+              deleteAttachmentData,
+              doesAttachmentExist,
+            }
+          );
+          conversation.set(newAttributes);
+        } else {
+          const { attributes } = conversation;
+          if (attributes.avatar && attributes.avatar.path) {
+            await deleteAttachmentData(attributes.avatar.path);
+          }
+          conversation.set({ avatar: null });
+        }
+
+        window.Signal.Data.updateConversation(conversation.attributes);
+
+        // expireTimer isn't in Storage Service so we have to rely on contact sync.
+        const { expireTimer } = details;
+        const isValidExpireTimer = typeof expireTimer === 'number';
+        if (isValidExpireTimer) {
+          await conversation.updateExpirationTimer(expireTimer, {
+            source: window.ConversationController.getOurConversationId(),
+            receivedAt: ev.receivedAtCounter,
+            fromSync: true,
+            isInitialSync,
+            reason: 'contact sync',
+          });
+        }
+
+        window.Whisper.events.trigger('incrementProgress');
+      } catch (error) {
+        log.error('onContactReceived error:', Errors.toLogFormat(error));
       }
-    } catch (error) {
+    });
-      log.error('onContactReceived error:', Errors.toLogFormat(error));
-    }
   }
 
   async function onGroupSyncComplete() {

ts/models/messages.ts

@@ -2045,7 +2045,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
     const idLog = conversation.idForLogging();
     await conversation.queueJob('handleDataMessage', async () => {
       log.info(
-        `handleDataMessage/${idLog}: processsing message ${message.idForLogging()}`
+        `handleDataMessage/${idLog}: processing message ${message.idForLogging()}`
       );
 
       if (
@@ -2603,49 +2603,44 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
                   expireTimer: initialMessage.expireTimer,
                 },
               });
-              conversation.set({ expireTimer: dataMessage.expireTimer });
-            }
 
-            // NOTE: Remove once the above calls this.model.updateExpirationTimer()
+              if (conversation.get('expireTimer') !== dataMessage.expireTimer) {
-            const { expireTimer } = dataMessage;
+                log.info('Incoming expirationTimerUpdate changed timer', {
-            const shouldLogExpireTimerChange =
+                  id: conversation.idForLogging(),
-              isExpirationTimerUpdate(message.attributes) || expireTimer;
+                  expireTimer: dataMessage.expireTimer || 'disabled',
-            if (shouldLogExpireTimerChange) {
+                  source: 'handleDataMessage/expirationTimerUpdate',
-              log.info("Update conversation 'expireTimer'", {
+                });
-                id: conversation.idForLogging(),
+                conversation.set({
-                expireTimer,
+                  expireTimer: dataMessage.expireTimer,
-                source: 'handleDataMessage',
-              });
-            }
-
-            if (!isEndSession(message.attributes)) {
-              if (dataMessage.expireTimer) {
-                if (
-                  dataMessage.expireTimer !== conversation.get('expireTimer')
-                ) {
-                  conversation.updateExpirationTimer(dataMessage.expireTimer, {
-                    source,
-                    receivedAt: message.get('received_at'),
-                    receivedAtMS: message.get('received_at_ms'),
-                    sentAt: message.get('sent_at'),
-                    fromGroupUpdate: isGroupUpdate(message.attributes),
-                    reason: `handleDataMessage(${this.idForLogging()})`,
-                  });
-                }
-              } else if (
-                conversation.get('expireTimer') &&
-                // We only turn off timers if it's not a group update
-                !isGroupUpdate(message.attributes)
-              ) {
-                conversation.updateExpirationTimer(undefined, {
-                  source,
-                  receivedAt: message.get('received_at'),
-                  receivedAtMS: message.get('received_at_ms'),
-                  sentAt: message.get('sent_at'),
-                  reason: `handleDataMessage(${this.idForLogging()})`,
                 });
               }
             }
+
+            // Note: For incoming expire timer updates (not normal messages that come
+            //   along with an expireTimer), the conversation will be updated by this
+            //   point and these calls will return early.
+            if (dataMessage.expireTimer) {
+              conversation.updateExpirationTimer(dataMessage.expireTimer, {
+                source,
+                receivedAt: message.get('received_at'),
+                receivedAtMS: message.get('received_at_ms'),
+                sentAt: message.get('sent_at'),
+                fromGroupUpdate: isGroupUpdate(message.attributes),
+                reason: `handleDataMessage(${this.idForLogging()})`,
+              });
+            } else if (
+              // We won't turn off timers for these kinds of messages:
+              !isGroupUpdate(message.attributes) &&
+              !isEndSession(message.attributes)
+            ) {
+              conversation.updateExpirationTimer(undefined, {
+                source,
+                receivedAt: message.get('received_at'),
+                receivedAtMS: message.get('received_at_ms'),
+                sentAt: message.get('sent_at'),
+                reason: `handleDataMessage(${this.idForLogging()})`,
+              });
+            }
           }
 
           if (initialMessage.profileKey) {

ts/textsecure/MessageReceiver.ts

@@ -2885,7 +2885,7 @@ export default class MessageReceiver
     envelope: ProcessedEnvelope,
     contacts: Proto.SyncMessage.IContacts
   ): Promise<void> {
-    log.info('MessageReceiver: handleContacts');
+    log.info(`MessageReceiver: handleContacts ${getEnvelopeId(envelope)}`);
     const { blob } = contacts;
     if (!blob) {
       throw new Error('MessageReceiver.handleContacts: blob field was missing');
@@ -2924,6 +2924,7 @@ export default class MessageReceiver
     groups: Proto.SyncMessage.IGroups
   ): Promise<void> {
     log.info('group sync');
+    log.info(`MessageReceiver: handleGroups ${getEnvelopeId(envelope)}`);
     const { blob } = groups;
 
     this.removeFromCache(envelope);