Calling stopFetching on GTMSessionFetcher to stop callbacks from bein… (#81)

schmidt-sebastian · web-flow · commit 5cb9446fab06 · 2017-06-15T09:43:31.000-07:00
* Calling stopFetching on GTMSeesionFetcher to stop callbacks from being called after deallocation.

Also fixing retain cycles in all task classes.

* Addressing build warnings

* Adjusting indentation
diff --git a/Firebase/Storage/FIRStorageDeleteTask.m b/Firebase/Storage/FIRStorageDeleteTask.m
@@ -21,6 +21,13 @@ @implementation FIRStorageDeleteTask {
   FIRStorageVoidError _completion;
 }
 
+@synthesize fetcher = _fetcher;
+@synthesize fetcherCompletion = _fetcherCompletion;
+
+- (void) dealloc {
+  [_fetcher stopFetching];
+}
+
 - (instancetype)initWithReference:(FIRStorageReference *)reference
                    fetcherService:(GTMSessionFetcherService *)service
                        completion:(FIRStorageVoidError)completion {
@@ -40,14 +47,26 @@ - (void)enqueue {
   _completion = nil;
 
   GTMSessionFetcher *fetcher = [self.fetcherService fetcherWithRequest:request];
+  _fetcher = fetcher;
+  
   fetcher.comment = @"DeleteTask";
-  [fetcher beginFetchWithCompletionHandler:^(NSData *_Nullable data, NSError *_Nullable error) {
+  
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Warc-retain-cycles"
+  _fetcherCompletion = ^(NSData *_Nullable data, NSError *_Nullable error) {
     if (!self.error) {
       self.error = [FIRStorageErrors errorWithServerError:error reference:self.reference];
     }
     if (callback) {
       callback(self.error);
     }
+    _fetcherCompletion = nil;
+  };
+#pragma clang diangostic pop
+
+  __weak FIRStorageDeleteTask* weakSelf = self;
+  [fetcher beginFetchWithCompletionHandler:^(NSData *_Nullable data, NSError *_Nullable error) {
+    weakSelf.fetcherCompletion(data, error);
   }];
 }
 
diff --git a/Firebase/Storage/FIRStorageDownloadTask.m b/Firebase/Storage/FIRStorageDownloadTask.m
@@ -23,6 +23,7 @@ @implementation FIRStorageDownloadTask
 
 @synthesize progress = _progress;
 @synthesize fetcher = _fetcher;
+@synthesize fetcherCompletion = _fetcherCompletion;
 
 - (instancetype)initWithReference:(FIRStorageReference *)reference
                    fetcherService:(GTMSessionFetcherService *)service
@@ -35,6 +36,10 @@ - (instancetype)initWithReference:(FIRStorageReference *)reference
   return self;
 }
 
+- (void) dealloc {
+  [_fetcher stopFetching];
+}
+
 - (void)enqueue {
   [self enqueueWithData:nil];
 }
@@ -60,9 +65,12 @@ - (void)enqueueWithData:(nullable NSData *)resumeData {
     fetcher.comment = @"Starting DownloadTask";
   }
 
+  __weak FIRStorageDownloadTask *weakSelf = self;
+
   [fetcher setResumeDataBlock:^(NSData *data) {
     if (data) {
-      _downloadData = data;
+      FIRStorageDownloadTask *strongSelf = weakSelf;
+      strongSelf->_downloadData = data;
     }
   }];
 
@@ -73,30 +81,31 @@ - (void)enqueueWithData:(nullable NSData *)resumeData {
     [fetcher setDestinationFileURL:_fileURL];
     [fetcher setDownloadProgressBlock:^(int64_t bytesWritten, int64_t totalBytesWritten,
                                         int64_t totalBytesExpectedToWrite) {
-      self.state = FIRStorageTaskStateProgress;
-      self.progress.completedUnitCount = totalBytesWritten;
-      self.progress.totalUnitCount = totalBytesExpectedToWrite;
-      FIRStorageTaskSnapshot *snapshot = self.snapshot;
-      [self fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:snapshot];
-      self.state = FIRStorageTaskStateRunning;
+      weakSelf.state = FIRStorageTaskStateProgress;
+      weakSelf.progress.completedUnitCount = totalBytesWritten;
+      weakSelf.progress.totalUnitCount = totalBytesExpectedToWrite;
+      FIRStorageTaskSnapshot *snapshot = weakSelf.snapshot;
+      [weakSelf fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:snapshot];
+      weakSelf.state = FIRStorageTaskStateRunning;
     }];
   } else {
     // Handle data downloads
     [fetcher setReceivedProgressBlock:^(int64_t bytesWritten, int64_t totalBytesWritten) {
-      self.state = FIRStorageTaskStateProgress;
-      self.progress.completedUnitCount = totalBytesWritten;
-      int64_t totalLength = [[self.fetcher response] expectedContentLength];
-      self.progress.totalUnitCount = totalLength;
-      FIRStorageTaskSnapshot *snapshot = self.snapshot;
-      [self fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:snapshot];
-      self.state = FIRStorageTaskStateRunning;
+      weakSelf.state = FIRStorageTaskStateProgress;
+      weakSelf.progress.completedUnitCount = totalBytesWritten;
+      int64_t totalLength = [[weakSelf.fetcher response] expectedContentLength];
+      weakSelf.progress.totalUnitCount = totalLength;
+      FIRStorageTaskSnapshot *snapshot = weakSelf.snapshot;
+      [weakSelf fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:snapshot];
+      weakSelf.state = FIRStorageTaskStateRunning;
     }];
   }
 
   _fetcher = fetcher;
 
-  self.state = FIRStorageTaskStateRunning;
-  [self.fetcher beginFetchWithCompletionHandler:^(NSData *data, NSError *error) {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Warc-retain-cycles"
+  _fetcherCompletion = ^(NSData *data, NSError *error) {
     // Fire last progress updates
     [self fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:self.snapshot];
 
@@ -106,6 +115,7 @@ - (void)enqueueWithData:(nullable NSData *)resumeData {
       self.error = [FIRStorageErrors errorWithServerError:error reference:self.reference];
       [self fireHandlersForStatus:FIRStorageTaskStatusFailure snapshot:self.snapshot];
       [self removeAllObservers];
+      _fetcherCompletion = nil;
       return;
     }
 
@@ -118,6 +128,13 @@ - (void)enqueueWithData:(nullable NSData *)resumeData {
 
     [self fireHandlersForStatus:FIRStorageTaskStatusSuccess snapshot:self.snapshot];
     [self removeAllObservers];
+    _fetcherCompletion = nil;
+  };
+#pragma clang diagnostic pop
+
+  self.state = FIRStorageTaskStateRunning;
+  [self.fetcher beginFetchWithCompletionHandler:^(NSData *data, NSError *error) {
+    weakSelf.fetcherCompletion(data, error);
   }];
 }
 
diff --git a/Firebase/Storage/FIRStorageGetMetadataTask.m b/Firebase/Storage/FIRStorageGetMetadataTask.m
@@ -26,6 +26,9 @@ @implementation FIRStorageGetMetadataTask {
   FIRStorageVoidMetadataError _completion;
 }
 
+@synthesize fetcher = _fetcher;
+@synthesize fetcherCompletion = _fetcherCompletion;
+
 - (instancetype)initWithReference:(FIRStorageReference *)reference
                    fetcherService:(GTMSessionFetcherService *)service
                        completion:(FIRStorageVoidMetadataError)completion {
@@ -36,6 +39,10 @@ - (instancetype)initWithReference:(FIRStorageReference *)reference
   return self;
 }
 
+- (void) dealloc {
+  [_fetcher stopFetching];
+}
+
 - (void)enqueue {
   NSMutableURLRequest *request = [self.baseRequest mutableCopy];
   request.HTTPMethod = @"GET";
@@ -45,15 +52,20 @@ - (void)enqueue {
   _completion = nil;
 
   GTMSessionFetcher *fetcher = [self.fetcherService fetcherWithRequest:request];
+  _fetcher = fetcher;
   fetcher.comment = @"GetMetadataTask";
-  [fetcher beginFetchWithCompletionHandler:^(NSData *data, NSError *error) {
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Warc-retain-cycles"
+  _fetcherCompletion = ^(NSData *data, NSError *error) {
     if (error) {
       if (!self.error) {
         self.error = [FIRStorageErrors errorWithServerError:error reference:self.reference];
       }
       if (callback) {
         callback(nil, self.error);
       }
+      _fetcherCompletion = nil;
       return;
     }
 
@@ -78,6 +90,13 @@ - (void)enqueue {
         callback(nil, self.error);
       }
     }
+    _fetcherCompletion = nil;
+  };
+#pragma clang diagnostic pop
+  
+  __weak FIRStorageGetMetadataTask *weakSelf = self;
+  [fetcher beginFetchWithCompletionHandler:^(NSData *data, NSError *error) {
+    weakSelf.fetcherCompletion(data, error);
   }];
 }
 
diff --git a/Firebase/Storage/FIRStorageUpdateMetadataTask.m b/Firebase/Storage/FIRStorageUpdateMetadataTask.m
@@ -24,6 +24,9 @@ @implementation FIRStorageUpdateMetadataTask {
   FIRStorageMetadata *_updateMetadata;
 }
 
+@synthesize fetcher = _fetcher;
+@synthesize fetcherCompletion = _fetcherCompletion;
+
 - (instancetype)initWithReference:(FIRStorageReference *)reference
                    fetcherService:(GTMSessionFetcherService *)service
                          metadata:(FIRStorageMetadata *)metadata
@@ -36,6 +39,10 @@ - (instancetype)initWithReference:(FIRStorageReference *)reference
   return self;
 }
 
+- (void) dealloc {
+  [_fetcher stopFetching];
+}
+
 - (void)enqueue {
   NSMutableURLRequest *request = [self.baseRequest mutableCopy];
   NSDictionary *updateDictionary = [_updateMetadata dictionaryRepresentation];
@@ -52,15 +59,19 @@ - (void)enqueue {
   _completion = nil;
 
   GTMSessionFetcher *fetcher = [self.fetcherService fetcherWithRequest:request];
-  fetcher.comment = @"UpdateMetadataTask";
-  [fetcher beginFetchWithCompletionHandler:^(NSData *data, NSError *error) {
+  _fetcher = fetcher;
+  
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Warc-retain-cycles"
+  _fetcherCompletion =^(NSData *data, NSError *error) {
     if (error) {
       if (!self.error) {
         self.error = [FIRStorageErrors errorWithServerError:error reference:self.reference];
       }
       if (callback) {
         callback(nil, self.error);
       }
+      _fetcherCompletion = nil;
       return;
     }
 
@@ -85,6 +96,15 @@ - (void)enqueue {
         callback(nil, self.error);
       }
     }
+    _fetcherCompletion = nil;
+  };
+#pragma clang diagnostic pop
+
+  fetcher.comment = @"UpdateMetadataTask";
+
+  __weak FIRStorageUpdateMetadataTask *weakSelf = self;
+  [fetcher beginFetchWithCompletionHandler:^(NSData *data, NSError *error) {
+    weakSelf.fetcherCompletion(data, error);
   }];
 }
 
diff --git a/Firebase/Storage/FIRStorageUploadTask.m b/Firebase/Storage/FIRStorageUploadTask.m
@@ -25,6 +25,7 @@
 @implementation FIRStorageUploadTask
 
 @synthesize progress = _progress;
+@synthesize fetcherCompletion = _fetcherCompletion;
 
 - (instancetype)initWithReference:(FIRStorageReference *)reference
                    fetcherService:(GTMSessionFetcherService *)service
@@ -62,6 +63,10 @@ - (instancetype)initWithReference:(FIRStorageReference *)reference
   return self;
 }
 
+- (void) dealloc {
+  [_uploadFetcher stopFetching];
+}
+
 - (void)enqueue {
   NSAssert([NSThread isMainThread], @"Upload attempting to execute on non main queue! Please only "
            @"execute this method on the main queue.");
@@ -103,23 +108,28 @@ - (void)enqueue {
   }
 
   uploadFetcher.maxRetryInterval = self.reference.storage.maxUploadRetryTime;
+  
+  __weak FIRStorageUploadTask* weakSelf = self;
 
   [uploadFetcher setSendProgressBlock:^(int64_t bytesSent, int64_t totalBytesSent,
                                         int64_t totalBytesExpectedToSend) {
-    self.state = FIRStorageTaskStateProgress;
-    self.progress.completedUnitCount = totalBytesSent;
-    self.progress.totalUnitCount = totalBytesExpectedToSend;
-    self.metadata = _uploadMetadata;
-    [self fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:self.snapshot];
-    self.state = FIRStorageTaskStateRunning;
+    weakSelf.state = FIRStorageTaskStateProgress;
+    weakSelf.progress.completedUnitCount = totalBytesSent;
+    weakSelf.progress.totalUnitCount = totalBytesExpectedToSend;
+    weakSelf.metadata = _uploadMetadata;
+    [weakSelf fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:weakSelf.snapshot];
+    weakSelf.state = FIRStorageTaskStateRunning;
   }];
 
   _uploadFetcher = uploadFetcher;
 
   // Process fetches
   self.state = FIRStorageTaskStateRunning;
-  [_uploadFetcher beginFetchWithCompletionHandler:^(NSData *_Nullable data,
-                                                    NSError *_Nullable error) {
+  
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Warc-retain-cycles"
+  _fetcherCompletion = ^(NSData *_Nullable data,
+                         NSError *_Nullable error) {
     // Fire last progress updates
     [self fireHandlersForStatus:FIRStorageTaskStatusProgress snapshot:self.snapshot];
 
@@ -130,6 +140,7 @@ - (void)enqueue {
       self.metadata = _uploadMetadata;
       [self fireHandlersForStatus:FIRStorageTaskStatusFailure snapshot:self.snapshot];
       [self removeAllObservers];
+      _fetcherCompletion = nil;
       return;
     }
 
@@ -156,6 +167,13 @@ - (void)enqueue {
 
     [self fireHandlersForStatus:FIRStorageTaskStatusSuccess snapshot:self.snapshot];
     [self removeAllObservers];
+    _fetcherCompletion = nil;
+  };
+#pragma clang diagnostic pop
+  
+  [_uploadFetcher beginFetchWithCompletionHandler:^(NSData *_Nullable data,
+                                                    NSError *_Nullable error) {
+    weakSelf.fetcherCompletion(data, error);
   }];
 }
 
diff --git a/Firebase/Storage/Private/FIRStorageTask_Private.h b/Firebase/Storage/Private/FIRStorageTask_Private.h
@@ -62,6 +62,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property(readonly, nonatomic) GTMSessionFetcherService *fetcherService;
 
+@property(readonly) GTMSessionFetcherCompletionHandler fetcherCompletion;
+
 /**
  * Creates a new FIRStorageTask initialized with a FIRStorageReference and GTMSessionFetcherService.
  * @param reference A FIRStorageReference the task will be performed on.
