[Merge M-87][Web Payment] Hold GlobalFrameRoutingId
Before this patch, multiple locations in Web Payment, e.g.,
PaymentAppServiceBridge, contained WebContents and
RenderFrameHost pointers that could become invalid and be used
after free.
This patch makes Web Payment store GlobalFrameRoutingId and
always retrieve and null check the RenderFrameHost from it. If
WebContents is needed and the RenderFrameHost is
current, then WebContents is retrieved from the
RenderFrameHost.
After this patch, Web Payment holds a GlobalFrameRoutingId, so the
RenderFrameHost and WebContents can be checked for validity before use.
(cherry picked from commit 8d385ac2244e1d1e733af5b3e23e2bc95b8c91f0)
Bug: 1125614
Change-Id: I084a062c2a7cad100a6d2d0994c1b0cc9b9efc5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422178
Commit-Queue: Rouslan Solomakhin <[email protected]>
Reviewed-by: Jochen Eisinger <[email protected]>
Reviewed-by: Danyao Wang <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#815307}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476778
Reviewed-by: Rouslan Solomakhin <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#430}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/browser/payments/android/payment_app_service_bridge.cc b/chrome/browser/payments/android/payment_app_service_bridge.cc
index fad98886..e5229f7 100644
--- a/chrome/browser/payments/android/payment_app_service_bridge.cc
+++ b/chrome/browser/payments/android/payment_app_service_bridge.cc
@@ -28,6 +28,7 @@
#include "components/url_formatter/elide_url.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "third_party/blink/public/mojom/payments/payment_app.mojom.h"
#include "third_party/blink/public/mojom/payments/payment_request.mojom.h"
@@ -44,16 +45,6 @@
using ::base::android::ScopedJavaGlobalRef;
using ::payments::mojom::PaymentMethodDataPtr;
-// Helper to get the PaymentAppService associated with |render_frame_host|'s
-// WebContents.
-payments::PaymentAppService* GetPaymentAppService(
- content::RenderFrameHost* render_frame_host) {
- content::WebContents* web_contents =
- content::WebContents::FromRenderFrameHost(render_frame_host);
- return payments::PaymentAppServiceFactory::GetForContext(
- web_contents ? web_contents->GetBrowserContext() : nullptr);
-}
-
void OnCanMakePaymentCalculated(const JavaRef<jobject>& jcallback,
bool can_make_payment) {
Java_PaymentAppServiceCallback_onCanMakePaymentCalculated(
@@ -102,13 +93,12 @@
scoped_refptr<payments::PaymentManifestWebDataService> web_data_service =
WebDataServiceFactory::GetPaymentManifestWebDataForProfile(
- Profile::FromBrowserContext(
- content::WebContents::FromRenderFrameHost(render_frame_host)
- ->GetBrowserContext()),
+ Profile::FromBrowserContext(render_frame_host->GetBrowserContext()),
ServiceAccessType::EXPLICIT_ACCESS);
payments::PaymentAppService* service =
- GetPaymentAppService(render_frame_host);
+ payments::PaymentAppServiceFactory::GetForContext(
+ render_frame_host->GetBrowserContext());
auto* bridge = payments::PaymentAppServiceBridge::Create(
service->GetNumberOfFactories(), render_frame_host, GURL(top_origin),
payments::android::PaymentRequestSpec::FromJavaPaymentRequestSpec(
@@ -174,6 +164,7 @@
PaymentAppCreatedCallback payment_app_created_callback,
PaymentAppCreationErrorCallback payment_app_creation_error_callback,
base::OnceClosure done_creating_payment_apps_callback) {
+ DCHECK(render_frame_host);
// Not using std::make_unique, because that requires a public constructor.
std::unique_ptr<PaymentAppServiceBridge> bridge(new PaymentAppServiceBridge(
number_of_factories, render_frame_host, top_origin, spec,
@@ -199,9 +190,9 @@
PaymentAppCreationErrorCallback payment_app_creation_error_callback,
base::OnceClosure done_creating_payment_apps_callback)
: number_of_pending_factories_(number_of_factories),
- web_contents_(
- content::WebContents::FromRenderFrameHost(render_frame_host)),
- render_frame_host_(render_frame_host),
+ frame_routing_id_(content::GlobalFrameRoutingId(
+ render_frame_host->GetProcess()->GetID(),
+ render_frame_host->GetRoutingID())),
top_origin_(top_origin),
frame_origin_(url_formatter::FormatUrlForSecurityDisplay(
render_frame_host->GetLastCommittedURL())),
@@ -226,7 +217,10 @@
}
content::WebContents* PaymentAppServiceBridge::GetWebContents() {
- return web_contents_;
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh && rfh->IsCurrent()
+ ? content::WebContents::FromRenderFrameHost(rfh)
+ : nullptr;
}
const GURL& PaymentAppServiceBridge::GetTopOrigin() {
return top_origin_;
@@ -242,7 +236,7 @@
content::RenderFrameHost* PaymentAppServiceBridge::GetInitiatorRenderFrameHost()
const {
- return render_frame_host_;
+ return content::RenderFrameHost::FromID(frame_routing_id_);
}
const std::vector<PaymentMethodDataPtr>&
@@ -258,8 +252,11 @@
// iframe attribute. The secure payment confirmation dialog displays the
// top-level origin in its UI before the user can click on the [Verify] button
// to invoke this authenticator.
- return std::make_unique<InternalAuthenticatorAndroid>(
- web_contents_->GetMainFrame());
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh && rfh->IsCurrent()
+ ? std::make_unique<InternalAuthenticatorAndroid>(
+ rfh->GetMainFrame())
+ : nullptr;
}
scoped_refptr<PaymentManifestWebDataService>
@@ -272,8 +269,10 @@
}
bool PaymentAppServiceBridge::IsOffTheRecord() const {
- Profile* profile =
- Profile::FromBrowserContext(web_contents_->GetBrowserContext());
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (!rfh)
+ return false;
+ Profile* profile = Profile::FromBrowserContext(rfh->GetBrowserContext());
return profile && profile->IsOffTheRecord();
}
diff --git a/chrome/browser/payments/android/payment_app_service_bridge.h b/chrome/browser/payments/android/payment_app_service_bridge.h
index 4db8db9..ed5ffad2 100644
--- a/chrome/browser/payments/android/payment_app_service_bridge.h
+++ b/chrome/browser/payments/android/payment_app_service_bridge.h
@@ -12,6 +12,7 @@
#include "base/callback_forward.h"
#include "base/memory/weak_ptr.h"
#include "components/payments/content/payment_app_factory.h"
+#include "content/public/browser/global_routing_id.h"
#include "url/gurl.h"
#include "url/origin.h"
@@ -103,8 +104,7 @@
base::OnceClosure done_creating_payment_apps_callback);
size_t number_of_pending_factories_;
- content::WebContents* web_contents_;
- content::RenderFrameHost* render_frame_host_;
+ content::GlobalFrameRoutingId frame_routing_id_;
const GURL top_origin_;
const GURL frame_origin_;
const url::Origin frame_security_origin_;
diff --git a/chrome/browser/payments/chrome_payment_request_delegate.cc b/chrome/browser/payments/chrome_payment_request_delegate.cc
index cb41179..edc94bd 100644
--- a/chrome/browser/payments/chrome_payment_request_delegate.cc
+++ b/chrome/browser/payments/chrome_payment_request_delegate.cc
@@ -34,6 +34,8 @@
#include "components/payments/content/payment_request_dialog.h"
#include "components/payments/core/payment_prefs.h"
#include "components/signin/public/identity_manager/identity_manager.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
@@ -63,10 +65,13 @@
} // namespace
ChromePaymentRequestDelegate::ChromePaymentRequestDelegate(
- content::WebContents* web_contents)
- : shown_dialog_(nullptr), web_contents_(web_contents) {}
+ content::RenderFrameHost* render_frame_host)
+ : shown_dialog_(nullptr),
+ frame_routing_id_(content::GlobalFrameRoutingId(
+ render_frame_host->GetProcess()->GetID(),
+ render_frame_host->GetRoutingID())) {}
-ChromePaymentRequestDelegate::~ChromePaymentRequestDelegate() {}
+ChromePaymentRequestDelegate::~ChromePaymentRequestDelegate() = default;
void ChromePaymentRequestDelegate::ShowDialog(
base::WeakPtr<PaymentRequest> request) {
@@ -116,7 +121,7 @@
// Autofill uses the original profile's PersonalDataManager to make data
// available in incognito, so PaymentRequest should do the same.
return autofill::PersonalDataManagerFactory::GetForProfile(
- Profile::FromBrowserContext(web_contents_->GetBrowserContext())
+ Profile::FromBrowserContext(GetBrowserContextOrNull())
->GetOriginalProfile());
}
@@ -125,22 +130,31 @@
}
bool ChromePaymentRequestDelegate::IsOffTheRecord() const {
- Profile* profile =
- Profile::FromBrowserContext(web_contents_->GetBrowserContext());
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (!rfh)
+ return false;
+ Profile* profile = Profile::FromBrowserContext(rfh->GetBrowserContext());
return profile && profile->IsOffTheRecord();
}
const GURL& ChromePaymentRequestDelegate::GetLastCommittedURL() const {
- return web_contents_->GetLastCommittedURL();
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh && rfh->IsCurrent()
+ ? content::WebContents::FromRenderFrameHost(rfh)
+ ->GetLastCommittedURL()
+ : GURL::EmptyGURL();
}
void ChromePaymentRequestDelegate::DoFullCardRequest(
const autofill::CreditCard& credit_card,
base::WeakPtr<autofill::payments::FullCardRequest::ResultDelegate>
result_delegate) {
- if (shown_dialog_)
- shown_dialog_->ShowCvcUnmaskPrompt(credit_card, result_delegate,
- web_contents_);
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (rfh && rfh->IsCurrent() && shown_dialog_) {
+ shown_dialog_->ShowCvcUnmaskPrompt(
+ credit_card, result_delegate,
+ content::WebContents::FromRenderFrameHost(rfh));
+ }
}
autofill::RegionDataLoader*
@@ -160,11 +174,14 @@
}
std::string ChromePaymentRequestDelegate::GetAuthenticatedEmail() const {
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (!rfh)
+ return std::string();
+
// Check if the profile is authenticated. Guest profiles or incognito
// windows may not have a sign in manager, and are considered not
// authenticated.
- Profile* profile =
- Profile::FromBrowserContext(web_contents_->GetBrowserContext());
+ Profile* profile = Profile::FromBrowserContext(rfh->GetBrowserContext());
signin::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(profile);
if (identity_manager && identity_manager->HasPrimaryAccount())
@@ -174,12 +191,16 @@
}
PrefService* ChromePaymentRequestDelegate::GetPrefService() {
- return Profile::FromBrowserContext(web_contents_->GetBrowserContext())
- ->GetPrefs();
+ return Profile::FromBrowserContext(GetBrowserContextOrNull())->GetPrefs();
}
bool ChromePaymentRequestDelegate::IsBrowserWindowActive() const {
- Browser* browser = chrome::FindBrowserWithWebContents(web_contents_);
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (!rfh || !rfh->IsCurrent())
+ return false;
+
+ Browser* browser = chrome::FindBrowserWithWebContents(
+ content::WebContents::FromRenderFrameHost(rfh));
return browser && browser->window() && browser->window()->IsActive();
}
@@ -190,21 +211,24 @@
// iframe attribute. The secure payment confirmation dialog displays the
// top-level origin in its UI before the user can click on the [Verify] button
// to invoke this authenticator.
- return std::make_unique<content::InternalAuthenticatorImpl>(
- web_contents_->GetMainFrame());
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh && rfh->IsCurrent()
+ ? std::make_unique<content::InternalAuthenticatorImpl>(
+ rfh->GetMainFrame())
+ : nullptr;
}
scoped_refptr<PaymentManifestWebDataService>
ChromePaymentRequestDelegate::GetPaymentManifestWebDataService() const {
return WebDataServiceFactory::GetPaymentManifestWebDataForProfile(
- Profile::FromBrowserContext(web_contents_->GetBrowserContext()),
+ Profile::FromBrowserContext(GetBrowserContextOrNull()),
ServiceAccessType::EXPLICIT_ACCESS);
}
PaymentRequestDisplayManager*
ChromePaymentRequestDelegate::GetDisplayManager() {
return PaymentRequestDisplayManagerFactory::GetForBrowserContext(
- web_contents_->GetBrowserContext());
+ GetBrowserContextOrNull());
}
void ChromePaymentRequestDelegate::EmbedPaymentHandlerWindow(
@@ -225,8 +249,11 @@
std::string
ChromePaymentRequestDelegate::GetInvalidSslCertificateErrorMessage() {
- return SslValidityChecker::GetInvalidSslCertificateErrorMessage(
- web_contents_);
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh && rfh->IsCurrent()
+ ? SslValidityChecker::GetInvalidSslCertificateErrorMessage(
+ content::WebContents::FromRenderFrameHost(rfh))
+ : "";
}
bool ChromePaymentRequestDelegate::SkipUiForBasicCard() const {
@@ -235,14 +262,19 @@
std::string ChromePaymentRequestDelegate::GetTwaPackageName() const {
#if defined(OS_CHROMEOS)
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (!rfh || !rfh->IsCurrent())
+ return "";
+
auto* apk_web_app_service = chromeos::ApkWebAppService::Get(
- Profile::FromBrowserContext(web_contents_->GetBrowserContext()));
+ Profile::FromBrowserContext(rfh->GetBrowserContext()));
if (!apk_web_app_service)
return "";
base::Optional<std::string> twa_package_name =
apk_web_app_service->GetPackageNameForWebApp(
- web_contents_->GetLastCommittedURL());
+ content::WebContents::FromRenderFrameHost(rfh)
+ ->GetLastCommittedURL());
return twa_package_name.has_value() ? twa_package_name.value() : "";
#else
@@ -254,4 +286,10 @@
return shown_dialog_.get();
}
+content::BrowserContext* ChromePaymentRequestDelegate::GetBrowserContextOrNull()
+ const {
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh ? rfh->GetBrowserContext() : nullptr;
+}
+
} // namespace payments
diff --git a/chrome/browser/payments/chrome_payment_request_delegate.h b/chrome/browser/payments/chrome_payment_request_delegate.h
index 260f7cb..b2007aa 100644
--- a/chrome/browser/payments/chrome_payment_request_delegate.h
+++ b/chrome/browser/payments/chrome_payment_request_delegate.h
@@ -12,10 +12,11 @@
#include "base/memory/weak_ptr.h"
#include "components/payments/content/content_payment_request_delegate.h"
#include "components/payments/content/secure_payment_confirmation_controller.h"
+#include "content/public/browser/global_routing_id.h"
namespace content {
-class WebContents;
-}
+class BrowserContext;
+} // namespace content
namespace payments {
@@ -23,7 +24,8 @@
class ChromePaymentRequestDelegate : public ContentPaymentRequestDelegate {
public:
- explicit ChromePaymentRequestDelegate(content::WebContents* web_contents);
+ explicit ChromePaymentRequestDelegate(
+ content::RenderFrameHost* render_frame_host);
~ChromePaymentRequestDelegate() override;
// PaymentRequestDelegate:
@@ -70,10 +72,13 @@
base::WeakPtr<PaymentRequestDialog> shown_dialog_;
private:
+ // Returns the browser context of the `render_frame_host_` or null if not
+ // available.
+ content::BrowserContext* GetBrowserContextOrNull() const;
+
std::unique_ptr<SecurePaymentConfirmationController> spc_dialog_;
- // Not owned but outlives the PaymentRequest object that owns this.
- content::WebContents* web_contents_;
+ content::GlobalFrameRoutingId frame_routing_id_;
DISALLOW_COPY_AND_ASSIGN(ChromePaymentRequestDelegate);
};
diff --git a/chrome/browser/payments/payment_request_factory.cc b/chrome/browser/payments/payment_request_factory.cc
index 55438fe..5b53d179 100644
--- a/chrome/browser/payments/payment_request_factory.cc
+++ b/chrome/browser/payments/payment_request_factory.cc
@@ -10,6 +10,8 @@
#include "base/no_destructor.h"
#include "chrome/browser/payments/chrome_payment_request_delegate.h"
#include "components/payments/content/payment_request_web_contents_manager.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
#include "mojo/public/cpp/bindings/message.h"
#include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-shared.h"
@@ -51,14 +53,11 @@
render_frame_host);
}
- content::WebContents* web_contents =
- content::WebContents::FromRenderFrameHost(render_frame_host);
- if (!web_contents)
- return;
- PaymentRequestWebContentsManager::GetOrCreateForWebContents(web_contents)
+ PaymentRequestWebContentsManager::GetOrCreateForWebContents(
+ content::WebContents::FromRenderFrameHost(render_frame_host))
->CreatePaymentRequest(
- render_frame_host, web_contents,
- std::make_unique<ChromePaymentRequestDelegate>(web_contents),
+ render_frame_host,
+ std::make_unique<ChromePaymentRequestDelegate>(render_frame_host),
std::move(receiver),
/*observer_for_testing=*/nullptr);
}
diff --git a/chrome/browser/ui/views/payments/payment_request_browsertest_base.cc b/chrome/browser/ui/views/payments/payment_request_browsertest_base.cc
index 07f4f7a5..c67084b 100644
--- a/chrome/browser/ui/views/payments/payment_request_browsertest_base.cc
+++ b/chrome/browser/ui/views/payments/payment_request_browsertest_base.cc
@@ -497,17 +497,17 @@
void PaymentRequestBrowserTestBase::CreatePaymentRequestForTest(
mojo::PendingReceiver<payments::mojom::PaymentRequest> receiver,
content::RenderFrameHost* render_frame_host) {
- content::WebContents* web_contents =
- content::WebContents::FromRenderFrameHost(render_frame_host);
- DCHECK(web_contents);
+ DCHECK(render_frame_host);
+ DCHECK(render_frame_host->IsCurrent());
std::unique_ptr<TestChromePaymentRequestDelegate> delegate =
std::make_unique<TestChromePaymentRequestDelegate>(
- web_contents, this /* observer */, &prefs_, is_incognito_,
+ render_frame_host, /*observer=*/this, &prefs_, is_incognito_,
is_valid_ssl_, is_browser_window_active_, skip_ui_for_basic_card_);
delegate_ = delegate.get();
- PaymentRequestWebContentsManager::GetOrCreateForWebContents(web_contents)
- ->CreatePaymentRequest(web_contents->GetMainFrame(), web_contents,
- std::move(delegate), std::move(receiver), this);
+ PaymentRequestWebContentsManager::GetOrCreateForWebContents(
+ content::WebContents::FromRenderFrameHost(render_frame_host))
+ ->CreatePaymentRequest(render_frame_host, std::move(delegate),
+ std::move(receiver), this);
}
void PaymentRequestBrowserTestBase::ClickOnDialogViewAndWait(
diff --git a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc
index 1f372ba3..1b33edc 100644
--- a/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view_browsertest.cc
@@ -73,7 +73,7 @@
test_delegate_ =
std::make_unique<TestSecurePaymentConfirmationPaymentRequestDelegate>(
- web_contents, model_.GetWeakPtr(), this);
+ web_contents->GetMainFrame(), model_.GetWeakPtr(), this);
ResetEventWaiter(DialogEvent::DIALOG_OPENED);
test_delegate_->ShowDialog(nullptr);
diff --git a/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc b/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc
index 055824a9..25231d7 100644
--- a/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc
+++ b/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc
@@ -9,14 +9,14 @@
namespace payments {
TestChromePaymentRequestDelegate::TestChromePaymentRequestDelegate(
- content::WebContents* web_contents,
+ content::RenderFrameHost* render_frame_host,
PaymentRequestDialogView::ObserverForTest* observer,
PrefService* pref_service,
bool is_off_the_record,
bool is_valid_ssl,
bool is_browser_window_active,
bool skip_ui_for_basic_card)
- : ChromePaymentRequestDelegate(web_contents),
+ : ChromePaymentRequestDelegate(render_frame_host),
region_data_loader_(nullptr),
observer_(observer),
pref_service_(pref_service),
diff --git a/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.h b/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.h
index 97e7ba17..240478d7 100644
--- a/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.h
+++ b/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.h
@@ -13,9 +13,9 @@
class PrefService;
-namespace content {
-class WebContents;
-}
+namespace payments {
+class RenderFrameHost;
+} // namespace payments
namespace payments {
@@ -26,7 +26,7 @@
public:
// This delegate does not own things passed as pointers.
TestChromePaymentRequestDelegate(
- content::WebContents* web_contents,
+ content::RenderFrameHost* render_frame_host,
PaymentRequestDialogView::ObserverForTest* observer,
PrefService* pref_service,
bool is_off_the_record,
diff --git a/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.cc b/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.cc
index 7b50da4..03e7e22 100644
--- a/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.cc
+++ b/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.cc
@@ -4,15 +4,21 @@
#include "chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+
namespace payments {
TestSecurePaymentConfirmationPaymentRequestDelegate::
TestSecurePaymentConfirmationPaymentRequestDelegate(
- content::WebContents* web_contents,
+ content::RenderFrameHost* render_frame_host,
base::WeakPtr<SecurePaymentConfirmationModel> model,
SecurePaymentConfirmationDialogView::ObserverForTest* observer)
- : ChromePaymentRequestDelegate(web_contents),
- web_contents_(web_contents),
+ : ChromePaymentRequestDelegate(render_frame_host),
+ frame_routing_id_(content::GlobalFrameRoutingId(
+ render_frame_host->GetProcess()->GetID(),
+ render_frame_host->GetRoutingID())),
model_(model),
dialog_view_(
(new SecurePaymentConfirmationDialogView(observer))->GetWeakPtr()) {}
@@ -22,8 +28,12 @@
void TestSecurePaymentConfirmationPaymentRequestDelegate::ShowDialog(
base::WeakPtr<PaymentRequest> request) {
- dialog_view_->ShowDialog(web_contents_, model_->GetWeakPtr(),
- base::DoNothing(), base::DoNothing());
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (rfh && rfh->IsCurrent()) {
+ dialog_view_->ShowDialog(content::WebContents::FromRenderFrameHost(rfh),
+ model_->GetWeakPtr(), base::DoNothing(),
+ base::DoNothing());
+ }
}
void TestSecurePaymentConfirmationPaymentRequestDelegate::CloseDialog() {
diff --git a/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.h b/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.h
index afe909d..14d654e0 100644
--- a/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.h
+++ b/chrome/browser/ui/views/payments/test_secure_payment_confirmation_payment_request_delegate.h
@@ -7,10 +7,11 @@
#include "chrome/browser/payments/chrome_payment_request_delegate.h"
#include "chrome/browser/ui/views/payments/secure_payment_confirmation_dialog_view.h"
+#include "content/public/browser/global_routing_id.h"
namespace content {
-class WebContents;
-}
+class RenderFrameHost;
+} // namespace content
namespace payments {
@@ -22,7 +23,7 @@
public:
// This delegate does not own things passed as pointers.
TestSecurePaymentConfirmationPaymentRequestDelegate(
- content::WebContents* web_contents,
+ content::RenderFrameHost* render_frame_host,
base::WeakPtr<SecurePaymentConfirmationModel> model,
SecurePaymentConfirmationDialogView::ObserverForTest* observer);
~TestSecurePaymentConfirmationPaymentRequestDelegate() override;
@@ -36,7 +37,7 @@
}
private:
- content::WebContents* web_contents_;
+ const content::GlobalFrameRoutingId frame_routing_id_;
base::WeakPtr<SecurePaymentConfirmationModel> model_;
base::WeakPtr<SecurePaymentConfirmationDialogView> dialog_view_;
};
diff --git a/chrome/test/payments/payment_request_test_controller_desktop.cc b/chrome/test/payments/payment_request_test_controller_desktop.cc
index adf8b06..6af280a 100644
--- a/chrome/test/payments/payment_request_test_controller_desktop.cc
+++ b/chrome/test/payments/payment_request_test_controller_desktop.cc
@@ -18,6 +18,10 @@
#include "components/payments/core/payment_prefs.h"
#include "components/payments/core/payment_request_delegate.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
+#include "content/public/browser/global_routing_id.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "third_party/blink/public/mojom/webauthn/authenticator.mojom.h"
@@ -47,14 +51,16 @@
class ChromePaymentRequestTestDelegate : public ChromePaymentRequestDelegate {
public:
- ChromePaymentRequestTestDelegate(content::WebContents* web_contents,
+ ChromePaymentRequestTestDelegate(content::RenderFrameHost* render_frame_host,
bool is_off_the_record,
bool valid_ssl,
PrefService* prefs,
const std::string& twa_package_name,
bool has_authenticator)
- : ChromePaymentRequestDelegate(web_contents),
- web_contents_(web_contents),
+ : ChromePaymentRequestDelegate(render_frame_host),
+ frame_routing_id_(content::GlobalFrameRoutingId(
+ render_frame_host->GetProcess()->GetID(),
+ render_frame_host->GetRoutingID())),
is_off_the_record_(is_off_the_record),
valid_ssl_(valid_ssl),
prefs_(prefs),
@@ -70,12 +76,14 @@
std::string GetTwaPackageName() const override { return twa_package_name_; }
std::unique_ptr<autofill::InternalAuthenticator> CreateInternalAuthenticator()
const override {
- return std::make_unique<TestAuthenticator>(web_contents_->GetMainFrame(),
- has_authenticator_);
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh ? std::make_unique<TestAuthenticator>(rfh->GetMainFrame(),
+ has_authenticator_)
+ : nullptr;
}
private:
- content::WebContents* web_contents_;
+ content::GlobalFrameRoutingId frame_routing_id_;
const bool is_off_the_record_;
const bool valid_ssl_;
PrefService* const prefs_;
@@ -240,25 +248,23 @@
base::WeakPtr<ContentPaymentRequestDelegate>* delegate_weakptr,
mojo::PendingReceiver<payments::mojom::PaymentRequest> receiver,
content::RenderFrameHost* render_frame_host) {
- content::WebContents* web_contents =
- content::WebContents::FromRenderFrameHost(render_frame_host);
- DCHECK(web_contents);
+ DCHECK(render_frame_host);
+ DCHECK(render_frame_host->IsCurrent());
auto delegate = std::make_unique<ChromePaymentRequestTestDelegate>(
- web_contents, is_off_the_record, valid_ssl, prefs, twa_package_name,
- has_authenticator);
+ render_frame_host, is_off_the_record, valid_ssl, prefs,
+ twa_package_name, has_authenticator);
*delegate_weakptr = delegate->GetContentWeakPtr();
PaymentRequestWebContentsManager* manager =
PaymentRequestWebContentsManager::GetOrCreateForWebContents(
- web_contents);
+ content::WebContents::FromRenderFrameHost(render_frame_host));
if (!twa_payment_app_method_name.empty()) {
AndroidAppCommunication::GetForBrowserContext(
- web_contents->GetBrowserContext())
+ render_frame_host->GetBrowserContext())
->SetAppForTesting(twa_package_name, twa_payment_app_method_name,
twa_payment_app_response);
}
- manager->CreatePaymentRequest(render_frame_host, web_contents,
- std::move(delegate), std::move(receiver),
- observer_for_test);
+ manager->CreatePaymentRequest(render_frame_host, std::move(delegate),
+ std::move(receiver), observer_for_test);
},
observer_converter_.get(), is_off_the_record_, valid_ssl_, prefs_.get(),
twa_package_name_, has_authenticator_, twa_payment_app_method_name_,
diff --git a/components/payments/content/android_payment_app_factory.cc b/components/payments/content/android_payment_app_factory.cc
index 6b6ef32..c8aeac25 100644
--- a/components/payments/content/android_payment_app_factory.cc
+++ b/components/payments/content/android_payment_app_factory.cc
@@ -225,8 +225,11 @@
AndroidPaymentAppFactory::~AndroidPaymentAppFactory() = default;
void AndroidPaymentAppFactory::Create(base::WeakPtr<Delegate> delegate) {
- auto app_finder = AppFinder::CreateAndSetOwnedBy(delegate->GetWebContents());
- app_finder->FindApps(communication_, delegate);
+ content::WebContents* web_contents = delegate->GetWebContents();
+ if (web_contents) {
+ auto app_finder = AppFinder::CreateAndSetOwnedBy(web_contents);
+ app_finder->FindApps(communication_, delegate);
+ }
}
} // namespace payments
diff --git a/components/payments/content/autofill_payment_app_unittest.cc b/components/payments/content/autofill_payment_app_unittest.cc
index 0949361..610e32e2 100644
--- a/components/payments/content/autofill_payment_app_unittest.cc
+++ b/components/payments/content/autofill_payment_app_unittest.cc
@@ -9,6 +9,7 @@
#include "base/macros.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/task/single_thread_task_executor.h"
#include "components/autofill/core/browser/address_normalizer.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/browser/data_model/autofill_profile.h"
@@ -307,7 +308,9 @@
TEST_F(AutofillPaymentAppTest, InvokePaymentApp_NormalizationBeforeUnmask) {
auto personal_data_manager =
std::make_unique<autofill::TestPersonalDataManager>();
- TestPaymentRequestDelegate delegate(personal_data_manager.get());
+ TestPaymentRequestDelegate delegate(
+ std::make_unique<base::SingleThreadTaskExecutor>(),
+ personal_data_manager.get());
delegate.DelayFullCardRequestCompletion();
delegate.test_address_normalizer()->DelayNormalization();
@@ -335,7 +338,9 @@
TEST_F(AutofillPaymentAppTest, InvokePaymentApp_UnmaskBeforeNormalization) {
auto personal_data_manager =
std::make_unique<autofill::TestPersonalDataManager>();
- TestPaymentRequestDelegate delegate(personal_data_manager.get());
+ TestPaymentRequestDelegate delegate(
+ std::make_unique<base::SingleThreadTaskExecutor>(),
+ personal_data_manager.get());
delegate.DelayFullCardRequestCompletion();
delegate.test_address_normalizer()->DelayNormalization();
diff --git a/components/payments/content/installable_payment_app_crawler.cc b/components/payments/content/installable_payment_app_crawler.cc
index c3fa3a5..e824dd0 100644
--- a/components/payments/content/installable_payment_app_crawler.cc
+++ b/components/payments/content/installable_payment_app_crawler.cc
@@ -40,20 +40,15 @@
InstallablePaymentAppCrawler::InstallablePaymentAppCrawler(
const url::Origin& merchant_origin,
content::RenderFrameHost* initiator_render_frame_host,
- content::WebContents* web_contents,
PaymentManifestDownloader* downloader,
PaymentManifestParser* parser,
PaymentManifestWebDataService* cache)
- : WebContentsObserver(web_contents),
- log_(web_contents),
+ : log_(content::WebContents::FromRenderFrameHost(
+ initiator_render_frame_host)),
merchant_origin_(merchant_origin),
- initiator_frame_routing_id_(
- initiator_render_frame_host &&
- initiator_render_frame_host->GetProcess()
- ? content::GlobalFrameRoutingId(
- initiator_render_frame_host->GetProcess()->GetID(),
- initiator_render_frame_host->GetRoutingID())
- : content::GlobalFrameRoutingId()),
+ initiator_frame_routing_id_(content::GlobalFrameRoutingId(
+ initiator_render_frame_host->GetProcess()->GetID(),
+ initiator_render_frame_host->GetRoutingID())),
downloader_(downloader),
parser_(parser),
number_of_payment_method_manifest_to_download_(0),
@@ -165,11 +160,13 @@
const std::vector<url::Origin>& supported_origins) {
number_of_payment_method_manifest_to_parse_--;
- if (web_contents() == nullptr)
+ auto* rfh = content::RenderFrameHost::FromID(initiator_frame_routing_id_);
+ if (!rfh)
return;
+
content::PermissionController* permission_controller =
content::BrowserContext::GetPermissionController(
- web_contents()->GetBrowserContext());
+ rfh->GetBrowserContext());
DCHECK(permission_controller);
for (const auto& web_app_manifest_url : default_applications) {
@@ -425,17 +422,28 @@
return false;
}
- // Stop if the web_contents is gone.
- if (web_contents() == nullptr) {
+ // If the initiator frame doesn't exists any more, e.g. the frame has
+ // navigated away, don't download the icon.
+ // TODO(crbug.com/1058840): Move this sanity check to ManifestIconDownloader
+ // after DownloadImage refactor is done.
+ auto* rfh = content::RenderFrameHost::FromID(initiator_frame_routing_id_);
+ if (!rfh || !rfh->IsCurrent()) {
log_.Warn(
"Cannot download icons after the webpage has been closed (web app "
"manifest \"" +
web_app_manifest_url.spec() + "\" for payment handler manifest \"" +
method_manifest_url.spec() + "\").");
+ // Post the result back asynchronously.
+ content::GetUIThreadTaskRunner({})->PostTask(
+ FROM_HERE,
+ base::BindOnce(
+ &InstallablePaymentAppCrawler::FinishCrawlingPaymentAppsIfReady,
+ weak_ptr_factory_.GetWeakPtr()));
return false;
}
- gfx::NativeView native_view = web_contents()->GetNativeView();
+ auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
+ gfx::NativeView native_view = web_contents->GetNativeView();
GURL best_icon_url = blink::ManifestIconSelector::FindBestMatchingIcon(
manifest_icons, IconSizeCalculator::IdealIconHeight(native_view),
IconSizeCalculator::MinimumIconHeight(),
@@ -451,26 +459,8 @@
number_of_web_app_icons_to_download_and_decode_++;
- // If the initiator frame doesn't exists any more, e.g. the frame has
- // navigated away, don't download the icon.
- // TODO(crbug.com/1058840): Move this sanity check to ManifestIconDownloader
- // after DownloadImage refactor is done.
- content::RenderFrameHost* render_frame_host =
- content::RenderFrameHost::FromID(initiator_frame_routing_id_);
- if (!render_frame_host || !render_frame_host->IsCurrent() ||
- content::WebContents::FromRenderFrameHost(render_frame_host) !=
- web_contents()) {
- // Post the result back asynchronously.
- content::GetUIThreadTaskRunner({})->PostTask(
- FROM_HERE,
- base::BindOnce(
- &InstallablePaymentAppCrawler::FinishCrawlingPaymentAppsIfReady,
- weak_ptr_factory_.GetWeakPtr()));
- return false;
- }
-
bool can_download_icon = content::ManifestIconDownloader::Download(
- web_contents(), downloader_->FindTestServerURL(best_icon_url),
+ web_contents, downloader_->FindTestServerURL(best_icon_url),
IconSizeCalculator::IdealIconHeight(native_view),
IconSizeCalculator::MinimumIconHeight(),
base::BindOnce(
diff --git a/components/payments/content/installable_payment_app_crawler.h b/components/payments/content/installable_payment_app_crawler.h
index 71861a8e..ed95a20 100644
--- a/components/payments/content/installable_payment_app_crawler.h
+++ b/components/payments/content/installable_payment_app_crawler.h
@@ -20,7 +20,6 @@
#include "components/payments/content/web_app_manifest.h"
#include "components/payments/core/payment_manifest_downloader.h"
#include "content/public/browser/global_routing_id.h"
-#include "content/public/browser/web_contents_observer.h"
#include "third_party/blink/public/mojom/payments/payment_request.mojom.h"
#include "url/origin.h"
@@ -28,7 +27,6 @@
namespace content {
class RenderFrameHost;
-class WebContents;
} // namespace content
namespace payments {
@@ -43,7 +41,7 @@
// Crawls installable web payment apps. First, fetches and parses the payment
// method manifests to get 'default_applications' manifest urls. Then, fetches
// and parses the web app manifests to get the installable payment apps' info.
-class InstallablePaymentAppCrawler : public content::WebContentsObserver {
+class InstallablePaymentAppCrawler {
public:
using FinishedCrawlingCallback = base::OnceCallback<void(
std::map<GURL, std::unique_ptr<WebAppInstallationInfo>>,
@@ -70,11 +68,10 @@
InstallablePaymentAppCrawler(
const url::Origin& merchant_origin,
content::RenderFrameHost* initiator_render_frame_host,
- content::WebContents* web_contents,
PaymentManifestDownloader* downloader,
PaymentManifestParser* parser,
PaymentManifestWebDataService* cache);
- ~InstallablePaymentAppCrawler() override;
+ ~InstallablePaymentAppCrawler();
// Starts the crawling process. All the url based payment methods in
// |request_method_data| will be crawled. A list of installable payment apps'
diff --git a/components/payments/content/payment_app_factory.h b/components/payments/content/payment_app_factory.h
index fc0cd3cc..e615a17 100644
--- a/components/payments/content/payment_app_factory.h
+++ b/components/payments/content/payment_app_factory.h
@@ -45,11 +45,20 @@
public:
virtual ~Delegate() = default;
+ // Returns the WebContents that initiated the PaymentRequest API, or null if
+ // the RenderFrameHost or WebContents has been deleted, which can happen
+ // when the page is being closed, for example.
virtual content::WebContents* GetWebContents() = 0;
+
virtual const GURL& GetTopOrigin() = 0;
virtual const GURL& GetFrameOrigin() = 0;
virtual const url::Origin& GetFrameSecurityOrigin() = 0;
+
+ // Returns the RenderFrameHost that initiated the PaymentRequest API, or
+ // null if the RenderFrameHost has been deleted, which can happen when the
+ // RenderFrameHost is being unloaded, for example.
virtual content::RenderFrameHost* GetInitiatorRenderFrameHost() const = 0;
+
virtual const std::vector<mojom::PaymentMethodDataPtr>& GetMethodData()
const = 0;
virtual std::unique_ptr<autofill::InternalAuthenticator>
diff --git a/components/payments/content/payment_request.cc b/components/payments/content/payment_request.cc
index 5b59b0d..d29822b 100644
--- a/components/payments/content/payment_request.cc
+++ b/components/payments/content/payment_request.cc
@@ -73,31 +73,27 @@
PaymentRequest::PaymentRequest(
content::RenderFrameHost* render_frame_host,
- content::WebContents* web_contents,
std::unique_ptr<ContentPaymentRequestDelegate> delegate,
PaymentRequestWebContentsManager* manager,
PaymentRequestDisplayManager* display_manager,
mojo::PendingReceiver<mojom::PaymentRequest> receiver,
ObserverForTest* observer_for_testing)
- : web_contents_(web_contents),
- // TODO(crbug.com/1058840): change to WeakPtr<RenderFrameHost> once
- // RenderFrameHost provides a WeakPtr API.
- initiator_frame_routing_id_(content::GlobalFrameRoutingId(
+ : initiator_frame_routing_id_(content::GlobalFrameRoutingId(
render_frame_host->GetProcess()->GetID(),
render_frame_host->GetRoutingID())),
- log_(web_contents_),
+ log_(web_contents()),
delegate_(std::move(delegate)),
manager_(manager),
display_manager_(display_manager),
display_handle_(nullptr),
top_level_origin_(url_formatter::FormatUrlForSecurityDisplay(
- web_contents_->GetLastCommittedURL())),
+ web_contents()->GetLastCommittedURL())),
frame_origin_(url_formatter::FormatUrlForSecurityDisplay(
render_frame_host->GetLastCommittedURL())),
frame_security_origin_(render_frame_host->GetLastCommittedOrigin()),
observer_for_testing_(observer_for_testing),
journey_logger_(delegate_->IsOffTheRecord(),
- ukm::GetSourceIdForWebContentsDocument(web_contents)) {
+ ukm::GetSourceIdForWebContentsDocument(web_contents())) {
receiver_.Bind(std::move(receiver));
// OnConnectionTerminated will be called when the Mojo pipe is closed. This
// will happen as a result of many renderer-side events (both successful and
@@ -108,7 +104,7 @@
&PaymentRequest::OnConnectionTerminated, weak_ptr_factory_.GetWeakPtr()));
payment_handler_host_ = std::make_unique<PaymentHandlerHost>(
- web_contents_, weak_ptr_factory_.GetWeakPtr());
+ web_contents(), weak_ptr_factory_.GetWeakPtr());
}
PaymentRequest::~PaymentRequest() = default;
@@ -176,9 +172,7 @@
return;
}
- // TODO(crbug.com/1058840): change to WeakPtr<RenderFrameHost> once
- // RenderFrameHost provides a WeakPtr API.
- content::RenderFrameHost* initiator_frame =
+ auto* initiator_frame =
content::RenderFrameHost::FromID(initiator_frame_routing_id_);
if (!initiator_frame) {
log_.Error(errors::kInvalidInitiatorFrame);
@@ -191,9 +185,8 @@
/*observer=*/weak_ptr_factory_.GetWeakPtr(),
delegate_->GetApplicationLocale());
state_ = std::make_unique<PaymentRequestState>(
- web_contents_, initiator_frame, top_level_origin_, frame_origin_,
- frame_security_origin_, spec(),
- /*delegate=*/weak_ptr_factory_.GetWeakPtr(),
+ initiator_frame, top_level_origin_, frame_origin_, frame_security_origin_,
+ spec(), /*delegate=*/weak_ptr_factory_.GetWeakPtr(),
delegate_->GetApplicationLocale(), delegate_->GetPersonalDataManager(),
delegate_.get(), &journey_logger_);
@@ -771,8 +764,8 @@
void PaymentRequest::RenderFrameDeleted(
content::RenderFrameHost* render_frame_host) {
- DCHECK(render_frame_host ==
- content::RenderFrameHost::FromID(initiator_frame_routing_id_));
+ DCHECK_EQ(render_frame_host,
+ content::RenderFrameHost::FromID(initiator_frame_routing_id_));
// RenderFrameHost is usually deleted explicitly before PaymentRequest
// destruction if the user closes the tab or browser window without closing
// the payment request dialog.
@@ -827,6 +820,13 @@
state_->selected_app()->UkmSourceId());
}
+content::WebContents* PaymentRequest::web_contents() {
+ auto* rfh = content::RenderFrameHost::FromID(initiator_frame_routing_id_);
+ return rfh && rfh->IsCurrent()
+ ? content::WebContents::FromRenderFrameHost(rfh)
+ : nullptr;
+}
+
void PaymentRequest::RecordFirstAbortReason(
JourneyLogger::AbortReason abort_reason) {
if (!has_recorded_completion_) {
@@ -848,8 +848,12 @@
void PaymentRequest::HasEnrolledInstrumentCallback(
bool has_enrolled_instrument) {
+ auto* rfh = content::RenderFrameHost::FromID(initiator_frame_routing_id_);
+ if (!rfh)
+ return;
+
if (!spec_ || CanMakePaymentQueryFactory::GetInstance()
- ->GetForContext(web_contents_->GetBrowserContext())
+ ->GetForContext(rfh->GetBrowserContext())
->CanQuery(top_level_origin_, frame_origin_,
spec_->query_for_quota())) {
RespondToHasEnrolledInstrumentQuery(has_enrolled_instrument,
diff --git a/components/payments/content/payment_request.h b/components/payments/content/payment_request.h
index ee8443a4..e394c3c 100644
--- a/components/payments/content/payment_request.h
+++ b/components/payments/content/payment_request.h
@@ -65,7 +65,6 @@
};
PaymentRequest(content::RenderFrameHost* render_frame_host,
- content::WebContents* web_contents,
std::unique_ptr<ContentPaymentRequestDelegate> delegate,
PaymentRequestWebContentsManager* manager,
PaymentRequestDisplayManager* display_manager,
@@ -136,9 +135,9 @@
// Called when the payment handler requests to open a payment handler window.
void OnPaymentHandlerOpenWindowCalled();
- content::WebContents* web_contents() { return web_contents_; }
+ content::WebContents* web_contents();
- const content::GlobalFrameRoutingId& initiator_frame_routing_id() {
+ const content::GlobalFrameRoutingId& initiator_frame_routing_id() const {
return initiator_frame_routing_id_;
}
@@ -205,7 +204,6 @@
void OnAbortResult(bool aborted);
- content::WebContents* web_contents_;
const content::GlobalFrameRoutingId initiator_frame_routing_id_;
DeveloperConsoleLogger log_;
std::unique_ptr<ContentPaymentRequestDelegate> delegate_;
diff --git a/components/payments/content/payment_request_state.cc b/components/payments/content/payment_request_state.cc
index 413e180..6c550ab 100644
--- a/components/payments/content/payment_request_state.cc
+++ b/components/payments/content/payment_request_state.cc
@@ -34,6 +34,7 @@
#include "components/payments/core/payment_request_data_util.h"
#include "components/payments/core/payments_experimental_features.h"
#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
#include "content/public/common/content_features.h"
namespace payments {
@@ -56,7 +57,6 @@
} // namespace
PaymentRequestState::PaymentRequestState(
- content::WebContents* web_contents,
content::RenderFrameHost* initiator_render_frame_host,
const GURL& top_level_origin,
const GURL& frame_origin,
@@ -67,8 +67,9 @@
autofill::PersonalDataManager* personal_data_manager,
ContentPaymentRequestDelegate* payment_request_delegate,
JourneyLogger* journey_logger)
- : web_contents_(web_contents),
- initiator_render_frame_host_(initiator_render_frame_host),
+ : frame_routing_id_(content::GlobalFrameRoutingId(
+ initiator_render_frame_host->GetProcess()->GetID(),
+ initiator_render_frame_host->GetRoutingID())),
top_origin_(top_level_origin),
frame_origin_(frame_origin),
frame_security_origin_(frame_security_origin),
@@ -83,9 +84,8 @@
profile_comparator_(app_locale, *spec) {
PopulateProfileCache();
- // |web_contents_| is null in unit tests.
PaymentAppService* service = PaymentAppServiceFactory::GetForContext(
- web_contents_ ? web_contents_->GetBrowserContext() : nullptr);
+ initiator_render_frame_host->GetBrowserContext());
number_of_payment_app_factories_ = service->GetNumberOfFactories();
service->Create(weak_ptr_factory_.GetWeakPtr());
@@ -95,7 +95,10 @@
PaymentRequestState::~PaymentRequestState() {}
content::WebContents* PaymentRequestState::GetWebContents() {
- return web_contents_;
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ return rfh && rfh->IsCurrent()
+ ? content::WebContents::FromRenderFrameHost(rfh)
+ : nullptr;
}
ContentPaymentRequestDelegate* PaymentRequestState::GetPaymentRequestDelegate()
@@ -129,7 +132,7 @@
content::RenderFrameHost* PaymentRequestState::GetInitiatorRenderFrameHost()
const {
- return initiator_render_frame_host_;
+ return content::RenderFrameHost::FromID(frame_routing_id_);
}
const std::vector<mojom::PaymentMethodDataPtr>&
diff --git a/components/payments/content/payment_request_state.h b/components/payments/content/payment_request_state.h
index 48f158c..f4614df6 100644
--- a/components/payments/content/payment_request_state.h
+++ b/components/payments/content/payment_request_state.h
@@ -21,6 +21,7 @@
#include "components/payments/content/service_worker_payment_app_factory.h"
#include "components/payments/core/journey_logger.h"
#include "components/payments/core/payments_profile_comparator.h"
+#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/payment_app_provider.h"
#include "content/public/browser/web_contents.h"
#include "third_party/blink/public/mojom/payments/payment_request.mojom.h"
@@ -97,8 +98,7 @@
const std::string& error_message)>;
// The `spec` parameter should not be null.
- PaymentRequestState(content::WebContents* web_contents,
- content::RenderFrameHost* initiator_render_frame_host,
+ PaymentRequestState(content::RenderFrameHost* initiator_render_frame_host,
const GURL& top_level_origin,
const GURL& frame_origin,
const url::Origin& frame_security_origin,
@@ -332,8 +332,7 @@
bool GetCanMakePaymentValue() const;
bool GetHasEnrolledInstrumentValue() const;
- content::WebContents* web_contents_;
- content::RenderFrameHost* initiator_render_frame_host_;
+ content::GlobalFrameRoutingId frame_routing_id_;
const GURL top_origin_;
const GURL frame_origin_;
const url::Origin frame_security_origin_;
diff --git a/components/payments/content/payment_request_state_unittest.cc b/components/payments/content/payment_request_state_unittest.cc
index bdf4385a..f417a35 100644
--- a/components/payments/content/payment_request_state_unittest.cc
+++ b/components/payments/content/payment_request_state_unittest.cc
@@ -20,7 +20,11 @@
#include "components/payments/content/payment_request_spec.h"
#include "components/payments/content/test_content_payment_request_delegate.h"
#include "components/payments/core/journey_logger.h"
+#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_web_contents_factory.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/mojom/payments/payment_request.mojom.h"
@@ -32,8 +36,10 @@
public PaymentRequestState::Delegate {
protected:
PaymentRequestStateTest()
- : num_on_selected_information_changed_called_(0),
- test_payment_request_delegate_(&test_personal_data_manager_),
+ : web_contents_(web_contents_factory_.CreateWebContents(&context_)),
+ num_on_selected_information_changed_called_(0),
+ test_payment_request_delegate_(/*task_executor=*/nullptr,
+ &test_personal_data_manager_),
journey_logger_(test_payment_request_delegate_.IsOffTheRecord(),
ukm::UkmRecorder::GetNewSourceID()),
address_(autofill::test::GetFullProfile()),
@@ -78,10 +84,9 @@
std::move(options), std::move(details), std::move(method_data),
/*observer=*/nullptr, "en-US");
PaymentAppServiceFactory::SetForTesting(
- std::make_unique<PaymentAppService>(/*context=*/nullptr));
+ std::make_unique<PaymentAppService>(&context_));
state_ = std::make_unique<PaymentRequestState>(
- /*web_contents=*/nullptr,
- /*render_frame_host=*/nullptr, GURL("https://example.com"),
+ web_contents_->GetMainFrame(), GURL("https://example.com"),
GURL("https://example.com/pay"),
url::Origin::Create(GURL("https://example.com")), spec_->AsWeakPtr(),
weak_ptr_factory_.GetWeakPtr(), "en-US", &test_personal_data_manager_,
@@ -136,6 +141,10 @@
private:
base::test::ScopedFeatureList scoped_feature_list_;
+ content::BrowserTaskEnvironment task_environment_;
+ content::TestBrowserContext context_;
+ content::TestWebContentsFactory web_contents_factory_;
+ content::WebContents* web_contents_; // Owned by `web_contents_factory_`.
std::unique_ptr<PaymentRequestState> state_;
std::unique_ptr<PaymentRequestSpec> spec_;
int num_on_selected_information_changed_called_;
diff --git a/components/payments/content/payment_request_web_contents_manager.cc b/components/payments/content/payment_request_web_contents_manager.cc
index 496711f..e87a043 100644
--- a/components/payments/content/payment_request_web_contents_manager.cc
+++ b/components/payments/content/payment_request_web_contents_manager.cc
@@ -30,12 +30,11 @@
void PaymentRequestWebContentsManager::CreatePaymentRequest(
content::RenderFrameHost* render_frame_host,
- content::WebContents* web_contents,
std::unique_ptr<ContentPaymentRequestDelegate> delegate,
mojo::PendingReceiver<payments::mojom::PaymentRequest> receiver,
PaymentRequest::ObserverForTest* observer_for_testing) {
auto new_request = std::make_unique<PaymentRequest>(
- render_frame_host, web_contents, std::move(delegate), /*manager=*/this,
+ render_frame_host, std::move(delegate), /*manager=*/this,
delegate->GetDisplayManager(), std::move(receiver), observer_for_testing);
PaymentRequest* request_ptr = new_request.get();
payment_requests_.insert(std::make_pair(request_ptr, std::move(new_request)));
diff --git a/components/payments/content/payment_request_web_contents_manager.h b/components/payments/content/payment_request_web_contents_manager.h
index ed41887..cb9dad7 100644
--- a/components/payments/content/payment_request_web_contents_manager.h
+++ b/components/payments/content/payment_request_web_contents_manager.h
@@ -53,7 +53,6 @@
// and the associated `web_contents`.
void CreatePaymentRequest(
content::RenderFrameHost* render_frame_host,
- content::WebContents* web_contents,
std::unique_ptr<ContentPaymentRequestDelegate> delegate,
mojo::PendingReceiver<payments::mojom::PaymentRequest> receiver,
PaymentRequest::ObserverForTest* observer_for_testing);
diff --git a/components/payments/content/payment_response_helper_unittest.cc b/components/payments/content/payment_response_helper_unittest.cc
index eb4a1e1..c0e3b80 100644
--- a/components/payments/content/payment_response_helper_unittest.cc
+++ b/components/payments/content/payment_response_helper_unittest.cc
@@ -11,6 +11,7 @@
#include "base/memory/weak_ptr.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/task/single_thread_task_executor.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/browser/data_model/autofill_profile.h"
#include "components/autofill/core/browser/data_model/credit_card.h"
@@ -27,7 +28,9 @@
public PaymentResponseHelper::Delegate {
protected:
PaymentResponseHelperTest()
- : test_payment_request_delegate_(&test_personal_data_manager_),
+ : test_payment_request_delegate_(
+ std::make_unique<base::SingleThreadTaskExecutor>(),
+ &test_personal_data_manager_),
address_(autofill::test::GetFullProfile()),
billing_addresses_({&address_}) {
test_personal_data_manager_.AddProfile(address_);
diff --git a/components/payments/content/secure_payment_confirmation_app_factory.cc b/components/payments/content/secure_payment_confirmation_app_factory.cc
index 04938a3d..f7a4345 100644
--- a/components/payments/content/secure_payment_confirmation_app_factory.cc
+++ b/components/payments/content/secure_payment_confirmation_app_factory.cc
@@ -73,7 +73,7 @@
mojom::SecurePaymentConfirmationRequestPtr request,
std::unique_ptr<autofill::InternalAuthenticator> authenticator,
bool is_available) {
- if (!delegate)
+ if (!delegate || !delegate->GetWebContents())
return;
if (!is_available && !base::FeatureList::IsEnabled(
diff --git a/components/payments/content/service_worker_payment_app_factory.cc b/components/payments/content/service_worker_payment_app_factory.cc
index 963a064..54da818 100644
--- a/components/payments/content/service_worker_payment_app_factory.cc
+++ b/components/payments/content/service_worker_payment_app_factory.cc
@@ -48,7 +48,8 @@
content::InstalledPaymentAppsFinder::PaymentApps apps,
ServiceWorkerPaymentAppFinder::InstallablePaymentApps installable_apps,
const std::string& error_message) {
- if (!delegate_ || !delegate_->GetSpec()) {
+ if (!delegate_ || !delegate_->GetSpec() || !delegate_->GetWebContents() ||
+ !delegate_->GetInitiatorRenderFrameHost()) {
FinishAndCleanup();
return;
}
@@ -180,14 +181,16 @@
ServiceWorkerPaymentAppFactory::~ServiceWorkerPaymentAppFactory() {}
void ServiceWorkerPaymentAppFactory::Create(base::WeakPtr<Delegate> delegate) {
- DCHECK(delegate->GetWebContents());
+ auto* rfh = delegate->GetInitiatorRenderFrameHost();
+ if (!rfh || !rfh->IsCurrent() || !delegate->GetWebContents())
+ return; // The frame or page is being unloaded.
+
auto creator = std::make_unique<ServiceWorkerPaymentAppCreator>(
/*owner=*/this, delegate);
ServiceWorkerPaymentAppCreator* creator_raw_pointer = creator.get();
creators_[creator_raw_pointer] = std::move(creator);
- ServiceWorkerPaymentAppFinder::GetOrCreateForCurrentDocument(
- delegate->GetInitiatorRenderFrameHost())
+ ServiceWorkerPaymentAppFinder::GetOrCreateForCurrentDocument(rfh)
->GetAllPaymentApps(
delegate->GetFrameSecurityOrigin(),
delegate->GetPaymentManifestWebDataService(),
diff --git a/components/payments/content/service_worker_payment_app_finder.cc b/components/payments/content/service_worker_payment_app_finder.cc
index 2050275..8fbd8c6 100644
--- a/components/payments/content/service_worker_payment_app_finder.cc
+++ b/components/payments/content/service_worker_payment_app_finder.cc
@@ -28,6 +28,7 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/payment_app_provider.h"
#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/stored_payment_app.h"
#include "content/public/browser/web_contents.h"
@@ -129,8 +130,11 @@
ServiceWorkerPaymentAppFinder::GetAllPaymentAppsCallback callback,
base::OnceClosure finished_using_resources_callback) {
DCHECK(!verifier_);
+ DCHECK(initiator_render_frame_host);
+ DCHECK(initiator_render_frame_host->IsCurrent());
downloader_ = std::move(downloader);
+
content::WebContents* web_contents =
content::WebContents::FromRenderFrameHost(initiator_render_frame_host);
parser_ = std::make_unique<PaymentManifestParser>(
@@ -142,10 +146,9 @@
if (may_crawl_for_installable_payment_apps &&
base::FeatureList::IsEnabled(
features::kWebPaymentsJustInTimePaymentApp)) {
- // Construct crawler in constructor to allow it observe the web_contents.
crawler_ = std::make_unique<InstallablePaymentAppCrawler>(
- merchant_origin, initiator_render_frame_host, web_contents,
- downloader_.get(), parser_.get(), cache_.get());
+ merchant_origin, initiator_render_frame_host, downloader_.get(),
+ parser_.get(), cache_.get());
if (ignore_port_in_origin_comparison_for_testing_)
crawler_->IgnorePortInOriginComparisonForTesting();
}
@@ -160,7 +163,7 @@
std::move(finished_using_resources_callback);
content::InstalledPaymentAppsFinder::GetInstance(
- web_contents->GetBrowserContext())
+ initiator_render_frame_host->GetBrowserContext())
->GetAllPaymentApps(base::BindOnce(
&SelfDeletingServiceWorkerPaymentAppFinder::OnGotAllPaymentApps,
weak_ptr_factory_.GetWeakPtr()));
@@ -402,6 +405,11 @@
GetAllPaymentAppsCallback callback,
base::OnceClosure finished_writing_cache_callback_for_testing) {
DCHECK(!requested_method_data.empty());
+
+ auto* rfh = content::RenderFrameHost::FromID(frame_routing_id_);
+ if (!rfh || !rfh->IsCurrent())
+ return;
+
// Do not look up payment handlers for ignored payment methods.
base::EraseIf(requested_method_data,
[&](const mojom::PaymentMethodDataPtr& method_data) {
@@ -416,8 +424,7 @@
return;
}
- content::WebContents* web_contents =
- content::WebContents::FromRenderFrameHost(rfh_);
+ auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
auto self_delete_factory =
SelfDeletingServiceWorkerPaymentAppFinder::CreateAndSetOwnedBy(
web_contents);
@@ -430,14 +437,13 @@
downloader = std::make_unique<payments::PaymentManifestDownloader>(
std::make_unique<DeveloperConsoleLogger>(web_contents),
content::BrowserContext::GetDefaultStoragePartition(
- web_contents->GetBrowserContext())
+ rfh->GetBrowserContext())
->GetURLLoaderFactoryForBrowserProcess());
}
self_delete_factory->GetAllPaymentApps(
- merchant_origin, rfh_, std::move(downloader), cache,
- requested_method_data, may_crawl_for_installable_payment_apps,
- std::move(callback),
+ merchant_origin, rfh, std::move(downloader), cache, requested_method_data,
+ may_crawl_for_installable_payment_apps, std::move(callback),
std::move(finished_writing_cache_callback_for_testing));
}
@@ -462,7 +468,9 @@
ServiceWorkerPaymentAppFinder::ServiceWorkerPaymentAppFinder(
content::RenderFrameHost* rfh)
- : rfh_(rfh),
+ : frame_routing_id_(
+ content::GlobalFrameRoutingId(rfh->GetProcess()->GetID(),
+ rfh->GetRoutingID())),
ignored_methods_({methods::kGooglePlayBilling}),
test_downloader_(nullptr) {}
diff --git a/components/payments/content/service_worker_payment_app_finder.h b/components/payments/content/service_worker_payment_app_finder.h
index 0f8b632f8..ea4d0c8 100644
--- a/components/payments/content/service_worker_payment_app_finder.h
+++ b/components/payments/content/service_worker_payment_app_finder.h
@@ -13,6 +13,7 @@
#include "base/callback.h"
#include "base/macros.h"
#include "components/payments/content/web_app_manifest.h"
+#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/installed_payment_apps_finder.h"
#include "content/public/browser/payment_app_provider.h"
#include "content/public/browser/render_document_host_user_data.h"
@@ -106,8 +107,8 @@
RENDER_DOCUMENT_HOST_USER_DATA_KEY_DECL();
- // |rfh_| owns this ServiceWorkerPaymentAppFinder, so it is always valid.
- content::RenderFrameHost* rfh_;
+ // The identifier of the frame that owns this ServiceWorkerPaymentAppFinder.
+ content::GlobalFrameRoutingId frame_routing_id_;
std::set<std::string> ignored_methods_;
std::unique_ptr<PaymentManifestDownloader> test_downloader_;
diff --git a/components/payments/content/test_content_payment_request_delegate.cc b/components/payments/content/test_content_payment_request_delegate.cc
index 7612f69..0a26df9 100644
--- a/components/payments/content/test_content_payment_request_delegate.cc
+++ b/components/payments/content/test_content_payment_request_delegate.cc
@@ -4,14 +4,17 @@
#include "components/payments/content/test_content_payment_request_delegate.h"
+#include <utility>
+
#include "components/payments/content/payment_manifest_web_data_service.h"
#include "components/payments/core/error_strings.h"
namespace payments {
TestContentPaymentRequestDelegate::TestContentPaymentRequestDelegate(
+ std::unique_ptr<base::SingleThreadTaskExecutor> task_executor,
autofill::PersonalDataManager* pdm)
- : core_delegate_(pdm) {}
+ : core_delegate_(std::move(task_executor), pdm) {}
TestContentPaymentRequestDelegate::~TestContentPaymentRequestDelegate() {}
diff --git a/components/payments/content/test_content_payment_request_delegate.h b/components/payments/content/test_content_payment_request_delegate.h
index 9c8d87c4..6f63a00 100644
--- a/components/payments/content/test_content_payment_request_delegate.h
+++ b/components/payments/content/test_content_payment_request_delegate.h
@@ -5,6 +5,8 @@
#ifndef COMPONENTS_PAYMENTS_CONTENT_TEST_CONTENT_PAYMENT_REQUEST_DELEGATE_H_
#define COMPONENTS_PAYMENTS_CONTENT_TEST_CONTENT_PAYMENT_REQUEST_DELEGATE_H_
+#include <memory>
+
#include "components/payments/content/content_payment_request_delegate.h"
#include "components/payments/content/payment_request_display_manager.h"
#include "components/payments/core/test_payment_request_delegate.h"
@@ -13,11 +15,16 @@
class PersonalDataManager;
} // namespace autofill
+namespace base {
+class SingleThreadTaskExecutor;
+} // namespace base
+
namespace payments {
class TestContentPaymentRequestDelegate : public ContentPaymentRequestDelegate {
public:
- explicit TestContentPaymentRequestDelegate(
+ TestContentPaymentRequestDelegate(
+ std::unique_ptr<base::SingleThreadTaskExecutor> task_executor,
autofill::PersonalDataManager* pdm);
~TestContentPaymentRequestDelegate() override;
diff --git a/components/payments/core/test_payment_request_delegate.cc b/components/payments/core/test_payment_request_delegate.cc
index 1598849..4eb3596c 100644
--- a/components/payments/core/test_payment_request_delegate.cc
+++ b/components/payments/core/test_payment_request_delegate.cc
@@ -4,14 +4,18 @@
#include "components/payments/core/test_payment_request_delegate.h"
+#include <utility>
+
#include "base/strings/utf_string_conversions.h"
#include "components/autofill/core/browser/personal_data_manager.h"
namespace payments {
TestPaymentRequestDelegate::TestPaymentRequestDelegate(
+ std::unique_ptr<base::SingleThreadTaskExecutor> task_executor,
autofill::PersonalDataManager* personal_data_manager)
- : personal_data_manager_(personal_data_manager),
+ : main_task_executor_(std::move(task_executor)),
+ personal_data_manager_(personal_data_manager),
locale_("en-US"),
last_committed_url_("https://shop.com"),
test_shared_loader_factory_(
diff --git a/components/payments/core/test_payment_request_delegate.h b/components/payments/core/test_payment_request_delegate.h
index 8d7dccb..a328a9f8 100644
--- a/components/payments/core/test_payment_request_delegate.h
+++ b/components/payments/core/test_payment_request_delegate.h
@@ -5,6 +5,7 @@
#ifndef COMPONENTS_PAYMENTS_CORE_TEST_PAYMENT_REQUEST_DELEGATE_H_
#define COMPONENTS_PAYMENTS_CORE_TEST_PAYMENT_REQUEST_DELEGATE_H_
+#include <memory>
#include <string>
#include "base/single_thread_task_runner.h"
@@ -22,7 +23,8 @@
class TestPaymentRequestDelegate : public PaymentRequestDelegate {
public:
- explicit TestPaymentRequestDelegate(
+ TestPaymentRequestDelegate(
+ std::unique_ptr<base::SingleThreadTaskExecutor> task_executor,
autofill::PersonalDataManager* personal_data_manager);
~TestPaymentRequestDelegate() override;
@@ -52,7 +54,7 @@
void CompleteFullCardRequest();
private:
- base::SingleThreadTaskExecutor main_task_executor_;
+ std::unique_ptr<base::SingleThreadTaskExecutor> main_task_executor_;
autofill::PersonalDataManager* personal_data_manager_;
std::string locale_;
const GURL last_committed_url_;
