Commit 5b51043c authored by Rouslan Solomakhin's avatar Rouslan Solomakhin Committed by Commit Bot

[Payments] Prohibit opening payments UI in background tab.

Before this patch, calling PaymentRequest.show() would bring the
background window to the foreground, which allows a page to open a
pop-under.

This patch adds a check for the browser window being active (in
foreground) in PaymentRequest.show(). If the window is not active (in
background), then PaymentRequest.show() promise is rejected with
"AbortError: User cancelled request." No UI is shown in that case.

After this patch, calling PaymentRequest.show() does not bring the
background window to the foreground, thus preventing opening a pop-under.

Bug: 768230
Change-Id: I2b90f9086ceca5ed7b7bdf8045e44d7e99d566d0
Reviewed-on: https://chromium-review.googlesource.com/681843Reviewed-by: default avataranthonyvd <anthonyvd@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504406}
parent 8b574458
...@@ -12,7 +12,10 @@ ...@@ -12,7 +12,10 @@
#include "chrome/browser/payments/ssl_validity_checker.h" #include "chrome/browser/payments/ssl_validity_checker.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/signin_manager_factory.h" #include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_dialogs.h" #include "chrome/browser/ui/browser_dialogs.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "components/autofill/core/browser/personal_data_manager.h" #include "components/autofill/core/browser/personal_data_manager.h"
#include "components/autofill/core/browser/region_combobox_model.h" #include "components/autofill/core/browser/region_combobox_model.h"
#include "components/autofill/core/browser/region_data_loader_impl.h" #include "components/autofill/core/browser/region_data_loader_impl.h"
...@@ -139,4 +142,9 @@ PrefService* ChromePaymentRequestDelegate::GetPrefService() { ...@@ -139,4 +142,9 @@ PrefService* ChromePaymentRequestDelegate::GetPrefService() {
->GetPrefs(); ->GetPrefs();
} }
bool ChromePaymentRequestDelegate::IsBrowserWindowActive() const {
Browser* browser = chrome::FindBrowserWithWebContents(web_contents_);
return browser && browser->window() && browser->window()->IsActive();
}
} // namespace payments } // namespace payments
...@@ -43,6 +43,7 @@ class ChromePaymentRequestDelegate : public PaymentRequestDelegate { ...@@ -43,6 +43,7 @@ class ChromePaymentRequestDelegate : public PaymentRequestDelegate {
ukm::UkmRecorder* GetUkmRecorder() override; ukm::UkmRecorder* GetUkmRecorder() override;
std::string GetAuthenticatedEmail() const override; std::string GetAuthenticatedEmail() const override;
PrefService* GetPrefService() override; PrefService* GetPrefService() override;
bool IsBrowserWindowActive() const override;
protected: protected:
// Reference to the dialog so that we can satisfy calls to CloseDialog(). This // Reference to the dialog so that we can satisfy calls to CloseDialog(). This
......
...@@ -52,6 +52,20 @@ class PaymentRequestNoShippingTest : public PaymentRequestBrowserTestBase { ...@@ -52,6 +52,20 @@ class PaymentRequestNoShippingTest : public PaymentRequestBrowserTestBase {
DISALLOW_COPY_AND_ASSIGN(PaymentRequestNoShippingTest); DISALLOW_COPY_AND_ASSIGN(PaymentRequestNoShippingTest);
}; };
IN_PROC_BROWSER_TEST_F(PaymentRequestNoShippingTest, InactiveBrowserWindow) {
SetBrowserWindowInactive();
ResetEventObserver(DialogEvent::DIALOG_CLOSED);
EXPECT_TRUE(content::ExecuteScript(
GetActiveWebContents(),
"(function() { document.getElementById('buy').click(); })();"));
WaitForObservedEvent();
ExpectBodyContains({"AbortError"});
}
IN_PROC_BROWSER_TEST_F(PaymentRequestNoShippingTest, OpenAndNavigateTo404) { IN_PROC_BROWSER_TEST_F(PaymentRequestNoShippingTest, OpenAndNavigateTo404) {
InvokePaymentRequestUI(); InvokePaymentRequestUI();
......
...@@ -79,7 +79,9 @@ PaymentRequestBrowserTestBase::PaymentRequestBrowserTestBase( ...@@ -79,7 +79,9 @@ PaymentRequestBrowserTestBase::PaymentRequestBrowserTestBase(
: test_file_path_(test_file_path), : test_file_path_(test_file_path),
delegate_(nullptr), delegate_(nullptr),
is_incognito_(false), is_incognito_(false),
is_valid_ssl_(true) {} is_valid_ssl_(true),
is_browser_window_active_(true) {}
PaymentRequestBrowserTestBase::~PaymentRequestBrowserTestBase() {} PaymentRequestBrowserTestBase::~PaymentRequestBrowserTestBase() {}
void PaymentRequestBrowserTestBase::SetUpCommandLine( void PaymentRequestBrowserTestBase::SetUpCommandLine(
...@@ -127,6 +129,10 @@ void PaymentRequestBrowserTestBase::SetInvalidSsl() { ...@@ -127,6 +129,10 @@ void PaymentRequestBrowserTestBase::SetInvalidSsl() {
is_valid_ssl_ = false; is_valid_ssl_ = false;
} }
void PaymentRequestBrowserTestBase::SetBrowserWindowInactive() {
is_browser_window_active_ = false;
}
void PaymentRequestBrowserTestBase::OnCanMakePaymentCalled() { void PaymentRequestBrowserTestBase::OnCanMakePaymentCalled() {
if (event_observer_) if (event_observer_)
event_observer_->Observe(DialogEvent::CAN_MAKE_PAYMENT_CALLED); event_observer_->Observe(DialogEvent::CAN_MAKE_PAYMENT_CALLED);
...@@ -465,7 +471,8 @@ void PaymentRequestBrowserTestBase::CreatePaymentRequestForTest( ...@@ -465,7 +471,8 @@ void PaymentRequestBrowserTestBase::CreatePaymentRequestForTest(
DCHECK(web_contents); DCHECK(web_contents);
std::unique_ptr<TestChromePaymentRequestDelegate> delegate = std::unique_ptr<TestChromePaymentRequestDelegate> delegate =
base::MakeUnique<TestChromePaymentRequestDelegate>( base::MakeUnique<TestChromePaymentRequestDelegate>(
web_contents, this /* observer */, is_incognito_, is_valid_ssl_); web_contents, this /* observer */, is_incognito_, is_valid_ssl_,
is_browser_window_active_);
delegate_ = delegate.get(); delegate_ = delegate.get();
PaymentRequestWebContentsManager::GetOrCreateForWebContents(web_contents) PaymentRequestWebContentsManager::GetOrCreateForWebContents(web_contents)
->CreatePaymentRequest(web_contents->GetMainFrame(), web_contents, ->CreatePaymentRequest(web_contents->GetMainFrame(), web_contents,
......
...@@ -99,6 +99,7 @@ class PaymentRequestBrowserTestBase ...@@ -99,6 +99,7 @@ class PaymentRequestBrowserTestBase
void SetIncognito(); void SetIncognito();
void SetInvalidSsl(); void SetInvalidSsl();
void SetBrowserWindowInactive();
// PaymentRequest::ObserverForTest: // PaymentRequest::ObserverForTest:
void OnCanMakePaymentCalled() override; void OnCanMakePaymentCalled() override;
...@@ -285,6 +286,7 @@ class PaymentRequestBrowserTestBase ...@@ -285,6 +286,7 @@ class PaymentRequestBrowserTestBase
TestChromePaymentRequestDelegate* delegate_; TestChromePaymentRequestDelegate* delegate_;
bool is_incognito_; bool is_incognito_;
bool is_valid_ssl_; bool is_valid_ssl_;
bool is_browser_window_active_;
service_manager::BinderRegistryWithArgs<content::RenderFrameHost*> registry_; service_manager::BinderRegistryWithArgs<content::RenderFrameHost*> registry_;
......
...@@ -12,12 +12,14 @@ TestChromePaymentRequestDelegate::TestChromePaymentRequestDelegate( ...@@ -12,12 +12,14 @@ TestChromePaymentRequestDelegate::TestChromePaymentRequestDelegate(
content::WebContents* web_contents, content::WebContents* web_contents,
PaymentRequestDialogView::ObserverForTest* observer, PaymentRequestDialogView::ObserverForTest* observer,
bool is_incognito, bool is_incognito,
bool is_valid_ssl) bool is_valid_ssl,
bool is_browser_window_active)
: ChromePaymentRequestDelegate(web_contents), : ChromePaymentRequestDelegate(web_contents),
region_data_loader_(nullptr), region_data_loader_(nullptr),
observer_(observer), observer_(observer),
is_incognito_(is_incognito), is_incognito_(is_incognito),
is_valid_ssl_(is_valid_ssl) {} is_valid_ssl_(is_valid_ssl),
is_browser_window_active_(is_browser_window_active) {}
void TestChromePaymentRequestDelegate::ShowDialog(PaymentRequest* request) { void TestChromePaymentRequestDelegate::ShowDialog(PaymentRequest* request) {
PaymentRequestDialogView* dialog_view = PaymentRequestDialogView* dialog_view =
...@@ -40,4 +42,9 @@ TestChromePaymentRequestDelegate::GetRegionDataLoader() { ...@@ -40,4 +42,9 @@ TestChromePaymentRequestDelegate::GetRegionDataLoader() {
return region_data_loader_; return region_data_loader_;
return ChromePaymentRequestDelegate::GetRegionDataLoader(); return ChromePaymentRequestDelegate::GetRegionDataLoader();
} }
bool TestChromePaymentRequestDelegate::IsBrowserWindowActive() const {
return is_browser_window_active_;
}
} // namespace payments } // namespace payments
...@@ -26,7 +26,8 @@ class TestChromePaymentRequestDelegate : public ChromePaymentRequestDelegate { ...@@ -26,7 +26,8 @@ class TestChromePaymentRequestDelegate : public ChromePaymentRequestDelegate {
content::WebContents* web_contents, content::WebContents* web_contents,
PaymentRequestDialogView::ObserverForTest* observer, PaymentRequestDialogView::ObserverForTest* observer,
bool is_incognito, bool is_incognito,
bool is_valid_ssl); bool is_valid_ssl,
bool is_browser_window_active);
void SetRegionDataLoader(autofill::RegionDataLoader* region_data_loader) { void SetRegionDataLoader(autofill::RegionDataLoader* region_data_loader) {
region_data_loader_ = region_data_loader; region_data_loader_ = region_data_loader;
...@@ -37,6 +38,7 @@ class TestChromePaymentRequestDelegate : public ChromePaymentRequestDelegate { ...@@ -37,6 +38,7 @@ class TestChromePaymentRequestDelegate : public ChromePaymentRequestDelegate {
bool IsIncognito() const override; bool IsIncognito() const override;
bool IsSslCertificateValid() override; bool IsSslCertificateValid() override;
autofill::RegionDataLoader* GetRegionDataLoader() override; autofill::RegionDataLoader* GetRegionDataLoader() override;
bool IsBrowserWindowActive() const override;
PaymentRequestDialogView* dialog_view() { PaymentRequestDialogView* dialog_view() {
return static_cast<PaymentRequestDialogView*>(dialog_); return static_cast<PaymentRequestDialogView*>(dialog_);
...@@ -47,8 +49,9 @@ class TestChromePaymentRequestDelegate : public ChromePaymentRequestDelegate { ...@@ -47,8 +49,9 @@ class TestChromePaymentRequestDelegate : public ChromePaymentRequestDelegate {
autofill::RegionDataLoader* region_data_loader_; autofill::RegionDataLoader* region_data_loader_;
PaymentRequestDialogView::ObserverForTest* observer_; PaymentRequestDialogView::ObserverForTest* observer_;
bool is_incognito_; const bool is_incognito_;
bool is_valid_ssl_; const bool is_valid_ssl_;
const bool is_browser_window_active_;
DISALLOW_COPY_AND_ASSIGN(TestChromePaymentRequestDelegate); DISALLOW_COPY_AND_ASSIGN(TestChromePaymentRequestDelegate);
}; };
......
...@@ -161,6 +161,14 @@ void PaymentRequest::Show() { ...@@ -161,6 +161,14 @@ void PaymentRequest::Show() {
return; return;
} }
if (!delegate_->IsBrowserWindowActive()) {
LOG(ERROR) << "Cannot show PaymentRequest UI in a background tab";
journey_logger_.SetNotShown(JourneyLogger::NOT_SHOWN_REASON_OTHER);
client_->OnError(mojom::PaymentErrorReason::USER_CANCEL);
OnConnectionTerminated();
return;
}
if (!state_->AreRequestedMethodsSupported()) { if (!state_->AreRequestedMethodsSupported()) {
journey_logger_.SetNotShown( journey_logger_.SetNotShown(
JourneyLogger::NOT_SHOWN_REASON_NO_SUPPORTED_PAYMENT_METHOD); JourneyLogger::NOT_SHOWN_REASON_NO_SUPPORTED_PAYMENT_METHOD);
......
...@@ -13,6 +13,8 @@ class PaymentRequest; ...@@ -13,6 +13,8 @@ class PaymentRequest;
class PaymentRequestDelegate : public PaymentRequestBaseDelegate { class PaymentRequestDelegate : public PaymentRequestBaseDelegate {
public: public:
~PaymentRequestDelegate() override {}
// Shows the Payment Request dialog for the given |request|. // Shows the Payment Request dialog for the given |request|.
virtual void ShowDialog(PaymentRequest* request) = 0; virtual void ShowDialog(PaymentRequest* request) = 0;
...@@ -23,6 +25,9 @@ class PaymentRequestDelegate : public PaymentRequestBaseDelegate { ...@@ -23,6 +25,9 @@ class PaymentRequestDelegate : public PaymentRequestBaseDelegate {
// Disables the dialog and shows an error message that the transaction has // Disables the dialog and shows an error message that the transaction has
// failed. // failed.
virtual void ShowErrorMessage() = 0; virtual void ShowErrorMessage() = 0;
// Returns whether the browser window is active.
virtual bool IsBrowserWindowActive() const = 0;
}; };
} // namespace payments } // namespace payments
......
...@@ -90,6 +90,10 @@ PrefService* TestPaymentRequestDelegate::GetPrefService() { ...@@ -90,6 +90,10 @@ PrefService* TestPaymentRequestDelegate::GetPrefService() {
return nullptr; return nullptr;
} }
bool TestPaymentRequestDelegate::IsBrowserWindowActive() const {
return true;
}
TestPaymentsClientDelegate::TestPaymentsClientDelegate() {} TestPaymentsClientDelegate::TestPaymentsClientDelegate() {}
TestPaymentsClientDelegate::~TestPaymentsClientDelegate() {} TestPaymentsClientDelegate::~TestPaymentsClientDelegate() {}
......
...@@ -73,6 +73,7 @@ class TestPaymentRequestDelegate : public PaymentRequestDelegate { ...@@ -73,6 +73,7 @@ class TestPaymentRequestDelegate : public PaymentRequestDelegate {
ukm::UkmRecorder* GetUkmRecorder() override; ukm::UkmRecorder* GetUkmRecorder() override;
std::string GetAuthenticatedEmail() const override; std::string GetAuthenticatedEmail() const override;
PrefService* GetPrefService() override; PrefService* GetPrefService() override;
bool IsBrowserWindowActive() const override;
TestAddressNormalizer* test_address_normalizer(); TestAddressNormalizer* test_address_normalizer();
void DelayFullCardRequestCompletion(); void DelayFullCardRequestCompletion();
......
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