Respond to CR.

Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m

@@ -72,7 +72,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)updateICloudStatus
 {
     __weak OWSBackupSettingsViewController *weakSelf = self;
-    [[self.backup checkCloudKitAccess]
+    [[self.backup ensureCloudKitAccess]
             .then(^{
                 OWSAssertIsOnMainThread();
 

Signal/src/ViewControllers/DebugUI/DebugUIBackup.m

@@ -88,7 +88,7 @@ NS_ASSUME_NONNULL_BEGIN
     OWSAssertDebug(success);
 
     NSString *recipientId = self.tsAccountManager.localNumber;
-    [[self.backup checkCloudKitAccess].then(^{
+    [[self.backup ensureCloudKitAccess].then(^{
         return
             [OWSBackupAPI saveTestFileToCloudObjcWithRecipientId:recipientId fileUrl:[NSURL fileURLWithPath:filePath]];
     }) retainUntilComplete];

Signal/src/util/Backup/OWSBackup.h

@@ -74,7 +74,7 @@ NSError *OWSBackupErrorWithDescription(NSString *description);
 
 - (void)allRecipientIdsWithManifestsInCloud:(OWSBackupStringListBlock)success failure:(OWSBackupErrorBlock)failure;
 
-- (AnyPromise *)checkCloudKitAccess;
+- (AnyPromise *)ensureCloudKitAccess;
 
 - (AnyPromise *)checkCanExportBackup;
 

Signal/src/util/Backup/OWSBackup.m

@@ -217,7 +217,7 @@ NSError *OWSBackupErrorWithDescription(NSString *description)
     _backupExportState = OWSBackupState_InProgress;
 
     self.backupExportJob = [[OWSBackupExportJob alloc] initWithDelegate:self recipientId:recipientId];
-    [self.backupExportJob startAsync];
+    [self.backupExportJob start];
 
     [self postDidChangeNotification];
 }
@@ -369,7 +369,7 @@ NSError *OWSBackupErrorWithDescription(NSString *description)
         self.backupExportJob = nil;
     } else if (self.shouldHaveBackupExport && !self.backupExportJob) {
         self.backupExportJob = [[OWSBackupExportJob alloc] initWithDelegate:self recipientId:recipientId];
-        [self.backupExportJob startAsync];
+        [self.backupExportJob start];
     }
 
     // Update the state flag.
@@ -423,10 +423,10 @@ NSError *OWSBackupErrorWithDescription(NSString *description)
 
 - (AnyPromise *)checkCanExportBackup
 {
-    return [self checkCloudKitAccess];
+    return [self ensureCloudKitAccess];
 }
 
-- (AnyPromise *)checkCloudKitAccess
+- (AnyPromise *)ensureCloudKitAccess
 {
     OWSAssertIsOnMainThread();
 
@@ -452,7 +452,7 @@ NSError *OWSBackupErrorWithDescription(NSString *description)
         return failWithUnexpectedError();
     }
 
-    return [[OWSBackupAPI checkCloudKitAccessObjc] retainUntilComplete];
+    return [OWSBackupAPI ensureCloudKitAccessObjc];
 }
 
 - (void)checkCanImportBackup:(OWSBackupBoolBlock)success failure:(OWSBackupErrorBlock)failure
@@ -484,7 +484,7 @@ NSError *OWSBackupErrorWithDescription(NSString *description)
         return failWithUnexpectedError();
     }
 
-    [[OWSBackupAPI checkCloudKitAccessObjc]
+    [[OWSBackupAPI ensureCloudKitAccessObjc]
             .thenInBackground(^{
                 return [OWSBackupAPI checkForManifestInCloudObjcWithRecipientId:recipientId];
             })

Signal/src/util/Backup/OWSBackupAPI.swift

@@ -202,23 +202,14 @@ import PromiseKit
     }
 
     public class func saveRecordToCloud(record: CKRecord) -> Promise<String> {
-        let (promise, resolver) = Promise<String>.pending()
+        return saveRecordToCloud(record: record,
-        saveRecordToCloud(record: record,
+                                 remainingRetries: maxRetries)
-                          remainingRetries: maxRetries,
-                          success: { (recordName) in
-                            resolver.fulfill(recordName)
-        },
-                          failure: { (error) in
-                            resolver.reject(error)
-        })
-        return promise.retainUntilComplete()
     }
 
     private class func saveRecordToCloud(record: CKRecord,
-                                         remainingRetries: Int,
+                                         remainingRetries: Int) -> Promise<String> {
-                                         success: @escaping (String) -> Void,
-                                         failure: @escaping (Error) -> Void) {
 
+        return Promise { resolver in
             let saveOperation = CKModifyRecordsOperation(recordsToSave: [record ], recordIDsToDelete: nil)
             saveOperation.modifyRecordsCompletionBlock = { (records, recordIds, error) in
 
@@ -228,26 +219,32 @@ import PromiseKit
                 switch outcome {
                 case .success:
                     let recordName = record.recordID.recordName
-                success(recordName)
+                    resolver.fulfill(recordName)
                 case .failureDoNotRetry(let outcomeError):
-                failure(outcomeError)
+                    resolver.reject(outcomeError)
                 case .failureRetryAfterDelay(let retryDelay):
                     DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: {
                         saveRecordToCloud(record: record,
-                                      remainingRetries: remainingRetries - 1,
+                                          remainingRetries: remainingRetries - 1)
-                                      success: success,
+                            .done { (recordName) in
-                                      failure: failure)
+                                resolver.fulfill(recordName)
+                            }.catch { (error) in
+                                resolver.reject(error)
+                            }.retainUntilComplete()
                     })
                 case .failureRetryWithoutDelay:
                     DispatchQueue.global().async {
                         saveRecordToCloud(record: record,
-                                      remainingRetries: remainingRetries - 1,
+                                          remainingRetries: remainingRetries - 1)
-                                      success: success,
+                            .done { (recordName) in
-                                      failure: failure)
+                                resolver.fulfill(recordName)
+                            }.catch { (error) in
+                                resolver.reject(error)
+                            }.retainUntilComplete()
                     }
                 case .unknownItem:
                     owsFailDebug("unexpected CloudKit response.")
-                failure(invalidServiceResponseError())
+                    resolver.reject(invalidServiceResponseError())
                 }
             }
             saveOperation.isAtomic = false
@@ -260,6 +257,7 @@ import PromiseKit
 
             database().add(saveOperation)
         }
+    }
 
     // Compare:
     // * An "upsert" creates a new record if none exists and
@@ -422,7 +420,7 @@ import PromiseKit
                             resolver.fulfill(record)
                         }.catch { (error) in
                             resolver.reject(error)
-                    }
+                        }.retainUntilComplete()
                 })
             case .failureRetryWithoutDelay:
                 DispatchQueue.global().async {
@@ -432,7 +430,7 @@ import PromiseKit
                             resolver.fulfill(record)
                         }.catch { (error) in
                             resolver.reject(error)
-                    }
+                        }.retainUntilComplete()
                 }
             case .unknownItem:
                 // Record not found.
@@ -440,7 +438,7 @@ import PromiseKit
             }
         }
         database().add(fetchOperation)
-        return promise.retainUntilComplete()
+        return promise
     }
 
     @objc
@@ -701,13 +699,13 @@ import PromiseKit
     }
 
     @objc
-    public class func checkCloudKitAccessObjc() -> AnyPromise {
+    public class func ensureCloudKitAccessObjc() -> AnyPromise {
-        return AnyPromise(checkCloudKitAccess())
+        return AnyPromise(ensureCloudKitAccess())
     }
 
-    public class func checkCloudKitAccess() -> Promise<Void> {
+    public class func ensureCloudKitAccess() -> Promise<Void> {
         let (promise, resolver) = Promise<Void>.pending()
-        CKContainer.default().accountStatus(completionHandler: { (accountStatus, error) in
+        CKContainer.default().accountStatus { (accountStatus, error) in
             if let error = error {
                 Logger.error("Unknown error: \(String(describing: error)).")
                 resolver.reject(error)
@@ -727,7 +725,7 @@ import PromiseKit
                 Logger.verbose("CloudKit access okay.")
                 resolver.fulfill(())
             }
-        })
+        }
         return promise
     }
 

Signal/src/util/Backup/OWSBackupExportJob.h

@@ -8,7 +8,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface OWSBackupExportJob : OWSBackupJob
 
-- (void)startAsync;
+- (void)start;
 
 @end
 

Signal/src/util/Backup/OWSBackupExportJob.m

@@ -337,7 +337,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark -
 
-- (void)startAsync
+- (void)start
 {
     OWSAssertIsOnMainThread();
 
@@ -347,24 +347,14 @@ NS_ASSUME_NONNULL_BEGIN
 
     [self updateProgressWithDescription:nil progress:nil];
 
-    [[self.backup checkCloudKitAccess]
+    [[self.backup ensureCloudKitAccess]
             .thenInBackground(^{
-                [self start];
-            })
-            .catch(^(NSError *error) {
-                [self failWithErrorDescription:
-                          NSLocalizedString(@"BACKUP_EXPORT_ERROR_COULD_NOT_EXPORT",
-                              @"Error indicating the backup export could not export the user's data.")];
-            }) retainUntilComplete];
-}
-
-- (void)start
-{
                 [self updateProgressWithDescription:NSLocalizedString(@"BACKUP_EXPORT_PHASE_CONFIGURATION",
                                                         @"Indicates that the backup export is being configured.")
                                            progress:nil];
 
-    [[self configureExport]
+                return [self configureExport];
+            })
             .thenInBackground(^{
                 return [self fetchAllRecords];
             })
@@ -415,7 +405,7 @@ NS_ASSUME_NONNULL_BEGIN
     //
     // We use an arbitrary request that requires authentication
     // to verify our account state.
-    AnyPromise *promise = [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) {
+    return [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) {
         TSRequest *currentSignedPreKey = [OWSRequestFactory currentSignedPreKeyRequest];
         [[TSNetworkManager sharedManager] makeRequest:currentSignedPreKey
             success:^(NSURLSessionDataTask *task, NSDictionary *responseObject) {
@@ -427,8 +417,6 @@ NS_ASSUME_NONNULL_BEGIN
                 resolve(error);
             }];
     }];
-    [promise retainUntilComplete];
-    return promise;
 }
 
 - (AnyPromise *)fetchAllRecords
@@ -439,7 +427,7 @@ NS_ASSUME_NONNULL_BEGIN
         return [AnyPromise promiseWithValue:OWSBackupErrorWithDescription(@"Backup export no longer active.")];
     }
 
-    AnyPromise *promise = [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) {
+    return [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) {
         [OWSBackupAPI fetchAllRecordNamesWithRecipientId:self.recipientId
             success:^(NSArray<NSString *> *recordNames) {
                 if (self.isComplete) {
@@ -452,8 +440,6 @@ NS_ASSUME_NONNULL_BEGIN
                 resolve(error);
             }];
     }];
-    [promise retainUntilComplete];
-    return promise;
 }
 
 - (AnyPromise *)exportDatabase
@@ -750,14 +736,14 @@ NS_ASSUME_NONNULL_BEGIN
     for (OWSBackupExportItem *item in self.unsavedDatabaseItems) {
         OWSAssertDebug(item.encryptedItem.filePath.length > 0);
 
-        promise = promise.thenInBackground(^{
+        promise
-            return [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) {
+            = promise
+                  .thenInBackground(^{
                       if (self.isComplete) {
-                    return resolve(OWSBackupErrorWithDescription(@"Backup export no longer active."));
+                          return [AnyPromise
+                              promiseWithValue:OWSBackupErrorWithDescription(@"Backup export no longer active.")];
                       }
-                resolve(@(1));
+
-            }]
-                .thenInBackground(^{
                       return [OWSBackupAPI
                           saveEphemeralDatabaseFileToCloudObjcWithRecipientId:self.recipientId
                                                                       fileUrl:[NSURL fileURLWithPath:item.encryptedItem
@@ -766,8 +752,6 @@ NS_ASSUME_NONNULL_BEGIN
                   .thenInBackground(^(NSString *recordName) {
                       item.recordName = recordName;
                       [self.savedDatabaseItems addObject:item];
-                    return [AnyPromise promiseWithValue:@(1)];
-                });
                   });
     }
     [self.unsavedDatabaseItems removeAllObjects];
@@ -880,7 +864,6 @@ NS_ASSUME_NONNULL_BEGIN
 
             OWSLogVerbose(
                 @"saved attachment: %@ as %@", attachmentExport.attachmentFilePath, attachmentExport.relativeFilePath);
-            return [AnyPromise promiseWithValue:@(1)];
         })
         .catchInBackground(^{
             if (![attachmentExport cleanUp]) {
@@ -914,7 +897,6 @@ NS_ASSUME_NONNULL_BEGIN
             self.manifestItem = exportItem;
 
             // All files have been saved to the cloud.
-            return [AnyPromise promiseWithValue:@(1)];
         });
 }
 

Signal/src/util/Backup/OWSBackupImportJob.m

@@ -77,7 +77,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
     [self updateProgressWithDescription:nil progress:nil];
 
     __weak OWSBackupImportJob *weakSelf = self;
-    [[self.backup checkCloudKitAccess]
+    [[self.backup ensureCloudKitAccess]
             .thenInBackground(^{
                 [weakSelf start];
             })

SignalServiceKit/src/Util/Promise+retainUntilComplete.swift

@@ -11,14 +11,12 @@ public extension AnyPromise {
      * promise to self retain, until it completes to avoid the risk it's GC'd before completion.
      */
     @objc
-    @discardableResult
+    func retainUntilComplete() {
-    func retainUntilComplete() -> AnyPromise {
         var retainCycle: AnyPromise? = self
         _ = self.ensure {
             assert(retainCycle != nil)
             retainCycle = nil
         }
-        return self
     }
 }
 
@@ -27,14 +25,12 @@ public extension PMKFinalizer {
      * Sometimes there isn't a straight forward candidate to retain a promise, in that case we tell the
      * promise to self retain, until it completes to avoid the risk it's GC'd before completion.
      */
-    @discardableResult
+    func retainUntilComplete() {
-    func retainUntilComplete() -> PMKFinalizer {
         var retainCycle: PMKFinalizer? = self
         _ = self.finally {
             assert(retainCycle != nil)
             retainCycle = nil
         }
-        return self
     }
 }
 
@@ -43,14 +39,12 @@ public extension Promise {
      * Sometimes there isn't a straight forward candidate to retain a promise, in that case we tell the 
      * promise to self retain, until it completes to avoid the risk it's GC'd before completion.
      */
-    @discardableResult
+    func retainUntilComplete() {
-    func retainUntilComplete() -> Promise<T> {
         var retainCycle: Promise<T>? = self
         _ = self.ensure {
             assert(retainCycle != nil)
             retainCycle = nil
         }
-        return self
     }
 }
 
@@ -59,13 +53,11 @@ public extension Guarantee {
      * Sometimes there isn't a straight forward candidate to retain a promise, in that case we tell the
      * promise to self retain, until it completes to avoid the risk it's GC'd before completion.
      */
-    @discardableResult
+    func retainUntilComplete() {
-    func retainUntilComplete() -> Guarantee<T> {
         var retainCycle: Guarantee<T>? = self
         _ = self.done { _ in
             assert(retainCycle != nil)
             retainCycle = nil
         }
-        return self
     }
 }