uber / uber-ios-sdk

Uber iOS SDK (beta)
https://developer.uber.com/docs
MIT License
375 stars 125 forks source link

Fix for a sporadic timing error in DeeplinkManager.swift #223

Closed lhasiuk closed 6 years ago

lhasiuk commented 6 years ago

This commit contains changes to DeeplinkManager.swift to fix another error we've seen where the SSO login process fails when a user tries it for the first time and the system dialog box appears. This case only happens sporadically.

The original problematic code sets a 0.25 second one-shot timer when it sees a UIApplicationDidBecomeActive notification. The timer is effectively canceled if a UIApplicationDidEnterBackground notification arrives within the programmed interval. Otherwise, the timer fires and forces a "deeplinkNotFollowed" error. The idea for the original code is probably to distinguish between the case where the application goes inactive and then active again when the system dialog is presented, but is waiting to see whether the user allows the process to continue or not, by determining whether the return to activity is followed by a notification indicating that the application went into the background (and, hence, that the deep link was followed) vs. the background notification not being received, an indication that the user canceled.

Timers for system notifications are never a reliable trigger, and the application exists in a strange state of being suspended but active while the system dialog is displayed. Because of this, the notifications are delivered to the calling application long after the user has dismissed the system dialog box, because no background task has been established in the calling code. While such measures might help, there is still an inherent unreliability in depending on the second event arriving in a specific amount of time.

Fortunately, for iOS 10 and later versions, there is a better way. A different form of the UIApplication API for opening a URL allows for a callback to let the application know whether the user allowed it or not. The proposed change adds code to use the latest API (which happens to be the only one which is not considered deprecated), and completely bypasses the need for hooking notifications and establishing timers. This makes it completely reliable under later iOS versions.

edjiang commented 6 years ago

Thanks @lhasiuk. I'm going to rebase your master.

edjiang commented 6 years ago

Hm, I got a permission denied when I tried to push your commit back to your own master (not sure how GitHub does permissions here), but I merged your commit into our master. Thanks!

lhasiuk commented 6 years ago

You're welcome!