Ensure fetcher authorization delays support stop callbacks.

Author: thomasvl

Sources/Core/GTMSessionFetcher.m

@@ -1827,13 +1827,26 @@ - (void)authorizer:(nullable id __unused)auth
     finishedWithError:(nullable NSError *)error {
   GTMSessionCheckNotSynchronized(self);
 
+  @synchronized(self) {
+    // If `stopFetching` was called, do nothing, since the fetch was in a delay state
+    // any needed callback already happened.
+    if (_userStoppedFetching) {
+      return;
+    }
+    // If it wasn't called, clear the `_delayState`, so a call after this point will
+    // trigger a callback as needed. This also ensure if this is going to error below
+    // a cancel callback couldn't also trigger.
+    _delayState = kDelayStateNotDelayed;
+
+    if (error == nil) {
+      _request = authorizedRequest;
+    }
+  }
+
   if (error != nil) {
     // We can't fetch without authorization
     [self failToBeginFetchWithError:(NSError *_Nonnull)error];
   } else {
-    @synchronized(self) {
-      _request = authorizedRequest;
-    }
     [self beginFetchMayDelay:NO mayAuthorize:NO mayDecorate:YES];
   }
 }
@@ -2080,7 +2093,12 @@ - (void)stopFetching {
     // `stopFetchReleasingCallbacks:` will dequeue it if there is a sevice throttled
     // delay, so the canceled callback needs to be directly triggered since the serivce
     // won't attempt to restart it.
-    triggerCallback = _delayState == kDelayStateServiceDelayed && self.stopFetchingTriggersCompletionHandler;
+    //
+    // And the authorization delay assumes all stop notifications needed will be done from
+    // from here.
+    triggerCallback =
+        (_delayState == kDelayStateServiceDelayed || _delayState == kDelayStateAuthorizing) &&
+        self.stopFetchingTriggersCompletionHandler;
   }  // @synchronized(self)
 
   if (triggerCallback) {

UnitTests/GTMSessionFetcherFetchingTest.m

@@ -1585,57 +1585,47 @@ - (void)testDelayedSyncAuthCancelFetchWithCallback_WithoutFetcherService {
 }
 
 - (void)testImmediateSyncAuthCancelFetchWithCallback {
-  XCTSkip(@"Has failed on CI, but not locally, needs investigation.");
   [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer syncAuthorizer]];
 }
 
 - (void)testImmediateSyncAuthCancelFetchWithCallback_WithoutFetcherService {
   _fetcherService = nil;
-  XCTSkip(@"Has failed on CI, but not locally, needs investigation.");
   [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer syncAuthorizer]];
 }
 
 - (void)testDelayedAsyncAuthCancelFetchWithCallback {
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizer]];
 }
 
 - (void)testDelayedAsyncAuthCancelFetchWithCallback_WithoutFetcherService {
   _fetcherService = nil;
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizer]];
 }
 
 - (void)testImmediateAsyncAuthCancelFetchWithCallback {
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizer]];
 }
 
 - (void)testImmediateAsyncAuthCancelFetchWithCallback_WithoutFetcherService {
   _fetcherService = nil;
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizer]];
 }
 
 - (void)testDelayedAsyncDelayedAuthCancelFetchWithCallback {
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizerDelayed:2]];
 }
 
 - (void)testDelayedAsyncDelayedAuthCancelFetchWithCallback_WithoutFetcherService {
   _fetcherService = nil;
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizerDelayed:2]];
 }
 
 - (void)testImmediateAsyncDelayedAuthCancelFetchWithCallback {
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizerDelayed:1]];
 }
 
 - (void)testImmediateAsyncDelayedAuthCancelFetchWithCallback_WithoutFetcherService {
   _fetcherService = nil;
-  XCTSkip(@"Currently fails, needs fixing.");
   [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizerDelayed:1]];
 }
 
@@ -1649,7 +1639,17 @@ - (void)internalCancelFetchWithCallback:(unsigned int)sleepTime
 #pragma clang diagnostic pop
   if (!_isServerRunning) return;
 
-  CREATE_START_STOP_NOTIFICATION_EXPECTATIONS(1, 1);
+  // If the authorizer is async, then the fetch won't fully begin, and there won't ever be
+  // a start notification (and thus stop notification).
+  int expectedNotifications = ((TestAuthorizer*)authorizer).isAsync ? 0 : 1;
+  XCTestExpectation *fetcherStartedExpectation = nil;
+  XCTestExpectation *fetcherStoppedExpectation = nil;
+  if (expectedNotifications) {
+    fetcherStartedExpectation =
+        [[XCTNSNotificationExpectation alloc] initWithName:kGTMSessionFetcherStartedNotification];
+    fetcherStoppedExpectation =
+        [[XCTNSNotificationExpectation alloc] initWithName:kGTMSessionFetcherStoppedNotification];
+  }
 
   FetcherNotificationsCounter *fnctr = [[FetcherNotificationsCounter alloc] init];
 
@@ -1674,19 +1674,17 @@ - (void)internalCancelFetchWithCallback:(unsigned int)sleepTime
   }
   [fetcher stopFetching];
 
-  WAIT_FOR_START_STOP_NOTIFICATION_EXPECTATIONS();
-
   [self waitForExpectationsWithTimeout:_timeoutInterval handler:nil];
 
   [self assertCallbacksReleasedForFetcher:fetcher];
 
   // Check the notifications.
-  XCTAssertEqual(fnctr.fetchStarted, 1, @"%@", fnctr.fetchersStartedDescriptions);
-  XCTAssertEqual(fnctr.fetchStopped, 1, @"%@", fnctr.fetchersStoppedDescriptions);
+  XCTAssertEqual(fnctr.fetchStarted, expectedNotifications, @"%@", fnctr.fetchersStartedDescriptions);
+  XCTAssertEqual(fnctr.fetchStopped, fnctr.fetchStarted, @"%@", fnctr.fetchersStoppedDescriptions);
   XCTAssertEqual(fnctr.fetchCompletionInvoked, 1);
 #if GTM_BACKGROUND_TASK_FETCHING
   [self waitForBackgroundTaskEndedNotifications:fnctr];
-  XCTAssertEqual(fnctr.backgroundTasksStarted.count, (NSUInteger)1);
+  XCTAssertEqual(fnctr.backgroundTasksStarted.count, (NSUInteger)expectedNotifications);
   XCTAssertEqualObjects(fnctr.backgroundTasksStarted, fnctr.backgroundTasksEnded);
 #endif
 }