Commit 833f851b authored by Rouslan Solomakhin's avatar Rouslan Solomakhin Committed by Commit Bot

[Payment Request] Require user gesture for skip-UI.

Before this patch, calling `new PaymentRequest(https://foo.com).show()`
without a user gesture could skip the payment sheet and go directly into
the payment handler for https://foo.com, i.e., the skip-UI flow.

This patch passes the user gesture flag from the renderer to the browser
and prevents the skip-UI flow for the cases when show() was called
without a user gesture.

After this patch, calling `new PaymentRequest(https://foo.com).show()`
without a user gesture will show the payment sheet, so the user will
have to explicitly provide the user gesture by tapping the "Pay" button
in the payment sheet before invoking the payment app.

Bug: 828427
Change-Id: Iaea5fe3ede1bbd4b68f2c06f63bde4bd36a38e43
Reviewed-on: https://chromium-review.googlesource.com/993412Reviewed-by: default avatarChris Palmer <palmer@chromium.org>
Reviewed-by: default avataranthonyvd <anthonyvd@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547862}
parent fb8db829
...@@ -36,7 +36,7 @@ public class PaymentRequestFactory implements InterfaceFactory<PaymentRequest> { ...@@ -36,7 +36,7 @@ public class PaymentRequestFactory implements InterfaceFactory<PaymentRequest> {
} }
@Override @Override
public void show() { public void show(boolean isUserGesture) {
if (mClient != null) { if (mClient != null) {
mClient.onError(PaymentErrorReason.USER_CANCEL); mClient.onError(PaymentErrorReason.USER_CANCEL);
mClient.close(); mClient.close();
......
...@@ -619,7 +619,7 @@ public class PaymentRequestImpl ...@@ -619,7 +619,7 @@ public class PaymentRequestImpl
* Called by the merchant website to show the payment request to the user. * Called by the merchant website to show the payment request to the user.
*/ */
@Override @Override
public void show() { public void show(boolean isUserGesture) {
if (mClient == null) return; if (mClient == null) return;
if (mUI != null) { if (mUI != null) {
...@@ -660,6 +660,7 @@ public class PaymentRequestImpl ...@@ -660,6 +660,7 @@ public class PaymentRequestImpl
mObservedTabModel.addObserver(mTabModelObserver); mObservedTabModel.addObserver(mTabModelObserver);
buildUI(chromeActivity); buildUI(chromeActivity);
if (!isUserGesture) mShouldSkipShowingPaymentRequestUi = false;
if (!mShouldSkipShowingPaymentRequestUi) mUI.show(); if (!mShouldSkipShowingPaymentRequestUi) mUI.show();
triggerPaymentAppUiSkipIfApplicable(); triggerPaymentAppUiSkipIfApplicable();
......
...@@ -151,7 +151,7 @@ void PaymentRequest::Init(mojom::PaymentRequestClientPtr client, ...@@ -151,7 +151,7 @@ void PaymentRequest::Init(mojom::PaymentRequestClientPtr client,
spec_->url_payment_method_identifiers().end()); spec_->url_payment_method_identifiers().end());
} }
void PaymentRequest::Show() { void PaymentRequest::Show(bool is_user_gesture) {
if (!client_.is_bound() || !binding_.is_bound()) { if (!client_.is_bound() || !binding_.is_bound()) {
LOG(ERROR) << "Attempted Show(), but binding(s) missing."; LOG(ERROR) << "Attempted Show(), but binding(s) missing.";
OnConnectionTerminated(); OnConnectionTerminated();
...@@ -177,6 +177,8 @@ void PaymentRequest::Show() { ...@@ -177,6 +177,8 @@ void PaymentRequest::Show() {
return; return;
} }
is_show_user_gesture_ = is_user_gesture;
// TODO(crbug.com/783811): Display a spinner when checking whether // TODO(crbug.com/783811): Display a spinner when checking whether
// the methods are supported asynchronously for better user experience. // the methods are supported asynchronously for better user experience.
state_->AreRequestedMethodsSupported( state_->AreRequestedMethodsSupported(
...@@ -356,7 +358,7 @@ bool PaymentRequest::IsIncognito() const { ...@@ -356,7 +358,7 @@ bool PaymentRequest::IsIncognito() const {
bool PaymentRequest::SatisfiesSkipUIConstraints() const { bool PaymentRequest::SatisfiesSkipUIConstraints() const {
return base::FeatureList::IsEnabled(features::kWebPaymentsSingleAppUiSkip) && return base::FeatureList::IsEnabled(features::kWebPaymentsSingleAppUiSkip) &&
base::FeatureList::IsEnabled(::features::kServiceWorkerPaymentApps) && base::FeatureList::IsEnabled(::features::kServiceWorkerPaymentApps) &&
state()->is_get_all_instruments_finished() && is_show_user_gesture_ && state()->is_get_all_instruments_finished() &&
state()->available_instruments().size() == 1 && state()->available_instruments().size() == 1 &&
spec()->stringified_method_data().size() == 1 && spec()->stringified_method_data().size() == 1 &&
!spec()->request_shipping() && !spec()->request_payer_name() && !spec()->request_shipping() && !spec()->request_payer_name() &&
......
...@@ -65,7 +65,7 @@ class PaymentRequest : public mojom::PaymentRequest, ...@@ -65,7 +65,7 @@ class PaymentRequest : public mojom::PaymentRequest,
std::vector<mojom::PaymentMethodDataPtr> method_data, std::vector<mojom::PaymentMethodDataPtr> method_data,
mojom::PaymentDetailsPtr details, mojom::PaymentDetailsPtr details,
mojom::PaymentOptionsPtr options) override; mojom::PaymentOptionsPtr options) override;
void Show() override; void Show(bool is_user_gesture) override;
void UpdateWith(mojom::PaymentDetailsPtr details) override; void UpdateWith(mojom::PaymentDetailsPtr details) override;
void NoUpdatedPaymentDetails() override; void NoUpdatedPaymentDetails() override;
void Abort() override; void Abort() override;
...@@ -165,6 +165,9 @@ class PaymentRequest : public mojom::PaymentRequest, ...@@ -165,6 +165,9 @@ class PaymentRequest : public mojom::PaymentRequest,
// Whether a completion was already recorded for this Payment Request. // Whether a completion was already recorded for this Payment Request.
bool has_recorded_completion_ = false; bool has_recorded_completion_ = false;
// Whether PaymentRequest.show() was invoked with a user gesture.
bool is_show_user_gesture_ = false;
base::WeakPtrFactory<PaymentRequest> weak_ptr_factory_; base::WeakPtrFactory<PaymentRequest> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(PaymentRequest); DISALLOW_COPY_AND_ASSIGN(PaymentRequest);
......
...@@ -817,7 +817,8 @@ ScriptPromise PaymentRequest::show(ScriptState* script_state) { ...@@ -817,7 +817,8 @@ ScriptPromise PaymentRequest::show(ScriptState* script_state) {
// TODO(crbug.com/825270): Reject with SecurityError DOMException if triggered // TODO(crbug.com/825270): Reject with SecurityError DOMException if triggered
// without user activation. // without user activation.
if (!Frame::HasTransientUserActivation(GetFrame())) { bool is_user_gesture = Frame::HasTransientUserActivation(GetFrame());
if (!is_user_gesture) {
UseCounter::Count(GetExecutionContext(), UseCounter::Count(GetExecutionContext(),
WebFeature::kPaymentRequestShowWithoutGesture); WebFeature::kPaymentRequestShowWithoutGesture);
} }
...@@ -830,7 +831,7 @@ ScriptPromise PaymentRequest::show(ScriptState* script_state) { ...@@ -830,7 +831,7 @@ ScriptPromise PaymentRequest::show(ScriptState* script_state) {
DOMException::Create(kInvalidStateError, "Page popups are suppressed")); DOMException::Create(kInvalidStateError, "Page popups are suppressed"));
} }
payment_provider_->Show(); payment_provider_->Show(is_user_gesture);
show_resolver_ = ScriptPromiseResolver::Create(script_state); show_resolver_ = ScriptPromiseResolver::Create(script_state);
return show_resolver_->Promise(); return show_resolver_->Promise();
......
...@@ -188,7 +188,7 @@ interface PaymentRequest { ...@@ -188,7 +188,7 @@ interface PaymentRequest {
PaymentOptions options); PaymentOptions options);
// Shows the user interface with the payment details. // Shows the user interface with the payment details.
Show(); Show(bool is_user_gesture);
// Updates the payment details in response to new shipping address or shipping // Updates the payment details in response to new shipping address or shipping
// option. // option.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment