chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc - Issue 2755093002: predictors: Mark before_first_contentful_paint for resources fetched before fcp.

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

Issue 2755093002: predictors: Mark before_first_contentful_paint for resources fetched before fcp. (Closed)

Patch Set: Respond to feedback. Created 3 years, 8 months 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

« chrome/browser/predictors/resource_prefetch_predictor.cc ('K') | « chrome/browser/predictors/resource_prefetch_predictor.proto ('k') | chrome/browser/predictors/resource_prefetch_predictor_tables.h » ('j') | chrome/test/data/predictors/subresource_fcp_order.html » ('J')
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 cf9bbc2877e3801e8a24ab6dffb402cec4a338d3..d05f2e2eec08098f775c0e0c43f86983c1e2cdbd 100644

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

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

@@ -54,6 +54,7 @@ const char kStylePath[] = "/handled-by-test/style.css";

const char kStylePath2[] = "/handled-by-test/style2.css";

const char kScriptPath[] = "/handled-by-test/script.js";

const char kScriptPath2[] = "/handled-by-test/script2.js";

+const char kScriptLongPath[] = "/handled-by-test/long-script.js";

const char kFontPath[] = "/handled-by-test/font.ttf";

const char kRedirectPath[] = "/handled-by-test/redirect.html";

const char kRedirectPath2[] = "/handled-by-test/redirect2.html";

@@ -72,6 +73,7 @@ const char kScriptXHRPath[] = "/predictors/xhr.js";

const char kHtmlIframePath[] = "/predictors/html_iframe.html";

const char kHtmlJavascriptRedirectPath[] =

"/predictors/javascript_redirect.html";

+const char kHtmlFcpOrderPath[] = "/predictors/subresource_fcp_order.html";

// Host, path.

const char* kScript[2] = {kFooHost, kScriptPath};

@@ -85,7 +87,9 @@ struct ResourceSummary {

is_no_store(false),

is_external(false),

is_observable(true),

- is_prohibited(false) {}

+ is_prohibited(false) {

+ request.before_first_contentful_paint = true;

+ }

ResourcePrefetchPredictor::URLRequestSummary request;

// Allows to update HTTP ETag.

@@ -99,6 +103,7 @@ struct ResourceSummary {

// A request with |is_prohibited| set to true makes the test that originates

// the request fail.

bool is_prohibited;

+ base::TimeDelta delay;

Benoit L 2017/04/25 15:30:37 nit: add a comment?

trevordixon 2017/04/25 20:23:55 Done.

};

struct RedirectEdge {

@@ -169,9 +174,12 @@ void SetValidNavigationID(NavigationID* navigation_id) {

}

void ModifySubresourceForComparison(URLRequestSummary* subresource,

- bool match_navigation_id) {

+ bool match_navigation_id,

+ bool match_before_first_contentful_paint) {

if (!match_navigation_id)

SetValidNavigationID(&subresource->navigation_id);

+ if (!match_before_first_contentful_paint)

+ subresource->before_first_contentful_paint = true;

if (subresource->resource_type == content::RESOURCE_TYPE_IMAGE &&

subresource->priority == net::LOWEST) {

// Fuzzy comparison for images because an image priority can be

@@ -185,15 +193,18 @@ void ModifySubresourceForComparison(URLRequestSummary* subresource,

// and fail the test if the expectation is not met.

void CompareSubresources(std::vector<URLRequestSummary> actual_subresources,

std::vector<URLRequestSummary> expected_subresources,

- bool match_navigation_id) {

+ bool match_navigation_id,

+ bool match_before_first_contentful_paint) {

// Duplicate resources can be observed in a single navigation but

// ResourcePrefetchPredictor only cares about the first occurrence of each.

RemoveDuplicateSubresources(&actual_subresources);

for (auto& subresource : actual_subresources)

- ModifySubresourceForComparison(&subresource, match_navigation_id);

+ ModifySubresourceForComparison(&subresource, match_navigation_id,

+ match_before_first_contentful_paint);

for (auto& subresource : expected_subresources)

- ModifySubresourceForComparison(&subresource, match_navigation_id);

+ ModifySubresourceForComparison(&subresource, match_navigation_id,

+ match_before_first_contentful_paint);

EXPECT_THAT(actual_subresources,

testing::UnorderedElementsAreArray(expected_subresources));

@@ -233,11 +244,14 @@ class LearningObserver : public TestObserver {

LearningObserver(ResourcePrefetchPredictor* predictor,

const size_t expected_url_visit_count,

const PageRequestSummary& expected_summary,

- bool match_navigation_id)

+ bool match_navigation_id,

+ bool match_before_first_contentful_paint)

: TestObserver(predictor),

url_visit_count_(expected_url_visit_count),

summary_(expected_summary),

- match_navigation_id_(match_navigation_id) {}

+ match_navigation_id_(match_navigation_id),

+ match_before_first_contentful_paint_(

+ match_before_first_contentful_paint) {}

// TestObserver:

void OnNavigationLearned(size_t url_visit_count,

@@ -248,7 +262,8 @@ class LearningObserver : public TestObserver {

for (const auto& resource : summary.subresource_requests)

current_navigation_ids_.insert(resource.navigation_id);

CompareSubresources(summary.subresource_requests,

- summary_.subresource_requests, match_navigation_id_);

+ summary_.subresource_requests, match_navigation_id_,

+ match_before_first_contentful_paint_);

run_loop_.Quit();

}

@@ -263,6 +278,7 @@ class LearningObserver : public TestObserver {

size_t url_visit_count_;

PageRequestSummary summary_;

bool match_navigation_id_;

+ bool match_before_first_contentful_paint_;

std::set<NavigationID> current_navigation_ids_;

DISALLOW_COPY_AND_ASSIGN(LearningObserver);

@@ -360,12 +376,18 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(false);

}

- void TestLearningAndPrefetching(const GURL& main_frame_url) {

+ void TestLearningAndPrefetching(

+ const GURL& main_frame_url,

+ bool match_before_first_contentful_paint = false) {

// Navigate to |main_frame_url| and check all the expectations.

- NavigateToURLAndCheckSubresources(main_frame_url);

+ NavigateToURLAndCheckSubresources(main_frame_url,

+ WindowOpenDisposition::CURRENT_TAB,

+ match_before_first_contentful_paint);

ClearCache();

// It is needed to have at least two resource hits to trigger prefetch.

- NavigateToURLAndCheckSubresources(main_frame_url);

+ NavigateToURLAndCheckSubresources(main_frame_url,

+ WindowOpenDisposition::CURRENT_TAB,

+ match_before_first_contentful_paint);

ClearCache();

// Prefetch all needed resources and change expectations so that all

// cacheable resources should be served from cache next navigation.

@@ -389,7 +411,8 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

void NavigateToURLAndCheckSubresources(

const GURL& navigation_url,

- WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB) {

+ WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB,

+ bool match_before_first_contentful_paint = false) {

GURL initial_url = GetLastClientSideRedirectEndpoint(navigation_url);

GURL main_frame_url = GetRedirectEndpoint(navigation_url);

std::vector<URLRequestSummary> url_request_summaries;

@@ -407,7 +430,7 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

predictor_, UpdateAndGetVisitCount(initial_url),

CreatePageRequestSummary(main_frame_url.spec(), initial_url.spec(),

url_request_summaries),

- match_navigation_id);

+ match_navigation_id, match_before_first_contentful_paint);

ui_test_utils::NavigateToURLWithDisposition(

browser(), navigation_url, disposition,

ui_test_utils::BROWSER_TEST_NONE);

@@ -672,6 +695,10 @@ class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {

// 0kB.

http_response->set_content(std::string(1024, ' '));

+ if (!summary.delay.is_zero()) {

Benoit L 2017/04/25 15:30:36 nit: no braces

trevordixon 2017/04/25 20:23:55 Done.

+ base::PlatformThread::Sleep(summary.delay);

+ }

return std::move(http_response);

}

@@ -742,6 +769,24 @@ IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest, Simple) {

internal::kResourcePrefetchPredictorPrefetchHitsSize, 4, 1);

}

+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest,

+ SubresourceFcpOrder) {

+ AddResource(GetURL(kStylePath), content::RESOURCE_TYPE_STYLESHEET,

+ net::HIGHEST);

+ AddResource(GetURL(kScriptPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);

+ ResourceSummary* long_script = AddResource(

+ GetURL(kScriptLongPath), content::RESOURCE_TYPE_SCRIPT, net::MEDIUM);

+ long_script->delay = base::TimeDelta::FromMilliseconds(1500);

+ long_script->request.before_first_contentful_paint = false;

+ ResourceSummary* image = AddResource(

+ GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);

+ image->request.before_first_contentful_paint = false;

+ TestLearningAndPrefetching(GetURL(kHtmlFcpOrderPath), true);

IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest, Redirect) {

GURL initial_url = GetURLWithHost(kFooHost, kRedirectPath);

GURL redirected_url =