Repository navigation
Fix CardScanGoogleLauncher attempting to reference unregistered launcher after PE is closed
#13261
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,8 +28,9 @@ internal class CardScanGoogleLauncher @VisibleForTesting constructor( | |
| private val _loadingState = MutableStateFlow(CardScanLoadingState.Loading) | ||
| override val loadingState: StateFlow<CardScanLoadingState> = _loadingState.asStateFlow() | ||
|
|
||
| @VisibleForTesting | ||
| lateinit var activityLauncher: ActivityResultLauncher<IntentSenderRequest> | ||
| private var deferredIntentSenderRequest: IntentSenderRequest? = null | ||
|
|
||
| private lateinit var activityLauncher: ActivityResultLauncher<IntentSenderRequest> | ||
|
|
||
| init { | ||
| paymentCardRecognitionClient.fetchIntent( | ||
|
|
@@ -55,15 +56,37 @@ internal class CardScanGoogleLauncher @VisibleForTesting constructor( | |
| paymentCardRecognitionClient.fetchIntent( | ||
| context = context, | ||
| onFailure = { e -> | ||
| _isLaunching = false | ||
| eventsReporter.onCardScanFailed(implementation, e) | ||
| }, | ||
| onSuccess = { intentSenderRequest -> | ||
| eventsReporter.onCardScanStarted("google_pay") | ||
| activityLauncher.launch(intentSenderRequest, options) | ||
| launchIntentSenderOrDefer(intentSenderRequest) | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| fun setLauncher(launcher: ActivityResultLauncher<IntentSenderRequest>) { | ||
| activityLauncher = launcher | ||
|
|
||
| val pending = deferredIntentSenderRequest ?: return | ||
|
|
||
| try { | ||
| launcher.launch(pending, options) | ||
| deferredIntentSenderRequest = null | ||
| } catch (_: IllegalStateException) { | ||
| _isLaunching = false | ||
| } | ||
| } | ||
|
|
||
| private fun launchIntentSenderOrDefer(intentSenderRequest: IntentSenderRequest) { | ||
| try { | ||
| activityLauncher.launch(intentSenderRequest, options) | ||
| } catch (_: IllegalStateException) { | ||
| deferredIntentSenderRequest = intentSenderRequest | ||
| } | ||
| } | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a ton of other issues with this scanner is built that I need to be resolved. A lot of the work (fetching intent, checking availability) is being done in the composition scope of the This PR fixes the initial crash but a good amount of follow-up work should be done. I'll open up tickets for these. |
||
|
|
||
| internal fun parseActivityResult(result: ActivityResult): CardScanResult { | ||
| val scanResult = when { | ||
| result.resultCode == Activity.RESULT_OK && result.data != null -> { | ||
|
|
@@ -125,7 +148,7 @@ internal class CardScanGoogleLauncher @VisibleForTesting constructor( | |
| onResult(launcher.parseActivityResult(result)) | ||
| } | ||
| return remember(activityLauncher) { | ||
| launcher.apply { this.activityLauncher = activityLauncher } | ||
| launcher.apply { setLauncher(activityLauncher) } | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker, but I wonder if we could fix this with coroutines. We'd pass
CoroutineScopefromrememberCardScanGoogleLauncherto this class and await anactivityLauncheron the main dispatcherThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, we should make this request in the view model scope rather than the composition scope, something I'll look to fix separately.