New issue
Advanced search Search tips

Issue 783811 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Make AreRequestedMethodsSupported asynchronous in payment_reqest_state

Project Member Reported by gogerald@chromium.org, Nov 10 2017

Issue description

changing 

request.canMakePayment().then(function(result) {
  request.show().then(response => {
    console.log(response);
    response.complete();
  });
});

to

request.show().then(response => {
  console.log(response);
  response.complete();
});

will immediately raise the following error: "Uncaught (in promise) NotSupportedError: The payment method is not supported".

 
Cc: -anthonyvd@chromium.org gogerald@chromium.org
Owner: anthonyvd@chromium.org
Status: Assigned (was: Untriaged)
Assigned to anthonyvd@ for UI changes (spinner before AreRequestedMethodsSupported return),
Anthony: Do you have suggestions on how to implement this? I'm digging into this for showing the spinner when a promise was passed into the show() method. This would happen when the final price is not known at the time of user gesture, for example. (My code so far: https://crrev.com/c/939669)
You could do something similar to what we do when the dialog is shown before all instruments are ready (https://cs.chromium.org/chromium/src/chrome/browser/ui/views/payments/payment_request_dialog_view.cc?rcl=39ab63b5f7e1f96e7dfe201dbb3e47b55ff403e3&l=83). Basically call ShowProcessingSpinner when show is called with a promise and let the resolution of that promise call HideProcessingSpinner.

We seem to be slowly heading into a situation where a *bunch* of stuff shows the spinner and assumes nothing else is also waiting before hiding the spinner and that could be bad when a few things race to hide it. Maybe we need something like {In|De}crementWaitingCounter and show the spinner when the counter > 0 and hide it when it's == 0, then replace all show calls to Increment and all hide calls to Decrement.
Ah! Excellent suggestion. I was doing the checking in the payment_request.cc file, but now I see that would be redundant with payment_request_dialog_view.cc.
Is  this bug already fixed due to the code in payment_request_dialog_view.cc, by the way?
Cc: -rouslan@chromium.org anthonyvd@chromium.org
Owner: rouslan@chromium.org
Status: Started (was: Assigned)
Fixing this in https://chromium-review.googlesource.com/939669
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2cae95abb040273c9c6ca2eda2827d4aa412f5a

commit d2cae95abb040273c9c6ca2eda2827d4aa412f5a
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Thu Aug 09 00:16:10 2018

[Desktop Payment Request] Show spinner while querying payment methods.

Before this patch, the Payment Request dialog on desktop would not be
shown after the merchant called PaymentRequest.show(). The dialog would
be shown only after the payment handlers have been queried.

This patch makes the Payment Request dialog UI appear with a
"Processing" spinner when PaymentRequest.show() is called.  When the
payment handler query has finished, the dialog hides the processing
spinner and becomes interactive. A few minor improvements flow from this
change:

- There's no longer a need for a hidden dialog that is eventually shown.
  The dialog is always shown after PaymentRequest.show().

- The tests are updated to expect the processing spinner before the
  "dialog opened" event.

- To better debug incorrect expectations in tests, the variable
  |events_| is renamed into |expected_events_| and the variable |event|
  is renamed into |actual_event| in test_event_waiter.h.

- Because the JourneyLogger is owned privately in PaymentRequest object
  and the "event shown" needs to be recorded before the "dialog opened"
  testing event is fired from the UI, the PaymentRequest object now
  exposes the RecordDialogShownEventInJourneyLogger() method to be
  called from the UI layer.

After this patch, the Payment Request dialog on desktop is shown in a
spinning state with "Processing" message after the merchant calls
PaymentRequest.show(), before the payment handlers have been queried.


Bug:  783811 
Change-Id: I820c9e8a093f74b30e5e7f29ef39675f60c17158
Reviewed-on: https://chromium-review.googlesource.com/1159183
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581731}
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/payments/chrome_payment_request_delegate.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/payments/chrome_payment_request_delegate.h
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/modifiers_browsertest.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_browsertest.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_browsertest_base.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_browsertest_base.h
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_can_make_payment_metrics_browsertest.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_completion_status_metrics_browsertest.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_no_update_with_browsertest.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/payment_request_payment_app_browsertest.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/components/autofill/core/browser/test_event_waiter.h
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/components/payments/content/payment_request.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/components/payments/content/payment_request.h
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/components/payments/content/payment_request_display_manager.h
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/components/payments/content/payment_request_state.cc
[modify] https://crrev.com/d2cae95abb040273c9c6ca2eda2827d4aa412f5a/components/payments/content/payment_request_state.h

Status: Fixed (was: Started)

Sign in to add a comment