Fix issue with PaymentLauncher configuration change

[jameswoo-stripe]

Summary

References #4739

Fixes an issue where configuration change of PaymentLauncherConfirmationActivity would cause a crash. This was because PaymentLauncherConfirmationActivity was launching a coroutine in the onCreate and would get called whenever there is a configuration change (e.g. screen rotation) and would emit a JobCancellationException. This exception was trying to be serialized as PaymentResult.Failed, but the exception contains a Job which cannot be serialized, causing a crash.

To fix this issue, we make sure that PaymentLauncherViewModel starts the coroutine so that it can survive configuration changes. However, additional changes needed to be made. The activity would also lose the ability to listen to the ActivityResultCallback after configuration change. This was fixed by adding a register method on the ViewModel.

Motivation

This issue will break all landscape mode PaymentLauncher confirmations.

Testing

• Added tests
• Modified tests
• Manually verified

Screenshots

Before

crash.mov

After

landscapepaymentlauncher.mov

Changelog

• [Fixed] [BUG] Crash when calling PaymentLauncher.confirm(params:) #4739

[github-actions]

Diffuse output:

[skyler-stripe]

LGTM. Before approving though, did you check to see if this works with dont keep activity on? Just want to make sure the payment doesn't try to do through twice and that it restores and redirects back correctly after being killed.

Assigning Chen explicitly so he can take a look too.

[carlosmuvi-stripe]

This keeps a reference to the activity internally. A viewModel cannot hold a reference to an Activity / any other view.

The Activity / Fragment gets destroyed and recreated on every configuration change, and a new instance gets the same instance of the ViewModel. Keeping a reference to the activity would prevent it from getting GCed here.

[carlosmuvi-stripe]

Update on this - Looks like we were already leaking by keeping an activity reference on the viewModel, so this seems out of the scope of this bugfix. Unblocking.

Blocked originally due to an existing memory leak.

[jameswoo-stripe]

LGTM. Before approving though, did you check to see if this works with dont keep activity on? Just want to make sure the payment doesn't try to do through twice and that it restores and redirects back correctly after being killed.

Assigning Chen explicitly so he can take a look too.

Checked DKA, works as expected. Thanks!

[carlosmuvi-stripe]

Unlocking this bugfix, but proposing a follow-up to avoid potential OOMs due to screen rotations.

[carlosmuvi-stripe]

looks like we're keeping a reference to authActivityStarterHost just to be able to run this logic right?

We're unlocking rotations with this PR, meaning every time the user rotates we're stacking an activity in memory that cannot be GCed, eventually running into an OOM / crash.

An important follow-up of this would be:

• Move authActivityStarterHost to activity
• Move this logic to the activity Communicate from viewModel to activity via a SharedFlow or LiveData<Event>

[skyler-stripe]

+1 to this. I'm not too thrilled about holding on to this AuthActivityStarterHost in the view model. I think flow is the way to go. We've been using those over LiveData.

[carlosmuvi-stripe]

LGTM, not holding an activity reference 👏