From bd440f087887be9d3c58a9c27d971e89bc0956af Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 15 Jun 2017 15:19:33 -0400 Subject: [PATCH] Respond to post-holiday code reviews. // FREEBIE --- Podfile | 4 +-- Podfile.lock | 9 ++---- .../Models/OWSMessagesBubblesSizeCalculator.m | 8 ++--- Signal/src/ProfileFetcherJob.swift | 29 +++++++++---------- .../ConversationView/MessagesViewController.m | 8 ++--- .../DebugUI/DebugUISessionState.m | 24 +++++++-------- .../DebugUI/DebugUIVerification.m | 2 +- .../FingerprintViewController.m | 8 ++--- .../FingerprintViewScanController.m | 2 +- .../SafetyNumberConfirmationAlert.swift | 2 +- Signal/src/call/CallService.swift | 6 ++-- Signal/src/views/OWSSystemMessageCell.m | 14 +-------- Signal/src/views/OWSUnreadIndicatorCell.m | 14 +-------- 13 files changed, 50 insertions(+), 80 deletions(-) diff --git a/Podfile b/Podfile index 64c1915dc..a33208ad2 100644 --- a/Podfile +++ b/Podfile @@ -5,8 +5,8 @@ target 'Signal' do pod 'SocketRocket', :git => 'https://github.com/facebook/SocketRocket.git' pod 'AxolotlKit', git: 'https://github.com/WhisperSystems/SignalProtocolKit.git' #pod 'AxolotlKit', path: '../SignalProtocolKit' - pod 'SignalServiceKit', git: 'https://github.com/WhisperSystems/SignalServiceKit.git' - #pod 'SignalServiceKit', path: '../SignalServiceKit' + #pod 'SignalServiceKit', git: 'https://github.com/WhisperSystems/SignalServiceKit.git' + pod 'SignalServiceKit', path: '../SignalServiceKit' pod 'JSQMessagesViewController', git: 'https://github.com/WhisperSystems/JSQMessagesViewController.git', branch: 'mkirk/position-edit-menu' #pod 'JSQMessagesViewController' path: '../JSQMessagesViewController' pod 'PureLayout' diff --git a/Podfile.lock b/Podfile.lock index 7b15cf529..a26c97191 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -112,7 +112,7 @@ DEPENDENCIES: - JSQMessagesViewController (from `https://github.com/WhisperSystems/JSQMessagesViewController.git`, branch `mkirk/position-edit-menu`) - PureLayout - Reachability - - SignalServiceKit (from `https://github.com/WhisperSystems/SignalServiceKit.git`) + - SignalServiceKit (from `../SignalServiceKit`) - SocketRocket (from `https://github.com/facebook/SocketRocket.git`) EXTERNAL SOURCES: @@ -122,7 +122,7 @@ EXTERNAL SOURCES: :branch: mkirk/position-edit-menu :git: https://github.com/WhisperSystems/JSQMessagesViewController.git SignalServiceKit: - :git: https://github.com/WhisperSystems/SignalServiceKit.git + :path: ../SignalServiceKit SocketRocket: :git: https://github.com/facebook/SocketRocket.git @@ -133,9 +133,6 @@ CHECKOUT OPTIONS: JSQMessagesViewController: :commit: 7054e4b13ee5bcd6d524adb6dc9a726e8c466308 :git: https://github.com/WhisperSystems/JSQMessagesViewController.git - SignalServiceKit: - :commit: a9bac8bce7ea3a0209024d0b0a937d45748aea97 - :git: https://github.com/WhisperSystems/SignalServiceKit.git SocketRocket: :commit: 877ac7438be3ad0b45ef5ca3969574e4b97112bf :git: https://github.com/facebook/SocketRocket.git @@ -161,6 +158,6 @@ SPEC CHECKSUMS: UnionFind: c33be5adb12983981d6e827ea94fc7f9e370f52d YapDatabase: cd911121580ff16675f65ad742a9eb0ab4d9e266 -PODFILE CHECKSUM: 89fd7aee1e2b0ca592ecc9dba0389e57b70f959b +PODFILE CHECKSUM: 6e5d90a9603eb043b395213fd8a29037d2276a8f COCOAPODS: 1.2.1 diff --git a/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m b/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m index 2be1073b7..b003c5c6c 100644 --- a/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m +++ b/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m @@ -38,8 +38,8 @@ NS_ASSUME_NONNULL_BEGIN @interface OWSMessagesBubblesSizeCalculator () -@property (nonatomic) OWSSystemMessageCell *referenceSystemMessageCell; -@property (nonatomic) OWSUnreadIndicatorCell *referenceUnreadIndicatorCell; +@property (nonatomic, readonly) OWSSystemMessageCell *referenceSystemMessageCell; +@property (nonatomic, readonly) OWSUnreadIndicatorCell *referenceUnreadIndicatorCell; @end @@ -120,7 +120,7 @@ NS_ASSUME_NONNULL_BEGIN } if (!self.referenceSystemMessageCell) { - self.referenceSystemMessageCell = [OWSSystemMessageCell new]; + _referenceSystemMessageCell = [OWSSystemMessageCell new]; } CGSize result = [self.referenceSystemMessageCell cellSizeForInteraction:interaction @@ -144,7 +144,7 @@ NS_ASSUME_NONNULL_BEGIN } if (!self.referenceUnreadIndicatorCell) { - self.referenceUnreadIndicatorCell = [OWSUnreadIndicatorCell new]; + _referenceUnreadIndicatorCell = [OWSUnreadIndicatorCell new]; } CGSize result = [self.referenceUnreadIndicatorCell cellSizeForInteraction:interaction diff --git a/Signal/src/ProfileFetcherJob.swift b/Signal/src/ProfileFetcherJob.swift index dc8910c68..619df461f 100644 --- a/Signal/src/ProfileFetcherJob.swift +++ b/Signal/src/ProfileFetcherJob.swift @@ -14,7 +14,7 @@ class ProfileFetcherJob: NSObject { let thread: TSThread - // This property is only accessed on the default global queue. + // This property is only accessed on the main queue. static var fetchDateMap = [String: Date]() public class func run(thread: TSThread, networkManager: TSNetworkManager) { @@ -31,7 +31,7 @@ class ProfileFetcherJob: NSObject { public func run() { AssertIsOnMainThread() - DispatchQueue.global().async { + DispatchQueue.main.async { for recipientId in self.thread.recipientIdentifiers { self.getProfile(recipientId: recipientId) } @@ -40,27 +40,24 @@ class ProfileFetcherJob: NSObject { public func getProfile(recipientId: String, remainingRetries: Int = 3) { - // Only throttle profile fetch in production builds in order to - // facilitate debugging. - if !_isDebugAssertConfiguration() { - if let lastDate = ProfileFetcherJob.fetchDateMap[recipientId] { - let lastTimeInterval = fabs(lastDate.timeIntervalSinceNow) - // Don't check a profile more often than every N minutes. - let kGetProfileMaxFrequencySeconds = 60.0 * 5.0 - if lastTimeInterval < kGetProfileMaxFrequencySeconds { - Logger.info("\(self.TAG) skipping getProfile: \(recipientId), lastTimeInterval: \(lastTimeInterval)") - return - } + if let lastDate = ProfileFetcherJob.fetchDateMap[recipientId] { + let lastTimeInterval = fabs(lastDate.timeIntervalSinceNow) + // Don't check a profile more often than every N minutes. + // + // Only throttle profile fetch in production builds in order to + // facilitate debugging. + let kGetProfileMaxFrequencySeconds = _isDebugAssertConfiguration() ? 0 : 60.0 * 5.0 + guard lastTimeInterval > kGetProfileMaxFrequencySeconds else { + Logger.info("\(self.TAG) skipping getProfile: \(recipientId), lastTimeInterval: \(lastTimeInterval)") + return } - ProfileFetcherJob.fetchDateMap[recipientId] = Date() } + ProfileFetcherJob.fetchDateMap[recipientId] = Date() Logger.error("\(self.TAG) getProfile: \(recipientId)") let request = OWSGetProfileRequest(recipientId: recipientId) - // We don't need to retainUntilComplete() since the success and failure - // handlers both close over a strong reference to self. self.networkManager.makeRequest( request, success: { (_: URLSessionDataTask?, responseObject: Any?) -> Void in diff --git a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m index c3c95660f..a4e3b87cc 100644 --- a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m +++ b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m @@ -685,7 +685,7 @@ typedef enum : NSUInteger { [result addObject:recipientId]; } } - return result; + return [result copy]; } - (void)ensureBannerState @@ -838,7 +838,7 @@ typedef enum : NSUInteger { preferredStyle:UIAlertControllerStyleActionSheet]; __weak MessagesViewController *weakSelf = self; - UIAlertAction *unblockAction = [UIAlertAction + UIAlertAction *verifyAction = [UIAlertAction actionWithTitle: NSLocalizedString(@"VERIFY_PRIVACY", @"Label for button or row which allows users to verify the safety number of another user.") @@ -846,7 +846,7 @@ typedef enum : NSUInteger { handler:^(UIAlertAction *_Nonnull action) { [weakSelf showConversationSettingsAndShowVerification:YES]; }]; - [actionSheetController addAction:unblockAction]; + [actionSheetController addAction:verifyAction]; UIAlertAction *dismissAction = [UIAlertAction actionWithTitle:NSLocalizedString(@"DISMISS_BUTTON_TEXT", @@ -882,7 +882,7 @@ typedef enum : NSUInteger { [OWSIdentityManager.sharedManager setVerificationState:OWSVerificationStateDefault identityKey:identityKey recipientId:recipientId - sendSyncMessage:YES]; + isUserInitiatedChange:YES]; } } diff --git a/Signal/src/ViewControllers/DebugUI/DebugUISessionState.m b/Signal/src/ViewControllers/DebugUI/DebugUISessionState.m index 6e19ae3ce..2a5fb9001 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUISessionState.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUISessionState.m @@ -90,10 +90,10 @@ NS_ASSUME_NONNULL_BEGIN NSData *identityKey = [identityManger identityKeyForRecipientId:recipientId]; [[OWSIdentityManager sharedManager] - setVerificationState:OWSVerificationStateDefault - identityKey:identityKey - recipientId:recipientId - sendSyncMessage:NO]; + setVerificationState:OWSVerificationStateDefault + identityKey:identityKey + recipientId:recipientId + isUserInitiatedChange:NO]; }]]; [alertController addAction:[UIAlertAction actionWithTitle:@"Verified" style:UIAlertActionStyleDefault @@ -101,10 +101,10 @@ NS_ASSUME_NONNULL_BEGIN NSData *identityKey = [identityManger identityKeyForRecipientId:recipientId]; [[OWSIdentityManager sharedManager] - setVerificationState:OWSVerificationStateVerified - identityKey:identityKey - recipientId:recipientId - sendSyncMessage:NO]; + setVerificationState:OWSVerificationStateVerified + identityKey:identityKey + recipientId:recipientId + isUserInitiatedChange:NO]; }]]; [alertController addAction:[UIAlertAction actionWithTitle:@"No Longer Verified" style:UIAlertActionStyleDefault @@ -112,10 +112,10 @@ NS_ASSUME_NONNULL_BEGIN NSData *identityKey = [identityManger identityKeyForRecipientId:recipientId]; [[OWSIdentityManager sharedManager] - setVerificationState:OWSVerificationStateNoLongerVerified - identityKey:identityKey - recipientId:recipientId - sendSyncMessage:NO]; + setVerificationState:OWSVerificationStateNoLongerVerified + identityKey:identityKey + recipientId:recipientId + isUserInitiatedChange:NO]; }]]; [[UIApplication sharedApplication].frontmostViewController presentViewController:alertController diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIVerification.m b/Signal/src/ViewControllers/DebugUI/DebugUIVerification.m index 9f6890c95..1529a5512 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIVerification.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIVerification.m @@ -69,7 +69,7 @@ NS_ASSUME_NONNULL_BEGIN [OWSIdentityManager.sharedManager setVerificationState:verificationState identityKey:identityKey recipientId:recipientId - sendSyncMessage:verificationState != OWSVerificationStateNoLongerVerified]; + isUserInitiatedChange:verificationState != OWSVerificationStateNoLongerVerified]; } @end diff --git a/Signal/src/ViewControllers/FingerprintViewController.m b/Signal/src/ViewControllers/FingerprintViewController.m index e7a193d90..2286dd8f0 100644 --- a/Signal/src/ViewControllers/FingerprintViewController.m +++ b/Signal/src/ViewControllers/FingerprintViewController.m @@ -510,10 +510,10 @@ typedef void (^CustomLayoutBlock)(); BOOL isVerified = [[OWSIdentityManager sharedManager] verificationStateForRecipientId:self.recipientId] == OWSVerificationStateVerified; [[OWSIdentityManager sharedManager] - setVerificationState:(isVerified ? OWSVerificationStateDefault : OWSVerificationStateVerified)identityKey - :self.identityKey - recipientId:self.recipientId - sendSyncMessage:YES]; + setVerificationState:(isVerified ? OWSVerificationStateDefault : OWSVerificationStateVerified)identityKey + :self.identityKey + recipientId:self.recipientId + isUserInitiatedChange:YES]; [self dismissViewControllerAnimated:YES completion:nil]; } diff --git a/Signal/src/ViewControllers/FingerprintViewScanController.m b/Signal/src/ViewControllers/FingerprintViewScanController.m index 30beaa332..c49b42c2c 100644 --- a/Signal/src/ViewControllers/FingerprintViewScanController.m +++ b/Signal/src/ViewControllers/FingerprintViewScanController.m @@ -205,7 +205,7 @@ NS_ASSUME_NONNULL_BEGIN [OWSIdentityManager.sharedManager setVerificationState:OWSVerificationStateVerified identityKey:identityKey recipientId:recipientId - sendSyncMessage:YES]; + isUserInitiatedChange:YES]; [viewController dismissViewControllerAnimated:true completion:nil]; }]]; UIAlertAction *dismissAction = diff --git a/Signal/src/ViewControllers/SafetyNumberConfirmationAlert.swift b/Signal/src/ViewControllers/SafetyNumberConfirmationAlert.swift index 8e47fc2c9..44f2ac1aa 100644 --- a/Signal/src/ViewControllers/SafetyNumberConfirmationAlert.swift +++ b/Signal/src/ViewControllers/SafetyNumberConfirmationAlert.swift @@ -55,7 +55,7 @@ class SafetyNumberConfirmationAlert: NSObject { Logger.info("\(self.TAG) Confirmed identity: \(untrustedIdentity)") OWSDispatch.sessionStoreQueue().async { - OWSIdentityManager.shared().setVerificationState(.default, identityKey: untrustedIdentity.identityKey, recipientId: untrustedIdentity.recipientId, sendSyncMessage: true) + OWSIdentityManager.shared().setVerificationState(.default, identityKey: untrustedIdentity.identityKey, recipientId: untrustedIdentity.recipientId, isUserInitiatedChange: true) DispatchQueue.main.async { completion(true) } diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index be252faed..e9a61740c 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -500,13 +500,13 @@ protocol CallServiceObserver: class { return } - + guard self.call == nil else { // TODO on iOS10+ we can use CallKit to swap calls rather than just returning busy immediately. Logger.info("\(TAG) receivedCallOffer for thread: \(thread) but we're already in call: \(call!)") - + handleLocalBusyCall(newCall, thread: thread) - + return } diff --git a/Signal/src/views/OWSSystemMessageCell.m b/Signal/src/views/OWSSystemMessageCell.m index 37c1eec5d..2ade8c5eb 100644 --- a/Signal/src/views/OWSSystemMessageCell.m +++ b/Signal/src/views/OWSSystemMessageCell.m @@ -40,21 +40,9 @@ NS_ASSUME_NONNULL_BEGIN return self; } -- (instancetype)init -{ - if (self = [super init]) { - [self commontInit]; - } - - return self; -} - - (void)commontInit { - if (self.imageView) { - // Don't init twice. - return; - } + OWSAssert(!self.imageView); [self setTranslatesAutoresizingMaskIntoConstraints:NO]; diff --git a/Signal/src/views/OWSUnreadIndicatorCell.m b/Signal/src/views/OWSUnreadIndicatorCell.m index c3599152e..2af3c7610 100644 --- a/Signal/src/views/OWSUnreadIndicatorCell.m +++ b/Signal/src/views/OWSUnreadIndicatorCell.m @@ -38,21 +38,9 @@ NS_ASSUME_NONNULL_BEGIN return self; } -- (instancetype)init -{ - if (self = [super init]) { - [self commontInit]; - } - - return self; -} - - (void)commontInit { - if (self.bannerView) { - // Don't init twice. - return; - } + OWSAssert(!self.bannerView); [self setTranslatesAutoresizingMaskIntoConstraints:NO];