Commit 375d0104 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

ExtraRequestCompleteInfo: s/ url / origin_of_final_url /.

ExtraRequestCompleteInfo relies only on origin-related parts of the url.
This means that we can replace the |GURL url| field with
|url::Origin origin_of_final_url|.  This helps sanitize the URL in the
future (possibly removing the path and/or query parts in some cases).

Bug: 973885
Change-Id: Iac54c27a5f8d3b1d80463f0092d31324cc928fd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894153
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716359}
parent 272696a3
......@@ -194,8 +194,8 @@ TEST_F(AndroidPageLoadMetricsObserverTest, LoadTimingInfo) {
const base::TimeTicks kNow = base::TimeTicks::Now();
load_timing_info->connect_timing.dns_start = kNow;
page_load_metrics::ExtraRequestCompleteInfo info(
GURL("https://ignored.com"), net::IPEndPoint(), frame_tree_node_id,
false, /* cached */
url::Origin::Create(GURL("https://ignored.com")), net::IPEndPoint(),
frame_tree_node_id, false, /* cached */
10 * 1024 /* size */, 0 /* original_network_content_length */,
nullptr
/* data_reduction_proxy_data */,
......
......@@ -124,15 +124,19 @@ bool GetIPAndPort(
// from the URL. If none was returned, try matching the hostname from the URL
// itself as it might be an IP address if it is a local network request, which
// is what we care about.
if (!ip_exists && extra_request_info.url.is_valid()) {
if (net::IsLocalhost(extra_request_info.url)) {
if (!ip_exists && !extra_request_info.origin_of_final_url.opaque()) {
// TODO(csharrison): https://crbug.com/1023042: Avoid the url::Origin->GURL
// conversion. Today the conversion is necessary, because net::IsLocalhost
// and EffectiveIntPort are only available for GURL.
GURL origin_of_final_url = extra_request_info.origin_of_final_url.GetURL();
if (net::IsLocalhost(origin_of_final_url)) {
*resource_ip = net::IPAddress::IPv4Localhost();
ip_exists = true;
} else {
ip_exists = net::ParseURLHostnameToAddress(extra_request_info.url.host(),
ip_exists = net::ParseURLHostnameToAddress(origin_of_final_url.host(),
resource_ip);
}
*resource_port = extra_request_info.url.EffectiveIntPort();
*resource_port = origin_of_final_url.EffectiveIntPort();
}
if (net::HostStringIsLocalhost(resource_ip->ToString())) {
......
......@@ -114,8 +114,9 @@ class LocalNetworkRequestsPageLoadMetricsObserverTest
net::IPAddress address;
ASSERT_TRUE(address.AssignFromIPLiteral(resource.host_ip));
page_load_metrics::ExtraRequestCompleteInfo request_info(
GURL(resource.url), net::IPEndPoint(address, resource.port),
-1 /* frame_tree_node_id */, !net_error /* was_cached */,
url::Origin::Create(GURL(resource.url)),
net::IPEndPoint(address, resource.port), -1 /* frame_tree_node_id */,
!net_error /* was_cached */,
(net_error ? 1024 * 20 : 0) /* raw_body_bytes */,
0 /* original_network_content_length */,
nullptr /* data_reduction_proxy_data */,
......@@ -785,8 +786,8 @@ TEST_F(LocalNetworkRequestsPageLoadMetricsObserverTest,
// Load a resource that has the IP address in the URL but returned an empty
// socket address for some reason.
PageLoadMetricsObserverTestHarness::tester()->SimulateLoadedResource(
{GURL(internal::kDiffSubnetRequest2.url), net::IPEndPoint(),
-1 /* frame_tree_node_id */, true /* was_cached */,
{url::Origin::Create(GURL(internal::kDiffSubnetRequest2.url)),
net::IPEndPoint(), -1 /* frame_tree_node_id */, true /* was_cached */,
1024 * 20 /* raw_body_bytes */, 0 /* original_network_content_length */,
nullptr /* data_reduction_proxy_data */,
content::ResourceType::kMainFrame, 0, nullptr /* load_timing_info */},
......@@ -812,7 +813,7 @@ TEST_F(LocalNetworkRequestsPageLoadMetricsObserverTest,
// Load a resource that doesn't have the IP address in the URL and returned an
// empty socket address (e.g., failed DNS resolution).
PageLoadMetricsObserverTestHarness::tester()->SimulateLoadedResource(
{GURL(internal::kPrivatePage.url), net::IPEndPoint(),
{url::Origin::Create(GURL(internal::kPrivatePage.url)), net::IPEndPoint(),
-1 /* frame_tree_node_id */, false /* was_cached */,
0 /* raw_body_bytes */, 0 /* original_network_content_length */,
nullptr /* data_reduction_proxy_data */,
......
......@@ -329,7 +329,8 @@ void MetricsWebContentsObserver::ResourceLoadComplete(
const content::mojom::CommonNetworkInfoPtr& network_info =
resource_load_info.network_info;
ExtraRequestCompleteInfo extra_request_complete_info(
resource_load_info.url, network_info->remote_endpoint.value(),
url::Origin::Create(resource_load_info.url),
network_info->remote_endpoint.value(),
render_frame_host->GetFrameTreeNodeId(), resource_load_info.was_cached,
resource_load_info.raw_body_bytes, original_content_length,
std::move(data_reduction_proxy_data), resource_load_info.resource_type,
......
......@@ -1380,7 +1380,8 @@ TEST_F(MetricsWebContentsObserverTest, OnLoadedResource_MainFrame) {
*CreateResourceLoadInfo(main_resource_url,
content::ResourceType::kMainFrame));
EXPECT_EQ(1u, loaded_resources().size());
EXPECT_EQ(main_resource_url, loaded_resources().back().url);
EXPECT_EQ(url::Origin::Create(main_resource_url),
loaded_resources().back().origin_of_final_url);
NavigateToUntrackedUrl();
......@@ -1391,7 +1392,8 @@ TEST_F(MetricsWebContentsObserverTest, OnLoadedResource_MainFrame) {
*CreateResourceLoadInfo(main_resource_url,
content::ResourceType::kMainFrame));
EXPECT_EQ(1u, loaded_resources().size());
EXPECT_EQ(main_resource_url, loaded_resources().back().url);
EXPECT_EQ(url::Origin::Create(main_resource_url),
loaded_resources().back().origin_of_final_url);
}
TEST_F(MetricsWebContentsObserverTest, OnLoadedResource_Subresource) {
......@@ -1405,7 +1407,8 @@ TEST_F(MetricsWebContentsObserverTest, OnLoadedResource_Subresource) {
content::ResourceType::kScript));
EXPECT_EQ(1u, loaded_resources().size());
EXPECT_EQ(loaded_resource_url, loaded_resources().back().url);
EXPECT_EQ(url::Origin::Create(loaded_resource_url),
loaded_resources().back().origin_of_final_url);
}
TEST_F(MetricsWebContentsObserverTest,
......
......@@ -227,7 +227,7 @@ void PageLoadMetricsObserverTester::SimulateLoadedResource(
}
content::mojom::ResourceLoadInfo resource_load_info;
resource_load_info.url = info.url;
resource_load_info.url = info.origin_of_final_url.GetURL();
resource_load_info.was_cached = info.was_cached;
resource_load_info.raw_body_bytes = info.raw_body_bytes;
resource_load_info.total_received_bytes =
......
......@@ -9,7 +9,7 @@
namespace page_load_metrics {
ExtraRequestCompleteInfo::ExtraRequestCompleteInfo(
const GURL& url,
const url::Origin& origin_of_final_url,
const net::IPEndPoint& remote_endpoint,
int frame_tree_node_id,
bool was_cached,
......@@ -20,7 +20,7 @@ ExtraRequestCompleteInfo::ExtraRequestCompleteInfo(
content::ResourceType detected_resource_type,
int net_error,
std::unique_ptr<net::LoadTimingInfo> load_timing_info)
: url(url),
: origin_of_final_url(origin_of_final_url),
remote_endpoint(remote_endpoint),
frame_tree_node_id(frame_tree_node_id),
was_cached(was_cached),
......@@ -33,7 +33,7 @@ ExtraRequestCompleteInfo::ExtraRequestCompleteInfo(
ExtraRequestCompleteInfo::ExtraRequestCompleteInfo(
const ExtraRequestCompleteInfo& other)
: url(other.url),
: origin_of_final_url(other.origin_of_final_url),
remote_endpoint(other.remote_endpoint),
frame_tree_node_id(other.frame_tree_node_id),
was_cached(other.was_cached),
......
......@@ -99,7 +99,7 @@ struct PageRenderData {
// load.
struct ExtraRequestCompleteInfo {
ExtraRequestCompleteInfo(
const GURL& url,
const url::Origin& origin_of_final_url,
const net::IPEndPoint& remote_endpoint,
int frame_tree_node_id,
bool was_cached,
......@@ -115,8 +115,11 @@ struct ExtraRequestCompleteInfo {
~ExtraRequestCompleteInfo();
// The URL for the request.
const GURL url;
// The origin of the final URL for the request (final = after redirects).
//
// The full URL is not available, because in some cases the path and query
// be sanitized away - see https://crbug.com/973885.
const url::Origin origin_of_final_url;
// The host (IP address) and port for the request.
const net::IPEndPoint remote_endpoint;
......
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