predictors: Mark before_first_contentful_paint for resources fetched before fcp.

chrome/browser/predictors/resource_prefetch_predictor.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 
 #include <map>
 #include <set>
 #include <utility>
 
@@ skipping 560 matching lines @@
       // corresponding to the navigation has not been created yet.
       if (!navigation_id.main_frame_url.is_empty())
         OnNavigationComplete(navigation_id);
       break;
     default:
       NOTREACHED() << "Unexpected initialization_state_: "
                    << initialization_state_;
   }
 }
 
+void ResourcePrefetchPredictor::RecordFirstContentfulPaint(
+    const NavigationID navigation_id) {
+  NavigationMap::iterator nav_it =
+      inflight_navigations_.find(navigation_id);
+  if (nav_it == inflight_navigations_.end()) return;

[alexilin, 2017/03/17 14:46:46]

nit:
Return statement should occupy a separate line.
Please, run `git cl format` to automatically reformat code according to style
guides.

[trevordixon, 2017/03/27 12:30:08]

Done.

+
+  for (auto& request_summary : nav_it->second->subresource_requests) {
+    request_summary.before_first_contentful_paint = true;

[alexilin, 2017/03/17 14:46:46]

You can't assume that Observer gets an event at the same time when first
contentful paint happens. 
The flow is following:
(1) Navigation start =====> (2) Navigation commit =====> (3) First contentful
paint =====> (4) PageLoadMetricsObserver::OnFirstContentfulPaint() is called

I've made a little experiment showing that the time between (3) and (4) could be
significant (>1s)
You need to use PageLoadTiming::first_contentful_paint to determine when (3)
exactly happened.

And it could be very tricky if OnLoad() event handlers complete between (3) and
(4). We'll write everything in the database assuming that all resources are
before fcp while some resources could be loaded after (3).

[trevordixon, 2017/03/27 12:30:08]

Maybe fixed. How likely is it that OnFirstContentfulPaint could be called after
onload? If it's sufficiently rare, it should work fine to ignore the
possibility.

[alexilin, 2017/03/27 15:31:59]

Well, it's definitely not rare for "fast" pages for which the time between fcp
and onload is short. Then it could happen that onload appears before
OnFirstContentfulPaint event. Examples: google properties, facebook, wikipedia. 
We don't target them, though. So, maybe, it's fine and isn't worth complexity.

+  }
+}
+
+
 void ResourcePrefetchPredictor::StartPrefetching(const GURL& url,
                                                  PrefetchOrigin origin) {
   TRACE_EVENT1("browser", "ResourcePrefetchPredictor::StartPrefetching", "url",
                url.spec());
   // Save prefetch start time to report prefetching duration.
   if (inflight_prefetches_.find(url) == inflight_prefetches_.end() &&
       IsUrlPrefetchable(url)) {
     inflight_prefetches_.insert(std::make_pair(url, base::TimeTicks::Now()));
   }
 
@@ skipping 548 matching lines @@
     data.set_primary_key(key);
     data.set_last_visit_time(base::Time::Now().ToInternalValue());
     size_t new_resources_size = new_resources.size();
     std::set<GURL> resources_seen;
     for (size_t i = 0; i < new_resources_size; ++i) {
       const URLRequestSummary& summary = new_resources[i];
       if (resources_seen.find(summary.resource_url) != resources_seen.end())
         continue;
 
       ResourceData* resource_to_add = data.add_resources();
+      resource_to_add->set_before_first_contentful_paint(
+          summary.before_first_contentful_paint);
       resource_to_add->set_resource_url(summary.resource_url.spec());
       resource_to_add->set_resource_type(
           static_cast<ResourceData::ResourceType>(summary.resource_type));
       resource_to_add->set_number_of_hits(1);
       resource_to_add->set_average_position(i + 1);
       resource_to_add->set_priority(
           static_cast<ResourceData::Priority>(summary.priority));
       resource_to_add->set_has_validators(summary.has_validators);
       resource_to_add->set_always_revalidate(summary.always_revalidate);
 
@@ skipping 273 matching lines @@
 TestObserver::~TestObserver() {
   predictor_->SetObserverForTesting(nullptr);
 }
 
 TestObserver::TestObserver(ResourcePrefetchPredictor* predictor)
     : predictor_(predictor) {
   predictor_->SetObserverForTesting(this);
 }
 
 }  // namespace predictors