Commit f36fc4ef authored by Tsuyoshi Horo's avatar Tsuyoshi Horo Committed by Commit Bot

Stop using callback to get frame_tree_node_id.

When I implemented SignedExchangeDevToolsProxy (crrev.com/c/1025492), we need to
use callback to get the frame_tree_node_id. It was because when Network Service
is not enabled the ID is not available while handling prefetch requests on the
IO thread.

But now, we don't need to support non-NetworkService mode (crbug.com/934009).
So we can stop using callback.

Bug: 934009
Change-Id: Idf5bfa1b59ce3499b8e9f27659a044ec52765dd7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863064
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarKunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706367}
parent f6573f6f
...@@ -51,6 +51,8 @@ const double kLoadingProgressDone = 1.0; ...@@ -51,6 +51,8 @@ const double kLoadingProgressDone = 1.0;
} // namespace } // namespace
const int FrameTreeNode::kFrameTreeNodeInvalidId = -1;
// This observer watches the opener of its owner FrameTreeNode and clears the // This observer watches the opener of its owner FrameTreeNode and clears the
// owner's opener if the opener is destroyed. // owner's opener if the opener is destroyed.
class FrameTreeNode::OpenerDestroyedObserver : public FrameTreeNode::Observer { class FrameTreeNode::OpenerDestroyedObserver : public FrameTreeNode::Observer {
......
...@@ -62,7 +62,7 @@ class CONTENT_EXPORT FrameTreeNode { ...@@ -62,7 +62,7 @@ class CONTENT_EXPORT FrameTreeNode {
virtual ~Observer() {} virtual ~Observer() {}
}; };
static const int kFrameTreeNodeInvalidId = -1; static const int kFrameTreeNodeInvalidId;
// Returns the FrameTreeNode with the given global |frame_tree_node_id|, // Returns the FrameTreeNode with the given global |frame_tree_node_id|,
// regardless of which FrameTree it is in. // regardless of which FrameTree it is in.
......
...@@ -32,7 +32,7 @@ PrefetchURLLoader::PrefetchURLLoader( ...@@ -32,7 +32,7 @@ PrefetchURLLoader::PrefetchURLLoader(
int32_t routing_id, int32_t routing_id,
int32_t request_id, int32_t request_id,
uint32_t options, uint32_t options,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
const network::ResourceRequest& resource_request, const network::ResourceRequest& resource_request,
network::mojom::URLLoaderClientPtr client, network::mojom::URLLoaderClientPtr client,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation, const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
...@@ -46,7 +46,7 @@ PrefetchURLLoader::PrefetchURLLoader( ...@@ -46,7 +46,7 @@ PrefetchURLLoader::PrefetchURLLoader(
base::WeakPtr<storage::BlobStorageContext> blob_storage_context, base::WeakPtr<storage::BlobStorageContext> blob_storage_context,
const std::string& accept_langs, const std::string& accept_langs,
RecursivePrefetchTokenGenerator recursive_prefetch_token_generator) RecursivePrefetchTokenGenerator recursive_prefetch_token_generator)
: frame_tree_node_id_getter_(frame_tree_node_id_getter), : frame_tree_node_id_(frame_tree_node_id),
resource_request_(resource_request), resource_request_(resource_request),
network_loader_factory_(std::move(network_loader_factory)), network_loader_factory_(std::move(network_loader_factory)),
client_binding_(this), client_binding_(this),
...@@ -158,7 +158,7 @@ void PrefetchURLLoader::OnReceiveResponse( ...@@ -158,7 +158,7 @@ void PrefetchURLLoader::OnReceiveResponse(
// network. (Until |this| calls the handler's FollowRedirect.) // network. (Until |this| calls the handler's FollowRedirect.)
signed_exchange_prefetch_handler_ = signed_exchange_prefetch_handler_ =
std::make_unique<SignedExchangePrefetchHandler>( std::make_unique<SignedExchangePrefetchHandler>(
frame_tree_node_id_getter_, resource_request_, response, frame_tree_node_id_, resource_request_, response,
mojo::ScopedDataPipeConsumerHandle(), std::move(loader_), mojo::ScopedDataPipeConsumerHandle(), std::move(loader_),
client_binding_.Unbind(), network_loader_factory_, client_binding_.Unbind(), network_loader_factory_,
url_loader_throttles_getter_, this, url_loader_throttles_getter_, this,
......
...@@ -54,14 +54,12 @@ class CONTENT_EXPORT PrefetchURLLoader : public network::mojom::URLLoader, ...@@ -54,14 +54,12 @@ class CONTENT_EXPORT PrefetchURLLoader : public network::mojom::URLLoader,
// |url_loader_throttles_getter| may be used when a prefetch handler needs to // |url_loader_throttles_getter| may be used when a prefetch handler needs to
// additionally create a request (e.g. for fetching certificate if the // additionally create a request (e.g. for fetching certificate if the
// prefetch was for a signed exchange). |frame_tree_node_id_getter| is called // prefetch was for a signed exchange).
// only on UI thread when NetworkService is not enabled, but can be also
// called on IO thread otherwise.
PrefetchURLLoader( PrefetchURLLoader(
int32_t routing_id, int32_t routing_id,
int32_t request_id, int32_t request_id,
uint32_t options, uint32_t options,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
const network::ResourceRequest& resource_request, const network::ResourceRequest& resource_request,
network::mojom::URLLoaderClientPtr client, network::mojom::URLLoaderClientPtr client,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation, const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
...@@ -127,7 +125,7 @@ class CONTENT_EXPORT PrefetchURLLoader : public network::mojom::URLLoader, ...@@ -127,7 +125,7 @@ class CONTENT_EXPORT PrefetchURLLoader : public network::mojom::URLLoader,
bool IsSignedExchangeHandlingEnabled(); bool IsSignedExchangeHandlingEnabled();
const base::RepeatingCallback<int(void)> frame_tree_node_id_getter_; const int frame_tree_node_id_;
// Set in the constructor and updated when redirected. // Set in the constructor and updated when redirected.
network::ResourceRequest resource_request_; network::ResourceRequest resource_request_;
......
...@@ -191,21 +191,18 @@ void PrefetchURLLoaderService::CreateLoaderAndStart( ...@@ -191,21 +191,18 @@ void PrefetchURLLoaderService::CreateLoaderAndStart(
->prefetched_signed_exchange_cache; ->prefetched_signed_exchange_cache;
} }
auto frame_tree_node_id_getter = base::BindRepeating(
[](int id) { return id; }, current_context.frame_tree_node_id);
// For now we strongly bind the loader to the request, while we can // For now we strongly bind the loader to the request, while we can
// also possibly make the new loader owned by the factory so that // also possibly make the new loader owned by the factory so that
// they can live longer than the client (i.e. run in detached mode). // they can live longer than the client (i.e. run in detached mode).
// TODO(kinuko): Revisit this. // TODO(kinuko): Revisit this.
mojo::MakeStrongBinding( mojo::MakeStrongBinding(
std::make_unique<PrefetchURLLoader>( std::make_unique<PrefetchURLLoader>(
routing_id, request_id, options, frame_tree_node_id_getter, routing_id, request_id, options, current_context.frame_tree_node_id,
resource_request, std::move(client), traffic_annotation, resource_request, std::move(client), traffic_annotation,
std::move(network_loader_factory_to_use), std::move(network_loader_factory_to_use),
base::BindRepeating( base::BindRepeating(
&PrefetchURLLoaderService::CreateURLLoaderThrottles, this, &PrefetchURLLoaderService::CreateURLLoaderThrottles, this,
resource_request, frame_tree_node_id_getter), resource_request, current_context.frame_tree_node_id),
browser_context_, signed_exchange_prefetch_metric_recorder_, browser_context_, signed_exchange_prefetch_metric_recorder_,
std::move(prefetched_signed_exchange_cache), blob_storage_context_, std::move(prefetched_signed_exchange_cache), blob_storage_context_,
accept_langs_, accept_langs_,
...@@ -314,8 +311,7 @@ base::UnguessableToken PrefetchURLLoaderService::GenerateRecursivePrefetchToken( ...@@ -314,8 +311,7 @@ base::UnguessableToken PrefetchURLLoaderService::GenerateRecursivePrefetchToken(
std::vector<std::unique_ptr<blink::URLLoaderThrottle>> std::vector<std::unique_ptr<blink::URLLoaderThrottle>>
PrefetchURLLoaderService::CreateURLLoaderThrottles( PrefetchURLLoaderService::CreateURLLoaderThrottles(
const network::ResourceRequest& request, const network::ResourceRequest& request,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter) { int frame_tree_node_id) {
int frame_tree_node_id = frame_tree_node_id_getter.Run();
return GetContentClient()->browser()->CreateURLLoaderThrottles( return GetContentClient()->browser()->CreateURLLoaderThrottles(
request, browser_context_, request, browser_context_,
base::BindRepeating(&WebContents::FromFrameTreeNodeId, base::BindRepeating(&WebContents::FromFrameTreeNodeId,
......
...@@ -101,9 +101,8 @@ class CONTENT_EXPORT PrefetchURLLoaderService final ...@@ -101,9 +101,8 @@ class CONTENT_EXPORT PrefetchURLLoaderService final
// For URLLoaderThrottlesGetter. // For URLLoaderThrottlesGetter.
std::vector<std::unique_ptr<blink::URLLoaderThrottle>> std::vector<std::unique_ptr<blink::URLLoaderThrottle>>
CreateURLLoaderThrottles( CreateURLLoaderThrottles(const network::ResourceRequest& request,
const network::ResourceRequest& request, int frame_tree_node_id);
base::RepeatingCallback<int(void)> frame_tree_node_id_getter);
scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter_; scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter_;
BrowserContext* browser_context_ = nullptr; BrowserContext* browser_context_ = nullptr;
......
...@@ -22,12 +22,12 @@ namespace content { ...@@ -22,12 +22,12 @@ namespace content {
SignedExchangeDevToolsProxy::SignedExchangeDevToolsProxy( SignedExchangeDevToolsProxy::SignedExchangeDevToolsProxy(
const GURL& outer_request_url, const GURL& outer_request_url,
const network::ResourceResponseHead& outer_response, const network::ResourceResponseHead& outer_response,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
base::Optional<const base::UnguessableToken> devtools_navigation_token, base::Optional<const base::UnguessableToken> devtools_navigation_token,
bool report_raw_headers) bool report_raw_headers)
: outer_request_url_(outer_request_url), : outer_request_url_(outer_request_url),
outer_response_(outer_response), outer_response_(outer_response),
frame_tree_node_id_getter_(frame_tree_node_id_getter), frame_tree_node_id_(frame_tree_node_id),
devtools_navigation_token_(devtools_navigation_token), devtools_navigation_token_(devtools_navigation_token),
devtools_enabled_(report_raw_headers) { devtools_enabled_(report_raw_headers) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
...@@ -43,7 +43,7 @@ void SignedExchangeDevToolsProxy::ReportError( ...@@ -43,7 +43,7 @@ void SignedExchangeDevToolsProxy::ReportError(
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
errors_.push_back(SignedExchangeError(message, std::move(error_field))); errors_.push_back(SignedExchangeError(message, std::move(error_field)));
WebContents* web_contents = WebContents* web_contents =
WebContents::FromFrameTreeNodeId(frame_tree_node_id_getter_.Run()); WebContents::FromFrameTreeNodeId(frame_tree_node_id_);
if (!web_contents) if (!web_contents)
return; return;
web_contents->GetMainFrame()->AddMessageToConsole( web_contents->GetMainFrame()->AddMessageToConsole(
...@@ -57,7 +57,7 @@ void SignedExchangeDevToolsProxy::CertificateRequestSent( ...@@ -57,7 +57,7 @@ void SignedExchangeDevToolsProxy::CertificateRequestSent(
return; return;
FrameTreeNode* frame_tree_node = FrameTreeNode* frame_tree_node =
FrameTreeNode::GloballyFindByID(frame_tree_node_id_getter_.Run()); FrameTreeNode::GloballyFindByID(frame_tree_node_id_);
if (!frame_tree_node) if (!frame_tree_node)
return; return;
...@@ -75,7 +75,7 @@ void SignedExchangeDevToolsProxy::CertificateResponseReceived( ...@@ -75,7 +75,7 @@ void SignedExchangeDevToolsProxy::CertificateResponseReceived(
return; return;
FrameTreeNode* frame_tree_node = FrameTreeNode* frame_tree_node =
FrameTreeNode::GloballyFindByID(frame_tree_node_id_getter_.Run()); FrameTreeNode::GloballyFindByID(frame_tree_node_id_);
if (!frame_tree_node) if (!frame_tree_node)
return; return;
...@@ -92,7 +92,7 @@ void SignedExchangeDevToolsProxy::CertificateRequestCompleted( ...@@ -92,7 +92,7 @@ void SignedExchangeDevToolsProxy::CertificateRequestCompleted(
return; return;
FrameTreeNode* frame_tree_node = FrameTreeNode* frame_tree_node =
FrameTreeNode::GloballyFindByID(frame_tree_node_id_getter_.Run()); FrameTreeNode::GloballyFindByID(frame_tree_node_id_);
if (!frame_tree_node) if (!frame_tree_node)
return; return;
...@@ -112,7 +112,7 @@ void SignedExchangeDevToolsProxy::OnSignedExchangeReceived( ...@@ -112,7 +112,7 @@ void SignedExchangeDevToolsProxy::OnSignedExchangeReceived(
ssl_info_opt = *ssl_info; ssl_info_opt = *ssl_info;
FrameTreeNode* frame_tree_node = FrameTreeNode* frame_tree_node =
FrameTreeNode::GloballyFindByID(frame_tree_node_id_getter_.Run()); FrameTreeNode::GloballyFindByID(frame_tree_node_id_);
if (!frame_tree_node) if (!frame_tree_node)
return; return;
......
...@@ -40,10 +40,6 @@ class SignedExchangeEnvelope; ...@@ -40,10 +40,6 @@ class SignedExchangeEnvelope;
// DevTools via the UI thread to show signed exchange related information. // DevTools via the UI thread to show signed exchange related information.
class CONTENT_EXPORT SignedExchangeDevToolsProxy { class CONTENT_EXPORT SignedExchangeDevToolsProxy {
public: public:
// |frame_tree_node_id_getter| callback will be called on the UI thread to get
// the frame tree node ID. Note: We are using callback beause when Network
// Service is not enabled the ID is not available while handling prefetch
// requests on the IO thread.
// When the signed exchange request is a navigation request, // When the signed exchange request is a navigation request,
// |devtools_navigation_token| can be used to find the matching request in // |devtools_navigation_token| can be used to find the matching request in
// DevTools. But when the signed exchange request is a prefetch request, the // DevTools. But when the signed exchange request is a prefetch request, the
...@@ -53,7 +49,7 @@ class CONTENT_EXPORT SignedExchangeDevToolsProxy { ...@@ -53,7 +49,7 @@ class CONTENT_EXPORT SignedExchangeDevToolsProxy {
SignedExchangeDevToolsProxy( SignedExchangeDevToolsProxy(
const GURL& outer_request_url, const GURL& outer_request_url,
const network::ResourceResponseHead& outer_response_head, const network::ResourceResponseHead& outer_response_head,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
base::Optional<const base::UnguessableToken> devtools_navigation_token, base::Optional<const base::UnguessableToken> devtools_navigation_token,
bool report_raw_headers); bool report_raw_headers);
~SignedExchangeDevToolsProxy(); ~SignedExchangeDevToolsProxy();
...@@ -79,7 +75,7 @@ class CONTENT_EXPORT SignedExchangeDevToolsProxy { ...@@ -79,7 +75,7 @@ class CONTENT_EXPORT SignedExchangeDevToolsProxy {
private: private:
const GURL outer_request_url_; const GURL outer_request_url_;
const network::ResourceResponseHead outer_response_; const network::ResourceResponseHead outer_response_;
const base::RepeatingCallback<int(void)> frame_tree_node_id_getter_; const int frame_tree_node_id_;
const base::Optional<const base::UnguessableToken> devtools_navigation_token_; const base::Optional<const base::UnguessableToken> devtools_navigation_token_;
const bool devtools_enabled_; const bool devtools_enabled_;
std::vector<SignedExchangeError> errors_; std::vector<SignedExchangeError> errors_;
......
...@@ -88,7 +88,7 @@ void VerifyCert(const scoped_refptr<net::X509Certificate>& certificate, ...@@ -88,7 +88,7 @@ void VerifyCert(const scoped_refptr<net::X509Certificate>& certificate,
const GURL& url, const GURL& url,
const std::string& ocsp_result, const std::string& ocsp_result,
const std::string& sct_list, const std::string& sct_list,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
VerifyCallback callback) { VerifyCallback callback) {
VerifyCallback wrapped_callback = mojo::WrapCallbackWithDefaultInvokeIfNotRun( VerifyCallback wrapped_callback = mojo::WrapCallbackWithDefaultInvokeIfNotRun(
std::move(callback), net::ERR_FAILED, net::CertVerifyResult(), std::move(callback), net::ERR_FAILED, net::CertVerifyResult(),
...@@ -97,8 +97,7 @@ void VerifyCert(const scoped_refptr<net::X509Certificate>& certificate, ...@@ -97,8 +97,7 @@ void VerifyCert(const scoped_refptr<net::X509Certificate>& certificate,
network::mojom::NetworkContext* network_context = network::mojom::NetworkContext* network_context =
g_network_context_for_testing; g_network_context_for_testing;
if (!network_context) { if (!network_context) {
auto* frame = auto* frame = FrameTreeNode::GloballyFindByID(frame_tree_node_id);
FrameTreeNode::GloballyFindByID(frame_tree_node_id_getter.Run());
if (!frame) if (!frame)
return; return;
...@@ -176,7 +175,7 @@ SignedExchangeHandler::SignedExchangeHandler( ...@@ -176,7 +175,7 @@ SignedExchangeHandler::SignedExchangeHandler(
std::unique_ptr<blink::SignedExchangeRequestMatcher> request_matcher, std::unique_ptr<blink::SignedExchangeRequestMatcher> request_matcher,
std::unique_ptr<SignedExchangeDevToolsProxy> devtools_proxy, std::unique_ptr<SignedExchangeDevToolsProxy> devtools_proxy,
SignedExchangeReporter* reporter, SignedExchangeReporter* reporter,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter) int frame_tree_node_id)
: is_secure_transport_(is_secure_transport), : is_secure_transport_(is_secure_transport),
has_nosniff_(has_nosniff), has_nosniff_(has_nosniff),
headers_callback_(std::move(headers_callback)), headers_callback_(std::move(headers_callback)),
...@@ -186,7 +185,7 @@ SignedExchangeHandler::SignedExchangeHandler( ...@@ -186,7 +185,7 @@ SignedExchangeHandler::SignedExchangeHandler(
request_matcher_(std::move(request_matcher)), request_matcher_(std::move(request_matcher)),
devtools_proxy_(std::move(devtools_proxy)), devtools_proxy_(std::move(devtools_proxy)),
reporter_(reporter), reporter_(reporter),
frame_tree_node_id_getter_(frame_tree_node_id_getter) { frame_tree_node_id_(frame_tree_node_id) {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"), TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("loading"),
"SignedExchangeHandler::SignedExchangeHandler"); "SignedExchangeHandler::SignedExchangeHandler");
...@@ -237,7 +236,8 @@ SignedExchangeHandler::~SignedExchangeHandler() = default; ...@@ -237,7 +236,8 @@ SignedExchangeHandler::~SignedExchangeHandler() = default;
SignedExchangeHandler::SignedExchangeHandler() SignedExchangeHandler::SignedExchangeHandler()
: is_secure_transport_(true), : is_secure_transport_(true),
has_nosniff_(true), has_nosniff_(true),
load_flags_(net::LOAD_NORMAL) {} load_flags_(net::LOAD_NORMAL),
frame_tree_node_id_(FrameTreeNode::kFrameTreeNodeInvalidId) {}
const GURL& SignedExchangeHandler::GetFallbackUrl() const { const GURL& SignedExchangeHandler::GetFallbackUrl() const {
return prologue_fallback_url_and_after_.fallback_url().url; return prologue_fallback_url_and_after_.fallback_url().url;
...@@ -521,7 +521,7 @@ void SignedExchangeHandler::OnCertReceived( ...@@ -521,7 +521,7 @@ void SignedExchangeHandler::OnCertReceived(
base::PostTask( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce(&VerifyCert, certificate, url, stapled_ocsp_response, base::BindOnce(&VerifyCert, certificate, url, stapled_ocsp_response,
sct_list_from_cert_cbor, frame_tree_node_id_getter_, sct_list_from_cert_cbor, frame_tree_node_id_,
base::BindOnce(&SignedExchangeHandler::OnVerifyCert, base::BindOnce(&SignedExchangeHandler::OnVerifyCert,
weak_factory_.GetWeakPtr()))); weak_factory_.GetWeakPtr())));
} }
......
...@@ -97,7 +97,7 @@ class CONTENT_EXPORT SignedExchangeHandler { ...@@ -97,7 +97,7 @@ class CONTENT_EXPORT SignedExchangeHandler {
std::unique_ptr<blink::SignedExchangeRequestMatcher> request_matcher, std::unique_ptr<blink::SignedExchangeRequestMatcher> request_matcher,
std::unique_ptr<SignedExchangeDevToolsProxy> devtools_proxy, std::unique_ptr<SignedExchangeDevToolsProxy> devtools_proxy,
SignedExchangeReporter* reporter, SignedExchangeReporter* reporter,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter); int frame_tree_node_id);
virtual ~SignedExchangeHandler(); virtual ~SignedExchangeHandler();
int64_t GetExchangeHeaderLength() const { return exchange_header_length_; } int64_t GetExchangeHeaderLength() const { return exchange_header_length_; }
...@@ -177,7 +177,7 @@ class CONTENT_EXPORT SignedExchangeHandler { ...@@ -177,7 +177,7 @@ class CONTENT_EXPORT SignedExchangeHandler {
// This is owned by SignedExchangeLoader which is the owner of |this|. // This is owned by SignedExchangeLoader which is the owner of |this|.
SignedExchangeReporter* reporter_; SignedExchangeReporter* reporter_;
base::RepeatingCallback<int(void)> frame_tree_node_id_getter_; const int frame_tree_node_id_;
base::TimeTicks cert_fetch_start_time_; base::TimeTicks cert_fetch_start_time_;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/path_service.h" #include "base/path_service.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "content/browser/frame_host/frame_tree_node.h"
#include "content/browser/web_package/signed_exchange_cert_fetcher_factory.h" #include "content/browser/web_package/signed_exchange_cert_fetcher_factory.h"
#include "content/browser/web_package/signed_exchange_devtools_proxy.h" #include "content/browser/web_package/signed_exchange_devtools_proxy.h"
#include "content/browser/web_package/signed_exchange_signature_verifier.h" #include "content/browser/web_package/signed_exchange_signature_verifier.h"
...@@ -336,7 +337,7 @@ class SignedExchangeHandlerTest ...@@ -336,7 +337,7 @@ class SignedExchangeHandlerTest
std::make_unique<blink::SignedExchangeRequestMatcher>( std::make_unique<blink::SignedExchangeRequestMatcher>(
net::HttpRequestHeaders(), std::string() /* accept_langs */), net::HttpRequestHeaders(), std::string() /* accept_langs */),
nullptr /* devtools_proxy */, nullptr /* reporter */, nullptr /* devtools_proxy */, nullptr /* reporter */,
base::RepeatingCallback<int(void)>()); FrameTreeNode::kFrameTreeNodeInvalidId);
} }
void WaitForHeader() { void WaitForHeader() {
......
...@@ -68,7 +68,7 @@ SignedExchangeLoader::SignedExchangeLoader( ...@@ -68,7 +68,7 @@ SignedExchangeLoader::SignedExchangeLoader(
std::unique_ptr<SignedExchangeReporter> reporter, std::unique_ptr<SignedExchangeReporter> reporter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
URLLoaderThrottlesGetter url_loader_throttles_getter, URLLoaderThrottlesGetter url_loader_throttles_getter,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
scoped_refptr<SignedExchangePrefetchMetricRecorder> metric_recorder, scoped_refptr<SignedExchangePrefetchMetricRecorder> metric_recorder,
const std::string& accept_langs) const std::string& accept_langs)
: outer_request_(outer_request), : outer_request_(outer_request),
...@@ -81,7 +81,7 @@ SignedExchangeLoader::SignedExchangeLoader( ...@@ -81,7 +81,7 @@ SignedExchangeLoader::SignedExchangeLoader(
reporter_(std::move(reporter)), reporter_(std::move(reporter)),
url_loader_factory_(std::move(url_loader_factory)), url_loader_factory_(std::move(url_loader_factory)),
url_loader_throttles_getter_(std::move(url_loader_throttles_getter)), url_loader_throttles_getter_(std::move(url_loader_throttles_getter)),
frame_tree_node_id_getter_(frame_tree_node_id_getter), frame_tree_node_id_(frame_tree_node_id),
metric_recorder_(std::move(metric_recorder)), metric_recorder_(std::move(metric_recorder)),
accept_langs_(accept_langs) { accept_langs_(accept_langs) {
DCHECK(outer_request_.url.is_valid()); DCHECK(outer_request_.url.is_valid());
...@@ -172,7 +172,7 @@ void SignedExchangeLoader::OnStartLoadingResponseBody( ...@@ -172,7 +172,7 @@ void SignedExchangeLoader::OnStartLoadingResponseBody(
std::move(cert_fetcher_factory), outer_request_.load_flags, std::move(cert_fetcher_factory), outer_request_.load_flags,
std::make_unique<blink::SignedExchangeRequestMatcher>( std::make_unique<blink::SignedExchangeRequestMatcher>(
outer_request_.headers, accept_langs_), outer_request_.headers, accept_langs_),
std::move(devtools_proxy_), reporter_.get(), frame_tree_node_id_getter_); std::move(devtools_proxy_), reporter_.get(), frame_tree_node_id_);
} }
void SignedExchangeLoader::OnComplete( void SignedExchangeLoader::OnComplete(
......
...@@ -72,7 +72,7 @@ class CONTENT_EXPORT SignedExchangeLoader final ...@@ -72,7 +72,7 @@ class CONTENT_EXPORT SignedExchangeLoader final
std::unique_ptr<SignedExchangeReporter> reporter, std::unique_ptr<SignedExchangeReporter> reporter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
URLLoaderThrottlesGetter url_loader_throttles_getter, URLLoaderThrottlesGetter url_loader_throttles_getter,
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
scoped_refptr<SignedExchangePrefetchMetricRecorder> metric_recorder, scoped_refptr<SignedExchangePrefetchMetricRecorder> metric_recorder,
const std::string& accept_langs); const std::string& accept_langs);
~SignedExchangeLoader() override; ~SignedExchangeLoader() override;
...@@ -172,7 +172,7 @@ class CONTENT_EXPORT SignedExchangeLoader final ...@@ -172,7 +172,7 @@ class CONTENT_EXPORT SignedExchangeLoader final
std::unique_ptr<SignedExchangeReporter> reporter_; std::unique_ptr<SignedExchangeReporter> reporter_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
URLLoaderThrottlesGetter url_loader_throttles_getter_; URLLoaderThrottlesGetter url_loader_throttles_getter_;
base::RepeatingCallback<int(void)> frame_tree_node_id_getter_; const int frame_tree_node_id_;
scoped_refptr<SignedExchangePrefetchMetricRecorder> metric_recorder_; scoped_refptr<SignedExchangePrefetchMetricRecorder> metric_recorder_;
base::Optional<net::SSLInfo> ssl_info_; base::Optional<net::SSLInfo> ssl_info_;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "content/browser/frame_host/frame_tree_node.h"
#include "content/browser/web_package/mock_signed_exchange_handler.h" #include "content/browser/web_package/mock_signed_exchange_handler.h"
#include "content/browser/web_package/signed_exchange_devtools_proxy.h" #include "content/browser/web_package/signed_exchange_devtools_proxy.h"
#include "content/browser/web_package/signed_exchange_prefetch_metric_recorder.h" #include "content/browser/web_package/signed_exchange_prefetch_metric_recorder.h"
...@@ -181,7 +182,7 @@ TEST_P(SignedExchangeLoaderTest, Simple) { ...@@ -181,7 +182,7 @@ TEST_P(SignedExchangeLoaderTest, Simple) {
false /* should_redirect_to_fallback */, nullptr /* devtools_proxy */, false /* should_redirect_to_fallback */, nullptr /* devtools_proxy */,
nullptr /* reporter */, CreateMockPingLoaderFactory(), nullptr /* reporter */, CreateMockPingLoaderFactory(),
base::BindRepeating(&SignedExchangeLoaderTest::ThrottlesGetter), base::BindRepeating(&SignedExchangeLoaderTest::ThrottlesGetter),
base::RepeatingCallback<int(void)>(), nullptr /* metric_recorder */, FrameTreeNode::kFrameTreeNodeInvalidId, nullptr /* metric_recorder */,
std::string() /* accept_langs */); std::string() /* accept_langs */);
EXPECT_CALL(mock_loader, PauseReadingBodyFromNet()); EXPECT_CALL(mock_loader, PauseReadingBodyFromNet());
......
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
namespace content { namespace content {
SignedExchangePrefetchHandler::SignedExchangePrefetchHandler( SignedExchangePrefetchHandler::SignedExchangePrefetchHandler(
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
const network::ResourceRequest& resource_request, const network::ResourceRequest& resource_request,
const network::ResourceResponseHead& response_head, const network::ResourceResponseHead& response_head,
mojo::ScopedDataPipeConsumerHandle response_body, mojo::ScopedDataPipeConsumerHandle response_body,
...@@ -51,14 +51,14 @@ SignedExchangePrefetchHandler::SignedExchangePrefetchHandler( ...@@ -51,14 +51,14 @@ SignedExchangePrefetchHandler::SignedExchangePrefetchHandler(
std::move(client), std::move(endpoints), url_loader_options, std::move(client), std::move(endpoints), url_loader_options,
false /* should_redirect_to_fallback */, false /* should_redirect_to_fallback */,
std::make_unique<SignedExchangeDevToolsProxy>( std::make_unique<SignedExchangeDevToolsProxy>(
resource_request.url, response_head, frame_tree_node_id_getter, resource_request.url, response_head, frame_tree_node_id,
base::nullopt /* devtools_navigation_token */, base::nullopt /* devtools_navigation_token */,
resource_request.report_raw_headers), resource_request.report_raw_headers),
SignedExchangeReporter::MaybeCreate( SignedExchangeReporter::MaybeCreate(resource_request.url,
resource_request.url, resource_request.referrer.spec(), response_head, resource_request.referrer.spec(),
frame_tree_node_id_getter), response_head, frame_tree_node_id),
std::move(url_loader_factory), loader_throttles_getter, std::move(url_loader_factory), loader_throttles_getter,
frame_tree_node_id_getter, std::move(metric_recorder), accept_langs); frame_tree_node_id, std::move(metric_recorder), accept_langs);
} }
SignedExchangePrefetchHandler::~SignedExchangePrefetchHandler() = default; SignedExchangePrefetchHandler::~SignedExchangePrefetchHandler() = default;
......
...@@ -45,7 +45,7 @@ class SignedExchangePrefetchHandler final ...@@ -45,7 +45,7 @@ class SignedExchangePrefetchHandler final
// |forwarding_client| is a pointer to the downstream client (typically who // |forwarding_client| is a pointer to the downstream client (typically who
// creates this handler). // creates this handler).
SignedExchangePrefetchHandler( SignedExchangePrefetchHandler(
base::RepeatingCallback<int(void)> frame_tree_node_id_getter, int frame_tree_node_id,
const network::ResourceRequest& resource_request, const network::ResourceRequest& resource_request,
const network::ResourceResponseHead& response, const network::ResourceResponseHead& response,
mojo::ScopedDataPipeConsumerHandle response_body, mojo::ScopedDataPipeConsumerHandle response_body,
......
...@@ -118,9 +118,8 @@ bool ShouldDowngradeReport(const char* result_string, ...@@ -118,9 +118,8 @@ bool ShouldDowngradeReport(const char* result_string,
return false; return false;
} }
void ReportResultOnUI(base::OnceCallback<int(void)> frame_tree_node_id_getter, void ReportResultOnUI(int frame_tree_node_id,
network::mojom::SignedExchangeReportPtr report) { network::mojom::SignedExchangeReportPtr report) {
int frame_tree_node_id = std::move(frame_tree_node_id_getter).Run();
FrameTreeNode* frame_tree_node = FrameTreeNode* frame_tree_node =
FrameTreeNode::GloballyFindByID(frame_tree_node_id); FrameTreeNode::GloballyFindByID(frame_tree_node_id);
if (!frame_tree_node) if (!frame_tree_node)
...@@ -146,23 +145,23 @@ std::unique_ptr<SignedExchangeReporter> SignedExchangeReporter::MaybeCreate( ...@@ -146,23 +145,23 @@ std::unique_ptr<SignedExchangeReporter> SignedExchangeReporter::MaybeCreate(
const GURL& outer_url, const GURL& outer_url,
const std::string& referrer, const std::string& referrer,
const network::ResourceResponseHead& response, const network::ResourceResponseHead& response,
base::OnceCallback<int(void)> frame_tree_node_id_getter) { int frame_tree_node_id) {
if (!signed_exchange_utils:: if (!signed_exchange_utils::
IsSignedExchangeReportingForDistributorsEnabled()) { IsSignedExchangeReportingForDistributorsEnabled()) {
return nullptr; return nullptr;
} }
return base::WrapUnique(new SignedExchangeReporter( return base::WrapUnique(new SignedExchangeReporter(
outer_url, referrer, response, std::move(frame_tree_node_id_getter))); outer_url, referrer, response, frame_tree_node_id));
} }
SignedExchangeReporter::SignedExchangeReporter( SignedExchangeReporter::SignedExchangeReporter(
const GURL& outer_url, const GURL& outer_url,
const std::string& referrer, const std::string& referrer,
const network::ResourceResponseHead& response, const network::ResourceResponseHead& response,
base::OnceCallback<int(void)> frame_tree_node_id_getter) int frame_tree_node_id)
: report_(network::mojom::SignedExchangeReport::New()), : report_(network::mojom::SignedExchangeReport::New()),
request_start_(response.load_timing.request_start), request_start_(response.load_timing.request_start),
frame_tree_node_id_getter_(std::move(frame_tree_node_id_getter)) { frame_tree_node_id_(frame_tree_node_id) {
report_->outer_url = outer_url; report_->outer_url = outer_url;
report_->referrer = referrer; report_->referrer = referrer;
report_->server_ip_address = response.remote_endpoint.address(); report_->server_ip_address = response.remote_endpoint.address();
...@@ -196,7 +195,6 @@ void SignedExchangeReporter::set_cert_url(const GURL& cert_url) { ...@@ -196,7 +195,6 @@ void SignedExchangeReporter::set_cert_url(const GURL& cert_url) {
void SignedExchangeReporter::ReportResultAndFinish( void SignedExchangeReporter::ReportResultAndFinish(
SignedExchangeLoadResult result) { SignedExchangeLoadResult result) {
DCHECK(report_); DCHECK(report_);
DCHECK(frame_tree_node_id_getter_);
const char* result_string = GetResultTypeString(result); const char* result_string = GetResultTypeString(result);
report_->success = result == SignedExchangeLoadResult::kSuccess; report_->success = result == SignedExchangeLoadResult::kSuccess;
...@@ -214,10 +212,9 @@ void SignedExchangeReporter::ReportResultAndFinish( ...@@ -214,10 +212,9 @@ void SignedExchangeReporter::ReportResultAndFinish(
report_->elapsed_time = base::TimeTicks::Now() - request_start_; report_->elapsed_time = base::TimeTicks::Now() - request_start_;
} }
base::PostTask( base::PostTask(FROM_HERE, {BrowserThread::UI},
FROM_HERE, {BrowserThread::UI}, base::BindOnce(&ReportResultOnUI, frame_tree_node_id_,
base::BindOnce(&ReportResultOnUI, std::move(frame_tree_node_id_getter_), std::move(report_)));
std::move(report_)));
} }
} // namespace content } // namespace content
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <memory> #include <memory>
#include <string> #include <string>
#include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "content/browser/web_package/signed_exchange_error.h" #include "content/browser/web_package/signed_exchange_error.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
...@@ -31,7 +30,7 @@ class CONTENT_EXPORT SignedExchangeReporter { ...@@ -31,7 +30,7 @@ class CONTENT_EXPORT SignedExchangeReporter {
const GURL& outer_url, const GURL& outer_url,
const std::string& referrer, const std::string& referrer,
const network::ResourceResponseHead& response, const network::ResourceResponseHead& response,
base::OnceCallback<int(void)> frame_tree_node_id_getter); int frame_tree_node_id);
~SignedExchangeReporter(); ~SignedExchangeReporter();
...@@ -44,15 +43,14 @@ class CONTENT_EXPORT SignedExchangeReporter { ...@@ -44,15 +43,14 @@ class CONTENT_EXPORT SignedExchangeReporter {
void ReportResultAndFinish(SignedExchangeLoadResult result); void ReportResultAndFinish(SignedExchangeLoadResult result);
private: private:
SignedExchangeReporter( SignedExchangeReporter(const GURL& outer_url,
const GURL& outer_url, const std::string& referrer,
const std::string& referrer, const network::ResourceResponseHead& response,
const network::ResourceResponseHead& response, int frame_tree_node_id);
base::OnceCallback<int(void)> frame_tree_node_id_getter);
network::mojom::SignedExchangeReportPtr report_; network::mojom::SignedExchangeReportPtr report_;
const base::TimeTicks request_start_; const base::TimeTicks request_start_;
base::OnceCallback<int(void)> frame_tree_node_id_getter_; const int frame_tree_node_id_;
net::IPAddress cert_server_ip_address_; net::IPAddress cert_server_ip_address_;
DISALLOW_COPY_AND_ASSIGN(SignedExchangeReporter); DISALLOW_COPY_AND_ASSIGN(SignedExchangeReporter);
......
...@@ -92,9 +92,6 @@ bool SignedExchangeRequestHandler::MaybeCreateLoaderForResponse( ...@@ -92,9 +92,6 @@ bool SignedExchangeRequestHandler::MaybeCreateLoaderForResponse(
network::mojom::URLLoaderClientPtr client; network::mojom::URLLoaderClientPtr client;
*client_request = mojo::MakeRequest(&client); *client_request = mojo::MakeRequest(&client);
base::RepeatingCallback<int(void)> frame_tree_node_id_getter =
base::BindRepeating([](int id) { return id; }, frame_tree_node_id_);
// This lets the SignedExchangeLoader directly returns an artificial redirect // This lets the SignedExchangeLoader directly returns an artificial redirect
// to the downstream client without going through ThrottlingURLLoader, which // to the downstream client without going through ThrottlingURLLoader, which
// means some checks like SafeBrowsing may not see the redirect. Given that // means some checks like SafeBrowsing may not see the redirect. Given that
...@@ -105,13 +102,12 @@ bool SignedExchangeRequestHandler::MaybeCreateLoaderForResponse( ...@@ -105,13 +102,12 @@ bool SignedExchangeRequestHandler::MaybeCreateLoaderForResponse(
url_loader->Unbind(), url_loader_options_, url_loader->Unbind(), url_loader_options_,
true /* should_redirect_to_fallback */, true /* should_redirect_to_fallback */,
std::make_unique<SignedExchangeDevToolsProxy>( std::make_unique<SignedExchangeDevToolsProxy>(
request.url, response_head, frame_tree_node_id_getter, request.url, response_head, frame_tree_node_id_,
devtools_navigation_token_, request.report_raw_headers), devtools_navigation_token_, request.report_raw_headers),
SignedExchangeReporter::MaybeCreate(request.url, request.referrer.spec(), SignedExchangeReporter::MaybeCreate(request.url, request.referrer.spec(),
response_head, response_head, frame_tree_node_id_),
frame_tree_node_id_getter), url_loader_factory_, url_loader_throttles_getter_, frame_tree_node_id_,
url_loader_factory_, url_loader_throttles_getter_, metric_recorder_, accept_langs_);
frame_tree_node_id_getter, metric_recorder_, accept_langs_);
*skip_other_interceptors = true; *skip_other_interceptors = true;
return true; return true;
......
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