-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix component startup time. #4137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,13 @@ + (void)load { | |
| [FIRDependency dependencyWithProtocol:@protocol(FIRAnalyticsInterop) isRequired:NO]; | ||
| FIRComponentCreationBlock creationBlock = | ||
| ^id _Nullable(FIRComponentContainer *container, BOOL *isCacheable) { | ||
| if (!container.app.isDefaultApp) { | ||
| // Only start for the default FIRApp. | ||
| FIRMessagingLoggerDebug(kFIRMessagingMessageCodeFIRApp001, | ||
| @"Firebase Messaging only works with the default app."); | ||
| return nil; | ||
| } | ||
|
|
||
| // Ensure it's cached so it returns the same instance every time messaging is called. | ||
| *isCacheable = YES; | ||
| id<FIRAnalyticsInterop> analytics = FIR_COMPONENT(FIRAnalyticsInterop, container); | ||
|
|
@@ -233,28 +240,19 @@ + (void)load { | |
| withInstanceID:[FIRInstanceID instanceID] | ||
| withUserDefaults:[GULUserDefaults standardUserDefaults]]; | ||
| [messaging start]; | ||
| [messaging configureNotificationSwizzlingIfEnabled]; | ||
| return messaging; | ||
| }; | ||
| FIRComponent *messagingProvider = | ||
| [FIRComponent componentWithProtocol:@protocol(FIRMessagingInstanceProvider) | ||
| instantiationTiming:FIRInstantiationTimingLazy | ||
| instantiationTiming:FIRInstantiationTimingEagerInDefaultApp | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change from lazy to eager? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It mimics the previous behaviour of |
||
| dependencies:@[ analyticsDep ] | ||
| creationBlock:creationBlock]; | ||
|
|
||
| return @[ messagingProvider ]; | ||
| } | ||
|
|
||
| + (void)configureWithApp:(FIRApp *)app { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to fix the same for other SDK as well, such as InstanceID, remote config etc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apologies for the delay - IID was fixed in #4030, will be addressing the other SDKs in a separate PR. |
||
| if (!app.isDefaultApp) { | ||
| // Only configure for the default FIRApp. | ||
| FIRMessagingLoggerDebug(kFIRMessagingMessageCodeFIRApp001, | ||
| @"Firebase Messaging only works with the default app."); | ||
| return; | ||
| } | ||
| [[FIRMessaging messaging] configureMessaging:app]; | ||
| } | ||
|
|
||
| - (void)configureMessaging:(FIRApp *)app { | ||
| - (void)configureNotificationSwizzlingIfEnabled { | ||
| // Swizzle remote-notification-related methods (app delegate and UNUserNotificationCenter) | ||
| if ([FIRMessagingRemoteNotificationsProxy canSwizzleMethods]) { | ||
| NSString *docsURLString = @"https://firebase.google.com/docs/cloud-messaging/ios/client" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why AlwaysEager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It mimics the previous behaviour removed on the previous line 1897:
https://github.com/firebase/firebase-ios-sdk/pull/4137/files#diff-f33c561b2a3430b947e6da076758c6d9L1897
every time
[FIRApp configure]is called, we called[FIRAuth authWithApp:]which instantiated the instance.The reason it's
AlwaysEagerand notEagerInDefaultAppis becauseFIRAuthworks with multipleFIRApps.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah,
FIRAuthworks with multipleFIRApps.