predictors: Mark before_first_contentful_paint for resources fetched before fcp.

chrome/browser/predictors/resource_prefetch_predictor_unittest.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 <iostream>
 #include <memory>
 #include <utility>
 
@@ skipping 2074 matching lines @@
       content::RESOURCE_TYPE_SCRIPT, 10, 0, 1, 2.1, net::MEDIUM, false, false);
   predictor_->host_table_cache_->insert(
       std::make_pair(google.primary_key(), google));
 
   predictor_->StartPrefetching(GURL(main_frame_url), PrefetchOrigin::EXTERNAL);
   predictor_->StopPrefetching(GURL(main_frame_url));
   histogram_tester_->ExpectTotalCount(
       internal::kResourcePrefetchPredictorPrefetchingDurationHistogram, 1);
 }
 
+TEST_F(ResourcePrefetchPredictorTest, TestRecordFirstContentfulPaint) {
+  using testing::_;
+  EXPECT_CALL(*mock_tables_.get(), UpdateRedirectData(_, _));
+  EXPECT_CALL(*mock_tables_.get(), UpdateOriginData(_));
+
+  const int kVisitCount = 1;
+  AddUrlToHistory("http://www.google.com", kVisitCount);
+
+  auto start = base::TimeTicks::Now();
+  auto second = base::TimeDelta::FromSeconds(1);
+
+  auto res1_time = start + 1 * second;

[alexilin, 2017/04/27 13:16:39]

nit:
You could use base::TimeTicks::FromInternalValue(x) instead of using Now() +
delta.

[trevordixon, 2017/04/28 09:44:06]

Done.

+  auto res2_time = start + 2 * second;
+  auto fcp_time = start + 3 * second;
+  auto res3_time = start + 4 * second;
+
+  URLRequestSummary main_frame =
+      CreateURLRequestSummary(1, "http://www.google.com");
+  predictor_->RecordURLRequest(main_frame);
+  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
+
+  URLRequestSummary resource1 = CreateURLRequestSummary(
+      1, "http://www.google.com", "http://google.com/style1.css",
+      content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", false);
+  resource1.response_time = res1_time;
+  predictor_->RecordURLResponse(resource1);
+  URLRequestSummary resource2 = CreateURLRequestSummary(
+      1, "http://www.google.com", "http://google.com/script1.js",
+      content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false);
+  resource2.response_time = res2_time;
+  predictor_->RecordURLResponse(resource2);
+  URLRequestSummary resource3 = CreateURLRequestSummary(
+      1, "http://www.google.com", "http://google.com/script2.js",
+      content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false);
+  resource3.response_time = res3_time;
+  resource3.before_first_contentful_paint = false;
+  predictor_->RecordURLResponse(resource3);
+
+  predictor_->RecordFirstContentfulPaint(main_frame.navigation_id, fcp_time);
+
+  StrictMock<MockResourcePrefetchPredictorObserver> mock_observer(
+      predictor_.get());
+  EXPECT_CALL(
+      mock_observer,
+      OnNavigationLearned(kVisitCount,
+                          CreatePageRequestSummary(
+                              "http://www.google.com", "http://www.google.com",
+                              {resource1, resource2, resource3})));
+
+  // Reset before_first_contentful_paint so we can see if the predictor can
+  // figure it out.
+  resource3.before_first_contentful_paint = true;

[alexilin, 2017/04/27 13:16:39]

Predictor doesn't keep references to URLRequestSummary that was passed to it as
well as gmock macros. So this change doesn't affect neither the state of the
predictor nor the set expectation. 
To make it work you need to call EXPECT_CALL(mock_observer,...) with correct
parameters first, then you need to reset before_first_contentful_paint and call
predictor_->RecordURLResponse(...) for all resources.

As an alternative, I'd suggest you to not use mock_observer and do not set
before_first_contentful_paint explicitly for any URLRequestSummary. The check
that the data passed to database is correct would be enough.

[trevordixon, 2017/04/28 09:44:06]

OK, removed.

+
+  PrefetchData host_data = CreatePrefetchData("www.google.com");
+  InitializeResourceData(host_data.add_resources(),
+                         "http://google.com/style1.css",
+                         content::RESOURCE_TYPE_STYLESHEET, 1, 0, 0, 1.0,
+                         net::MEDIUM, false, false);
+  InitializeResourceData(
+      host_data.add_resources(), "http://google.com/script1.js",
+      content::RESOURCE_TYPE_SCRIPT, 1, 0, 0, 2.0, net::MEDIUM, false, false);
+  ResourceData* resource3_rd = host_data.add_resources();
+  InitializeResourceData(resource3_rd, "http://google.com/script2.js",
+                         content::RESOURCE_TYPE_SCRIPT, 1, 0, 0, 3.0,
+                         net::MEDIUM, false, false);
+  resource3_rd->set_before_first_contentful_paint(false);
+  EXPECT_CALL(*mock_tables_.get(),
+              UpdateResourceData(host_data, PREFETCH_KEY_TYPE_HOST));
+
+  predictor_->RecordMainFrameLoadComplete(main_frame.navigation_id);
+  profile_->BlockUntilHistoryProcessesPendingRequests();
+}
+
 }  // namespace predictors