predictors: Make speculative_prefetch_predictor work with PlzNavigate

chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

 // Copyright 2016 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 <cstddef>
+#include <set>
+
 #include "base/command_line.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor_factory.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
-#include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/render_process_host.h"
+#include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace predictors {
 
 namespace {
 
 const char kImageMime[] = "image/png";
@@ skipping 76 matching lines @@
       std::unique(subresources->begin(), subresources->end(),
                   [](const URLRequestSummary& x, const URLRequestSummary& y) {
                     return x.resource_url == y.resource_url;
                   }),
       subresources->end());
 }
 
 // Fill a NavigationID with "empty" data that does not trigger
 // the is_valid DCHECK(). Allows comparing.
 void SetValidNavigationID(NavigationID* navigation_id) {
-  navigation_id->render_process_id = 0;
-  navigation_id->render_frame_id = 0;
+  navigation_id->session_id = 0;
   navigation_id->main_frame_url = GURL("http://127.0.0.1");
 }
 
 void ModifySubresourceForComparison(URLRequestSummary* subresource,
                                     bool match_navigation_id) {
   if (!match_navigation_id)
     SetValidNavigationID(&subresource->navigation_id);
   if (subresource->resource_type == content::RESOURCE_TYPE_IMAGE &&
       subresource->priority == net::LOWEST) {
     // Fuzzy comparison for images because an image priority can be
@@ skipping 38 matching lines @@
         url_visit_count_(expected_url_visit_count),
         summary_(expected_summary),
         match_navigation_id_(match_navigation_id) {}
 
   // TestObserver:
   void OnNavigationLearned(size_t url_visit_count,
                            const PageRequestSummary& summary) override {
     EXPECT_EQ(url_visit_count, url_visit_count_);
     EXPECT_EQ(summary.main_frame_url, summary_.main_frame_url);
     EXPECT_EQ(summary.initial_url, summary_.initial_url);
+    for (const auto& resource : summary.subresource_requests)
+      current_navigation_ids_.insert(resource.navigation_id);
     CompareSubresources(summary.subresource_requests,
                         summary_.subresource_requests, match_navigation_id_);
     run_loop_.Quit();
   }
 
   void Wait() { run_loop_.Run(); }
 
+  std::set<NavigationID>& navigation_ids() { return current_navigation_ids_; }

[Benoit L, 2016/12/20 13:20:49]

nit: This is not enforced by the style guide, but I prefer it when the accessor
has the same name as the member variable (minus the trailing _).

[ahemery, 2016/12/20 14:06:35]

Done.

+
  private:
   base::RunLoop run_loop_;
   size_t url_visit_count_;
   PageRequestSummary summary_;
   bool match_navigation_id_;
+  std::set<NavigationID> current_navigation_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(LearningObserver);
 };
 
 // Helper class to track and allow waiting for a single OnPrefetchingFinished
 // event. No learning events should be fired while this observer is active.
 class PrefetchingObserver : public TestObserver {
  public:
   PrefetchingObserver(ResourcePrefetchPredictor* predictor,
                       const GURL& expected_main_frame_url)
@@ skipping 23 matching lines @@
  protected:
   using URLRequestSummary = ResourcePrefetchPredictor::URLRequestSummary;
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitchASCII(
         switches::kSpeculativeResourcePrefetching,
         switches::kSpeculativeResourcePrefetchingEnabled);
   }
 
   void SetUpOnMainThread() override {
+    set_current_host_name("127.0.0.1");
+    host_resolver()->AddRule("*", "127.0.0.1");

[Benoit L, 2016/12/20 13:20:49]

nit: Can you add a comment saying why this is needed?
I guess it's for multi-origin loads, right?

[ahemery, 2016/12/20 14:06:35]

Done.

     embedded_test_server()->RegisterRequestHandler(
         base::Bind(&ResourcePrefetchPredictorBrowserTest::HandleRedirectRequest,
                    base::Unretained(this)));
     embedded_test_server()->RegisterRequestHandler(
         base::Bind(&ResourcePrefetchPredictorBrowserTest::HandleResourceRequest,
                    base::Unretained(this)));
     ASSERT_TRUE(embedded_test_server()->Start());
     predictor_ =
         ResourcePrefetchPredictorFactory::GetForProfile(browser()->profile());
     ASSERT_TRUE(predictor_);
@@ skipping 17 matching lines @@
       const GURL& main_frame_url,
       WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB) {
     GURL endpoint_url = GetRedirectEndpoint(main_frame_url);
     std::vector<URLRequestSummary> url_request_summaries;
     for (const auto& kv : resources_) {
       if (kv.second.is_no_store || !kv.second.should_be_recorded)
         continue;
       url_request_summaries.push_back(
           GetURLRequestSummaryForResource(endpoint_url, kv.second));
     }
+
+    bool match_navigation_id = true;
+    if (disposition != WindowOpenDisposition::CURRENT_TAB)
+      match_navigation_id = false;
+
     LearningObserver observer(
         predictor_, UpdateAndGetVisitCount(main_frame_url),
         CreatePageRequestSummary(endpoint_url.spec(), main_frame_url.spec(),
                                  url_request_summaries),
-        true);  // Matching navigation id by default
+        match_navigation_id);
     ui_test_utils::NavigateToURLWithDisposition(
         browser(), main_frame_url, disposition,
         ui_test_utils::BROWSER_TEST_NONE);
     observer.Wait();
     for (auto& kv : resources_) {
       if (!kv.second.is_no_store && kv.second.should_be_recorded)
         kv.second.request.was_cached = true;
     }
+    for (const auto& nav : observer.navigation_ids())
+      navigation_id_history_.insert(nav);
   }
 
   void PrefetchURL(const GURL& main_frame_url) {
     PrefetchingObserver observer(predictor_, main_frame_url);
     predictor_->StartPrefetching(main_frame_url, PrefetchOrigin::EXTERNAL);
     observer.Wait();
     for (auto& kv : resources_) {
       if (!kv.second.is_no_store && kv.second.should_be_recorded)
         kv.second.request.was_cached = true;
     }
@@ skipping 35 matching lines @@
                         const std::vector<RedirectEdge>& redirect_chain) {
     ASSERT_FALSE(redirect_chain.empty());
     GURL current = initial_url;
     for (const auto& edge : redirect_chain) {
       auto result = redirects_.insert(std::make_pair(current, edge));
       EXPECT_TRUE(result.second) << current << " already has a redirect.";
       current = edge.url;
     }
   }
 
+  void ClearResources() { resources_.clear(); }
+
   void ClearCache() {
     chrome::ClearCache(browser());
     for (auto& kv : resources_)
       kv.second.request.was_cached = false;
   }
 
   // Shortcut for convenience.
   GURL GetURL(const std::string& path) const {
-    return embedded_test_server()->GetURL(path);
+    return embedded_test_server()->GetURL(current_host_name_, path);
   }
 
   void EnableHttpsServer() {
     ASSERT_FALSE(https_server_);
     https_server_ = base::MakeUnique<net::EmbeddedTestServer>(
         net::EmbeddedTestServer::TYPE_HTTPS);
     https_server()->AddDefaultHandlers(
         base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
     https_server()->RegisterRequestHandler(
         base::Bind(&ResourcePrefetchPredictorBrowserTest::HandleRedirectRequest,
                    base::Unretained(this)));
     https_server()->RegisterRequestHandler(
         base::Bind(&ResourcePrefetchPredictorBrowserTest::HandleResourceRequest,
                    base::Unretained(this)));
     ASSERT_TRUE(https_server()->Start());
   }
 
   // Returns the embedded test server working over HTTPS. Must be enabled by
   // calling EnableHttpsServer() before use.
   const net::EmbeddedTestServer* https_server() const {
     return https_server_.get();
   }
 
   net::EmbeddedTestServer* https_server() { return https_server_.get(); }
 
+  void set_current_host_name(const std::string& new_host) {
+    current_host_name_ = new_host;
+  }
+
+  std::size_t get_navigation_ids_history_size() {

[Benoit L, 2016/12/20 13:20:49]

nit: getters don't have the initial "get_" in the name.

[ahemery, 2016/12/20 14:06:35]

Done.

+    return navigation_id_history_.size();
+  }
+
  private:
   // ResourcePrefetchPredictor needs to be initialized before the navigation
   // happens otherwise this navigation will be ignored by predictor.
   void EnsurePredictorInitialized() {
     if (predictor_->initialization_state_ ==
         ResourcePrefetchPredictor::INITIALIZED) {
       return;
     }
 
     InitializationObserver observer(predictor_);
     if (predictor_->initialization_state_ ==
         ResourcePrefetchPredictor::NOT_INITIALIZED) {
       predictor_->StartInitialization();
     }
     observer.Wait();
   }
 
   URLRequestSummary GetURLRequestSummaryForResource(
       const GURL& main_frame_url,
       const ResourceSummary& resource_summary) const {
     URLRequestSummary summary(resource_summary.request);
     content::WebContents* web_contents =
         browser()->tab_strip_model()->GetActiveWebContents();
-    int process_id = web_contents->GetRenderProcessHost()->GetID();
-    int frame_id = web_contents->GetMainFrame()->GetRoutingID();
     summary.navigation_id =
-        CreateNavigationID(process_id, frame_id, main_frame_url.spec());
+        NavigationID(web_contents, main_frame_url, base::TimeTicks::Now());
     return summary;
   }
 
   GURL GetRedirectEndpoint(const GURL& initial_url) const {
     GURL current = initial_url;
     while (true) {
       auto it = redirects_.find(current);
       if (it == redirects_.end())
         break;
       current = it->second.url;
     }
     return current;
   }
 
   std::unique_ptr<net::test_server::HttpResponse> HandleResourceRequest(
       const net::test_server::HttpRequest& request) const {
-    auto resource_it = resources_.find(request.GetURL());
+    GURL resource_url = request.GetURL();
+    GURL::Replacements replace_host;
+    replace_host.SetHostStr(current_host_name_);
+    resource_url = resource_url.ReplaceComponents(replace_host);
+
+    auto resource_it = resources_.find(resource_url);
     if (resource_it == resources_.end())
       return nullptr;
 
     const ResourceSummary& summary = resource_it->second;
     if (summary.is_external)
       return nullptr;
 
     auto http_response =
         base::MakeUnique<net::test_server::BasicHttpResponse>();
     http_response->set_code(net::HTTP_OK);
@@ skipping 21 matching lines @@
     if (redirect_it == redirects_.end())
       return nullptr;
 
     auto http_response =
         base::MakeUnique<net::test_server::BasicHttpResponse>();
     http_response->set_code(redirect_it->second.code);
     http_response->AddCustomHeader("Location", redirect_it->second.url.spec());
     return std::move(http_response);
   }
 
-  size_t UpdateAndGetVisitCount(const GURL& main_frame_url) {
+  std::size_t UpdateAndGetVisitCount(const GURL& main_frame_url) {
     return ++visit_count_[main_frame_url];
   }
 
+  std::string current_host_name_;
   ResourcePrefetchPredictor* predictor_;
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
   std::map<GURL, ResourceSummary> resources_;
   std::map<GURL, RedirectEdge> redirects_;
   std::map<GURL, size_t> visit_count_;
+  std::set<NavigationID> navigation_id_history_;
+};
+
+// This browser test override is specifically used to have ONLY

[Benoit L, 2016/12/20 13:20:49]

What is the issue here?

[ahemery, 2016/12/20 14:06:35]

After more than 2 calls to NavigateToURLAndCheckSubresources(...) the predictor
actually starts prefetching the resources, tagging them as "was_cached = true"
and causing mismatches in the subsequent calls. Alternative would be another
boolean match_caching_information. We would still need ClearCache() calls
because server caching is trickier (fills in mime type, etc.)

[Benoit L, 2016/12/21 12:38:25]

Not is you set the flags to "external only". This flag can be set everywhere,
making the code simpler.

[alexilin, 2016/12/21 13:08:12]

To clarify: are you saying that we can change this switch directly in the test
code? I'm not sure that this is true, because we have to set it before the
ResourcePrefetchPredictor creation.

+// the learning part of the prefetcher without the actual prefetching
+// Used to avoid having caching issues for matcher
+class ResourcePrefetchPredictorLearningBrowserTest
+    : public ResourcePrefetchPredictorBrowserTest {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitchASCII(
+        switches::kSpeculativeResourcePrefetching,
+        switches::kSpeculativeResourcePrefetchingLearning);
+  }
 };
 
 IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest, Simple) {
   // These resources have default priorities that correspond to
   // blink::typeToPriority function.
   AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
   AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,
               net::HIGHEST);
   AddResource(GetURL(kScriptPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
   AddResource(GetURL(kFontPath), content::RESOURCE_TYPE_FONT_RESOURCE,
@@ skipping 35 matching lines @@
                      https_server()->GetURL(kHtmlSubresourcesPath)}});
   AddResource(https_server()->GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE,
               net::LOWEST);
   AddResource(https_server()->GetURL(kStylePath),
               content::RESOURCE_TYPE_STYLESHEET, net::HIGHEST);
   AddResource(https_server()->GetURL(kScriptPath),
               content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
   AddResource(https_server()->GetURL(kFontPath),
               content::RESOURCE_TYPE_FONT_RESOURCE, net::HIGHEST);
   NavigateToURLAndCheckSubresources(GetURL(kRedirectPath));
   // TODO(alexilin): Test learning and prefetching once crbug.com/650246 is

[Benoit L, 2016/12/20 13:20:49]

Since this is the bug you are fixing, can you see how to address the TODO at the
same time?

[ahemery, 2016/12/20 14:06:35]

Seen with alexilin@, removed the comment and fixed the test.

   // fixed.
 }
 
 IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
                        JavascriptDocumentWrite) {
   auto externalScript =
       AddExternalResource(GetURL(kScriptDocumentWritePath),
                           content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
   externalScript->request.mime_type = kJavascriptMime;
   AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
@@ skipping 56 matching lines @@
   AddResource(GetURL(kImagePath2), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
   AddResource(GetURL(kStylePath2), content::RESOURCE_TYPE_STYLESHEET,
               net::HIGHEST);
   AddResource(GetURL(kScriptPath2), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
   // Included from <iframe src="html_subresources.html"> and not recored.
   AddUnrecordedResources({GetURL(kImagePath), GetURL(kStylePath),
                           GetURL(kScriptPath), GetURL(kFontPath)});
   TestLearningAndPrefetching(GetURL(kHtmlIframePath));
 }
 
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,
+                       CrossSiteLearning) {
+  set_current_host_name("foo.com");
+  AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
+  AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,
+              net::HIGHEST);
+  AddResource(GetURL(kScriptPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  AddResource(GetURL(kFontPath), content::RESOURCE_TYPE_FONT_RESOURCE,
+              net::HIGHEST);
+  TestLearningAndPrefetching(GetURL(kHtmlSubresourcesPath));
+  ClearResources();
+
+  set_current_host_name("bar.com");
+  AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
+  AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,
+              net::HIGHEST);
+  AddResource(GetURL(kScriptPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  AddResource(GetURL(kFontPath), content::RESOURCE_TYPE_FONT_RESOURCE,
+              net::HIGHEST);
+  TestLearningAndPrefetching(GetURL(kHtmlSubresourcesPath));
+}
+
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorLearningBrowserTest,
+                       TabIdBehavingAsExpected) {
+  AddResource(GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
+  AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,
+              net::HIGHEST);
+  AddResource(GetURL(kScriptPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);
+  AddResource(GetURL(kFontPath), content::RESOURCE_TYPE_FONT_RESOURCE,
+              net::HIGHEST);
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath));
+  EXPECT_EQ(get_navigation_ids_history_size(), 1U);
+  ClearCache();
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath));
+  EXPECT_EQ(get_navigation_ids_history_size(), 1U);
+  ClearCache();
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath),
+                                    WindowOpenDisposition::NEW_FOREGROUND_TAB);
+  EXPECT_EQ(get_navigation_ids_history_size(), 2U);
+  ClearCache();
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath),
+                                    WindowOpenDisposition::NEW_WINDOW);
+  EXPECT_EQ(get_navigation_ids_history_size(), 3U);
+  ClearCache();
+  NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath),
+                                    WindowOpenDisposition::NEW_POPUP);
+  EXPECT_EQ(get_navigation_ids_history_size(), 4U);
+}
+
 }  // namespace predictors