Skip to content

GitLab

T
tangled
Commit ee7090f8 authored by Doug Arnett's avatar Doug Arnett Committed by Commit Bot
Browse files
Options

[Previews] Adds DeferAllScript Preview support to PreviewsContentUtil 

Adds Allowed/Committed PreviewsState -> PreviewType support in
PreviewsContentUtil for DeferAllScript preview. It is prioritized
at this level above ResourceLoadingHints and NoScript since they are
supported on older clients and also assuming it is a better option
if supported by the client. The optimization guide logic may affect
priority separately as well.

Bug: 965277
Change-Id: I27b24ad4076f9b424cc9c5041e2d7a36027b12f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1669983
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: default avatarRobert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671878}
parent 355e7bdc
Show whitespace changes
Inline Side-by-side
Showing with 107 additions and 4 deletions
chrome/browser/previews/previews_content_util.cc
View file @ ee7090f8
......@@ -273,6 +273,12 @@ content::PreviewsState DetermineAllowedClientPreviewsState(
// Check PageHint preview types first.
bool should_load_page_hints = false;
if (previews_decider->ShouldAllowPreviewAtNavigationStart(
previews_data, url, is_reload,
previews::PreviewsType::DEFER_ALL_SCRIPT)) {
previews_state |= content::DEFER_ALL_SCRIPT_ON;
should_load_page_hints = true;
}
if (previews_decider->ShouldAllowPreviewAtNavigationStart(
previews_data, url, is_reload,
previews::PreviewsType::RESOURCE_LOADING_HINTS)) {
......@@ -383,6 +389,21 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
// Make priority decision among allowed client preview types that can be
// decided at Commit time.
if (previews_state & content::DEFER_ALL_SCRIPT_ON) {
// DeferAllScript was allowed for the original URL but only continue with it
// if the committed URL has HTTPS scheme and is allowed by decider.
if (is_https && previews_decider &&
previews_decider->ShouldCommitPreview(
previews_data, url, previews::PreviewsType::DEFER_ALL_SCRIPT)) {
LogCommittedPreview(previews_data, PreviewsType::DEFER_ALL_SCRIPT);
return content::DEFER_ALL_SCRIPT_ON;
}
// Remove DEFER_ALL_SCRIPT_ON from |previews_state| since we decided not to
// commit to it.
previews_state = previews_state & ~content::DEFER_ALL_SCRIPT_ON;
}
if (previews_state & content::RESOURCE_LOADING_HINTS_ON) {
// Resource loading hints was chosen for the original URL but only continue
// with it if the committed URL has HTTPS scheme and is allowed by decider.
......@@ -505,6 +526,8 @@ previews::PreviewsType GetMainFramePreviewsType(
return previews::PreviewsType::LITE_PAGE_REDIRECT;
if (previews_state & content::SERVER_LITE_PAGE_ON)
return previews::PreviewsType::LITE_PAGE;
if (previews_state & content::DEFER_ALL_SCRIPT_ON)
return previews::PreviewsType::DEFER_ALL_SCRIPT;
if (previews_state & content::RESOURCE_LOADING_HINTS_ON)
return previews::PreviewsType::RESOURCE_LOADING_HINTS;
if (previews_state & content::NOSCRIPT_ON)
......
chrome/browser/previews/previews_content_util_unittest.cc
View file @ ee7090f8
......@@ -71,7 +71,8 @@ class PreviewEnabledPreviewsDecider : public PreviewsDecider {
const GURL& url,
PreviewsType type) const override {
EXPECT_TRUE(type == PreviewsType::NOSCRIPT ||
type == PreviewsType::RESOURCE_LOADING_HINTS);
type == PreviewsType::RESOURCE_LOADING_HINTS ||
type == PreviewsType::DEFER_ALL_SCRIPT);
return IsEnabled(type);
}
......@@ -218,6 +219,31 @@ TEST_F(PreviewsContentUtilTest,
enabled_previews_decider(), nullptr));
}
TEST_F(PreviewsContentUtilTest,
DetermineAllowedClientPreviewsStateDeferAllScript) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitFromCommandLine("Previews,DeferAllScript",
std::string());
PreviewsUserData user_data(1);
bool is_reload = false;
bool previews_triggering_logic_already_ran = false;
bool is_data_saver_user = true;
// Allowed for start of HTTPS navigation.
EXPECT_LT(0,
content::DEFER_ALL_SCRIPT_ON &
previews::CallDetermineAllowedClientPreviewsState(
&user_data, GURL("https://www.google.com"), is_reload,
previews_triggering_logic_already_ran, is_data_saver_user,
enabled_previews_decider(), nullptr));
// Allowed for start of HTTP navigation.
EXPECT_LT(0,
content::DEFER_ALL_SCRIPT_ON &
previews::CallDetermineAllowedClientPreviewsState(
&user_data, GURL("http://www.google.com"), is_reload,
previews_triggering_logic_already_ran, is_data_saver_user,
enabled_previews_decider(), nullptr));
}
TEST_F(PreviewsContentUtilTest,
DetermineAllowedClientPreviewsStateResourceLoadingHints) {
base::test::ScopedFeatureList scoped_feature_list;
......@@ -322,7 +348,8 @@ TEST_F(PreviewsContentUtilTest,
TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitFromCommandLine(
"Previews,NoScriptPreviews,ResourceLoadingHints", std::string());
"Previews,NoScriptPreviews,ResourceLoadingHints,DeferAllScript",
std::string());
PreviewsUserData user_data(1);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G);
base::HistogramTester histogram_tester;
......@@ -371,6 +398,19 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) {
// Try different navigation ECT value.
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
// DeferAllScript has precedence over NoScript and ResourceLoadingHints.
EXPECT_EQ(content::DEFER_ALL_SCRIPT_ON,
previews::DetermineCommittedClientPreviewsState(
&user_data, GURL("https://www.google.com"),
content::DEFER_ALL_SCRIPT_ON | content::NOSCRIPT_ON |
content::RESOURCE_LOADING_HINTS_ON,
enabled_previews_decider(), nullptr));
histogram_tester.ExpectBucketCount(
"Previews.Triggered.EffectiveConnectionType2.DeferAllScript",
static_cast<int>(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G), 1);
histogram_tester.ExpectTotalCount(
"Previews.Triggered.EffectiveConnectionType2", 4);
// RESOURCE_LOADING_HINTS has precedence over NoScript.
EXPECT_EQ(content::RESOURCE_LOADING_HINTS_ON,
previews::DetermineCommittedClientPreviewsState(
......@@ -381,7 +421,7 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) {
"Previews.Triggered.EffectiveConnectionType2.ResourceLoadingHints",
static_cast<int>(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G), 1);
histogram_tester.ExpectTotalCount(
"Previews.Triggered.EffectiveConnectionType2", 4);
"Previews.Triggered.EffectiveConnectionType2", 5);
// Only NoScript:
EXPECT_EQ(content::NOSCRIPT_ON,
......@@ -390,6 +430,36 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) {
content::NOSCRIPT_ON, enabled_previews_decider(), nullptr));
}
TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsStateForHttp) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitFromCommandLine(
"Previews,NoScriptPreviews,ResourceLoadingHints,DeferAllScript",
std::string());
PreviewsUserData user_data(1);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G);
base::HistogramTester histogram_tester;
// Verify that currently these previews do not commit on HTTP.
EXPECT_EQ(content::PREVIEWS_OFF,
previews::DetermineCommittedClientPreviewsState(
&user_data, GURL("http://www.google.com"),
content::NOSCRIPT_ON | content::RESOURCE_LOADING_HINTS_ON |
content::DEFER_ALL_SCRIPT_ON,
enabled_previews_decider(), nullptr));
histogram_tester.ExpectTotalCount(
"Previews.Triggered.EffectiveConnectionType2", 0);
// Ensure one does commit on HTTPS (to confirm test setup).
EXPECT_NE(content::PREVIEWS_OFF,
previews::DetermineCommittedClientPreviewsState(
&user_data, GURL("https://www.google.com"),
content::NOSCRIPT_ON | content::RESOURCE_LOADING_HINTS_ON |
content::DEFER_ALL_SCRIPT_ON,
enabled_previews_decider(), nullptr));
histogram_tester.ExpectTotalCount(
"Previews.Triggered.EffectiveConnectionType2", 1);
}
TEST_F(PreviewsContentUtilTest,
DetermineCommittedClientPreviewsStateNoScriptCheckIfStillAllowed) {
base::test::ScopedFeatureList scoped_feature_list;
......@@ -437,6 +507,8 @@ TEST_F(PreviewsContentUtilTest, GetMainFramePreviewsType) {
previews::GetMainFramePreviewsType(content::RESOURCE_LOADING_HINTS_ON));
EXPECT_EQ(previews::PreviewsType::LITE_PAGE_REDIRECT,
previews::GetMainFramePreviewsType(content::LITE_PAGE_REDIRECT_ON));
EXPECT_EQ(previews::PreviewsType::DEFER_ALL_SCRIPT,
previews::GetMainFramePreviewsType(content::DEFER_ALL_SCRIPT_ON));
// NONE cases:
EXPECT_EQ(previews::PreviewsType::NONE,
......@@ -456,10 +528,14 @@ TEST_F(PreviewsContentUtilTest, GetMainFramePreviewsType) {
EXPECT_EQ(previews::PreviewsType::RESOURCE_LOADING_HINTS,
previews::GetMainFramePreviewsType(
content::NOSCRIPT_ON | content::RESOURCE_LOADING_HINTS_ON));
EXPECT_EQ(previews::PreviewsType::DEFER_ALL_SCRIPT,
previews::GetMainFramePreviewsType(
content::NOSCRIPT_ON | content::RESOURCE_LOADING_HINTS_ON |
content::DEFER_ALL_SCRIPT_ON));
EXPECT_EQ(previews::PreviewsType::LITE_PAGE_REDIRECT,
previews::GetMainFramePreviewsType(
content::NOSCRIPT_ON | content::RESOURCE_LOADING_HINTS_ON |
content::LITE_PAGE_REDIRECT_ON));
content::DEFER_ALL_SCRIPT_ON | content::LITE_PAGE_REDIRECT_ON));
}
class PreviewsContentSimulatedNavigationTest
......
components/previews/core/previews_experiments.h
View file @ ee7090f8
......@@ -15,6 +15,8 @@
namespace previews {
// Types of previews. This enum must remain synchronized with the enum
// |PreviewsType| in tools/metrics/histograms/enums.xml.
enum class PreviewsType {
// Used to indicate that there is no preview type.
NONE = 0,
......
tools/metrics/histograms/enums.xml
View file @ ee7090f8
......@@ -47713,6 +47713,8 @@ Called by update_net_trust_anchors.py.-->
<int value="5" label="NoScript"/>
<int value="6" label="Unspecified"/>
<int value="7" label="ResourceLoadingHints"/>
<int value="8" label="LitePageRedirect"/>
<int value="9" label="DeferAllScript"/>
</enum>
<enum name="PreviewsUserOmniboxAction">
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