Commit 4f2ba63c authored by Danyao Wang's avatar Danyao Wang Committed by Commit Bot

[Payment Request][Desktop] Download manifest icon in the correct frame.

This CL plumbs RenderFrameHost into InstallablePaymentAppCrawler so it
can be provided to ManifestIconDownloader to download the icon in the
correct frame that initiated the PaymentRequest.

The ManifestIconDownloader was previously changed to expect the global
frame routing ID instead of RenderFrameHost* (crrev.com/c/2085752).
This will be changed to use RenderFrameHost* directly in a subsequent
CL per a post-commit comment.

After this change, all payments code can also be cleaned up to look
up WebContents* from the RenderFrameHost* instead of retaining both.
This will also be cleaned up in a subsequent CL.

Bug: 1055360
Change-Id: Ic431b04ff88d1ac0235172391c9697bbd0ec8fa9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097242Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749149}
parent 4d5b4529
......@@ -423,7 +423,10 @@ static void JNI_ServiceWorkerPaymentAppBridge_GetAllPaymentApps(
content::WebContents::FromJavaWebContents(jweb_contents);
payments::ServiceWorkerPaymentAppFinder::GetInstance()->GetAllPaymentApps(
url::Origin::FromJavaObject(jorigin), web_contents,
url::Origin::FromJavaObject(jorigin),
// TODO(crbug.com/1055360): plumb the RenderFrameHost from Java side.
nullptr, /* initiator_render_frame_host */
web_contents,
WebDataServiceFactory::GetPaymentManifestWebDataForProfile(
Profile::FromBrowserContext(web_contents->GetBrowserContext()),
ServiceAccessType::EXPLICIT_ACCESS),
......
......@@ -185,7 +185,8 @@ class ServiceWorkerPaymentAppFinderBrowserTest : public InProcessBrowserTest {
base::RunLoop run_loop;
ServiceWorkerPaymentAppFinder::GetInstance()->GetAllPaymentApps(
url::Origin::Create(GURL("https://chromium.org")), web_contents,
url::Origin::Create(GURL("https://chromium.org")),
web_contents->GetMainFrame(), web_contents,
WebDataServiceFactory::GetPaymentManifestWebDataForProfile(
Profile::FromBrowserContext(context),
ServiceAccessType::EXPLICIT_ACCESS),
......
......@@ -15,10 +15,13 @@
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/manifest_icon_downloader.h"
#include "content/public/browser/payment_app_provider.h"
#include "content/public/browser/permission_controller.h"
#include "content/public/browser/permission_type.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 "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "third_party/blink/public/common/manifest/manifest.h"
......@@ -33,6 +36,7 @@ namespace payments {
// TODO(crbug.com/782270): Add integration tests for this class.
InstallablePaymentAppCrawler::InstallablePaymentAppCrawler(
const url::Origin& merchant_origin,
content::RenderFrameHost* initiator_render_frame_host,
content::WebContents* web_contents,
PaymentManifestDownloader* downloader,
PaymentManifestParser* parser,
......@@ -40,6 +44,7 @@ InstallablePaymentAppCrawler::InstallablePaymentAppCrawler(
: WebContentsObserver(web_contents),
log_(web_contents),
merchant_origin_(merchant_origin),
initiator_render_frame_host_(initiator_render_frame_host),
downloader_(downloader),
parser_(parser),
number_of_payment_method_manifest_to_download_(0),
......@@ -401,6 +406,13 @@ void InstallablePaymentAppCrawler::DownloadAndDecodeWebAppIcon(
}
number_of_web_app_icons_to_download_and_decode_++;
content::GlobalFrameRoutingId frame_routing_id;
if (initiator_render_frame_host_) {
frame_routing_id = content::GlobalFrameRoutingId(
initiator_render_frame_host_->GetProcess()->GetID(),
initiator_render_frame_host_->GetRoutingID());
}
bool can_download_icon = content::ManifestIconDownloader::Download(
web_contents(), downloader_->FindTestServerURL(best_icon_url),
IconSizeCalculator::IdealIconHeight(native_view),
......@@ -409,7 +421,8 @@ void InstallablePaymentAppCrawler::DownloadAndDecodeWebAppIcon(
&InstallablePaymentAppCrawler::OnPaymentWebAppIconDownloadAndDecoded,
weak_ptr_factory_.GetWeakPtr(), method_manifest_url,
web_app_manifest_url),
false /* square_only */);
false, /* square_only */
frame_routing_id);
DCHECK(can_download_icon);
}
......
......@@ -26,6 +26,7 @@
class GURL;
namespace content {
class RenderFrameHost;
class WebContents;
}
......@@ -40,18 +41,21 @@ class InstallablePaymentAppCrawler : public content::WebContentsObserver {
std::map<GURL, std::unique_ptr<WebAppInstallationInfo>>,
const std::string& error_message)>;
// |merchant_origin| should be the origin of the iframe that created the
// |merchant_origin| is the origin of the iframe that created the
// PaymentRequest object. It is used by security features like
// 'Sec-Fetch-Site' and 'Cross-Origin-Resource-Policy'.
// |initiator_render_frame_host| is the iframe for |merchant_origin|.
//
// The owner of InstallablePaymentAppCrawler owns |downloader|, |parser| and
// |cache|. They should live until |finished_using_resources| parameter to
// Start() method is called.
InstallablePaymentAppCrawler(const url::Origin& merchant_origin,
content::WebContents* web_contents,
PaymentManifestDownloader* downloader,
PaymentManifestParser* parser,
PaymentManifestWebDataService* cache);
InstallablePaymentAppCrawler(
const url::Origin& merchant_origin,
content::RenderFrameHost* initiator_render_frame_host,
content::WebContents* web_contents,
PaymentManifestDownloader* downloader,
PaymentManifestParser* parser,
PaymentManifestWebDataService* cache);
~InstallablePaymentAppCrawler() override;
// Starts the crawling process. All the url based payment methods in
......@@ -107,6 +111,7 @@ class InstallablePaymentAppCrawler : public content::WebContentsObserver {
DeveloperConsoleLogger log_;
const url::Origin merchant_origin_;
content::RenderFrameHost* initiator_render_frame_host_;
PaymentManifestDownloader* downloader_;
PaymentManifestParser* parser_;
FinishedCrawlingCallback callback_;
......
......@@ -21,6 +21,7 @@ class AutofillProfile;
} // namespace autofill
namespace content {
class RenderFrameHost;
class WebContents;
} // namespace content
......@@ -46,6 +47,7 @@ class PaymentAppFactory {
virtual const GURL& GetTopOrigin() = 0;
virtual const GURL& GetFrameOrigin() = 0;
virtual const url::Origin& GetFrameSecurityOrigin() = 0;
virtual content::RenderFrameHost* GetInitiatorRenderFrameHost() const = 0;
virtual const std::vector<autofill::AutofillProfile*>&
GetBillingProfiles() = 0;
virtual bool IsRequestedAutofillDataAvailable() = 0;
......
......@@ -97,6 +97,7 @@ PaymentRequest::PaymentRequest(
mojo::PendingReceiver<mojom::PaymentRequest> receiver,
ObserverForTest* observer_for_testing)
: web_contents_(web_contents),
initiator_render_frame_host_(render_frame_host),
log_(web_contents_),
delegate_(std::move(delegate)),
manager_(manager),
......@@ -186,8 +187,8 @@ void PaymentRequest::Init(
std::move(options), std::move(details), std::move(method_data),
/*observer=*/this, delegate_->GetApplicationLocale());
state_ = std::make_unique<PaymentRequestState>(
web_contents_, top_level_origin_, frame_origin_, frame_security_origin_,
spec_.get(),
web_contents_, initiator_render_frame_host_, top_level_origin_,
frame_origin_, frame_security_origin_, spec_.get(),
/*delegate=*/this, delegate_->GetApplicationLocale(),
delegate_->GetPersonalDataManager(), delegate_.get(),
base::BindRepeating(&PaymentRequest::SetInvokedServiceWorkerIdentity,
......
......@@ -184,6 +184,7 @@ class PaymentRequest : public mojom::PaymentRequest,
bool warn_localhost_or_file);
content::WebContents* web_contents_;
content::RenderFrameHost* initiator_render_frame_host_;
DeveloperConsoleLogger log_;
std::unique_ptr<ContentPaymentRequestDelegate> delegate_;
// |manager_| owns this PaymentRequest.
......
......@@ -33,6 +33,7 @@
#include "components/payments/core/payment_app.h"
#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/common/content_features.h"
namespace payments {
......@@ -56,6 +57,7 @@ void PostStatusCallback(PaymentRequestState::StatusCallback callback,
PaymentRequestState::PaymentRequestState(
content::WebContents* web_contents,
content::RenderFrameHost* initiator_render_frame_host,
const GURL& top_level_origin,
const GURL& frame_origin,
const url::Origin& frame_security_origin,
......@@ -67,6 +69,7 @@ PaymentRequestState::PaymentRequestState(
const ServiceWorkerPaymentApp::IdentityCallback& sw_identity_callback,
JourneyLogger* journey_logger)
: web_contents_(web_contents),
initiator_render_frame_host_(initiator_render_frame_host),
top_origin_(top_level_origin),
frame_origin_(frame_origin),
frame_security_origin_(frame_security_origin),
......@@ -118,6 +121,11 @@ const url::Origin& PaymentRequestState::GetFrameSecurityOrigin() {
return frame_security_origin_;
}
content::RenderFrameHost* PaymentRequestState::GetInitiatorRenderFrameHost()
const {
return initiator_render_frame_host_;
}
const std::vector<autofill::AutofillProfile*>&
PaymentRequestState::GetBillingProfiles() {
return shipping_profiles_;
......
......@@ -33,6 +33,10 @@ class PersonalDataManager;
class RegionDataLoader;
} // namespace autofill
namespace content {
class RenderFrameHost;
} // namespace content
namespace payments {
class ContentPaymentRequestDelegate;
......@@ -112,6 +116,7 @@ class PaymentRequestState : public PaymentAppFactory::Delegate,
PaymentRequestState(
content::WebContents* web_contents,
content::RenderFrameHost* initiator_render_frame_host,
const GURL& top_level_origin,
const GURL& frame_origin,
const url::Origin& frame_security_origin,
......@@ -131,6 +136,7 @@ class PaymentRequestState : public PaymentAppFactory::Delegate,
const GURL& GetTopOrigin() override;
const GURL& GetFrameOrigin() override;
const url::Origin& GetFrameSecurityOrigin() override;
content::RenderFrameHost* GetInitiatorRenderFrameHost() const override;
const std::vector<autofill::AutofillProfile*>& GetBillingProfiles() override;
bool IsRequestedAutofillDataAvailable() override;
bool MayCrawlForInstallablePaymentApps() override;
......@@ -336,6 +342,7 @@ class PaymentRequestState : public PaymentAppFactory::Delegate,
SectionSelectionStatus selection_status);
content::WebContents* web_contents_;
content::RenderFrameHost* initiator_render_frame_host_;
const GURL top_origin_;
const GURL frame_origin_;
const url::Origin frame_security_origin_;
......
......@@ -80,7 +80,8 @@ class PaymentRequestStateTest : public testing::Test,
PaymentAppServiceFactory::SetForTesting(
std::make_unique<PaymentAppService>());
state_ = std::make_unique<PaymentRequestState>(
/*web_contents=*/nullptr, GURL("https://example.com"),
/*web_contents=*/nullptr,
/*render_frame_host=*/nullptr, GURL("https://example.com"),
GURL("https://example.com/pay"),
url::Origin::Create(GURL("https://example.com")), spec_.get(), this,
"en-US", &test_personal_data_manager_, &test_payment_request_delegate_,
......
......@@ -126,7 +126,8 @@ void ServiceWorkerPaymentAppFactory::Create(base::WeakPtr<Delegate> delegate) {
ServiceWorkerPaymentAppCreator* creator_raw_pointer = creator.get();
creators_[creator_raw_pointer] = std::move(creator);
ServiceWorkerPaymentAppFinder::GetInstance()->GetAllPaymentApps(
delegate->GetFrameSecurityOrigin(), delegate->GetWebContents(),
delegate->GetFrameSecurityOrigin(),
delegate->GetInitiatorRenderFrameHost(), delegate->GetWebContents(),
delegate->GetPaymentRequestDelegate()->GetPaymentManifestWebDataService(),
delegate->GetSpec()->method_data(),
delegate->MayCrawlForInstallablePaymentApps(),
......
......@@ -24,6 +24,7 @@
#include "components/payments/core/payment_manifest_downloader.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/stored_payment_app.h"
#include "content/public/browser/web_contents.h"
......@@ -99,6 +100,7 @@ class SelfDeletingServiceWorkerPaymentAppFinder {
// until |finished_using_resources_callback| has run.
void GetAllPaymentApps(
const url::Origin& merchant_origin,
content::RenderFrameHost* initiator_render_frame_host,
content::WebContents* web_contents,
std::unique_ptr<PaymentManifestDownloader> downloader,
scoped_refptr<PaymentManifestWebDataService> cache,
......@@ -120,8 +122,8 @@ class SelfDeletingServiceWorkerPaymentAppFinder {
features::kWebPaymentsJustInTimePaymentApp)) {
// Construct crawler in constructor to allow it observe the web_contents.
crawler_ = std::make_unique<InstallablePaymentAppCrawler>(
merchant_origin, web_contents, downloader_.get(), parser_.get(),
cache_.get());
merchant_origin, initiator_render_frame_host, web_contents,
downloader_.get(), parser_.get(), cache_.get());
if (ignore_port_in_origin_comparison_for_testing_)
crawler_->IgnorePortInOriginComparisonForTesting();
}
......@@ -284,6 +286,7 @@ ServiceWorkerPaymentAppFinder* ServiceWorkerPaymentAppFinder::GetInstance() {
void ServiceWorkerPaymentAppFinder::GetAllPaymentApps(
const url::Origin& merchant_origin,
content::RenderFrameHost* initiator_render_frame_host,
content::WebContents* web_contents,
scoped_refptr<PaymentManifestWebDataService> cache,
const std::vector<mojom::PaymentMethodDataPtr>& requested_method_data,
......@@ -306,9 +309,9 @@ void ServiceWorkerPaymentAppFinder::GetAllPaymentApps(
}
self_delete_factory->GetAllPaymentApps(
merchant_origin, web_contents, std::move(downloader), cache,
requested_method_data, may_crawl_for_installable_payment_apps,
std::move(callback),
merchant_origin, initiator_render_frame_host, web_contents,
std::move(downloader), cache, requested_method_data,
may_crawl_for_installable_payment_apps, std::move(callback),
std::move(finished_writing_cache_callback_for_testing));
}
......
......@@ -27,6 +27,7 @@ struct DefaultSingletonTraits;
} // namespace base
namespace content {
class RenderFrameHost;
class WebContents;
} // namespace content
......@@ -70,6 +71,7 @@ class ServiceWorkerPaymentAppFinder {
// The method should be called on the UI thread.
void GetAllPaymentApps(
const url::Origin& merchant_origin,
content::RenderFrameHost* initiator_render_frame_host,
content::WebContents* web_contents,
scoped_refptr<PaymentManifestWebDataService> cache,
const std::vector<mojom::PaymentMethodDataPtr>& requested_method_data,
......
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