Commit 96903cd1 authored by Doug Arnett's avatar Doug Arnett Committed by Commit Bot

[Previews] Adds DeferAllScript support to Decider and OptGuide

Adds new opt guide proto optimization type for DeferAllScript with
whitelist checking for it from PreviewsDeciderImpl thru PreviewsHints.

Bug: 965277
Change-Id: I7be59e6eda9b64d310027ac2c3ddfb7f91ac76eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1673754Reviewed-by: default avatarRobert Ogden <robertogden@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672122}
parent c68f0923
......@@ -118,6 +118,9 @@ enum OptimizationType {
LITE_PAGE_REDIRECT = 3;
// This optimization does nothing (no-op).
OPTIMIZATION_NONE = 4;
// This optimization defers execution of parser-blocking script until after
// parsing completes.
DEFER_ALL_SCRIPT = 5;
}
// Presents semantics for how page load URLs should be matched.
......
......@@ -55,7 +55,8 @@ bool HintsFetcher::FetchOptimizationGuideServiceHints(
optimization_guide::proto::NOSCRIPT);
get_hints_request_->add_supported_optimizations(
optimization_guide::proto::RESOURCE_LOADING);
// TODO(dougarnett): Add DEFER_ALL_SCRIPT once supported.
get_hints_request_->add_supported_optimizations(
optimization_guide::proto::DEFER_ALL_SCRIPT);
static_assert(static_cast<int>(PreviewsType::DEFER_ALL_SCRIPT) + 1 ==
static_cast<int>(PreviewsType::LAST),
"PreviewsType has been updated, make sure Optimization Guide "
......
......@@ -51,11 +51,11 @@ bool ShouldCheckOptimizationHints(PreviewsType type) {
case PreviewsType::NOSCRIPT:
case PreviewsType::RESOURCE_LOADING_HINTS:
case PreviewsType::LITE_PAGE_REDIRECT:
case PreviewsType::DEFER_ALL_SCRIPT:
return true;
// These types do not have server optimization hints.
case PreviewsType::OFFLINE:
case PreviewsType::LITE_PAGE:
case PreviewsType::DEFER_ALL_SCRIPT:
return false;
case PreviewsType::NONE:
case PreviewsType::UNSPECIFIED:
......@@ -333,7 +333,8 @@ PreviewsEligibilityReason PreviewsDeciderImpl::DeterminePreviewEligibility(
return ShouldAllowPreviewPerOptimizationHints(previews_data, url, type,
passed_reasons);
} else if (type == PreviewsType::RESOURCE_LOADING_HINTS ||
type == PreviewsType::NOSCRIPT) {
type == PreviewsType::NOSCRIPT ||
type == PreviewsType::DEFER_ALL_SCRIPT) {
return PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER;
}
}
......@@ -373,7 +374,8 @@ bool PreviewsDeciderImpl::ShouldCommitPreview(PreviewsUserData* previews_data,
PreviewsType type) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(PreviewsType::NOSCRIPT == type ||
PreviewsType::RESOURCE_LOADING_HINTS == type);
PreviewsType::RESOURCE_LOADING_HINTS == type ||
PreviewsType::DEFER_ALL_SCRIPT == type);
if (previews_black_list_ && !blacklist_ignored_) {
std::vector<PreviewsEligibilityReason> passed_reasons;
// The blacklist will disallow certain hosts for periods of time based on
......@@ -412,7 +414,8 @@ PreviewsDeciderImpl::ShouldAllowPreviewPerOptimizationHints(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(type == PreviewsType::LITE_PAGE_REDIRECT ||
type == PreviewsType::NOSCRIPT ||
type == PreviewsType::RESOURCE_LOADING_HINTS);
type == PreviewsType::RESOURCE_LOADING_HINTS ||
type == PreviewsType::DEFER_ALL_SCRIPT);
// For LitePageRedirect, ensure it is not blacklisted for this request, and
// hints have been fully loaded.
//
......@@ -446,7 +449,8 @@ PreviewsDeciderImpl::ShouldCommitPreviewPerOptimizationHints(
std::vector<PreviewsEligibilityReason>* passed_reasons) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(type == PreviewsType::NOSCRIPT ||
type == PreviewsType::RESOURCE_LOADING_HINTS);
type == PreviewsType::RESOURCE_LOADING_HINTS ||
type == PreviewsType::DEFER_ALL_SCRIPT);
if (!previews_opt_guide_ || !previews_opt_guide_->has_hints())
return PreviewsEligibilityReason::OPTIMIZATION_HINTS_NOT_AVAILABLE;
......
......@@ -193,14 +193,16 @@ class TestPreviewsOptimizationGuide : public PreviewsOptimizationGuide {
PreviewsType type,
net::EffectiveConnectionType* ect_threshold) const override {
EXPECT_TRUE(type == PreviewsType::NOSCRIPT ||
type == PreviewsType::RESOURCE_LOADING_HINTS);
type == PreviewsType::RESOURCE_LOADING_HINTS ||
type == PreviewsType::DEFER_ALL_SCRIPT);
// Use default ect trigger threshold for the preview type.
*ect_threshold = previews::params::GetECTThresholdForPreview(type);
if (type == PreviewsType::NOSCRIPT) {
return url.host().compare("whitelisted.example.com") == 0 ||
url.host().compare("noscript_only_whitelisted.example.com") == 0;
}
if (type == PreviewsType::RESOURCE_LOADING_HINTS) {
if (type == PreviewsType::RESOURCE_LOADING_HINTS ||
type == PreviewsType::DEFER_ALL_SCRIPT) {
return url.host().compare("whitelisted.example.com") == 0;
}
return false;
......@@ -425,6 +427,7 @@ class PreviewsDeciderImplTest : public ProtoDatabaseProviderTestBase {
allowed_types[static_cast<int>(PreviewsType::LITE_PAGE_REDIRECT)] = 0;
allowed_types[static_cast<int>(PreviewsType::NOSCRIPT)] = 0;
allowed_types[static_cast<int>(PreviewsType::RESOURCE_LOADING_HINTS)] = 0;
allowed_types[static_cast<int>(PreviewsType::DEFER_ALL_SCRIPT)] = 0;
std::unique_ptr<TestPreviewsDeciderImpl> previews_decider_impl =
std::make_unique<TestPreviewsDeciderImpl>(&clock_);
......@@ -1186,7 +1189,7 @@ TEST_F(PreviewsDeciderImplTest, OptimizationGuidePreviewsAllowedWithoutHints) {
scoped_feature_list.InitWithFeatures(
{features::kPreviews, features::kLitePageServerPreviews,
features::kOptimizationHints, features::kNoScriptPreviews,
features::kResourceLoadingHints},
features::kResourceLoadingHints, features::kDeferAllScriptPreviews},
{});
InitializeUIService();
......@@ -1203,6 +1206,11 @@ TEST_F(PreviewsDeciderImplTest, OptimizationGuidePreviewsAllowedWithoutHints) {
&user_data, GURL("https://whitelisted.example.com"), false,
PreviewsType::RESOURCE_LOADING_HINTS));
// DeferAllScript is allowed before commit without hints.
EXPECT_TRUE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://whitelisted.example.com"), false,
PreviewsType::DEFER_ALL_SCRIPT));
// LitePageRedirect is not allowed before commit without hints.
EXPECT_FALSE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://whitelisted.example.com"), false,
......@@ -1221,6 +1229,9 @@ TEST_F(PreviewsDeciderImplTest, OptimizationGuidePreviewsAllowedWithoutHints) {
EXPECT_TRUE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://whitelisted.example.com"), false,
PreviewsType::RESOURCE_LOADING_HINTS));
EXPECT_TRUE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://whitelisted.example.com"), false,
PreviewsType::DEFER_ALL_SCRIPT));
EXPECT_TRUE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://whitelisted.example.com"), false,
PreviewsType::LITE_PAGE_REDIRECT));
......@@ -1412,6 +1423,185 @@ TEST_F(PreviewsDeciderImplTest, ResourceLoadingHintsCommitTimeWhitelistCheck) {
}
}
TEST_F(PreviewsDeciderImplTest, DeferAllScriptNotAllowedByDefault) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatures(
{features::kPreviews, features::kOptimizationHints}, {});
InitializeUIService();
InitializeOptimizationGuideHints();
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
EXPECT_FALSE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://www.google.com"), false,
PreviewsType::DEFER_ALL_SCRIPT));
}
TEST_F(PreviewsDeciderImplTest,
DeferAllScriptDisallowedWithoutOptimizationHints) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatures(
{features::kPreviews, features::kDeferAllScriptPreviews},
{features::kOptimizationHints});
InitializeUIService();
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
EXPECT_FALSE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://whitelisted.example.com"), false,
PreviewsType::DEFER_ALL_SCRIPT));
histogram_tester.ExpectUniqueSample(
"Previews.EligibilityReason",
static_cast<int>(
PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER),
1);
histogram_tester.ExpectUniqueSample(
"Previews.EligibilityReason.DeferAllScript",
static_cast<int>(
PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER),
1);
}
TEST_F(PreviewsDeciderImplTest, DeferAllScriptAllowedByFeatureAndWhitelist) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatures(
{features::kPreviews, features::kDeferAllScriptPreviews,
features::kOptimizationHints},
{});
InitializeUIService();
InitializeOptimizationGuideHints();
for (const auto& test_ect : {net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
net::EFFECTIVE_CONNECTION_TYPE_3G}) {
ReportEffectiveConnectionType(test_ect);
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
// Check whitelisted URL.
EXPECT_TRUE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://whitelisted.example.com"), false,
PreviewsType::DEFER_ALL_SCRIPT));
EXPECT_EQ(test_ect, user_data.navigation_ect());
histogram_tester.ExpectUniqueSample(
"Previews.EligibilityReason.DeferAllScript",
static_cast<int>(PreviewsEligibilityReason::ALLOWED), 1);
}
}
TEST_F(PreviewsDeciderImplTest,
DeferAllScriptAllowedByFeatureWithoutKnownHints) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatures(
{features::kPreviews, features::kDeferAllScriptPreviews,
features::kOptimizationHints},
{});
InitializeUIService();
InitializeOptimizationGuideHints();
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
// Verify preview allowed initially for url without known hints.
EXPECT_TRUE(previews_decider_impl()->ShouldAllowPreviewAtNavigationStart(
&user_data, GURL("https://www.google.com"), false,
PreviewsType::DEFER_ALL_SCRIPT));
histogram_tester.ExpectBucketCount(
"Previews.EligibilityReason.DeferAllScript",
static_cast<int>(PreviewsEligibilityReason::ALLOWED), 1);
}
TEST_F(PreviewsDeciderImplTest, DeferAllScriptCommitTimeWhitelistCheck) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatures(
{features::kPreviews, features::kDeferAllScriptPreviews,
features::kOptimizationHints},
{});
InitializeUIService();
InitializeOptimizationGuideHints();
// First verify not allowed for non-whitelisted url.
{
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G);
EXPECT_FALSE(previews_decider_impl()->ShouldCommitPreview(
&user_data, GURL("https://www.google.com"),
PreviewsType::DEFER_ALL_SCRIPT));
histogram_tester.ExpectUniqueSample(
"Previews.EligibilityReason.DeferAllScript",
static_cast<int>(
PreviewsEligibilityReason::HOST_NOT_WHITELISTED_BY_SERVER),
1);
}
// Now verify preview for whitelisted url.
{
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G);
EXPECT_TRUE(previews_decider_impl()->ShouldCommitPreview(
&user_data, GURL("https://whitelisted.example.com"),
PreviewsType::DEFER_ALL_SCRIPT));
// Expect no eligibility logging.
histogram_tester.ExpectTotalCount(
"Previews.EligibilityReason.DeferAllScript", 0);
}
// Verify preview not allowed for whitelisted url when network is not slow.
{
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_4G);
EXPECT_FALSE(previews_decider_impl()->ShouldCommitPreview(
&user_data, GURL("https://whitelisted.example.com"),
PreviewsType::DEFER_ALL_SCRIPT));
histogram_tester.ExpectUniqueSample(
"Previews.EligibilityReason.DeferAllScript",
static_cast<int>(PreviewsEligibilityReason::NETWORK_NOT_SLOW), 1);
}
// Verify preview not allowed for whitelisted url for unknown network quality.
{
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
EXPECT_FALSE(previews_decider_impl()->ShouldCommitPreview(
&user_data, GURL("https://whitelisted.example.com"),
PreviewsType::DEFER_ALL_SCRIPT));
histogram_tester.ExpectUniqueSample(
"Previews.EligibilityReason.DeferAllScript",
static_cast<int>(PreviewsEligibilityReason::DEVICE_OFFLINE), 1);
}
// Verify preview not allowed for session limited ECT threshold.
{
base::test::ScopedFeatureList nested_scoped_list;
nested_scoped_list.InitAndEnableFeatureWithParameters(
features::kSlowPageTriggering,
{{"session_max_ect_trigger", "Offline"}});
base::HistogramTester histogram_tester;
PreviewsUserData user_data(kDefaultPageId);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G);
EXPECT_FALSE(previews_decider_impl()->ShouldCommitPreview(
&user_data, GURL("https://whitelisted.example.com"),
PreviewsType::DEFER_ALL_SCRIPT));
histogram_tester.ExpectUniqueSample(
"Previews.EligibilityReason.DeferAllScript",
static_cast<int>(
PreviewsEligibilityReason::NETWORK_NOT_SLOW_FOR_SESSION),
1);
}
}
TEST_F(PreviewsDeciderImplTest, LogPreviewNavigationPassInCorrectParams) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(features::kPreviews);
......
......@@ -125,6 +125,8 @@ base::Optional<PreviewsType> ConvertProtoOptimizationTypeToPreviewsType(
return PreviewsType::LITE_PAGE_REDIRECT;
case optimization_guide::proto::OPTIMIZATION_NONE:
return PreviewsType::NONE;
case optimization_guide::proto::DEFER_ALL_SCRIPT:
return PreviewsType::DEFER_ALL_SCRIPT;
}
}
......@@ -143,6 +145,8 @@ bool IsEnabledOptimizationType(
case optimization_guide::proto::OPTIMIZATION_NONE:
// Always consider enabled to allow as no-op optimization.
return true;
case optimization_guide::proto::DEFER_ALL_SCRIPT:
return previews::params::IsDeferAllScriptPreviewsEnabled();
}
}
......
......@@ -517,9 +517,9 @@ TEST_F(PreviewsHintsTest, IsWhitelistedOutParams) {
}
TEST_F(PreviewsHintsTest,
IsWhitelistedForSecondOptimizationNoScriptWithFirstDisabled) {
IsWhitelistedForSecondOptimizationDeferAllScriptWithFirstDisabled) {
base::test::ScopedFeatureList scoped_list;
scoped_list.InitWithFeatures({features::kNoScriptPreviews},
scoped_list.InitWithFeatures({features::kDeferAllScriptPreviews},
{features::kResourceLoadingHints});
optimization_guide::proto::Configuration config;
......@@ -528,7 +528,7 @@ TEST_F(PreviewsHintsTest,
hint1->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
hint1->set_version("someversion");
// Page hint with NOSCRIPT optimization
// Page hint with RESOURCE_LOADING and DEFER_ALL_SCRIPT optimizations
optimization_guide::proto::PageHint* page_hint1 = hint1->add_page_hints();
page_hint1->set_page_pattern("/has_multiple_optimizations/");
optimization_guide::proto::Optimization* optimization1 =
......@@ -537,7 +537,8 @@ TEST_F(PreviewsHintsTest,
optimization_guide::proto::RESOURCE_LOADING);
optimization_guide::proto::Optimization* optimization2 =
page_hint1->add_whitelisted_optimizations();
optimization2->set_optimization_type(optimization_guide::proto::NOSCRIPT);
optimization2->set_optimization_type(
optimization_guide::proto::DEFER_ALL_SCRIPT);
ParseConfig(config);
......@@ -547,7 +548,7 @@ TEST_F(PreviewsHintsTest,
std::string serialized_hint_version_string;
EXPECT_TRUE(MaybeLoadHintAndCheckIsWhitelisted(
GURL("https://www.somedomain.org/has_multiple_optimizations/"),
PreviewsType::NOSCRIPT, &inflation_percent, &ect_threshold,
PreviewsType::DEFER_ALL_SCRIPT, &inflation_percent, &ect_threshold,
&serialized_hint_version_string));
EXPECT_FALSE(MaybeLoadHintAndCheckIsWhitelisted(
GURL("https://www.somedomain.org/has_multiple_optimizations/"),
......@@ -559,7 +560,7 @@ TEST_F(PreviewsHintsTest,
IsWhitelistedForSecondOptimizationResourceLoadingWithFirstDisabled) {
base::test::ScopedFeatureList scoped_list;
scoped_list.InitWithFeatures({features::kResourceLoadingHints},
{features::kNoScriptPreviews});
{features::kDeferAllScriptPreviews});
optimization_guide::proto::Configuration config;
optimization_guide::proto::Hint* hint1 = config.add_hints();
......@@ -567,12 +568,13 @@ TEST_F(PreviewsHintsTest,
hint1->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
hint1->set_version("someversion");
// Page hint with NOSCRIPT optimization
// Page hint with RESOURCE_LOADING and DEFER_ALL_SCRIPT optimizations
optimization_guide::proto::PageHint* page_hint1 = hint1->add_page_hints();
page_hint1->set_page_pattern("/has_multiple_optimizations/");
optimization_guide::proto::Optimization* optimization1 =
page_hint1->add_whitelisted_optimizations();
optimization1->set_optimization_type(optimization_guide::proto::NOSCRIPT);
optimization1->set_optimization_type(
optimization_guide::proto::DEFER_ALL_SCRIPT);
optimization_guide::proto::Optimization* optimization2 =
page_hint1->add_whitelisted_optimizations();
optimization2->set_optimization_type(
......
......@@ -672,6 +672,41 @@ TEST_F(PreviewsOptimizationGuideTest,
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_2G, ect_threshold);
}
TEST_F(PreviewsOptimizationGuideTest,
ProcessHintsWithValidCommandLineOverrideForDeferAllScript) {
base::test::ScopedFeatureList scoped_list;
scoped_list.InitWithFeatures({features::kDeferAllScriptPreviews}, {});
optimization_guide::proto::Configuration config;
optimization_guide::proto::Hint* hint = config.add_hints();
hint->set_key("somedomain.org");
hint->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
optimization_guide::proto::PageHint* page_hint = hint->add_page_hints();
page_hint->set_page_pattern("defer_default_2g");
optimization_guide::proto::Optimization* optimization =
page_hint->add_whitelisted_optimizations();
optimization->set_optimization_type(
optimization_guide::proto::DEFER_ALL_SCRIPT);
std::string encoded_config;
config.SerializeToString(&encoded_config);
base::Base64Encode(encoded_config, &encoded_config);
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kHintsProtoOverride, encoded_config);
CreateServiceAndGuide();
// Verify page matches and ECT thresholds.
PreviewsUserData user_data(kDefaultPageId);
net::EffectiveConnectionType ect_threshold;
EXPECT_TRUE(guide()->GetHintsForTesting());
EXPECT_TRUE(MaybeLoadOptimizationHintsAndCheckIsWhitelisted(
&user_data, GURL("https://somedomain.org/defer_default_2g"),
PreviewsType::DEFER_ALL_SCRIPT, &ect_threshold));
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_2G, ect_threshold);
}
TEST_F(PreviewsOptimizationGuideTest,
ProcessHintsWithValidCommandLineOverrideAndPreexistingData) {
base::test::ScopedFeatureList scoped_list;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment