Fix container instantiation timing, IID startup. (#4030)

* Fix container instantiation timing, IID startup.

The container instantiation could happen at the wrong time, before all
components have been added to the container. This fixes it, and also
moves IID to use the proper instantiation timing instead of relying on
the `configureWithApp:` call. This is part continuation of #3147, where
I'll be fixing the rest of the SDKs in follow up PRs.

* Typo

* Further work on instantiation timing.

This moves the instantiation of components to after the FirebaseApp is
completely configured and assigned, as it should be. This also exposed
a recursive issue with IID calling the Messaging singleton, which in
turn called IID and instantiated multiple instances.

A change was made to break the cycle: the Messaging `isAutoInitEnabled`
call was changed to a static method vs an instance method, allowing IID
to call it during initialization without instantiating the Messaging
instance (since it doesn't have a direct dependency on it).

* Revert unintentional formatting of entire FIRMessaging file.

* Fix tests.

* Format InstanceID.m

* Remove unused import.

* Fix ComponentContainer test.

* Minor PR feedback.

Author: ryanwilson

Example/Core/Tests/FIRComponentContainerTest.m

@@ -207,6 +207,11 @@ - (FIRComponentContainer *)containerWithRegistrants:(NSArray<Class> *)registrant
   FIRComponentContainer *container = [[FIRComponentContainer alloc] initWithApp:_hostApp
                                                                     registrants:allRegistrants];
   _hostApp.container = container;
+
+  // Instantiate all the components that were eagerly registered now that all other properties are
+  // configured.
+  [container instantiateEagerComponents];
+
   return container;
 }
 

Example/Messaging/Tests/FIRInstanceIDWithFCMTest.m

@@ -58,11 +58,10 @@ - (void)tearDown {
 }
 
 - (void)testFCMAutoInitEnabled {
-  NSString *const kFIRMessagingTestsAutoInit = @"com.messaging.test_autoInit";
-  NSUserDefaults *defaults = [[NSUserDefaults alloc] initWithSuiteName:kFIRMessagingTestsAutoInit];
+  // Use the standardUserDefaults since that's what IID expects and depends on.
+  NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
   FIRMessaging *messaging = [FIRMessagingTestUtilities messagingForTestsWithUserDefaults:defaults];
   id classMock = OCMClassMock([FIRMessaging class]);
-  OCMStub([classMock messaging]).andReturn(messaging);
   OCMStub([_mockFirebaseApp isDataCollectionDefaultEnabled]).andReturn(YES);
   messaging.autoInitEnabled = YES;
   XCTAssertTrue(

Example/Messaging/Tests/FIRMessagingTest.m

@@ -37,6 +37,9 @@ @interface FIRMessaging ()
 @property(nonatomic, readwrite, strong) NSData *apnsTokenData;
 @property(nonatomic, readwrite, strong) FIRInstanceID *instanceID;
 
+// Expose autoInitEnabled static method for IID.
++ (BOOL)isAutoInitEnabledWithUserDefaults:(NSUserDefaults *)userDefaults;
+
 // Direct Channel Methods
 - (void)updateAutomaticClientConnection;
 - (BOOL)shouldBeConnectedAutomatically;
@@ -129,6 +132,26 @@ - (void)testAutoInitEnableGlobalDefaultFalse {
   [bundleMock stopMocking];
 }
 
+- (void)testAutoInitEnabledMatchesStaticMethod {
+  // Flag is set to YES in user defaults.
+  NSUserDefaults *defaults = self.messaging.messagingUserDefaults;
+  [defaults setObject:@YES forKey:kFIRMessagingUserDefaultsKeyAutoInitEnabled];
+
+  XCTAssertTrue(self.messaging.isAutoInitEnabled);
+  XCTAssertEqual(self.messaging.isAutoInitEnabled,
+                 [FIRMessaging isAutoInitEnabledWithUserDefaults:defaults]);
+}
+
+- (void)testAutoInitDisabledMatchesStaticMethod {
+  // Flag is set to NO in user defaults.
+  NSUserDefaults *defaults = self.messaging.messagingUserDefaults;
+  [defaults setObject:@NO forKey:kFIRMessagingUserDefaultsKeyAutoInitEnabled];
+
+  XCTAssertFalse(self.messaging.isAutoInitEnabled);
+  XCTAssertEqual(self.messaging.isAutoInitEnabled,
+                 [FIRMessaging isAutoInitEnabledWithUserDefaults:defaults]);
+}
+
 #pragma mark - Direct Channel Establishment Testing
 
 #if TARGET_OS_IOS || TARGET_OS_TV

Firebase/Core/FIRApp.m

@@ -191,6 +191,10 @@ + (void)configureWithName:(NSString *)name options:(FIROptions *)options {
     }
 
     [FIRApp addAppToAppDictionary:app];
+
+    // The FIRApp instance is ready to go, `sDefaultApp` is assigned, other SDKs are now ready to be
+    // instantiated.
+    [app.container instantiateEagerComponents];
     [FIRApp sendNotificationsToSDKs:app];
   }
 }

Firebase/Core/FIRComponentContainer.m

@@ -32,6 +32,9 @@ @interface FIRComponentContainer ()
 /// Cached instances of components that requested to be cached.
 @property(nonatomic, strong) NSMutableDictionary<NSString *, id> *cachedInstances;
 
+/// Protocols of components that have requested to be eagerly instantiated.
+@property(nonatomic, strong, nullable) NSMutableArray<Protocol *> *eagerProtocolsToInstantiate;
+
 @end
 
 @implementation FIRComponentContainer
@@ -74,6 +77,9 @@ - (instancetype)initWithApp:(FIRApp *)app registrants:(NSMutableSet<Class> *)all
 }
 
 - (void)populateComponentsFromRegisteredClasses:(NSSet<Class> *)classes forApp:(FIRApp *)app {
+  // Keep track of any components that need to eagerly instantiate after all components are added.
+  self.eagerProtocolsToInstantiate = [[NSMutableArray alloc] init];
+
   // Loop through the verified component registrants and populate the components array.
   for (Class<FIRLibrary> klass in classes) {
     // Loop through all the components being registered and store them as appropriate.
@@ -92,24 +98,38 @@ - (void)populateComponentsFromRegisteredClasses:(NSSet<Class> *)classes forApp:(
       // Store the creation block for later usage.
       self.components[protocolName] = component.creationBlock;
 
-      // Instantiate the instance if it has requested to be instantiated.
+      // Queue any protocols that should be eagerly instantiated. Don't instantiate them yet
+      // because they could depend on other components that haven't been added to the components
+      // array yet.
       BOOL shouldInstantiateEager =
           (component.instantiationTiming == FIRInstantiationTimingAlwaysEager);
       BOOL shouldInstantiateDefaultEager =
           (component.instantiationTiming == FIRInstantiationTimingEagerInDefaultApp &&
            [app isDefaultApp]);
       if (shouldInstantiateEager || shouldInstantiateDefaultEager) {
-        @synchronized(self) {
-          [self instantiateInstanceForProtocol:component.protocol
-                                     withBlock:component.creationBlock];
-        }
+        [self.eagerProtocolsToInstantiate addObject:component.protocol];
       }
     }
   }
 }
 
 #pragma mark - Instance Creation
 
+- (void)instantiateEagerComponents {
+  // After all components are registered, instantiate the ones that are requesting eager
+  // instantiation.
+  @synchronized(self) {
+    for (Protocol *protocol in self.eagerProtocolsToInstantiate) {
+      // Get an instance for the protocol, which will instantiate it since it couldn't have been
+      // cached yet. Ignore the instance coming back since we don't need it.
+      __unused id unusedInstance = [self instanceForProtocol:protocol];
+    }
+
+    // All eager instantiation is complete, clear the stored property now.
+    self.eagerProtocolsToInstantiate = nil;
+  }
+}
+
 /// Instantiate an instance of a class that conforms to the specified protocol.
 /// This will:
 ///   - Call the block to create an instance if possible,

Firebase/Core/Private/FIRComponentContainerInternal.h

@@ -31,6 +31,9 @@ NS_ASSUME_NONNULL_BEGIN
 /// protocol wasn't registered, or if the instance couldn't instantiate for the provided app.
 - (nullable id)instanceForProtocol:(Protocol *)protocol NS_SWIFT_NAME(instance(for:));
 
+/// Instantiates all the components that have registered as "eager" after initialization.
+- (void)instantiateEagerComponents;
+
 /// Remove all of the cached instances stored and allow them to clean up after themselves.
 - (void)removeAllCachedInstances;
 

Firebase/InstanceID/FIRInstanceID.m

@@ -22,6 +22,7 @@
 #import <FirebaseCore/FIRLibrary.h>
 #import <FirebaseCore/FIROptions.h>
 #import <GoogleUtilities/GULAppEnvironmentUtil.h>
+#import <GoogleUtilities/GULUserDefaults.h>
 #import "FIRInstanceID+Private.h"
 #import "FIRInstanceIDAuthService.h"
 #import "FIRInstanceIDCheckinPreferences.h"
@@ -67,8 +68,8 @@
 static NSString *const kAPSEnvironmentDevelopmentValue = @"development";
 /// FIRMessaging selector that returns the current FIRMessaging auto init
 /// enabled flag.
-static NSString *const kFIRInstanceIDFCMSelectorAutoInitEnabled = @"isAutoInitEnabled";
-static NSString *const kFIRInstanceIDFCMSelectorInstance = @"messaging";
+static NSString *const kFIRInstanceIDFCMSelectorAutoInitEnabled =
+    @"isAutoInitEnabledWithUserDefaults:";
 
 static NSString *const kFIRInstanceIDAPNSTokenType = @"APNSTokenType";
 static NSString *const kFIRIIDAppReadyToConfigureSDKNotification =
@@ -77,8 +78,6 @@
 static NSString *const kFIRIIDErrorDomain = @"com.firebase.instanceid";
 static NSString *const kFIRIIDServiceInstanceID = @"InstanceID";
 
-static NSInteger const kFIRIIDErrorCodeInstanceIDFailed = -121;
-
 typedef void (^FIRInstanceIDKeyPairHandler)(FIRInstanceIDKeyPair *keyPair, NSError *error);
 
 /**
@@ -639,41 +638,41 @@ + (void)load {
 + (nonnull NSArray<FIRComponent *> *)componentsToRegister {
   FIRComponentCreationBlock creationBlock =
       ^id _Nullable(FIRComponentContainer *container, BOOL *isCacheable) {
+    // InstanceID only works with the default app.
+    if (!container.app.isDefaultApp) {
+      // Only configure for the default FIRApp.
+      FIRInstanceIDLoggerDebug(kFIRInstanceIDMessageCodeFIRApp002,
+                               @"Firebase Instance ID only works with the default app.");
+      return nil;
+    }
+
     // Ensure it's cached so it returns the same instance every time instanceID is called.
     *isCacheable = YES;
     FIRInstanceID *instanceID = [[FIRInstanceID alloc] initPrivately];
     [instanceID start];
+    [instanceID configureInstanceIDWithOptions:container.app.options];
     return instanceID;
   };
   FIRComponent *instanceIDProvider =
       [FIRComponent componentWithProtocol:@protocol(FIRInstanceIDInstanceProvider)
-                      instantiationTiming:FIRInstantiationTimingLazy
+                      instantiationTiming:FIRInstantiationTimingEagerInDefaultApp
                              dependencies:@[]
                             creationBlock:creationBlock];
   return @[ instanceIDProvider ];
 }
 
-+ (void)configureWithApp:(FIRApp *)app {
-  if (!app.isDefaultApp) {
-    // Only configure for the default FIRApp.
-    FIRInstanceIDLoggerDebug(kFIRInstanceIDMessageCodeFIRApp002,
-                             @"Firebase Instance ID only works with the default app.");
-    return;
-  }
-  [[FIRInstanceID instanceID] configureInstanceIDWithOptions:app.options app:app];
-}
-
-- (void)configureInstanceIDWithOptions:(FIROptions *)options app:(FIRApp *)firApp {
+- (void)configureInstanceIDWithOptions:(FIROptions *)options {
   NSString *GCMSenderID = options.GCMSenderID;
   if (!GCMSenderID.length) {
     FIRInstanceIDLoggerError(kFIRInstanceIDMessageCodeFIRApp000,
                              @"Firebase not set up correctly, nil or empty senderID.");
-    [FIRInstanceID exitWithReason:@"GCM_SENDER_ID must not be nil or empty." forFirebaseApp:firApp];
-    return;
+    [NSException raise:kFIRIIDErrorDomain
+                format:@"Could not configure Firebase InstanceID. GCMSenderID must not be nil or "
+                       @"empty."];
   }
 
   self.fcmSenderID = GCMSenderID;
-  self.firebaseAppID = firApp.options.googleAppID;
+  self.firebaseAppID = options.googleAppID;
 
   // FCM generates a FCM token during app start for sending push notification to device.
   // This is not needed for app extension.
@@ -682,26 +681,6 @@ - (void)configureInstanceIDWithOptions:(FIROptions *)options app:(FIRApp *)firAp
   }
 }
 
-+ (NSError *)configureErrorWithReason:(nonnull NSString *)reason {
-  NSString *description =
-      [NSString stringWithFormat:@"Configuration failed for service %@.", kFIRIIDServiceInstanceID];
-  if (!reason.length) {
-    reason = @"Unknown reason";
-  }
-
-  NSDictionary *userInfo =
-      @{NSLocalizedDescriptionKey : description, NSLocalizedFailureReasonErrorKey : reason};
-
-  return [NSError errorWithDomain:kFIRIIDErrorDomain
-                             code:kFIRIIDErrorCodeInstanceIDFailed
-                         userInfo:userInfo];
-}
-
-+ (void)exitWithReason:(nonnull NSString *)reason forFirebaseApp:(FIRApp *)firebaseApp {
-  [NSException raise:kFIRIIDErrorDomain
-              format:@"Could not configure Firebase InstanceID. %@", reason];
-}
-
 // This is used to start any operations when we receive FirebaseSDK setup notification
 // from FIRCore.
 - (void)didCompleteConfigure {
@@ -738,29 +717,20 @@ - (BOOL)isFCMAutoInitEnabled {
     return NO;
   }
 
-  // Messaging doesn't have the singleton method, auto init should be enabled since FCM exists.
-  SEL instanceSelector = NSSelectorFromString(kFIRInstanceIDFCMSelectorInstance);
-  if (![messagingClass respondsToSelector:instanceSelector]) {
-    return YES;
-  }
-
-  // Get FIRMessaging shared instance.
-  IMP messagingInstanceIMP = [messagingClass methodForSelector:instanceSelector];
-  id (*getMessagingInstance)(id, SEL) = (void *)messagingInstanceIMP;
-  id messagingInstance = getMessagingInstance(messagingClass, instanceSelector);
-
-  // Messaging doesn't have the property, auto init should be enabled since FCM exists.
+  // Messaging doesn't have the class method, auto init should be enabled since FCM exists.
   SEL autoInitSelector = NSSelectorFromString(kFIRInstanceIDFCMSelectorAutoInitEnabled);
-  if (![messagingInstance respondsToSelector:autoInitSelector]) {
+  if (![messagingClass respondsToSelector:autoInitSelector]) {
     return YES;
   }
 
-  // Get autoInitEnabled method.
-  IMP isAutoInitEnabledIMP = [messagingInstance methodForSelector:autoInitSelector];
-  BOOL (*isAutoInitEnabled)(id, SEL) = (BOOL(*)(id, SEL))isAutoInitEnabledIMP;
+  // Get the autoInitEnabled class method.
+  IMP isAutoInitEnabledIMP = [messagingClass methodForSelector:autoInitSelector];
+  BOOL(*isAutoInitEnabled)
+  (Class, SEL, GULUserDefaults *) = (BOOL(*)(id, SEL, GULUserDefaults *))isAutoInitEnabledIMP;
 
   // Check FCM's isAutoInitEnabled property.
-  return isAutoInitEnabled(messagingInstance, autoInitSelector);
+  return isAutoInitEnabled(messagingClass, autoInitSelector,
+                           [GULUserDefaults standardUserDefaults]);
 }
 
 // Actually makes InstanceID instantiate both the IID and Token-related subsystems.

Firebase/Messaging/FIRMessaging.m

@@ -541,9 +541,21 @@ - (void)setAPNSToken:(NSData *)apnsToken type:(FIRMessagingAPNSTokenType)type {
 #pragma mark - FCM
 
 - (BOOL)isAutoInitEnabled {
+  // Defer to the class method since we're just reading from regular userDefaults and we need to
+  // read this from IID without instantiating the Messaging singleton.
+  return [[self class] isAutoInitEnabledWithUserDefaults:_messagingUserDefaults];
+}
+
+/// Checks if Messaging auto-init is enabled in the user defaults instance passed in. This is
+/// exposed as a class property for IID to fetch the property without instantiating an instance of
+/// Messaging. Since Messaging can only be used with the default FIRApp, we can have one point of
+/// entry without context of which FIRApp instance is being used.
+/// ** THIS METHOD IS DEPENDED ON INTERNALLY BY IID USING REFLECTION. PLEASE DO NOT CHANGE THE
+///  SIGNATURE, AS IT WOULD BREAK AUTOINIT FUNCTIONALITY WITHIN IID. **
++ (BOOL)isAutoInitEnabledWithUserDefaults:(GULUserDefaults *)userDefaults {
   // Check storage
   id isAutoInitEnabledObject =
-      [_messagingUserDefaults objectForKey:kFIRMessagingUserDefaultsKeyAutoInitEnabled];
+      [userDefaults objectForKey:kFIRMessagingUserDefaultsKeyAutoInitEnabled];
   if (isAutoInitEnabledObject) {
     return [isAutoInitEnabledObject boolValue];
   }