chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc - Issue 2587443002: predictors: Make speculative_prefetch_predictor work with PlzNavigate

Unified Diff: chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

Issue 2587443002: predictors: Make speculative_prefetch_predictor work with PlzNavigate (Closed)

Patch Set: Updates after lizeb@ second review Created 4 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « chrome/browser/predictors/resource_prefetch_predictor.cc ('k') | chrome/browser/predictors/resource_prefetch_predictor_test_util.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

diff --git a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

index 70c3131eee72b3618091e2b5e499350e753622f6..76401d8b9aa25ace8c020ba64ca956bf1a8c3023 100644

--- a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

+++ b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

@@ -2,6 +2,10 @@

// Use of this source code is governed by a BSD-style license that can be

// found in the LICENSE file.

+#include <stddef.h>

+#include <set>

#include "base/command_line.h"

#include "chrome/browser/predictors/resource_prefetch_predictor.h"

#include "chrome/browser/predictors/resource_prefetch_predictor_factory.h"

@@ -13,8 +17,7 @@

#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"

@@ -111,8 +114,7 @@ void RemoveDuplicateSubresources(std::vector<URLRequestSummary>* subresources) {

// 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");

}

@@ -171,6 +173,8 @@ class LearningObserver : public TestObserver {

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();

@@ -178,11 +182,16 @@ class LearningObserver : public TestObserver {

void Wait() { run_loop_.Run(); }

+ std::set<NavigationID>& current_navigation_ids() {

+ return current_navigation_ids_;

+ }

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);

};

@@ -226,6 +235,10 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

}

void SetUpOnMainThread() override {

+ set_current_host_name("127.0.0.1");

+ // Resolving all hosts to local allows us to have

+ // cross domains navigations (matching url_visit_count_, etc)

alexilin 2016/12/20 14:31:44 nit: add period to the end of the sentence.

ahemery 2016/12/20 14:44:56 Done.

+ host_resolver()->AddRule("*", "127.0.0.1");

embedded_test_server()->RegisterRequestHandler(

base::Bind(&ResourcePrefetchPredictorBrowserTest::HandleRedirectRequest,

base::Unretained(this)));

@@ -263,11 +276,16 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

url_request_summaries.push_back(

GetURLRequestSummaryForResource(endpoint_url, kv.second));

}

+ bool match_navigation_id = true;

alexilin 2016/12/21 13:08:12 bool match_navigation_id = disposition == WindowOp

alexilin 2016/12/22 10:32:29 Still unresolved

ahemery 2016/12/22 12:09:47 Done. Sorry missed this one

+ 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);

@@ -276,6 +294,8 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

if (!kv.second.is_no_store && kv.second.should_be_recorded)

kv.second.request.was_cached = true;

}

+ for (const auto& nav : observer.current_navigation_ids())

+ navigation_id_history_.insert(nav);

}

void PrefetchURL(const GURL& main_frame_url) {

@@ -331,6 +351,8 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

}

+ void ClearResources() { resources_.clear(); }

void ClearCache() {

chrome::ClearCache(browser());

for (auto& kv : resources_)

@@ -339,7 +361,7 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

// 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() {

@@ -365,6 +387,12 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

net::EmbeddedTestServer* https_server() { return https_server_.get(); }

+ void set_current_host_name(const std::string& new_host) {

+ current_host_name_ = new_host;

+ }

+ size_t navigation_ids_history_size() { return navigation_id_history_.size(); }

private:

// ResourcePrefetchPredictor needs to be initialized before the navigation

// happens otherwise this navigation will be ignored by predictor.

@@ -388,10 +416,8 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

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;

}

@@ -408,7 +434,12 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

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;

@@ -454,11 +485,26 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

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

+// the learning part of the prefetcher without the actual prefetching

alexilin 2016/12/20 14:31:44 nit: periods as well.

ahemery 2016/12/20 14:44:57 Done.

+// 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) {

@@ -500,7 +546,7 @@ IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest, RedirectChain) {

}

IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,

- LearningAfterHttpToHttpsRedirect) {

+ HttpToHttpsRedirect) {

EnableHttpsServer();

AddRedirectChain(GetURL(kRedirectPath),

{{net::HTTP_MOVED_PERMANENTLY,

@@ -513,9 +559,7 @@ IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,

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

- // fixed.

+ TestLearningAndPrefetching(GetURL(kRedirectPath));

alexilin 2016/12/20 14:31:44 It's great that this test works now!

ahemery 2016/12/20 14:44:56 Done.

}

IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,

@@ -591,4 +635,53 @@ IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,

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(navigation_ids_history_size(), 1U);

+ ClearCache();

+ NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath));

+ EXPECT_EQ(navigation_ids_history_size(), 1U);

+ ClearCache();

+ NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath),

+ WindowOpenDisposition::NEW_FOREGROUND_TAB);

+ EXPECT_EQ(navigation_ids_history_size(), 2U);

+ ClearCache();

+ NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath),

+ WindowOpenDisposition::NEW_WINDOW);

+ EXPECT_EQ(navigation_ids_history_size(), 3U);

+ ClearCache();

+ NavigateToURLAndCheckSubresources(GetURL(kHtmlSubresourcesPath),

+ WindowOpenDisposition::NEW_POPUP);

+ EXPECT_EQ(navigation_ids_history_size(), 4U);

} // namespace predictors