Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Example/Messaging/Tests/FIRInstanceIDWithFCMTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
24 changes: 24 additions & 0 deletions Example/Messaging/Tests/FIRMessagingTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#import <FirebaseInstanceID/FirebaseInstanceID.h>
#import <FirebaseAnalyticsInterop/FIRAnalyticsInterop.h>
#import <FirebaseMessaging/FIRMessaging.h>
#import <GoogleUtilities/GULUserDefaults.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import doesn't seem to be needed since the class uses NSUserDefaults directly


#import "Example/Messaging/Tests/FIRMessagingTestUtilities.h"
#import "Firebase/Messaging/FIRMessaging_Private.h"
Expand All @@ -37,6 +38,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;
Expand Down Expand Up @@ -129,6 +133,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
Expand Down
4 changes: 4 additions & 0 deletions Firebase/Core/FIRApp.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}
Expand Down
31 changes: 26 additions & 5 deletions Firebase/Core/FIRComponentContainer.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -92,24 +98,39 @@ - (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, empty and clear the stored property now.
[self.eagerProtocolsToInstantiate removeAllObjects];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assigning nil is sufficient to release all the contents of the array.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed, thanks.

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,
Expand Down
3 changes: 3 additions & 0 deletions Firebase/Core/Private/FIRComponentContainerInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
82 changes: 26 additions & 56 deletions Firebase/InstanceID/FIRInstanceID.m
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 =
Expand All @@ -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);

/**
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a warning or even an error?

Feel free to ignore--I see that this is just copypasta from the prior state.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also a "this code should never be run" so maybe it should be an error, but I'll leave it as is since it was copied from before.

@"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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
13 changes: 12 additions & 1 deletion Firebase/Messaging/FIRMessaging.m
Original file line number Diff line number Diff line change
Expand Up @@ -541,9 +541,20 @@ - (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. PLEASE DO NOT CHANGE THE SIGNATURE. **
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my read it looks like isAutoInitEnabled is depended upon by Messaging. isAutoInitEnabledWithUserDefaults is only depended upon by the tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, I missed the call.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't strongly enough indicate that the call is made reflectively so it won't necessarily show up in find usages.

+ (BOOL)isAutoInitEnabledWithUserDefaults:(GULUserDefaults *)userDefaults {
// Check storage
id isAutoInitEnabledObject =
[_messagingUserDefaults objectForKey:kFIRMessagingUserDefaultsKeyAutoInitEnabled];
[userDefaults objectForKey:kFIRMessagingUserDefaultsKeyAutoInitEnabled];
if (isAutoInitEnabledObject) {
return [isAutoInitEnabledObject boolValue];
}
Expand Down