Skip to content
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

Fixes an issue where app hangs on startup with both Unity notifications and Firebase in the project #130

Merged
merged 8 commits into from
Nov 24, 2021

Conversation

itsneil
Copy link
Collaborator

@itsneil itsneil commented Nov 12, 2021

https://fogbugz.unity3d.com/f/cases/1375744/

We need to move the setting of a delegate out of the load method in order to fix the above case.

@itsneil
Copy link
Collaborator Author

itsneil commented Nov 17, 2021

After discussions with Alexey, it was noted that setting the delegate in load is a bad bad idea.

https://developer.apple.com/documentation/objectivec/nsobject/1418815-load?language=objc
https://www.mikeash.com/pyblog/friday-qa-2009-05-22-objective-c-class-loading-and-initialization.html

Adding them to the PR

@itsneil
Copy link
Collaborator Author

itsneil commented Nov 17, 2021

Sorry for the git mess, GUI client... When squashed it will be sensible.

@aurimasc
Copy link
Collaborator

@vaidasma after the changes the concerning points are push notifications and app launch by clicking the notification. By changing the place when we set the delegate we risk losing info about notifications delivered while app was in background (or killed entirely) and not receiving information about the notification that was used to launch it.

@alexey-unity
Copy link
Contributor

are push notifications and app launch by clicking the notification

if that change results in issues or we worry about resulting in the issues - should we try +initialize ? this happens when app runtime is fully setup

@Pavel-Unity
Copy link

  • the way the notification package do its initialization seems won't work in UaaL scenario when another app is fully running so there will be no UIApplicationDidFinishLaunchingNotification even broad casted
  • according to https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/1649522-delegate?language=objc Notifications that cause your app to be launched or delivered shortly after (application:didFinishLaunchingWithOptions) methods finish executing
    but we are attaching initialization code not to apps delegate callback but to message broadcasted UIApplicationDidFinishLaunchingNotification.
    The question is it guaranteed that UIApplicationDidFinishLaunchingNotification will be broadcasted before UNUserNotificationCenter.delegate calls start to happen

@itsneil
Copy link
Collaborator Author

itsneil commented Nov 19, 2021

Given https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/1649522-delegate?language=objc

To guarantee that your app responds to all actionable notifications, you must set the value of this property before your app finishes launching. For an iOS app, this means updating this property in the application:willFinishLaunchingWithOptions: or application:didFinishLaunchingWithOptions: method of the app delegate.

And in order to allow UaaL to function correctly, I have decided to have the notifications init on the broadcast of kUnityWillFinishLaunchingWithOptions.

When launching an app, the AppDelegate's methods are called and then the notification posted shortly afterward, we cannot guarantee the timing of the native lifecycle notifications, but we always post kUnityWillFinishLaunchingWithOptions from the AppDelegate. This could guarantee that the notification stuff is set up late enough not to cause locks and early enough not to miss notifications.

This would also be a good thing to document to all plugin writers as a good place to init your plugin?

Copy link
Contributor

@vaidasma vaidasma left a comment

Choose a reason for hiding this comment

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

Verified as fixed:
• No longer crashes upon killing the app and launching again (with FIrebase used in the project)
• Application does not demonstrate any unwanted behavior when putting it to background/foreground, locking/unlocking
• Notifications Authorization is successful
• Local/push notifications are received

Tested with Notification projects merged into the user's project from Fogbugz
Device used: iPhone 12 (OS:15.2)

Note: during testing, it has been noticed that authorization on launch no longer works. Unrelated to the current fix.

@aurimasc
Copy link
Collaborator

@vaidasma after the changes the concerning points are push notifications and app launch by clicking the notification. By changing the place when we set the delegate we risk losing info about notifications delivered while app was in background (or killed entirely) and not receiving information about the notification that was used to launch it.

@aurimasc aurimasc merged commit 437f031 into master Nov 24, 2021
@aurimasc aurimasc deleted the fix-ios-hang-on-app-relaunch branch November 24, 2021 14:59
@vaidasma
Copy link
Contributor

Above issue by design: caused by "Provisional" flag in Default Notification Authorization Settings

@seagullua
Copy link

When the fix will become available in Unity Package Manager?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants