 Chromium Code Reviews
 Chromium Code Reviews Issue 2755093002:
  predictors: Mark before_first_contentful_paint for resources fetched before fcp.  (Closed)
    
  
    Issue 2755093002:
  predictors: Mark before_first_contentful_paint for resources fetched before fcp.  (Closed) 
  | Index: chrome/browser/predictors/resource_prefetch_predictor.cc | 
| diff --git a/chrome/browser/predictors/resource_prefetch_predictor.cc b/chrome/browser/predictors/resource_prefetch_predictor.cc | 
| index ad3e824f5ea5adadb37ada20bb5fbf40412e3ae5..dad0022eed3df82ab850245a52158e2992373901 100644 | 
| --- a/chrome/browser/predictors/resource_prefetch_predictor.cc | 
| +++ b/chrome/browser/predictors/resource_prefetch_predictor.cc | 
| @@ -417,6 +417,8 @@ void ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(bool state) { | 
| ResourcePrefetchPredictor::URLRequestSummary::URLRequestSummary() | 
| : resource_type(content::RESOURCE_TYPE_LAST_TYPE), | 
| priority(net::IDLE), | 
| + response_time(base::TimeTicks()), | 
| + before_first_contentful_paint(false), | 
| was_cached(false), | 
| has_validators(false), | 
| always_revalidate(false) {} | 
| @@ -427,6 +429,8 @@ ResourcePrefetchPredictor::URLRequestSummary::URLRequestSummary( | 
| resource_url(other.resource_url), | 
| resource_type(other.resource_type), | 
| priority(other.priority), | 
| + response_time(other.response_time), | 
| + before_first_contentful_paint(other.before_first_contentful_paint), | 
| mime_type(other.mime_type), | 
| was_cached(other.was_cached), | 
| redirect_url(other.redirect_url), | 
| @@ -445,6 +449,7 @@ bool ResourcePrefetchPredictor::URLRequestSummary::SummarizeResponse( | 
| if (!request_info) | 
| return false; | 
| + summary->response_time = base::TimeTicks::Now(); | 
| summary->resource_url = request.original_url(); | 
| content::ResourceType resource_type_from_request = | 
| request_info->GetResourceType(); | 
| @@ -469,7 +474,9 @@ bool ResourcePrefetchPredictor::URLRequestSummary::SummarizeResponse( | 
| ResourcePrefetchPredictor::PageRequestSummary::PageRequestSummary( | 
| const GURL& i_main_frame_url) | 
| - : main_frame_url(i_main_frame_url), initial_url(i_main_frame_url) {} | 
| + : main_frame_url(i_main_frame_url), | 
| + initial_url(i_main_frame_url), | 
| + first_contentful_paint(base::TimeTicks::Max()) {} | 
| ResourcePrefetchPredictor::PageRequestSummary::PageRequestSummary( | 
| const PageRequestSummary& other) = default; | 
| @@ -591,6 +598,15 @@ void ResourcePrefetchPredictor::RecordMainFrameLoadComplete( | 
| } | 
| } | 
| +void ResourcePrefetchPredictor::RecordFirstContentfulPaint( | 
| + const NavigationID& navigation_id, | 
| + const base::TimeTicks& first_contentful_paint) { | 
| + NavigationMap::iterator nav_it = inflight_navigations_.find(navigation_id); | 
| + if (nav_it != inflight_navigations_.end()) { | 
| + nav_it->second->first_contentful_paint = first_contentful_paint; | 
| + } | 
| +} | 
| + | 
| void ResourcePrefetchPredictor::StartPrefetching(const GURL& url, | 
| PrefetchOrigin origin) { | 
| TRACE_EVENT1("browser", "ResourcePrefetchPredictor::StartPrefetching", "url", | 
| @@ -770,6 +786,12 @@ void ResourcePrefetchPredictor::OnNavigationComplete( | 
| std::unique_ptr<PageRequestSummary> summary = std::move(nav_it->second); | 
| inflight_navigations_.erase(nav_it); | 
| + // Set before_first_contentful paint for each resource. | 
| + for (auto& request_summary : summary->subresource_requests) { | 
| + request_summary.before_first_contentful_paint = | 
| + request_summary.response_time < summary->first_contentful_paint; | 
| + } | 
| + | 
| const GURL& initial_url = summary->initial_url; | 
| ResourcePrefetchPredictor::Prediction prediction; | 
| bool has_data = GetPrefetchData(initial_url, &prediction); | 
| @@ -1186,6 +1208,8 @@ void ResourcePrefetchPredictor::LearnNavigation( | 
| resource_to_add->set_average_position(i + 1); | 
| resource_to_add->set_priority( | 
| static_cast<ResourceData::Priority>(summary.priority)); | 
| + resource_to_add->set_before_first_contentful_paint( | 
| 
alexilin
2017/04/21 13:49:05
It has been lost :(
 
trevordixon
2017/04/25 12:46:09
Oh no, I did a bad merge. Restored.
 | 
| + summary.before_first_contentful_paint); | 
| resource_to_add->set_has_validators(summary.has_validators); | 
| resource_to_add->set_always_revalidate(summary.always_revalidate); | 
| @@ -1235,6 +1259,8 @@ void ResourcePrefetchPredictor::LearnNavigation( | 
| old_resource->set_priority( | 
| static_cast<ResourceData::Priority>(new_summary.priority)); | 
| + old_resource->set_before_first_contentful_paint( | 
| 
alexilin
2017/04/21 13:49:05
Ditto.
 
trevordixon
2017/04/25 12:46:09
Fixed.
 | 
| + new_summary.before_first_contentful_paint); | 
| int position = new_index[resource_url] + 1; | 
| int total = | 
| @@ -1262,6 +1288,8 @@ void ResourcePrefetchPredictor::LearnNavigation( | 
| resource_to_add->set_average_position(i + 1); | 
| resource_to_add->set_priority( | 
| static_cast<ResourceData::Priority>(summary.priority)); | 
| + resource_to_add->set_before_first_contentful_paint( | 
| + summary.before_first_contentful_paint); | 
| resource_to_add->set_has_validators(new_resources[i].has_validators); | 
| resource_to_add->set_always_revalidate( | 
| new_resources[i].always_revalidate); |