[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 <[email protected]>
Reviewed-by: anthonyvd <[email protected]>
Commit-Queue: Rouslan Solomakhin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#581731}diff --git a/components/payments/content/payment_request.cc b/components/payments/content/payment_request.cc
index 081bedc..dbc6518 100644
--- a/components/payments/content/payment_request.cc
+++ b/components/payments/content/payment_request.cc
@@ -93,16 +93,8 @@
LOG(ERROR) << "SSL certificate is not valid";
if (!allowed_origin || invalid_ssl) {
- // Don't show UI. Resolve .canMakepayment() with "false". Reject .show()
- // with "NotSupportedError".
- spec_ = std::make_unique<PaymentRequestSpec>(
- mojom::PaymentOptions::New(), mojom::PaymentDetails::New(),
- std::vector<mojom::PaymentMethodDataPtr>(), this,
- delegate_->GetApplicationLocale());
- state_ = std::make_unique<PaymentRequestState>(
- web_contents_, top_level_origin_, frame_origin_, spec_.get(), this,
- delegate_->GetApplicationLocale(), delegate_->GetPersonalDataManager(),
- delegate_.get(), &journey_logger_);
+ // Intentionally don't set |spec_| and |state_|. Don't show UI. Resolve
+ // .canMakepayment() with "false". Reject .show() with "NotSupportedError".
return;
}
@@ -178,10 +170,16 @@
return;
}
+ if (!state_) {
+ // SSL is not valid.
+ AreRequestedMethodsSupportedCallback(false);
+ return;
+ }
+
is_show_user_gesture_ = is_user_gesture;
- // TODO(crbug.com/783811): Display a spinner when checking whether
- // the methods are supported asynchronously for better user experience.
+ display_handle_->Show(this);
+
state_->AreRequestedMethodsSupported(
base::BindOnce(&PaymentRequest::AreRequestedMethodsSupportedCallback,
weak_ptr_factory_.GetWeakPtr()));
@@ -205,10 +203,8 @@
void PaymentRequest::AreRequestedMethodsSupportedCallback(
bool methods_supported) {
if (methods_supported) {
- journey_logger_.SetEventOccurred(JourneyLogger::EVENT_SHOWN);
-
- DCHECK(display_handle_);
- display_handle_->Show(this);
+ if (SatisfiesSkipUIConstraints())
+ Pay();
} else {
journey_logger_.SetNotShown(
JourneyLogger::NOT_SHOWN_REASON_NO_SUPPORTED_PAYMENT_METHOD);
@@ -285,10 +281,11 @@
if (observer_for_testing_)
observer_for_testing_->OnCanMakePaymentCalled();
- if (!delegate_->GetPrefService()->GetBoolean(kCanMakePaymentEnabled)) {
+ if (!delegate_->GetPrefService()->GetBoolean(kCanMakePaymentEnabled) ||
+ !state_) {
CanMakePaymentCallback(/*can_make_payment=*/false);
} else {
- state()->CanMakePayment(
+ state_->CanMakePayment(
base::BindOnce(&PaymentRequest::CanMakePaymentCallback,
weak_ptr_factory_.GetWeakPtr()));
}
@@ -389,6 +386,10 @@
display_handle_.reset();
}
+void PaymentRequest::RecordDialogShownEventInJourneyLogger() {
+ journey_logger_.SetEventOccurred(JourneyLogger::EVENT_SHOWN);
+}
+
bool PaymentRequest::IsIncognito() const {
return delegate_->IsIncognito();
}
@@ -415,10 +416,10 @@
}
void PaymentRequest::CanMakePaymentCallback(bool can_make_payment) {
- if (CanMakePaymentQueryFactory::GetInstance()
- ->GetForContext(web_contents_->GetBrowserContext())
- ->CanQuery(top_level_origin_, frame_origin_,
- spec()->stringified_method_data())) {
+ if (!spec_ || CanMakePaymentQueryFactory::GetInstance()
+ ->GetForContext(web_contents_->GetBrowserContext())
+ ->CanQuery(top_level_origin_, frame_origin_,
+ spec_->stringified_method_data())) {
RespondToCanMakePaymentQuery(can_make_payment, false);
} else if (OriginSecurityChecker::IsOriginLocalhostOrFile(frame_origin_)) {
RespondToCanMakePaymentQuery(can_make_payment, true);
