predictors: Move more methods from ResourcePrefetchPredictor into LoadingDataCollector.

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 17 matching lines @@
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_job.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::StrictMock;
 using testing::UnorderedElementsAre;
 
 namespace predictors {
 
-using URLRequestSummary = ResourcePrefetchPredictor::URLRequestSummary;
-using PageRequestSummary = ResourcePrefetchPredictor::PageRequestSummary;
 using PrefetchDataMap = std::map<std::string, PrefetchData>;
 using RedirectDataMap = std::map<std::string, RedirectData>;
 using OriginDataMap = std::map<std::string, OriginData>;
 using ManifestDataMap = std::map<std::string, precache::PrecacheManifest>;
 
-template <typename T>
-class FakeGlowplugKeyValueTable : public GlowplugKeyValueTable<T> {
- public:
-  FakeGlowplugKeyValueTable() : GlowplugKeyValueTable<T>("") {}
-  void GetAllData(std::map<std::string, T>* data_map,
-                  sql::Connection* db) const override {
-    *data_map = data_;
-  }
-  void UpdateData(const std::string& key,
-                  const T& data,
-                  sql::Connection* db) override {
-    data_[key] = data;
-  }
-  void DeleteData(const std::vector<std::string>& keys,
-                  sql::Connection* db) override {
-    for (const auto& key : keys)
-      data_.erase(key);
-  }
-  void DeleteAllData(sql::Connection* db) override { data_.clear(); }
-
-  std::map<std::string, T> data_;
-};
-
-class MockResourcePrefetchPredictorTables
-    : public ResourcePrefetchPredictorTables {
- public:
-  MockResourcePrefetchPredictorTables() = default;
-
-  void ScheduleDBTask(const tracked_objects::Location& from_here,
-                      DBTask task) override {
-    ExecuteDBTaskOnDBThread(std::move(task));
-  }
-
-  void ExecuteDBTaskOnDBThread(DBTask task) override {
-    std::move(task).Run(nullptr);
-  }
-
-  GlowplugKeyValueTable<PrefetchData>* url_resource_table() override {
-    return &url_resource_table_;
-  }
-
-  GlowplugKeyValueTable<RedirectData>* url_redirect_table() override {
-    return &url_redirect_table_;
-  }
-
-  GlowplugKeyValueTable<PrefetchData>* host_resource_table() override {
-    return &host_resource_table_;
-  }
-
-  GlowplugKeyValueTable<RedirectData>* host_redirect_table() override {
-    return &host_redirect_table_;
-  }
-
-  GlowplugKeyValueTable<precache::PrecacheManifest>* manifest_table() override {
-    return &manifest_table_;
-  }
-
-  GlowplugKeyValueTable<OriginData>* origin_table() override {
-    return &origin_table_;
-  }
-
-  FakeGlowplugKeyValueTable<PrefetchData> url_resource_table_;
-  FakeGlowplugKeyValueTable<RedirectData> url_redirect_table_;
-  FakeGlowplugKeyValueTable<PrefetchData> host_resource_table_;
-  FakeGlowplugKeyValueTable<RedirectData> host_redirect_table_;
-  FakeGlowplugKeyValueTable<precache::PrecacheManifest> manifest_table_;
-  FakeGlowplugKeyValueTable<OriginData> origin_table_;
-
- protected:
-  ~MockResourcePrefetchPredictorTables() override = default;
-};
-
-class MockResourcePrefetchPredictorObserver : public TestObserver {
- public:
-  explicit MockResourcePrefetchPredictorObserver(
-      ResourcePrefetchPredictor* predictor)
-      : TestObserver(predictor) {}
-
-  MOCK_METHOD2(
-      OnNavigationLearned,
-      void(size_t url_visit_count,
-           const ResourcePrefetchPredictor::PageRequestSummary& summary));
-};
-
 class ResourcePrefetchPredictorTest : public testing::Test {
  public:
   ResourcePrefetchPredictorTest();
   ~ResourcePrefetchPredictorTest() override;
   void SetUp() override;
   void TearDown() override;
 
  protected:
   void AddUrlToHistory(const std::string& url, int visit_count) {
     HistoryServiceFactory::GetForProfile(profile_.get(),
                                          ServiceAccessType::EXPLICIT_ACCESS)->
         AddPageWithDetails(
             GURL(url),
             base::string16(),
             visit_count,
             0,
             base::Time::Now(),
             false,
             history::SOURCE_BROWSED);
     profile_->BlockUntilHistoryProcessesPendingRequests();
   }
 
-  URLRequestSummary CreateRedirectRequestSummary(
-      SessionID::id_type session_id,
-      const std::string& main_frame_url,
-      const std::string& redirect_url) {
-    URLRequestSummary summary =
-        CreateURLRequestSummary(session_id, main_frame_url);
-    summary.redirect_url = GURL(redirect_url);
-    return summary;
-  }
-
   void InitializePredictor() {
     loading_predictor_->StartInitialization();
     base::RunLoop loop;
     loop.RunUntilIdle();  // Runs the DB lookup.
     profile_->BlockUntilHistoryProcessesPendingRequests();
   }
 
   void ResetPredictor(bool small_db = true) {
     LoadingPredictorConfig config;
     PopulateTestConfig(&config, small_db);
@@ skipping 39 matching lines @@
 
   ASSERT_TRUE(profile_->CreateHistoryService(true, false));
   profile_->BlockUntilHistoryProcessesPendingRequests();
   EXPECT_TRUE(HistoryServiceFactory::GetForProfile(
       profile_.get(), ServiceAccessType::EXPLICIT_ACCESS));
   // Initialize the predictor with empty data.
   ResetPredictor();
   EXPECT_EQ(predictor_->initialization_state_,
             ResourcePrefetchPredictor::NOT_INITIALIZED);
   InitializePredictor();
-  EXPECT_TRUE(predictor_->inflight_navigations_.empty());
   EXPECT_EQ(predictor_->initialization_state_,
             ResourcePrefetchPredictor::INITIALIZED);
 
   url_request_job_factory_.Reset();
   url_request_context_.set_job_factory(&url_request_job_factory_);
 
   histogram_tester_.reset(new base::HistogramTester());
 }
 
 void ResourcePrefetchPredictorTest::TearDown() {
@@ skipping 189 matching lines @@
   mock_tables_->host_redirect_table_.data_ = test_host_redirect_data_;
   mock_tables_->manifest_table_.data_ = test_manifest_data_;
   mock_tables_->origin_table_.data_ = test_origin_data_;
 
   ResetPredictor();
   InitializePredictor();
 
   // Test that the internal variables correctly initialized.
   EXPECT_EQ(predictor_->initialization_state_,
             ResourcePrefetchPredictor::INITIALIZED);
-  EXPECT_TRUE(predictor_->inflight_navigations_.empty());
 
   // Integrity of the cache and the backend storage is checked on TearDown.
 }
 
 // Single navigation but history count is low, so should not record url data.
 TEST_F(ResourcePrefetchPredictorTest, NavigationLowHistoryCount) {
   const int kVisitCount = 1;
   AddUrlToHistory("https://www.google.com", kVisitCount);
 
   URLRequestSummary main_frame =
       CreateURLRequestSummary(1, "http://www.google.com");
-  predictor_->RecordURLRequest(main_frame);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
 
   URLRequestSummary main_frame_redirect = CreateRedirectRequestSummary(
       1, "http://www.google.com", "https://www.google.com");
-  predictor_->RecordURLRedirect(main_frame_redirect);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
   main_frame = CreateURLRequestSummary(1, "https://www.google.com");
 
   // Now add a few subresources.
   URLRequestSummary resource1 = CreateURLRequestSummary(
       1, "https://www.google.com", "https://google.com/style1.css",
       content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", false);
-  predictor_->RecordURLResponse(resource1);
   URLRequestSummary resource2 = CreateURLRequestSummary(
       1, "https://www.google.com", "https://google.com/script1.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false);
-  predictor_->RecordURLResponse(resource2);
   URLRequestSummary resource3 = CreateURLRequestSummary(
       1, "https://www.google.com", "https://google.com/script2.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false);
-  predictor_->RecordURLResponse(resource3);
+
+  auto page_summary = CreatePageRequestSummary(
+      "https://www.google.com", "http://www.google.com",
+      {resource1, resource2, resource3});
 
   StrictMock<MockResourcePrefetchPredictorObserver> mock_observer(predictor_);
-  EXPECT_CALL(
-      mock_observer,
-      OnNavigationLearned(kVisitCount,
-                          CreatePageRequestSummary(
-                              "https://www.google.com", "http://www.google.com",
-                              {resource1, resource2, resource3})));
+  EXPECT_CALL(mock_observer, OnNavigationLearned(kVisitCount, page_summary));
 
-  predictor_->RecordMainFrameLoadComplete(main_frame.navigation_id);
+  predictor_->RecordPageRequestSummary(
+      base::MakeUnique<PageRequestSummary>(page_summary));
   profile_->BlockUntilHistoryProcessesPendingRequests();
 
   PrefetchData host_data = CreatePrefetchData("www.google.com");
   InitializeResourceData(host_data.add_resources(),
                          "https://google.com/style1.css",
                          content::RESOURCE_TYPE_STYLESHEET, 1, 0, 0, 1.0,
                          net::MEDIUM, false, false);
   InitializeResourceData(
       host_data.add_resources(), "https://google.com/script1.js",
       content::RESOURCE_TYPE_SCRIPT, 1, 0, 0, 2.0, net::MEDIUM, false, false);
@@ skipping 18 matching lines @@
 }
 
 // Single navigation that will be recorded. Will check for duplicate
 // resources and also for number of resources saved.
 TEST_F(ResourcePrefetchPredictorTest, NavigationUrlNotInDB) {
   const int kVisitCount = 4;
   AddUrlToHistory("http://www.google.com", kVisitCount);
 
   URLRequestSummary main_frame =
       CreateURLRequestSummary(1, "http://www.google.com");
-  predictor_->RecordURLRequest(main_frame);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
 
   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));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/script1.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/script2.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/script1.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", true));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/image1.png",
       content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/image2.png",
       content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/style2.css",
       content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true));
-  predictor_->RecordURLResponse(resources.back());
 
   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);
   no_store.is_no_store = true;
-  predictor_->RecordURLResponse(no_store);
 
   auto redirected = CreateURLRequestSummary(
       1, "http://www.google.com", "http://reader.google.com/style.css",
       content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true);
   redirected.redirect_url = GURL("http://dev.null.google.com/style.css");
 
-  predictor_->RecordURLRedirect(redirected);
+  auto page_summary = CreatePageRequestSummary(
+      "http://www.google.com", "http://www.google.com", resources);
+  page_summary.UpdateOrAddToOrigins(no_store);
+  page_summary.UpdateOrAddToOrigins(redirected);
+
   redirected.is_no_store = true;
   redirected.request_url = redirected.redirect_url;
   redirected.redirect_url = GURL();
-
-  predictor_->RecordURLResponse(redirected);
+  page_summary.UpdateOrAddToOrigins(redirected);
 
   StrictMock<MockResourcePrefetchPredictorObserver> mock_observer(predictor_);
-  EXPECT_CALL(mock_observer,
-              OnNavigationLearned(
-                  kVisitCount, CreatePageRequestSummary("http://www.google.com",
-                                                        "http://www.google.com",
-                                                        resources)));
+  EXPECT_CALL(mock_observer, OnNavigationLearned(kVisitCount, page_summary));
 
-  predictor_->RecordMainFrameLoadComplete(main_frame.navigation_id);
+  predictor_->RecordPageRequestSummary(
+      base::MakeUnique<PageRequestSummary>(page_summary));
   profile_->BlockUntilHistoryProcessesPendingRequests();
 
   PrefetchData url_data = CreatePrefetchData("http://www.google.com/");
   InitializeResourceData(url_data.add_resources(),
                          "http://google.com/style1.css",
                          content::RESOURCE_TYPE_STYLESHEET, 1, 0, 0, 1.0,
                          net::MEDIUM, false, false);
   InitializeResourceData(url_data.add_resources(),
                          "http://google.com/style2.css",
                          content::RESOURCE_TYPE_STYLESHEET, 1, 0, 0, 7.0,
@@ skipping 47 matching lines @@
 
   mock_tables_->url_resource_table_.data_ = test_url_data_;
   mock_tables_->host_resource_table_.data_ = test_host_data_;
 
   ResetPredictor();
   InitializePredictor();
 
   URLRequestSummary main_frame = CreateURLRequestSummary(
       1, "http://www.google.com", "http://www.google.com",
       content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-  predictor_->RecordURLRequest(main_frame);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
 
   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));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/script1.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/script2.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/script1.js",
       content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", true));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/image1.png",
       content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/image2.png",
       content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false));
-  predictor_->RecordURLResponse(resources.back());
   resources.push_back(CreateURLRequestSummary(
       1, "http://www.google.com", "http://google.com/style2.css",
       content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", true));
-  predictor_->RecordURLResponse(resources.back());
   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);
   no_store.is_no_store = true;
-  predictor_->RecordURLResponse(no_store);
+
+  auto page_summary = CreatePageRequestSummary(
+      "http://www.google.com", "http://www.google.com", resources);
+  page_summary.UpdateOrAddToOrigins(no_store);
 
   StrictMock<MockResourcePrefetchPredictorObserver> mock_observer(predictor_);
-  EXPECT_CALL(mock_observer,
-              OnNavigationLearned(
-                  kVisitCount, CreatePageRequestSummary("http://www.google.com",
-                                                        "http://www.google.com",
-                                                        resources)));
+  EXPECT_CALL(mock_observer, OnNavigationLearned(kVisitCount, page_summary));
 
-  predictor_->RecordMainFrameLoadComplete(main_frame.navigation_id);
+  predictor_->RecordPageRequestSummary(
+      base::MakeUnique<PageRequestSummary>(page_summary));
   profile_->BlockUntilHistoryProcessesPendingRequests();
 
   PrefetchData url_data = CreatePrefetchData("http://www.google.com/");
   InitializeResourceData(url_data.add_resources(),
                          "http://google.com/style1.css",
                          content::RESOURCE_TYPE_STYLESHEET, 4, 2, 0, 1.0,
                          net::MEDIUM, false, false);
   InitializeResourceData(url_data.add_resources(),
                          "http://google.com/style2.css",
                          content::RESOURCE_TYPE_STYLESHEET, 1, 0, 0, 7.0,
@@ skipping 62 matching lines @@
   mock_tables_->url_resource_table_.data_ = test_url_data_;
   mock_tables_->host_resource_table_.data_ = test_host_data_;
   mock_tables_->origin_table_.data_ = test_origin_data_;
 
   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);
-  predictor_->RecordURLRequest(main_frame);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
 
   URLRequestSummary resource1 = CreateURLRequestSummary(
       1, "http://www.nike.com", "http://nike.com/style1.css",
       content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", false);
-  predictor_->RecordURLResponse(resource1);
   URLRequestSummary resource2 = CreateURLRequestSummary(
       1, "http://www.nike.com", "http://nike.com/image2.png",
       content::RESOURCE_TYPE_IMAGE, net::MEDIUM, "image/png", false);
-  predictor_->RecordURLResponse(resource2);
+
+  auto page_summary = CreatePageRequestSummary(
+      "http://www.nike.com", "http://www.nike.com", {resource1, resource2});
 
   StrictMock<MockResourcePrefetchPredictorObserver> mock_observer(predictor_);
-  EXPECT_CALL(mock_observer,
-              OnNavigationLearned(
-                  kVisitCount, CreatePageRequestSummary(
-                                   "http://www.nike.com", "http://www.nike.com",
-                                   {resource1, resource2})));
+  EXPECT_CALL(mock_observer, OnNavigationLearned(kVisitCount, page_summary));
 
-  predictor_->RecordMainFrameLoadComplete(main_frame.navigation_id);
+  predictor_->RecordPageRequestSummary(
+      base::MakeUnique<PageRequestSummary>(page_summary));
   profile_->BlockUntilHistoryProcessesPendingRequests();
 
   PrefetchData url_data = CreatePrefetchData("http://www.nike.com/");
   InitializeResourceData(url_data.add_resources(), "http://nike.com/style1.css",
                          content::RESOURCE_TYPE_STYLESHEET, 1, 0, 0, 1.0,
                          net::MEDIUM, false, false);
   InitializeResourceData(url_data.add_resources(), "http://nike.com/image2.png",
                          content::RESOURCE_TYPE_IMAGE, 1, 0, 0, 2.0,
                          net::MEDIUM, false, false);
   PrefetchDataMap expected_url_resource_data = test_url_data_;
@@ skipping 30 matching lines @@
   OriginDataMap expected_origin_data = test_origin_data_;
   expected_origin_data.erase("google.com");
   expected_origin_data["www.nike.com"] = origin_data;
   EXPECT_EQ(mock_tables_->origin_table_.data_, expected_origin_data);
 }
 
 TEST_F(ResourcePrefetchPredictorTest, RedirectUrlNotInDB) {
   const int kVisitCount = 4;
   AddUrlToHistory("https://facebook.com/google", kVisitCount);
 
   URLRequestSummary fb1 = CreateURLRequestSummary(1, "http://fb.com/google");

[alexilin, 2017/06/23 16:55:05]

nit:
fb1, fb2, fb3, fb_end are not used. Feel free to delete.

[trevordixon, 2017/06/27 21:28:55]

Done.

-  predictor_->RecordURLRequest(fb1);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
 
   URLRequestSummary fb2 = CreateRedirectRequestSummary(
       1, "http://fb.com/google", "http://facebook.com/google");
-  predictor_->RecordURLRedirect(fb2);
   URLRequestSummary fb3 = CreateRedirectRequestSummary(
       1, "http://facebook.com/google", "https://facebook.com/google");
-  predictor_->RecordURLRedirect(fb3);
   NavigationID fb_end = CreateNavigationID(1, "https://facebook.com/google");
 
+  auto page_summary = CreatePageRequestSummary(
+      "https://facebook.com/google", "http://fb.com/google",
+      std::vector<URLRequestSummary>());
+
   StrictMock<MockResourcePrefetchPredictorObserver> mock_observer(predictor_);
-  EXPECT_CALL(
-      mock_observer,
-      OnNavigationLearned(kVisitCount, CreatePageRequestSummary(
-                                           "https://facebook.com/google",
-                                           "http://fb.com/google",
-                                           std::vector<URLRequestSummary>())));
+  EXPECT_CALL(mock_observer, OnNavigationLearned(kVisitCount, page_summary));
 
-  predictor_->RecordMainFrameLoadComplete(fb_end);
+  predictor_->RecordPageRequestSummary(
+      base::MakeUnique<PageRequestSummary>(page_summary));
   profile_->BlockUntilHistoryProcessesPendingRequests();
 
   RedirectData url_redirect_data = CreateRedirectData("http://fb.com/google");
   InitializeRedirectStat(url_redirect_data.add_redirect_endpoints(),
                          "https://facebook.com/google", 1, 0, 0);
   EXPECT_EQ(
       mock_tables_->url_redirect_table_.data_,
       RedirectDataMap({{url_redirect_data.primary_key(), url_redirect_data}}));
 
   RedirectData host_redirect_data = CreateRedirectData("fb.com");
   InitializeRedirectStat(host_redirect_data.add_redirect_endpoints(),
                          "facebook.com", 1, 0, 0);
   EXPECT_EQ(mock_tables_->host_redirect_table_.data_,
             RedirectDataMap(
                 {{host_redirect_data.primary_key(), host_redirect_data}}));
 }
 
 // Tests that redirect is recorded correctly for URL already present in
 // the database cache.
 TEST_F(ResourcePrefetchPredictorTest, RedirectUrlInDB) {
   const int kVisitCount = 7;
   AddUrlToHistory("https://facebook.com/google", kVisitCount);
 
   mock_tables_->url_redirect_table_.data_ = test_url_redirect_data_;
   mock_tables_->host_redirect_table_.data_ = test_host_redirect_data_;
 
   ResetPredictor();
   InitializePredictor();
 
   URLRequestSummary fb1 = CreateURLRequestSummary(1, "http://fb.com/google");

[alexilin, 2017/06/23 16:55:05]

nit:
ditto

[trevordixon, 2017/06/27 21:28:55]

Done.

-  predictor_->RecordURLRequest(fb1);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
 
   URLRequestSummary fb2 = CreateRedirectRequestSummary(
       1, "http://fb.com/google", "http://facebook.com/google");
-  predictor_->RecordURLRedirect(fb2);
   URLRequestSummary fb3 = CreateRedirectRequestSummary(
       1, "http://facebook.com/google", "https://facebook.com/google");
-  predictor_->RecordURLRedirect(fb3);
   NavigationID fb_end = CreateNavigationID(1, "https://facebook.com/google");
 
+  auto page_summary = CreatePageRequestSummary(
+      "https://facebook.com/google", "http://fb.com/google",
+      std::vector<URLRequestSummary>());
+
   StrictMock<MockResourcePrefetchPredictorObserver> mock_observer(predictor_);
-  EXPECT_CALL(
-      mock_observer,
-      OnNavigationLearned(kVisitCount, CreatePageRequestSummary(
-                                           "https://facebook.com/google",
-                                           "http://fb.com/google",
-                                           std::vector<URLRequestSummary>())));
+  EXPECT_CALL(mock_observer, OnNavigationLearned(kVisitCount, page_summary));
 
-  predictor_->RecordMainFrameLoadComplete(fb_end);
+  predictor_->RecordPageRequestSummary(
+      base::MakeUnique<PageRequestSummary>(page_summary));
   profile_->BlockUntilHistoryProcessesPendingRequests();
 
   RedirectData url_redirect_data = CreateRedirectData("http://fb.com/google");
   InitializeRedirectStat(url_redirect_data.add_redirect_endpoints(),
                          "https://facebook.com/google", 6, 1, 0);
   // Existing redirect to https://facebook.com/login will be deleted because of
   // too many consecutive misses.
   RedirectDataMap expected_url_redirect_data = test_url_redirect_data_;
   expected_url_redirect_data[url_redirect_data.primary_key()] =
       url_redirect_data;
@@ skipping 225 matching lines @@
   EXPECT_EQ(mock_tables_->manifest_table_.data_, manifests);
 
   predictor_->DeleteAllUrls();
   EXPECT_TRUE(mock_tables_->url_resource_table_.data_.empty());
   EXPECT_TRUE(mock_tables_->host_resource_table_.data_.empty());
   EXPECT_TRUE(mock_tables_->url_redirect_table_.data_.empty());
   EXPECT_TRUE(mock_tables_->host_redirect_table_.data_.empty());
   EXPECT_TRUE(mock_tables_->manifest_table_.data_.empty());
 }
 
-TEST_F(ResourcePrefetchPredictorTest, OnMainFrameRequest) {
-  URLRequestSummary summary1 = CreateURLRequestSummary(
-      1, "http://www.google.com", "http://www.google.com",
-      content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-  URLRequestSummary summary2 = CreateURLRequestSummary(
-      2, "http://www.google.com", "http://www.google.com",
-      content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-  URLRequestSummary summary3 = CreateURLRequestSummary(
-      3, "http://www.yahoo.com", "http://www.yahoo.com",
-      content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-
-  predictor_->OnMainFrameRequest(summary1);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
-  predictor_->OnMainFrameRequest(summary2);
-  EXPECT_EQ(2U, predictor_->inflight_navigations_.size());
-  predictor_->OnMainFrameRequest(summary3);
-  EXPECT_EQ(3U, predictor_->inflight_navigations_.size());
-
-  // Insert another with same navigation id. It should replace.
-  URLRequestSummary summary4 = CreateURLRequestSummary(
-      1, "http://www.nike.com", "http://www.nike.com",
-      content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-  URLRequestSummary summary5 = CreateURLRequestSummary(
-      2, "http://www.google.com", "http://www.google.com",
-      content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-
-  predictor_->OnMainFrameRequest(summary4);
-  EXPECT_EQ(3U, predictor_->inflight_navigations_.size());
-
-  // Change this creation time so that it will go away on the next insert.
-  summary5.navigation_id.creation_time = base::TimeTicks::Now() -
-      base::TimeDelta::FromDays(1);
-  predictor_->OnMainFrameRequest(summary5);
-  EXPECT_EQ(3U, predictor_->inflight_navigations_.size());
-
-  URLRequestSummary summary6 = CreateURLRequestSummary(
-      4, "http://www.shoes.com", "http://www.shoes.com",
-      content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-  predictor_->OnMainFrameRequest(summary6);
-  EXPECT_EQ(3U, predictor_->inflight_navigations_.size());
-
-  EXPECT_TRUE(predictor_->inflight_navigations_.find(summary3.navigation_id) !=
-              predictor_->inflight_navigations_.end());
-  EXPECT_TRUE(predictor_->inflight_navigations_.find(summary4.navigation_id) !=
-              predictor_->inflight_navigations_.end());
-  EXPECT_TRUE(predictor_->inflight_navigations_.find(summary6.navigation_id) !=
-              predictor_->inflight_navigations_.end());
-}
-
-TEST_F(ResourcePrefetchPredictorTest, OnMainFrameRedirect) {
-  URLRequestSummary yahoo = CreateURLRequestSummary(1, "http://yahoo.com");
-
-  URLRequestSummary bbc1 = CreateURLRequestSummary(2, "http://bbc.com");
-  URLRequestSummary bbc2 =
-      CreateRedirectRequestSummary(2, "http://bbc.com", "https://www.bbc.com");
-  NavigationID bbc_end = CreateNavigationID(2, "https://www.bbc.com");
-
-  URLRequestSummary youtube1 = CreateURLRequestSummary(3, "http://youtube.com");
-  URLRequestSummary youtube2 = CreateRedirectRequestSummary(
-      3, "http://youtube.com", "https://youtube.com");
-  NavigationID youtube_end = CreateNavigationID(3, "https://youtube.com");
-
-  URLRequestSummary nyt1 = CreateURLRequestSummary(4, "http://nyt.com");
-  URLRequestSummary nyt2 =
-      CreateRedirectRequestSummary(4, "http://nyt.com", "http://nytimes.com");
-  URLRequestSummary nyt3 = CreateRedirectRequestSummary(4, "http://nytimes.com",
-                                                        "http://m.nytimes.com");
-  NavigationID nyt_end = CreateNavigationID(4, "http://m.nytimes.com");
-
-  URLRequestSummary fb1 = CreateURLRequestSummary(5, "http://fb.com");
-  URLRequestSummary fb2 =
-      CreateRedirectRequestSummary(5, "http://fb.com", "http://facebook.com");
-  URLRequestSummary fb3 = CreateRedirectRequestSummary(5, "http://facebook.com",
-                                                       "https://facebook.com");
-  URLRequestSummary fb4 = CreateRedirectRequestSummary(
-      5, "https://facebook.com",
-      "https://m.facebook.com/?refsrc=https%3A%2F%2Fwww.facebook.com%2F&_rdr");
-  NavigationID fb_end = CreateNavigationID(
-      5,
-      "https://m.facebook.com/?refsrc=https%3A%2F%2Fwww.facebook.com%2F&_rdr");
-
-  // Redirect with empty redirect_url will be deleted.
-  predictor_->OnMainFrameRequest(yahoo);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
-  predictor_->OnMainFrameRedirect(yahoo);
-  EXPECT_TRUE(predictor_->inflight_navigations_.empty());
-
-  // Redirect without previous request works fine.
-  // predictor_->OnMainFrameRequest(bbc1) missing.
-  predictor_->OnMainFrameRedirect(bbc2);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
-  EXPECT_EQ(bbc1.navigation_id.main_frame_url,
-            predictor_->inflight_navigations_[bbc_end]->initial_url);
-
-  // http://youtube.com -> https://youtube.com.
-  predictor_->OnMainFrameRequest(youtube1);
-  EXPECT_EQ(2U, predictor_->inflight_navigations_.size());
-  predictor_->OnMainFrameRedirect(youtube2);
-  EXPECT_EQ(2U, predictor_->inflight_navigations_.size());
-  EXPECT_EQ(youtube1.navigation_id.main_frame_url,
-            predictor_->inflight_navigations_[youtube_end]->initial_url);
-
-  // http://nyt.com -> http://nytimes.com -> http://m.nytimes.com.
-  predictor_->OnMainFrameRequest(nyt1);
-  EXPECT_EQ(3U, predictor_->inflight_navigations_.size());
-  predictor_->OnMainFrameRedirect(nyt2);
-  predictor_->OnMainFrameRedirect(nyt3);
-  EXPECT_EQ(3U, predictor_->inflight_navigations_.size());
-  EXPECT_EQ(nyt1.navigation_id.main_frame_url,
-            predictor_->inflight_navigations_[nyt_end]->initial_url);
-
-  // http://fb.com -> http://facebook.com -> https://facebook.com ->
-  // https://m.facebook.com/?refsrc=https%3A%2F%2Fwww.facebook.com%2F&_rdr.
-  predictor_->OnMainFrameRequest(fb1);
-  EXPECT_EQ(4U, predictor_->inflight_navigations_.size());
-  predictor_->OnMainFrameRedirect(fb2);
-  predictor_->OnMainFrameRedirect(fb3);
-  predictor_->OnMainFrameRedirect(fb4);
-  EXPECT_EQ(4U, predictor_->inflight_navigations_.size());
-  EXPECT_EQ(fb1.navigation_id.main_frame_url,
-            predictor_->inflight_navigations_[fb_end]->initial_url);
-}
-
-TEST_F(ResourcePrefetchPredictorTest, OnSubresourceResponse) {
-  // If there is no inflight navigation, nothing happens.
-  URLRequestSummary resource1 = CreateURLRequestSummary(
-      1, "http://www.google.com", "http://google.com/style1.css",
-      content::RESOURCE_TYPE_STYLESHEET, net::MEDIUM, "text/css", false);
-  predictor_->OnSubresourceResponse(resource1);
-  EXPECT_TRUE(predictor_->inflight_navigations_.empty());
-
-  // Add an inflight navigation.
-  URLRequestSummary main_frame1 = CreateURLRequestSummary(
-      1, "http://www.google.com", "http://www.google.com",
-      content::RESOURCE_TYPE_MAIN_FRAME, net::MEDIUM, std::string(), false);
-  predictor_->OnMainFrameRequest(main_frame1);
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
-
-  // Now add a few subresources.
-  URLRequestSummary resource2 = CreateURLRequestSummary(
-      1, "http://www.google.com", "http://google.com/script1.js",
-      content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false);
-  URLRequestSummary resource3 = CreateURLRequestSummary(
-      1, "http://www.google.com", "http://google.com/script2.js",
-      content::RESOURCE_TYPE_SCRIPT, net::MEDIUM, "text/javascript", false);
-  predictor_->OnSubresourceResponse(resource1);
-  predictor_->OnSubresourceResponse(resource2);
-  predictor_->OnSubresourceResponse(resource3);
-
-  EXPECT_EQ(1U, predictor_->inflight_navigations_.size());
-  EXPECT_EQ(3U, predictor_->inflight_navigations_[main_frame1.navigation_id]
-                    ->subresource_requests.size());
-  EXPECT_EQ(resource1,
-            predictor_->inflight_navigations_[main_frame1.navigation_id]
-                ->subresource_requests[0]);
-  EXPECT_EQ(resource2,
-            predictor_->inflight_navigations_[main_frame1.navigation_id]
-                ->subresource_requests[1]);
-  EXPECT_EQ(resource3,
-            predictor_->inflight_navigations_[main_frame1.navigation_id]
-                ->subresource_requests[2]);
-}
-
 TEST_F(ResourcePrefetchPredictorTest, SummarizeResponse) {

[alexilin, 2017/06/23 16:55:05]

nit:
LoadingDataCollectorTest is the better place for all Summarize* tests.

[trevordixon, 2017/06/27 21:28:55]

Done.

   net::HttpResponseInfo response_info;
   response_info.headers =
       MakeResponseHeaders("HTTP/1.1 200 OK\n\nSome: Headers\n");
   response_info.was_cached = true;
   url_request_job_factory_.set_response_info(response_info);
 
   GURL url("http://www.google.com/cat.png");
   std::unique_ptr<net::URLRequest> request =
       CreateURLRequest(url_request_context_, url, net::MEDIUM,
                        content::RESOURCE_TYPE_IMAGE, true);
@@ skipping 295 matching lines @@
                          content::RESOURCE_TYPE_FONT_RESOURCE, 10, 0, 1, 2.1,
                          net::MEDIUM, false, false);
   predictor_->url_resource_data_->UpdateData(www_google_url.primary_key(),
                                              www_google_url);
 
   urls.clear();
   EXPECT_TRUE(predictor_->GetPrefetchData(main_frame_url, &prediction));
   EXPECT_THAT(urls, UnorderedElementsAre(GURL(font_url)));
 }
 
-TEST_F(ResourcePrefetchPredictorTest, TestRecordFirstContentfulPaint) {

[alexilin, 2017/06/23 16:55:05]

nit:
Could you add some simple test to check that FCP is actually written to the
database?

[trevordixon, 2017/06/27 21:28:55]

Still working on this.

-  auto res1_time = base::TimeTicks::FromInternalValue(1);
-  auto res2_time = base::TimeTicks::FromInternalValue(2);
-  auto fcp_time = base::TimeTicks::FromInternalValue(3);
-  auto res3_time = base::TimeTicks::FromInternalValue(4);
-
-  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;
-  predictor_->RecordURLResponse(resource3);
-
-  predictor_->RecordFirstContentfulPaint(main_frame.navigation_id, fcp_time);
-
-  predictor_->RecordMainFrameLoadComplete(main_frame.navigation_id);
-  profile_->BlockUntilHistoryProcessesPendingRequests();
-
-  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_EQ(mock_tables_->host_resource_table_.data_,
-            PrefetchDataMap({{host_data.primary_key(), host_data}}));
-}
-
 }  // namespace predictors