predictors: Mark before_first_contentful_paint for resources fetched before fcp.

chrome/browser/predictors/resource_prefetch_predictor_unittest.cc

Index: chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc b/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
index 763076926d1ef95a785eaee4ae3a0df0f0bec6cf..c260e79d6d42c9f65d5ef27437619d37d7c97620 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
+++ b/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
@@ -2092,4 +2092,77 @@ TEST_F(ResourcePrefetchPredictorTest, TestPrefetchingDurationHistogram) {
       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