firebase / firebase-cpp-sdk

Firebase C++ SDK
http://firebase.google.com
Apache License 2.0
272 stars 111 forks source link

applicationDidBecomeActive broke when I add Firebase SDK into the project #6

Closed AGulev closed 5 years ago

AGulev commented 5 years ago

If I add firebase cpp sdk into my project and bundle this project for ios, my applicationDidBecomeActive in UIApplicationDelegate doesn't work anymore (even without sdk initialization).

ios sdk version 5.20.2 Analytics firebase_cpp_sdk 5.7.0

stewartmiles commented 5 years ago

Hi @AGulev when you say it broke do you mean that it is no longer called? I suspect something is going on with swizzling. Are you able to reproduce the failure using the Analytics sample in this github repo? Perhaps there are some other frameworks that are swizzling and conflicting with Firebase?

Cheers, Stewart

AGulev commented 5 years ago

Hi @stewartmiles , yes, it's no longer called.

As I see in logs swizzling applied only after initialization of firebase. But in my case, I've tried to remove all client code (do not call initialization) and the issue has still reproduced even when I just add libraries to the project.

stewartmiles commented 5 years ago

@AGulev right, swizzling is applied when Obj-C modules are loaded since it needs to happen before UIApplicationMain is executed.

Are you able to reproduce this in our analytics sample application?

AGulev commented 5 years ago

@stewartmiles I'll try today later

AGulev commented 5 years ago

Hi @stewartmiles , I've made more investigations.

I can't reproduce it in a clean example because in our application we have our own hook system for the events. We have a modular architecture, and we need to provide the possibility of connecting different external libraries without changing the main codebase.

The strange part: we have a problem only with applicationDidBecomeActive, but applicationDidEnterBackground, applicationWillResignActive and applicationWillEnterForeground works fine.

I've read Firebase code and found this place, it seems like a place of a possible reason for me: https://github.com/firebase/firebase-cpp-sdk/blob/f799d80750cd570a69e25585097cf15303ce6dd9/app/src/invites/ios/invites_ios_startup.mm#L107

I can't catch invocation of applicationDidBecomeActive() method in forwardInvocation() delegate method but i am sure that in respondsToSelector() returned YES for applicationDidBecomeActive selector.

As a next step, I'll try to build and debug my own version of the firebase. I'll give you know about my finds. I would be appreciated if you have some thought or advice for me according to the info above.

Thanks, Alex

AGulev commented 5 years ago

Hi @stewartmiles , I have made an example where you can reproduce the same issue. Pay attention to "applicationDidBecomeActive" - it doesn't work, but without firebase lib, everything works fine.

I think the same problem with other methods from this function: https://github.com/firebase/firebase-cpp-sdk/blob/f799d80750cd570a69e25585097cf15303ce6dd9/app/src/invites/ios/invites_ios_startup.mm#L128

Example: example.zip

Thanks, Alex

jonsimantov commented 5 years ago

Hi @AGulev,

I took your source file (ios_main.mm) and tried it in the C++ Analytics quickstart on my device, using the latest Firebase C++ SDK (6.0.0), and could not reproduce your issue. The applicationDidBecomeActive event occurred as expected.

I ran the app, went back to the home screen, waited a moment, then went back into the app, and got these log messages in order: applicationDidBecomeActive applicationWillResignActive applicationDidEnterBackground applicationWillEnterForeground applicationDidBecomeActive

What Xcode version are you using to build your app? And what iOS version are you running on?

Any other log messages of note while your app is initializing?

AGulev commented 5 years ago

Hi @jonsimantov , This is a new example (libs included) with the same logic and the same bug (using the newest SDK 6.0.0): https://www.dropbox.com/s/sn9so5vz3c7fa8s/new_test.zip?dl=0

XCode Version 10.2.1 (10E1001) iOS 12.2 iPhone XR, also iOS 12.2 iPhone X, and few other

Log:

2019-05-13 13:52:39.627410+0200 AnalyticsExample[51868:3287816]  - <AppMeasurement>[I-ACS036002] Analytics screen reporting is enabled. Call +[FIRAnalytics setScreenName:setScreenClass:] to set the screen name or override the default screen class name. To disable screen reporting, set the flag FirebaseScreenReportingEnabled to NO (boolean) in the Info.plist
2019-05-13 13:52:39.687048+0200 AnalyticsExample[51868:3287705] Unknown class FoodPickerViewController in Interface Builder file.
2019-05-13 13:52:39.690296+0200 AnalyticsExample[51868:3287705] [Application] The app delegate must implement the window property if it wants to use a main storyboard file.
2019-05-13 13:52:42.628079+0200 AnalyticsExample[51868:3287705] applicationWillResignActive
2019-05-13 13:52:45.083823+0200 AnalyticsExample[51868:3287705] applicationDidEnterBackground
2019-05-13 13:52:48.530749+0200 AnalyticsExample[51868:3287705] applicationWillEnterForeground

The same example without Firebase SDK (just remove firebase.framework from "Linked Frameworks and Libraries"):

2019-05-13 13:56:33.929465+0200 AnalyticsExample[51926:3295387]  - <AppMeasurement>[I-ACS036002] Analytics screen reporting is enabled. Call +[FIRAnalytics setScreenName:setScreenClass:] to set the screen name or override the default screen class name. To disable screen reporting, set the flag FirebaseScreenReportingEnabled to NO (boolean) in the Info.plist
2019-05-13 13:56:33.994624+0200 AnalyticsExample[51926:3295271] Unknown class FoodPickerViewController in Interface Builder file.
2019-05-13 13:56:33.997509+0200 AnalyticsExample[51926:3295271] [Application] The app delegate must implement the window property if it wants to use a main storyboard file.
2019-05-13 13:56:34.006359+0200 AnalyticsExample[51926:3295271] applicationDidBecomeActive
2019-05-13 13:56:36.327774+0200 AnalyticsExample[51926:3295271] applicationWillResignActive
2019-05-13 13:56:38.746447+0200 AnalyticsExample[51926:3295271] applicationDidEnterBackground
2019-05-13 13:56:40.523397+0200 AnalyticsExample[51926:3295271] applicationWillEnterForeground
2019-05-13 13:56:40.807612+0200 AnalyticsExample[51926:3295271] applicationDidBecomeActive
AGulev commented 5 years ago

Hi, Any updates about this issue?

a-maurice commented 5 years ago

Hi @AGulev,

Looking at your example, it seems like the problem is in the underlying iOS SDK, as it doesn't involve the C++ SDK at all. I'll ping that team about this issue, but feel free to file an issue at https://github.com/firebase/firebase-ios-sdk as well.

AGulev commented 5 years ago

Hi @a-maurice , I don't think so. My example works fine if I remove firebase.framework (which is a part of firebase cpp sdk), as I understand the problem with this hook https://github.com/firebase/firebase-cpp-sdk/blob/f799d80750cd570a69e25585097cf15303ce6dd9/app/src/invites/ios/invites_ios_startup.mm#L128

Thanks, Alex

a-maurice commented 5 years ago

Oh, I see. Sorry, was thrown off by the iOS quickstart, and missed that it was the C++ framework being linked (or not). We'll keep looking into this.

a-maurice commented 5 years ago

So, the problem seems to be when we swizzle applicationDidBecomeActive, we handle if the AppDelegate we are replacing was implementing that function, however that isn't the case here, since it is instead using the forward invocation logic to forward it instead. We will look into handling this case in the underlying code, but for a workaround if you added applicationDidBecomeActive to the AppDelegateProxy implementation, that should be called without problem in the meantime.

AGulev commented 5 years ago

Which methods should I add? I mean your are swizzling more than one method, right?

patm1987 commented 5 years ago

Hi @AGulev,

Sorry for the delay in the response! The issue as you've reported it should be fixed right now, let us know if you're still running into issues: https://github.com/firebase/firebase-cpp-sdk/commit/08cced48269c0fa1309feee4c2e3e06ec98aef0d

Feel free to open this back up if you still need help! I know this doesn't answer which methods you need to add, but hopefully you can go back to your old logic.

--Patrick