Merge branch 'charlesmchen/profileAvatarDownloads' into release/2.21.0

SignalMessaging/profiles/OWSProfileManager.m

@@ -785,12 +785,9 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
     return nil;
 }
 
-- (void)downloadAvatarForUserProfile:(OWSUserProfile *)userProfileParameter
+- (void)downloadAvatarForUserProfile:(OWSUserProfile *)userProfile
 {
-    OWSAssert(userProfileParameter);
-
-    // Make a local copy.
-    OWSUserProfile *userProfile = [userProfileParameter copy];
+    OWSAssert(userProfile);
 
     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
         if (userProfile.avatarUrlPath.length < 1) {
@@ -817,6 +814,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
             [self.currentAvatarDownloads addObject:userProfile.recipientId];
         }
 
+        DDLogVerbose(@"%@ downloading profile avatar: %@", self.logTag, userProfile.uniqueId);
+
         NSString *tempDirectory = NSTemporaryDirectory();
         NSString *tempFilePath = [tempDirectory stringByAppendingPathComponent:fileName];
 
@@ -920,7 +919,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
 
         [userProfile updateWithProfileName:profileName
                              avatarUrlPath:avatarUrlPath
-                            avatarFileName:userProfile.avatarFileName // use existing file name if already downloaded
                               dbConnection:self.dbConnection
                                 completion:nil];
 
@@ -931,18 +929,17 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640;
             OWSUserProfile *localUserProfile = self.localUserProfile;
             OWSAssert(localUserProfile);
 
-            // Don't clear avatarFileName optimistically.
-            // * The profile avatar probably isn't out of sync.
-            // * If the profile avatar is out of sync, it can be synced on next app launch.
-            // * We don't want to touch local avatar state until we've
-            //   downloaded the latest avatar by downloadAvatarForUserProfile.
             [localUserProfile updateWithProfileName:profileName
                                       avatarUrlPath:avatarUrlPath
                                        dbConnection:self.dbConnection
                                          completion:nil];
         }
 
-        if (userProfile.avatarUrlPath.length > 0 && userProfile.avatarFileName.length == 0) {
+        // Whenever we change avatarUrlPath, OWSUserProfile clears avatarFileName.
+        // So if avatarUrlPath is set and avatarFileName is not set, we should to
+        // download this avatar. downloadAvatarForUserProfile will de-bounce
+        // downloads.
+        if (userProfile.avatarUrlPath.length > 0 && userProfile.avatarFileName.length < 1) {
             [self downloadAvatarForUserProfile:userProfile];
         }
     });

SignalMessaging/profiles/OWSUserProfile.h

@@ -1,5 +1,5 @@
 //
-//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
 //
 
 #import <SignalServiceKit/TSYapDatabaseObject.h>

SignalMessaging/profiles/OWSUserProfile.m

@@ -109,12 +109,14 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId";
         BOOL didChange;
         if (_avatarUrlPath == nil && avatarUrlPath == nil) {
             didChange = NO;
-        } else if (_avatarUrlPath != nil && avatarUrlPath != nil) {
+        } else if (_avatarUrlPath != nil || avatarUrlPath != nil) {
             didChange = YES;
         } else {
             didChange = [_avatarUrlPath isEqualToString:avatarUrlPath];
         }
 
+        _avatarUrlPath = avatarUrlPath;
+
         if (didChange) {
             // If the avatarURL changed, the avatarFileName can't be valid.
             // Clear it.
@@ -137,7 +139,9 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId";
         dbConnection:(YapDatabaseConnection *)dbConnection
           completion:(nullable OWSUserProfileCompletion)completion
 {
-    NSDictionary *beforeSnapshot = self.dictionaryValue;
+    // self might be the latest instance, so take a "before" snapshot
+    // before any changes have been made.
+    __block NSDictionary *beforeSnapshot = [self.dictionaryValue copy];
 
     changeBlock(self);
 
@@ -146,12 +150,21 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId";
         NSString *collection = [[self class] collection];
         OWSUserProfile *_Nullable latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection];
         if (latestInstance) {
+            // If self is NOT the latest instance, take a new "before" snapshot
+            // before updating.
+            if (self != latestInstance) {
+                beforeSnapshot = [latestInstance.dictionaryValue copy];
+            }
+
             changeBlock(latestInstance);
 
-            NSDictionary *afterSnapshot = latestInstance.dictionaryValue;
+            NSDictionary *afterSnapshot = [latestInstance.dictionaryValue copy];
+
             if ([beforeSnapshot isEqual:afterSnapshot]) {
-                DDLogVerbose(
-                    @"%@ Ignoring redundant update in %s: %@", self.logTag, functionName, self.debugDescription);
+                DDLogVerbose(@"%@ Ignoring redundant update in %s: %@",
+                    self.logTag,
+                    functionName,
+                    self.debugDescription);
                 didChange = NO;
             } else {
                 [latestInstance saveWithTransaction:transaction];