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

Simplify Android #288

Open
kristoffer-zliide opened this issue Jan 9, 2025 · 1 comment
Open

Simplify Android #288

kristoffer-zliide opened this issue Jan 9, 2025 · 1 comment

Comments

@kristoffer-zliide
Copy link

We have our own button that launches the payment sheets, "advanced" usage I suppose. We're updating from v.2 to v.3, and wondering how to deal with the changes. Our v.2 code looks like this:

void _onPayPressed() async {
  final result = await _payClient.showPaymentSelector(
    PayProvider.google_pay,
    _paymentItems,
  );
  // Code send the resulting token to your server / PSP
}

From what we understand, we need to change it to this on Android:

void _onPayPressed() async {
  final completer = Completer<Map<String, dynamic>>();

  // From the README
  const eventChannel =
      EventChannel('plugins.flutter.io/pay/payment_result');
  final _paymentResultSubscription = eventChannel
      .receiveBroadcastStream()
      .map((result) => jsonDecode(result as String) as Map<String, dynamic>)
      .listen((result) {
        // TODO: Send the resulting Google Pay token to your server / PSP
        completer.complete(result);
      }, onError: (error) {
        // TODO: Handle errors
        completer.completeError(error);
      });

  await _payClient.showPaymentSelector(
    PayProvider.google_pay,
    _paymentItems,
  );

  final result = await completer.future;

  _paymentResultSubscription.cancel();
  _paymentResultSubscription = null;

  // Code send the resulting token to your server / PSP
}

Is that correct, or are we still missing something?

If it is indeed correct, why wasn't this just rolled into the package like this, so users of the package won't have to deal with it?

The PR related to the changes indicates that the main activity risks being destroyed during the payment flow. Can that really happen?? Or are we seeing ghosts? Google Pay is just a sheet, the activity hosting Flutter is visible behind it, so I'd be very surprised if Android would dispose of it. We are unable to recreate such behavior by enabling "Don't keep activities" in "Developer options". How do you test it?

And even if the activity is destroyed, are these changes enough? Wouldn't we need to also listen for events again after the activity is recreated, e.g. at app startup? I suspect that the solution with subscribing in initState won't work unless the button is on the app's first page. This might not just be an issue for "advanced" usage.

@JlUgia
Copy link
Member

JlUgia commented Jan 13, 2025

Hi @kristoffer-zliide, thank you for your thorough analysis. You bring up great points.

[As a side thought, I'm less confident that "advanced" is an appropriate way to describe the alternative, more customizable integration path. Something to consider]

Similar concerns and ideas were discussed during assessment and code change efforts. See #282 if you prefer to check out the conclusions, and #274 contains a brief API design assessment. Feel free to comment with further suggestions and proposals.
Specifically, to your questions:

  1. Code proposal: Your proposal looks good. My only reservation is that the subscription does not match its state with the underlying widget hosting it, which in turn, is coupled with an instance of the plugin engine, which at the moment is tied to an Android activity. We've explored bespoke engine caching strategies overriding the default behavior, though preferred to stay within the boundaries of API behavior that may change later on and diverge with the default library. I'm curious as to whether the subscription in your proposal may be left stranded in an scenario where the activity is recycled prior to a result being return (see the next point).
  2. Activities that get destroyed: You'll find more background information about this at Add an event stream to communicate with Android's native end #282 and the issues linked there. Unfortunately, those destruction events are rather real (and the share of cases per active users, though low, are there – see Crash when calling DataResult multiple times on the same instance #277 or Critical Crash on Payment Success Handling in Google Pay Flutter Plugin Affecting 350+ Users #274). Regardless of how lightweight the pay sheet might be, congestion, and hence resource exhaustion, is generated by anything running on the device. The pay process may not be at the front of the queue for candidate processes to be recycled, but that's different from having guarantees that it'll be kept alive. In any case, enabling "Don't keep activities" is enough to trigger this behavior. Make sure to temporarily navigate to a different app process right after starting the payment process with Google Pay, and then come back to the application.
  3. Resiliency against activity destruction: This changes are effective against activity destruction events for the cases explored. These events produced a re-creation of the host activity on the Flutter end, which in turn re-generates the state of the widget. In any case, and should this behavior suffer from any changes, the subscription can be tied to a more suitable lifecycle event in the widget.

I'm curious to know more about how your proposal behaves against the re-creation events explored above. If the subscription is adequately recreated before results are returned, when destroyed in the middle of the payment process (after showing the pay sheet, and before a result is returned), we should consider being more lax and reduce ties to the widget's lifecycle.

Let me know if that makes sense.

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

No branches or pull requests

2 participants