Commit a05e7112 authored by Rouslan Solomakhin's avatar Rouslan Solomakhin Committed by Commit Bot

[Web Payment] Remove delegate dependency.

Before this patch, C++ service worker payment app depended on
PaymentRequestDelegate class, whose concrete implementation in
ChromePaymentRequestDelegate depended on desktop-specific "Views"
frameework, thus preventing usage of the C++ service worker payment app
from Android.

This patch replaces the PaymentRequestDelegate dependency from the C++
service worker with one boolean and one closure.

After this patch, C++ service worker payment app has fewer dependencies
on desktop specific code.

Design doc: https://bit.ly/cross-platform-pay-app-factory

Bug: 1063118
Change-Id: Ic1629d93ad09a3b1e3c6d54e0d36b5f747bcde13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2196424
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: default avatarDanyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769574}
parent 7526ec49
......@@ -14,7 +14,6 @@
#include "components/payments/content/service_worker_payment_app.h"
#include "components/payments/core/autofill_payment_app.h"
#include "components/payments/core/features.h"
#include "components/payments/core/mock_payment_request_delegate.h"
#include "content/public/browser/stored_payment_app.h"
#include "content/public/browser/supported_delegations.h"
#include "content/public/browser/web_contents.h"
......@@ -88,7 +87,8 @@ class PaymentAppTest : public testing::TestWithParam<RequiredPaymentOptions>,
return std::make_unique<ServiceWorkerPaymentApp>(
&browser_context_, GURL("https://testmerchant.com"),
GURL("https://testmerchant.com/bobpay"), spec_.get(),
std::move(stored_app), &delegate_,
std::move(stored_app), /*is_incognito=*/false,
/*show_processing_spinner=*/base::DoNothing(),
/*identity_callback=*/
base::BindRepeating([](const url::Origin&,
int64_t) { /* Intentionally left blank. */ }));
......@@ -119,7 +119,8 @@ class PaymentAppTest : public testing::TestWithParam<RequiredPaymentOptions>,
return std::make_unique<ServiceWorkerPaymentApp>(
web_contents_, GURL("https://merchant.example"),
GURL("https://merchant.example/iframe"), spec_.get(),
std::move(installable_app), "https://pay.example", &delegate_,
std::move(installable_app), "https://pay.example",
/*is_incognito=*/false, /*show_processing_spinner=*/base::DoNothing(),
/*identity_callback=*/
base::BindRepeating([](const url::Origin&,
int64_t) { /* Intentionally left blank. */ }));
......@@ -178,7 +179,6 @@ class PaymentAppTest : public testing::TestWithParam<RequiredPaymentOptions>,
autofill::AutofillProfile address_;
autofill::CreditCard local_card_;
std::vector<autofill::AutofillProfile*> billing_profiles_;
MockPaymentRequestDelegate delegate_;
RequiredPaymentOptions required_options_;
std::unique_ptr<PaymentRequestSpec> spec_;
......@@ -207,8 +207,8 @@ TEST_P(PaymentAppTest, SortApps) {
// Add an expired card.
autofill::CreditCard expired_card = local_credit_card();
expired_card.SetExpirationYear(2016);
AutofillPaymentApp expired_cc_app("visa", expired_card,
billing_profiles(), "en-US", nullptr);
AutofillPaymentApp expired_cc_app("visa", expired_card, billing_profiles(),
"en-US", nullptr);
apps.push_back(&expired_cc_app);
// Add a non-preselectable sw based payment app.
......
......@@ -17,7 +17,6 @@
#include "components/payments/content/payment_request_converter.h"
#include "components/payments/core/features.h"
#include "components/payments/core/method_strings.h"
#include "components/payments/core/payment_request_delegate.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/payment_app_provider.h"
#include "content/public/browser/web_contents.h"
......@@ -35,7 +34,8 @@ ServiceWorkerPaymentApp::ServiceWorkerPaymentApp(
const GURL& frame_origin,
const PaymentRequestSpec* spec,
std::unique_ptr<content::StoredPaymentApp> stored_payment_app_info,
PaymentRequestDelegate* payment_request_delegate,
bool is_incognito,
const base::RepeatingClosure& show_processing_spinner,
const IdentityCallback& identity_callback)
: PaymentApp(0, PaymentApp::Type::SERVICE_WORKER_APP),
browser_context_(browser_context),
......@@ -44,7 +44,8 @@ ServiceWorkerPaymentApp::ServiceWorkerPaymentApp(
spec_(spec),
stored_payment_app_info_(std::move(stored_payment_app_info)),
delegate_(nullptr),
payment_request_delegate_(payment_request_delegate),
is_incognito_(is_incognito),
show_processing_spinner_(show_processing_spinner),
identity_callback_(identity_callback),
can_make_payment_result_(false),
has_enrolled_instrument_result_(false),
......@@ -76,14 +77,16 @@ ServiceWorkerPaymentApp::ServiceWorkerPaymentApp(
const PaymentRequestSpec* spec,
std::unique_ptr<WebAppInstallationInfo> installable_payment_app_info,
const std::string& enabled_method,
PaymentRequestDelegate* payment_request_delegate,
bool is_incognito,
const base::RepeatingClosure& show_processing_spinner,
const IdentityCallback& identity_callback)
: PaymentApp(0, PaymentApp::Type::SERVICE_WORKER_APP),
top_origin_(top_origin),
frame_origin_(frame_origin),
spec_(spec),
delegate_(nullptr),
payment_request_delegate_(payment_request_delegate),
is_incognito_(is_incognito),
show_processing_spinner_(show_processing_spinner),
identity_callback_(identity_callback),
can_make_payment_result_(false),
has_enrolled_instrument_result_(false),
......@@ -130,7 +133,7 @@ void ServiceWorkerPaymentApp::ValidateCanMakePayment(
// Returns true if we are in incognito (avoiding sending the event to the
// payment handler).
if (payment_request_delegate_->IsIncognito()) {
if (is_incognito_) {
OnCanMakePaymentEventSkipped(std::move(callback));
return;
}
......@@ -254,7 +257,7 @@ void ServiceWorkerPaymentApp::InvokePaymentApp(Delegate* delegate) {
weak_ptr_factory_.GetWeakPtr()));
}
payment_request_delegate_->ShowProcessingSpinner();
show_processing_spinner_.Run();
}
void ServiceWorkerPaymentApp::OnPaymentAppWindowClosed() {
......
......@@ -7,7 +7,7 @@
#include <stdint.h>
#include "base/callback_forward.h"
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "components/payments/content/payment_request_spec.h"
#include "components/payments/content/web_app_manifest.h"
......@@ -29,8 +29,6 @@ class Origin;
namespace payments {
class PaymentRequestDelegate;
// Represents a service worker based payment app.
class ServiceWorkerPaymentApp : public PaymentApp {
public:
......@@ -45,7 +43,8 @@ class ServiceWorkerPaymentApp : public PaymentApp {
const GURL& frame_origin,
const PaymentRequestSpec* spec,
std::unique_ptr<content::StoredPaymentApp> stored_payment_app_info,
PaymentRequestDelegate* payment_request_delegate,
bool is_incognito,
const base::RepeatingClosure& show_processing_spinner,
const IdentityCallback& identity_callback);
// This constructor is used for a payment app that has not been installed in
......@@ -56,8 +55,9 @@ class ServiceWorkerPaymentApp : public PaymentApp {
const GURL& frame_origin,
const PaymentRequestSpec* spec,
std::unique_ptr<WebAppInstallationInfo> installable_payment_app_info,
const std::string& enabled_methods,
PaymentRequestDelegate* payment_request_delegate,
const std::string& enabled_method,
bool is_incognito,
const base::RepeatingClosure& show_processing_spinner,
const IdentityCallback& identity_callback);
~ServiceWorkerPaymentApp() override;
......@@ -134,8 +134,11 @@ class ServiceWorkerPaymentApp : public PaymentApp {
// PaymentRequestState which also owns PaymentResponseHelper.
Delegate* delegate_;
// Weak pointer that must outlive this object.
PaymentRequestDelegate* payment_request_delegate_;
bool is_incognito_;
// Disables user interaction by showing a spinner. Used when the app is
// invoked.
base::RepeatingClosure show_processing_spinner_;
// The callback that is notified of service worker registration identifier
// after the service worker is installed.
......
......@@ -64,12 +64,17 @@ class ServiceWorkerPaymentAppCreator {
return;
}
base::RepeatingClosure show_processing_spinner = base::BindRepeating(
&PaymentRequestDelegate::ShowProcessingSpinner,
delegate_->GetPaymentRequestDelegate()->GetWeakPtr());
for (auto& installed_app : apps) {
auto app = std::make_unique<ServiceWorkerPaymentApp>(
delegate_->GetWebContents()->GetBrowserContext(),
delegate_->GetTopOrigin(), delegate_->GetFrameOrigin(),
delegate_->GetSpec(), std::move(installed_app.second),
delegate_->GetPaymentRequestDelegate(),
delegate_->GetPaymentRequestDelegate()->IsIncognito(),
show_processing_spinner,
base::BindRepeating(
&PaymentAppFactory::Delegate::OnPaymentAppInstalled, delegate_));
app->ValidateCanMakePayment(base::BindOnce(
......@@ -84,7 +89,8 @@ class ServiceWorkerPaymentAppCreator {
delegate_->GetWebContents(), delegate_->GetTopOrigin(),
delegate_->GetFrameOrigin(), delegate_->GetSpec(),
std::move(installable_app.second), installable_app.first.spec(),
delegate_->GetPaymentRequestDelegate(),
delegate_->GetPaymentRequestDelegate()->IsIncognito(),
show_processing_spinner,
base::BindRepeating(
&PaymentAppFactory::Delegate::OnPaymentAppInstalled, delegate_));
app->ValidateCanMakePayment(base::BindOnce(
......
......@@ -8,7 +8,6 @@
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
#include "components/payments/core/mock_payment_request_delegate.h"
#include "content/public/browser/stored_payment_app.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_browser_context.h"
......@@ -115,7 +114,8 @@ class ServiceWorkerPaymentAppTest : public testing::Test,
app_ = std::make_unique<ServiceWorkerPaymentApp>(
&browser_context_, GURL("https://testmerchant.com"),
GURL("https://testmerchant.com/bobpay"), spec_.get(),
std::move(stored_app), &delegate_,
std::move(stored_app), /*is_incognito=*/false,
/*show_processing_spinner=*/base::DoNothing(),
base::BindRepeating(
[](const url::Origin& origin,
int64_t registration_id) { /* Intentionally left blank. */ }));
......@@ -132,7 +132,6 @@ class ServiceWorkerPaymentAppTest : public testing::Test,
}
private:
MockPaymentRequestDelegate delegate_;
content::BrowserTaskEnvironment task_environment_;
content::TestBrowserContext browser_context_;
......
......@@ -46,8 +46,11 @@ jumbo_static_library("core") {
"payment_method_data.h",
"payment_prefs.cc",
"payment_prefs.h",
"payment_request_base_delegate.h",
"payment_request_data_util.cc",
"payment_request_data_util.h",
"payment_request_delegate.cc",
"payment_request_delegate.h",
"payment_shipping_option.cc",
"payment_shipping_option.h",
"payments_experimental_features.cc",
......@@ -63,8 +66,6 @@ jumbo_static_library("core") {
"payment_options.cc",
"payment_options.h",
"payment_options_provider.h",
"payment_request_base_delegate.h",
"payment_request_delegate.h",
"payment_response.cc",
"payment_response.h",
"payments_profile_comparator.cc",
......@@ -121,8 +122,6 @@ jumbo_static_library("method_strings") {
jumbo_static_library("test_support") {
testonly = true
sources = [
"mock_payment_request_delegate.cc",
"mock_payment_request_delegate.h",
"payments_test_util.cc",
"payments_test_util.h",
"test_payment_manifest_downloader.cc",
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/payments/core/mock_payment_request_delegate.h"
namespace payments {
MockPaymentRequestDelegate::MockPaymentRequestDelegate() = default;
MockPaymentRequestDelegate::~MockPaymentRequestDelegate() = default;
} // namespace payments
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_PAYMENTS_CORE_MOCK_PAYMENT_REQUEST_DELEGATE_H_
#define COMPONENTS_PAYMENTS_CORE_MOCK_PAYMENT_REQUEST_DELEGATE_H_
#include "components/payments/core/payment_request_delegate.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace payments {
class MockPaymentRequestDelegate : public PaymentRequestDelegate {
public:
MockPaymentRequestDelegate();
~MockPaymentRequestDelegate() override;
MOCK_METHOD1(ShowDialog, void(PaymentRequest* request));
MOCK_METHOD0(RetryDialog, void());
MOCK_METHOD0(CloseDialog, void());
MOCK_METHOD0(ShowErrorMessage, void());
MOCK_METHOD0(ShowProcessingSpinner, void());
MOCK_CONST_METHOD0(IsBrowserWindowActive, bool());
MOCK_METHOD0(GetPersonalDataManager, autofill::PersonalDataManager*());
MOCK_CONST_METHOD0(GetApplicationLocale, const std::string&());
MOCK_CONST_METHOD0(IsIncognito, bool());
MOCK_CONST_METHOD0(GetLastCommittedURL, const GURL&());
MOCK_METHOD2(
DoFullCardRequest,
void(const autofill::CreditCard& credit_card,
base::WeakPtr<autofill::payments::FullCardRequest::ResultDelegate>
result_delegate));
MOCK_METHOD0(GetAddressNormalizer, autofill::AddressNormalizer*());
MOCK_METHOD0(GetRegionDataLoader, autofill::RegionDataLoader*());
MOCK_METHOD0(GetUkmRecorder, ukm::UkmRecorder*());
MOCK_CONST_METHOD0(GetAuthenticatedEmail, std::string());
MOCK_METHOD0(GetPrefService, PrefService*());
private:
DISALLOW_COPY_AND_ASSIGN(MockPaymentRequestDelegate);
};
} // namespace payments
#endif // COMPONENTS_PAYMENTS_CORE_MOCK_PAYMENT_REQUEST_DELEGATE_H_
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/payments/core/payment_request_delegate.h"
namespace payments {
PaymentRequestDelegate::PaymentRequestDelegate() = default;
PaymentRequestDelegate::~PaymentRequestDelegate() = default;
base::WeakPtr<PaymentRequestDelegate> PaymentRequestDelegate::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}
} // namespace payments
......@@ -5,6 +5,7 @@
#ifndef COMPONENTS_PAYMENTS_CORE_PAYMENT_REQUEST_DELEGATE_H_
#define COMPONENTS_PAYMENTS_CORE_PAYMENT_REQUEST_DELEGATE_H_
#include "base/memory/weak_ptr.h"
#include "components/payments/core/payment_request_base_delegate.h"
namespace payments {
......@@ -13,7 +14,8 @@ class PaymentRequest;
class PaymentRequestDelegate : public PaymentRequestBaseDelegate {
public:
~PaymentRequestDelegate() override {}
PaymentRequestDelegate();
~PaymentRequestDelegate() override;
// Shows the Payment Request dialog for the given |request|.
virtual void ShowDialog(PaymentRequest* request) = 0;
......@@ -33,6 +35,12 @@ class PaymentRequestDelegate : public PaymentRequestBaseDelegate {
// Returns whether the browser window is active.
virtual bool IsBrowserWindowActive() const = 0;
// Returns a weak pointer to this delegate.
base::WeakPtr<PaymentRequestDelegate> GetWeakPtr();
private:
base::WeakPtrFactory<PaymentRequestDelegate> weak_ptr_factory_{this};
};
} // namespace payments
......
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