predictors: Refactor resource_prefetch_predictor triggering.

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>
 
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
 #include "base/time/time.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor_tables.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h"
+#include "chrome/browser/predictors/resource_prefetcher_manager.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/sessions/core/session_id.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::ContainerEq;
 using testing::Pointee;
 using testing::SetArgPointee;
 using testing::StrictMock;
 using testing::UnorderedElementsAre;
 
 namespace predictors {
 
+namespace {
+constexpr int kDefaultMode = LoadingPredictorConfig::LEARNING |
+                             LoadingPredictorConfig::PREFETCHING_FOR_EXTERNAL;
+}  // namespace
+
 using URLRequestSummary = ResourcePrefetchPredictor::URLRequestSummary;
 using PageRequestSummary = ResourcePrefetchPredictor::PageRequestSummary;
 using PrefetchDataMap = ResourcePrefetchPredictorTables::PrefetchDataMap;
 using RedirectDataMap = ResourcePrefetchPredictorTables::RedirectDataMap;
 using ManifestDataMap = ResourcePrefetchPredictorTables::ManifestDataMap;
 using OriginDataMap = ResourcePrefetchPredictorTables::OriginDataMap;
 
 scoped_refptr<net::HttpResponseHeaders> MakeResponseHeaders(
     const char* headers) {
   return make_scoped_refptr(new net::HttpResponseHeaders(
@@ skipping 117 matching lines @@
   explicit MockResourcePrefetchPredictorObserver(
       ResourcePrefetchPredictor* predictor)
       : TestObserver(predictor) {}
 
   MOCK_METHOD2(
       OnNavigationLearned,
       void(size_t url_visit_count,
            const ResourcePrefetchPredictor::PageRequestSummary& summary));
 };
 
+// Does nothing when a prefetch starts.
+class MockResourcePrefetcherManager : public ResourcePrefetcherManager {
+ public:
+  using ResourcePrefetcherManager::ResourcePrefetcherManager;
+  MOCK_METHOD2(MaybeAddPrefetch,
+               void(const GURL& main_frame_url, const std::vector<GURL>& urls));
+
+ protected:
+  ~MockResourcePrefetcherManager() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockResourcePrefetcherManager);
+};
+
 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(),
@@ skipping 33 matching lines @@
         false, true, content::PREVIEWS_OFF);
     request->Start();
     return request;
   }
 
   void InitializePredictor() {
     predictor_->StartInitialization();
     base::RunLoop loop;
     loop.RunUntilIdle();  // Runs the DB lookup.
     profile_->BlockUntilHistoryProcessesPendingRequests();
+
+    mock_prefetch_manager_ = nullptr;
+    if (predictor_->prefetch_manager_) {
+      mock_prefetch_manager_ = new StrictMock<MockResourcePrefetcherManager>(
+          predictor_.get(), predictor_->config_,
+          predictor_->profile_->GetRequestContext());
+      predictor_->set_mock_resource_prefetcher_manager(mock_prefetch_manager_);
+    }
   }
 
-  void ResetPredictor() {
+  void ResetPredictor(int mode = kDefaultMode) {

[alexilin, 2017/05/23 11:01:39]

It changes the current behavior of tests. Why not leave the default mode and
enable PREFETCHING if it's needed? 

[Benoit L, 2017/05/29 16:45:14]

Done.

     LoadingPredictorConfig config;
     config.max_urls_to_track = 3;
     config.max_hosts_to_track = 2;
     config.min_url_visit_count = 2;
     config.max_resources_per_entry = 4;
     config.max_consecutive_misses = 2;
     config.max_redirect_consecutive_misses = 2;
     config.min_resource_confidence_to_trigger_prefetch = 0.5;
     config.is_url_learning_enabled = true;
     config.is_manifests_enabled = true;
     config.is_origin_learning_enabled = true;
+    config.mode = mode;
 
-    config.mode |= LoadingPredictorConfig::LEARNING;
-    predictor_.reset(new ResourcePrefetchPredictor(config, profile_.get()));
+    predictor_ =
+        base::MakeUnique<ResourcePrefetchPredictor>(config, profile_.get());
     predictor_->set_mock_tables(mock_tables_);
   }
 
   void InitializeSampleData();
   void TestRedirectStatusHistogram(
       const std::string& predictor_initial_key,
       const std::string& predictor_key,
       const std::string& navigation_initial_url,
       const std::string& navigation_url,
       ResourcePrefetchPredictor::RedirectStatus expected_status);
 
   content::TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<TestingProfile> profile_;
   net::TestURLRequestContext url_request_context_;
 
   std::unique_ptr<ResourcePrefetchPredictor> predictor_;
   scoped_refptr<StrictMock<MockResourcePrefetchPredictorTables>> mock_tables_;
+  scoped_refptr<StrictMock<MockResourcePrefetcherManager>>
+      mock_prefetch_manager_;
 
   PrefetchDataMap test_url_data_;
   PrefetchDataMap test_host_data_;
   RedirectDataMap test_url_redirect_data_;
   RedirectDataMap test_host_redirect_data_;
   ManifestDataMap test_manifest_data_;
   OriginDataMap test_origin_data_;
 
   MockURLRequestJobFactory url_request_job_factory_;
   EmptyURLRequestDelegate url_request_delegate_;
@@ skipping 1814 matching lines @@
 
 TEST_F(ResourcePrefetchPredictorTest,
        TestRedirectStatusRedirectCorrectlyPredicted) {
   TestRedirectStatusHistogram(
       "google.com", "www.google.com", "http://google.com?query=cats",
       "http://www.google.com?query=cats",
       ResourcePrefetchPredictor::RedirectStatus::REDIRECT_CORRECTLY_PREDICTED);
 }
 
 TEST_F(ResourcePrefetchPredictorTest, TestPrefetchingDurationHistogram) {
-  // Prefetching duration for an url without resources in the database

[alexilin, 2017/05/23 11:01:39]

Why changing this test?
We wanted to record prefetching duration even when prefetching is disabled.

-  // shouldn't be recorded.
-  const std::string main_frame_url = "http://google.com/?query=cats";
-  predictor_->StartPrefetching(GURL(main_frame_url), HintOrigin::EXTERNAL);
-  predictor_->StopPrefetching(GURL(main_frame_url));
-  histogram_tester_->ExpectTotalCount(
-      internal::kResourcePrefetchPredictorPrefetchingDurationHistogram, 0);
+  // Fill the database to record a duration.
+  for (const auto& kv : test_host_data_)

[alexilin, 2017/05/23 11:01:39]

nit:
predictor_->host_table_cache_ = test_host_data_;

+    predictor_->host_table_cache_->insert(kv);
 
-  // Fill the database to record a duration.
-  PrefetchData google = CreatePrefetchData("google.com", 1);
-  InitializeResourceData(
-      google.add_resources(), "https://cdn.google.com/script.js",
-      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));
+  using testing::_;
+  EXPECT_CALL(*mock_prefetch_manager_.get(), MaybeAddPrefetch(_, _));
 
-  predictor_->StartPrefetching(GURL(main_frame_url), HintOrigin::EXTERNAL);
-  predictor_->StopPrefetching(GURL(main_frame_url));
+  const GURL url = GURL("http://www.facebook.com/cats");
+  predictor_->StartPrefetching(url, HintOrigin::EXTERNAL);
+  predictor_->StopPrefetching(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(_));
 
   auto res1_time = base::TimeTicks::FromInternalValue(1);
@@ skipping 37 matching lines @@
                          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();
 }
 
+TEST_F(ResourcePrefetchPredictorTest, TestCanPrefetchUrlForOrigin) {
+  for (const auto& kv : test_host_data_)

[alexilin, 2017/05/23 11:01:39]

ditto

+    predictor_->host_table_cache_->insert(kv);
+
+  const GURL url = GURL("http://www.facebook.com/cats");
+  EXPECT_TRUE(predictor_->CanPrefetchUrlForOrigin(url, HintOrigin::EXTERNAL));
+
+  // Unknown host.
+  EXPECT_FALSE(predictor_->CanPrefetchUrlForOrigin(GURL("https://unknown.com"),
+                                                   HintOrigin::EXTERNAL));
+  // Wrong origin.
+  EXPECT_FALSE(
+      predictor_->CanPrefetchUrlForOrigin(url, HintOrigin::NAVIGATION));
+
+  // Prefetching not enabled.
+  EXPECT_CALL(*mock_tables_.get(),
+              GetAllData(Pointee(ContainerEq(PrefetchDataMap())),
+                         Pointee(ContainerEq(PrefetchDataMap())),
+                         Pointee(ContainerEq(RedirectDataMap())),
+                         Pointee(ContainerEq(RedirectDataMap())),
+                         Pointee(ContainerEq(ManifestDataMap())),
+                         Pointee(ContainerEq(OriginDataMap()))));
+  ResetPredictor(LoadingPredictorConfig::LEARNING);
+  InitializePredictor();
+  for (const auto& kv : test_host_data_)

[alexilin, 2017/05/23 11:01:39]

ditto

+    predictor_->host_table_cache_->insert(kv);
+
+  EXPECT_FALSE(predictor_->CanPrefetchUrlForOrigin(url, HintOrigin::EXTERNAL));
+}
+
 }  // namespace predictors