Fix race condition where checkin is not reset properly when app first installed

[charlotteliang]

This fixes #8491.

There's a race condition when checkin sometimes not reset properly during app first install, then delete token request contains an old chekcin that causes token is not deleted properly.

[google-oss-bot]

Coverage Report

Affected SDKs

• FirebaseMessaging-iOS-FirebaseMessaging.framework

  SDK overall coverage changed from ? (9719717) to 65.00% (bd904c7) by ?.

  Click to show coverage changes in 29 files.

  Filename	Base (9719717)	Head (bd904c7)	Diff
  FIRMessaging.m	?	52.62%	?
  FIRMessagingAPNSInfo.m	?	87.23%	?
  FIRMessagingAnalytics.m	?	80.00%	?
  FIRMessagingAuthKeychain.m	?	88.64%	?
  FIRMessagingAuthService.m	?	92.17%	?
  FIRMessagingBackupExcludedPlist.m	?	86.30%	?
  FIRMessagingCheckinPreferences.m	?	98.18%	?
  FIRMessagingCheckinService.m	?	91.76%	?
  FIRMessagingCheckinStore.m	?	83.22%	?
  FIRMessagingContextManagerService.m	?	66.99%	?
  FIRMessagingExtensionHelper.m	?	83.47%	?
  FIRMessagingKeychain.m	?	93.89%	?
  FIRMessagingLogger.m	?	100.00%	?
  FIRMessagingPendingTopicsList.m	?	89.93%	?
  FIRMessagingPersistentSyncMessage.m	?	38.10%	?
  FIRMessagingPubSub.m	?	53.85%	?
  FIRMessagingRemoteNotificationsProxy.m	?	73.64%	?
  FIRMessagingRmqManager.m	?	58.57%	?
  FIRMessagingSyncMessageManager.m	?	73.08%	?
  FIRMessagingTokenDeleteOperation.m	?	9.64%	?
  FIRMessagingTokenFetchOperation.m	?	75.32%	?
  FIRMessagingTokenInfo.m	?	82.78%	?
  FIRMessagingTokenManager.m	?	43.05%	?
  FIRMessagingTokenOperation.m	?	96.84%	?
  FIRMessagingTokenStore.m	?	68.42%	?
  FIRMessagingTopicOperation.m	?	0.00%	?
  FIRMessagingUtilities.m	?	59.89%	?
  NSDictionary+FIRMessaging.m	?	26.47%	?
  NSError+FIRMessaging.m	?	100.00%	?

Test Logs

• Head (bd904c70): https://github.com/firebase/firebase-ios-sdk/actions/runs/1117678751

[paulb777]

Please take a look at the watchOS CI failure.

[maksymmalyhin]

Could you please provide a bit more info on how the race condition occurs and causes the issue as well as how the changes fix it?

Is it possible to add a tests that triggers the race condition with the old code and succeeds with the changes?

[charlotteliang]

@maksymmalyhin When detect newly install, client try to delete previous checkin from keychain. However, the reset function is in checkinStore class, but when we sent token request, we still load the cache value from authService class. These classes are added from the refactor when we try to remove IID dependency. So we let authService completely own checkinStore and handle the proper reset from authService class.
Let me know if that makes sense and happy to discuss more offline.

[charlotteliang]

@maksymmalyhin We had deleteToken Swift API call in place, however, this case is not 100% reproducible and also it only happens during newly installed app.

[maksymmalyhin]

Thank you for the explanation. LGTM on CI green.

[spundun]

@chliangGoogle Does this mean that the race condition is between these two threads of execution?
[pre-condition] app just got reinstalled and reloaded
[thread 1] detect reinstall -> reset checkin credentials in storage through checkinStore class
[thread 2] authService class is loaded -> reads and caches checkin credentials

[charlotteliang]

@spundun That is correct.