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

Crash bug when processing SSO return under Xcode 9.3 #233

Merged
merged 8 commits into from
Apr 25, 2018

Conversation

lhasiuk
Copy link
Contributor

@lhasiuk lhasiuk commented Apr 5, 2018

Fixed an issue that appears when compiled with Xcode 9.3, where the various "annotation" parameters are declared as "Any", which means that they cannot be nil. With the new version of the compiler, a nil value causes a runtime crash, and the source of this parameter, the application's incoming value for options[UIApplicationOpenURLOptionsAnnotationKey] is typically nil. Passing a nil value as annotation directly actually results in the following compilation error: "Null passed to a callee that requires a non-null argument", as further evidence of the issue. The fix is to make annotation an optional parameter ("Any?" instead of "Any") in the various places it is used.

lhasiuk and others added 8 commits January 30, 2018 17:33
"<App Name>" wants to open "Uber"
Cancel / Open

would cause SSO to fail because the system dialog triggers the UIApplicationDidBecomeActive notification.
The fix is to make sure that the notification is preceded by a UIApplicationWillEnterForeground notification,
which happens only when the there's been a context switch to the Uber app.

Fixed a related bug where, if the user were to select Cancel in the system dialog box (above), the library
would attempt to use fallback to a web view, when the correct handling would be to stop attempting to log
in and return an error to the caller.
…s to appear as if the user did not follow them when they actually did.
…arious "annotation" parameters are declared as "Any", which means that they cannot be nil. With the new version of the compiler, a nil value causes a runtime crash, and the source of this parameter, the application's incoming value for options[UIApplicationOpenURLOptionsAnnotationKey] is typically nil. Passing a nil value as annotation directly actually results in the following compilation error: "Null passed to a callee that requires a non-null argument", as further evidence of the issue. The fix is to make annotation an optional parameter ("Any?" instead of "Any") in the various places it is used.
…tead of "Any?" (i.e. optional, since it can be nil).

Fixed a compilation issue in UberCoreTests.
@lhasiuk
Copy link
Contributor Author

lhasiuk commented Apr 6, 2018

Sorry about yesterday's pull request... I hadn't checked the test code to make sure it compiled. Today's fixes that and catches a few more places where "annotation" was required to be non-nil.

@edjiang
Copy link
Contributor

edjiang commented Apr 9, 2018

No problem, thanks @lhasiuk!

@martyla
Copy link

martyla commented Apr 25, 2018

Any blockers on merging this?

@edjiang
Copy link
Contributor

edjiang commented Apr 25, 2018

No blockers -- just my time. Sorry! 😅

I'll bump the version later today.

@edjiang edjiang merged commit 40f60ee into uber:master Apr 25, 2018
edjiang pushed a commit that referenced this pull request Apr 27, 2018
* Fixed an issue that appears when compiled with Xcode 9.3, where the various "annotation" parameters are declared as "Any", which means that they cannot be nil.  With the new version of the compiler, a nil value causes a runtime crash, and the source of this parameter, the application's incoming value for options[UIApplicationOpenURLOptionsAnnotationKey] is typically nil. Passing a nil value as annotation directly actually results in the following compilation error: "Null passed to a callee that requires a non-null argument", as further evidence of the issue.  The fix is to make annotation an optional parameter ("Any?" instead of "Any") in the various places it is used.

* Fixed a few more places where "annotation" was declared as "Any", instead of "Any?" (i.e. optional, since it can be nil).
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.

3 participants