Skip to content

GitLab

T
tangled
Commit 7003a957 authored by Alexandr Ilin's avatar Alexandr Ilin Committed by Commit Bot
Browse files
Options

predictors: Remove unused fields from URLRequestSummary 

Many fields of predictors::URLRequestSummary are not used after the
ResourcePrefetchPredictor was deprecated. This CL removes obsolete
members from the class.

Bug: 816545
Change-Id: I405e4a640a23f587691d7cc0ac7a6fa2341e3dbc
Reviewed-on: https://chromium-review.googlesource.com/960665Reviewed-by: default avatarBenoit L <lizeb@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543037}
parent 457e24ff
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 119 additions and 267 deletions
chrome/browser/net/loading_predictor_observer.cc
View file @ 7003a957
......@@ -101,7 +101,6 @@ void LoadingPredictorObserver::OnRequestStarted(
return;
auto summary = std::make_unique<URLRequestSummary>();
summary->resource_url = request->original_url();
summary->resource_type = resource_type;
BrowserThread::PostTask(
......
chrome/browser/predictors/loading_data_collector.cc
View file @ 7003a957
......@@ -54,30 +54,14 @@ bool IsNoStore(const net::URLRequest& response) {
} // namespace
OriginRequestSummary::OriginRequestSummary()
: origin(),
always_access_network(false),
accessed_network(false),
first_occurrence(0) {}
OriginRequestSummary::OriginRequestSummary() = default;
OriginRequestSummary::OriginRequestSummary(const OriginRequestSummary& other) =
default;
OriginRequestSummary::~OriginRequestSummary() = default;
OriginRequestSummary::~OriginRequestSummary() {}
URLRequestSummary::URLRequestSummary()
: resource_type(content::RESOURCE_TYPE_LAST_TYPE),
priority(net::IDLE),
before_first_contentful_paint(false),
was_cached(false),
has_validators(false),
always_revalidate(false),
is_no_store(false),
network_accessed(false) {}
URLRequestSummary::URLRequestSummary() = default;
URLRequestSummary::URLRequestSummary(const URLRequestSummary& other) = default;
URLRequestSummary::~URLRequestSummary() {}
URLRequestSummary::~URLRequestSummary() = default;
// static
bool URLRequestSummary::SummarizeResponse(const net::URLRequest& request,
......@@ -87,28 +71,17 @@ bool URLRequestSummary::SummarizeResponse(const net::URLRequest& request,
if (!request_info)
return false;
// This method is called when the response is started, so this field reflects
// the time at which the response began, not when it finished, as would
// arguably be ideal. This means if firstContentfulPaint happens after the
// response has started, but before it's finished, we will erroneously mark
// the resource as having been loaded before firstContentfulPaint. This is
// a rare and insignificant enough occurrence that we opt to record the time
// here for the sake of simplicity.
summary->response_time = base::TimeTicks::Now();
summary->resource_url = request.original_url();
summary->request_url = request.url();
content::ResourceType resource_type_from_request =
request_info->GetResourceType();
summary->priority = request.priority();
request.GetMimeType(&summary->mime_type);
summary->was_cached = request.was_cached();
std::string mime_type;
request.GetMimeType(&mime_type);
summary->resource_type = LoadingDataCollector::GetResourceType(
resource_type_from_request, summary->mime_type);
resource_type_from_request, mime_type);
scoped_refptr<net::HttpResponseHeaders> headers =
request.response_info().headers;
if (headers.get()) {
summary->has_validators = headers->HasValidators();
// RFC 2616, section 14.9.
summary->always_revalidate =
headers->HasHeaderValue("cache-control", "no-cache") ||
......@@ -118,13 +91,6 @@ bool URLRequestSummary::SummarizeResponse(const net::URLRequest& request,
}
summary->network_accessed = request.response_info().network_accessed;
net::LoadTimingInfo timing_info;
request.GetLoadTimingInfo(&timing_info);
const auto& connect_timing = timing_info.connect_timing;
summary->connect_duration =
(connect_timing.dns_end - connect_timing.dns_start) +
(connect_timing.connect_end - connect_timing.connect_start);
return true;
}
......@@ -156,7 +122,7 @@ void PageRequestSummary::UpdateOrAddToOrigins(
it->second.accessed_network |= request_summary.network_accessed;
}
PageRequestSummary::~PageRequestSummary() {}
PageRequestSummary::~PageRequestSummary() = default;
// static
content::ResourceType LoadingDataCollector::GetResourceTypeFromMimeType(
......@@ -313,7 +279,7 @@ LoadingDataCollector::LoadingDataCollector(
stats_collector_(stats_collector),
config_(config) {}
LoadingDataCollector::~LoadingDataCollector() {}
LoadingDataCollector::~LoadingDataCollector() = default;
void LoadingDataCollector::RecordURLRequest(const URLRequestSummary& request) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
......
chrome/browser/predictors/loading_data_collector.h
View file @ 7003a957
......@@ -13,7 +13,6 @@
#include "chrome/browser/predictors/loading_predictor_config.h"
#include "chrome/browser/predictors/resource_prefetch_common.h"
#include "content/public/common/resource_type.h"
#include "net/base/request_priority.h"
#include "url/gurl.h"
namespace net {
......@@ -33,9 +32,9 @@ struct OriginRequestSummary {
~OriginRequestSummary();
GURL origin;
bool always_access_network;
bool accessed_network;
int first_occurrence;
bool always_access_network = false;
bool accessed_network = false;
int first_occurrence = 0;
};
// Stores the data that we need to get from the URLRequest.
......@@ -45,27 +44,13 @@ struct URLRequestSummary {
~URLRequestSummary();
NavigationID navigation_id;
GURL resource_url;
GURL request_url; // URL after all redirects.
content::ResourceType resource_type;
net::RequestPriority priority;
base::TimeTicks response_time;
bool before_first_contentful_paint;
// Only for responses.
std::string mime_type;
bool was_cached;
GURL redirect_url; // Empty unless request was redirected to a valid url.
content::ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE;
bool has_validators;
bool always_revalidate;
bool is_no_store;
bool network_accessed;
// The time spent looking up the host's DNS address and establishing the
// connection. This can be zero if the request was cached or if the existing
// connection was reused.
base::TimeDelta connect_duration;
bool always_revalidate = false;
bool is_no_store = false;
bool network_accessed = false;
// Initializes a |URLRequestSummary| from a |URLRequest| response.
// Returns true for success. Note: NavigationID is NOT initialized
......
chrome/browser/predictors/loading_test_util.cc
View file @ 7003a957
......@@ -103,27 +103,17 @@ PageRequestSummary CreatePageRequestSummary(
URLRequestSummary CreateURLRequestSummary(SessionID::id_type tab_id,
const std::string& main_frame_url,
const std::string& resource_url,
const std::string& request_url,
content::ResourceType resource_type,
net::RequestPriority priority,
const std::string& mime_type,
bool was_cached,
const std::string& redirect_url,
bool has_validators,
bool always_revalidate) {
URLRequestSummary summary;
summary.navigation_id = CreateNavigationID(tab_id, main_frame_url);
summary.resource_url =
resource_url.empty() ? GURL(main_frame_url) : GURL(resource_url);
summary.request_url = summary.resource_url;
summary.resource_type = resource_type;
summary.priority = priority;
summary.before_first_contentful_paint = true;
summary.mime_type = mime_type;
summary.was_cached = was_cached;
summary.request_url =
request_url.empty() ? GURL(main_frame_url) : GURL(request_url);
if (!redirect_url.empty())
summary.redirect_url = GURL(redirect_url);
summary.has_validators = has_validators;
summary.resource_type = resource_type;
summary.always_revalidate = always_revalidate;
summary.is_no_store = false;
summary.network_accessed = true;
......@@ -295,12 +285,10 @@ std::ostream& operator<<(std::ostream& os, const PageRequestSummary& summary) {
}
std::ostream& operator<<(std::ostream& os, const URLRequestSummary& summary) {
return os << "[" << summary.navigation_id << "," << summary.resource_url
<< "," << summary.resource_type << "," << summary.priority << ","
<< summary.before_first_contentful_paint << "," << summary.mime_type
<< "," << summary.was_cached << "," << summary.redirect_url << ","
<< summary.has_validators << "," << summary.always_revalidate
<< "]";
return os << "[" << summary.navigation_id << "," << summary.request_url << ","
<< summary.redirect_url << "," << summary.resource_type << ","
<< summary.always_revalidate << "," << summary.is_no_store << ","
<< summary.network_accessed << "]";
}
std::ostream& operator<<(std::ostream& os, const NavigationID& navigation_id) {
......@@ -351,15 +339,12 @@ bool operator==(const PageRequestSummary& lhs, const PageRequestSummary& rhs) {
bool operator==(const URLRequestSummary& lhs, const URLRequestSummary& rhs) {
return lhs.navigation_id == rhs.navigation_id &&
lhs.resource_url == rhs.resource_url &&
lhs.resource_type == rhs.resource_type &&
lhs.priority == rhs.priority && lhs.mime_type == rhs.mime_type &&
lhs.before_first_contentful_paint ==
rhs.before_first_contentful_paint &&
lhs.was_cached == rhs.was_cached &&
lhs.request_url == rhs.request_url &&
lhs.redirect_url == rhs.redirect_url &&
lhs.has_validators == rhs.has_validators &&
lhs.always_revalidate == rhs.always_revalidate;
lhs.resource_type == rhs.resource_type &&
lhs.always_revalidate == rhs.always_revalidate &&
lhs.is_no_store == rhs.is_no_store &&
lhs.network_accessed == rhs.network_accessed;
}
bool operator==(const OriginRequestSummary& lhs,
......
chrome/browser/predictors/loading_test_util.h
View file @ 7003a957
......@@ -70,13 +70,9 @@ PageRequestSummary CreatePageRequestSummary(
URLRequestSummary CreateURLRequestSummary(
SessionID::id_type tab_id,
const std::string& main_frame_url,
const std::string& resource_url = std::string(),
const std::string& request_url = std::string(),
content::ResourceType resource_type = content::RESOURCE_TYPE_MAIN_FRAME,
net::RequestPriority priority = net::MEDIUM,
const std::string& mime_type = std::string(),
bool was_cached = false,
const std::string& redirect_url = std::string(),
bool has_validators = false,
bool always_revalidate = false);
URLRequestSummary CreateRedirectRequestSummary(
......
chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
View file @ 7003a957
......@@ -57,12 +57,9 @@ void ResourcePrefetchPredictorTabHelper::DidLoadResourceFromMemoryCache(
URLRequestSummary summary;
summary.navigation_id = NavigationID(web_contents());
summary.resource_url = url;
summary.request_url = url;
summary.mime_type = mime_type;
summary.resource_type =
LoadingDataCollector::GetResourceType(resource_type, mime_type);
summary.was_cached = true;
auto* collector = loading_predictor->loading_data_collector();
collector->RecordURLResponse(summary);
}
......
chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
View file @ 7003a957
......@@ -253,35 +253,35 @@ TEST_F(ResourcePrefetchPredictorTest, NavigationUrlNotInDB) {
std::vector<URLRequestSummary> resources;
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/style1.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/script2.js",
content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", true));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/image1.png",
content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/image2.png",
content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
content::RESOURCE_TYPE_STYLESHEET));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/script2.js",
content::RESOURCE_TYPE_SCRIPT));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/image1.png",
content::RESOURCE_TYPE_IMAGE));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/image2.png",
content::RESOURCE_TYPE_IMAGE));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/style2.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true));
content::RESOURCE_TYPE_STYLESHEET));
auto no_store = CreateURLRequestSummary(
1, "http://www.google.com",
"http://static.google.com/style2-no-store.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true);
auto no_store =
CreateURLRequestSummary(1, "http://www.google.com",
"http://static.google.com/style2-no-store.css",
content::RESOURCE_TYPE_STYLESHEET);
no_store.is_no_store = true;
auto redirected = CreateURLRequestSummary(
1, "http://www.google.com", "http://reader.google.com/style.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true);
content::RESOURCE_TYPE_STYLESHEET);
redirected.redirect_url = GURL("http://dev.null.google.com/style.css");
auto page_summary = CreatePageRequestSummary(
......@@ -331,34 +331,34 @@ TEST_F(ResourcePrefetchPredictorTest, NavigationUrlInDB) {
URLRequestSummary main_frame = CreateURLRequestSummary(
1, "http://www.google.com", "http://www.google.com",
content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
content::RESOURCE_TYPE_MAIN_FRAME);
std::vector<URLRequestSummary> resources;
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/style1.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/script2.js",
content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", true));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/image1.png",
content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/image2.png",
content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
content::RESOURCE_TYPE_STYLESHEET));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/script2.js",
content::RESOURCE_TYPE_SCRIPT));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/script1.js",
content::RESOURCE_TYPE_SCRIPT));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/image1.png",
content::RESOURCE_TYPE_IMAGE));
resources.push_back(CreateURLRequestSummary(1, "http://www.google.com",
"http://google.com/image2.png",
content::RESOURCE_TYPE_IMAGE));
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", "http://google.com/style2.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true));
auto no_store = CreateURLRequestSummary(
1, "http://www.google.com",
"http://static.google.com/style2-no-store.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true);
content::RESOURCE_TYPE_STYLESHEET));
auto no_store =
CreateURLRequestSummary(1, "http://www.google.com",
"http://static.google.com/style2-no-store.css",
content::RESOURCE_TYPE_STYLESHEET);
no_store.is_no_store = true;
auto page_summary = CreatePageRequestSummary(
......@@ -397,16 +397,16 @@ TEST_F(ResourcePrefetchPredictorTest, NavigationUrlNotInDBAndDBFull) {
ResetPredictor();
InitializePredictor();
URLRequestSummary main_frame = CreateURLRequestSummary(
1, "http://www.nike.com", "http://www.nike.com",
content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
URLRequestSummary main_frame =
CreateURLRequestSummary(1, "http://www.nike.com", "http://www.nike.com",
content::RESOURCE_TYPE_MAIN_FRAME);
URLRequestSummary resource1 = CreateURLRequestSummary(
1, "http://www.nike.com", "http://nike.com/style1.css",
content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", false);
content::RESOURCE_TYPE_STYLESHEET);
URLRequestSummary resource2 = CreateURLRequestSummary(
1, "http://www.nike.com", "http://nike.com/image2.png",
content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false);
content::RESOURCE_TYPE_IMAGE);
auto page_summary = CreatePageRequestSummary(
"http://www.nike.com", "http://www.nike.com", {resource1, resource2});
......@@ -448,8 +448,7 @@ TEST_F(ResourcePrefetchPredictorTest,
const int num_resources = predictor_->config_.max_origins_per_entry + 10;
for (int i = 1; i <= num_resources; ++i) {
resources.push_back(CreateURLRequestSummary(
1, "http://www.google.com", gen(i), content::RESOURCE_TYPE_SCRIPT,
net::MEDIUM, "text/javascript", false));
1, "http://www.google.com", gen(i), content::RESOURCE_TYPE_SCRIPT));
}
auto page_summary = CreatePageRequestSummary(
......
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