Commit 77cb17ab authored by Sophie Chang's avatar Sophie Chang Committed by Commit Bot

Deprecate ECT histogram and extend date for CC-NoTransform one

Bug: 1144811,1144812
Change-Id: I3b28456a40e9d553cff5c0ad85532a520fa1d2f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2512523Reviewed-by: default avatarMichael Crouse <mcrouse@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823290}
parent 903860b6
...@@ -116,18 +116,6 @@ blink::PreviewsState DetermineAllowedClientPreviewsState( ...@@ -116,18 +116,6 @@ blink::PreviewsState DetermineAllowedClientPreviewsState(
return previews_state; return previews_state;
} }
void LogCommittedPreview(previews::PreviewsUserData* previews_data,
PreviewsType type) {
net::EffectiveConnectionType navigation_ect = previews_data->navigation_ect();
UMA_HISTOGRAM_ENUMERATION("Previews.Triggered.EffectiveConnectionType2",
navigation_ect,
net::EFFECTIVE_CONNECTION_TYPE_LAST);
base::UmaHistogramEnumeration(
base::StringPrintf("Previews.Triggered.EffectiveConnectionType2.%s",
GetStringNameForType(type).c_str()),
navigation_ect, net::EFFECTIVE_CONNECTION_TYPE_LAST);
}
// Records the result of the coin flip in PreviewsUserData and UKM. This may be // Records the result of the coin flip in PreviewsUserData and UKM. This may be
// called multiple times during a navigation (like on redirects), but once // called multiple times during a navigation (like on redirects), but once
// called with one |result|, that value is not expected to change. // called with one |result|, that value is not expected to change.
...@@ -259,7 +247,6 @@ blink::PreviewsState DetermineCommittedClientPreviewsState( ...@@ -259,7 +247,6 @@ blink::PreviewsState DetermineCommittedClientPreviewsState(
if (previews_decider && previews_decider->ShouldCommitPreview( if (previews_decider && previews_decider->ShouldCommitPreview(
previews_data, navigation_handle, previews_data, navigation_handle,
previews::PreviewsType::DEFER_ALL_SCRIPT)) { previews::PreviewsType::DEFER_ALL_SCRIPT)) {
LogCommittedPreview(previews_data, PreviewsType::DEFER_ALL_SCRIPT);
return blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON; return blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON;
} }
// Remove DEFER_ALL_SCRIPT_ON from |previews_state| since we decided not to // Remove DEFER_ALL_SCRIPT_ON from |previews_state| since we decided not to
...@@ -275,7 +262,6 @@ blink::PreviewsState DetermineCommittedClientPreviewsState( ...@@ -275,7 +262,6 @@ blink::PreviewsState DetermineCommittedClientPreviewsState(
previews_decider->ShouldCommitPreview( previews_decider->ShouldCommitPreview(
previews_data, navigation_handle, previews_data, navigation_handle,
previews::PreviewsType::RESOURCE_LOADING_HINTS)) { previews::PreviewsType::RESOURCE_LOADING_HINTS)) {
LogCommittedPreview(previews_data, PreviewsType::RESOURCE_LOADING_HINTS);
return blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON; return blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON;
} }
// Remove RESOURCE_LOADING_HINTS_ON from |previews_state| since we decided // Remove RESOURCE_LOADING_HINTS_ON from |previews_state| since we decided
...@@ -290,7 +276,6 @@ blink::PreviewsState DetermineCommittedClientPreviewsState( ...@@ -290,7 +276,6 @@ blink::PreviewsState DetermineCommittedClientPreviewsState(
if (previews_decider && previews_decider->ShouldCommitPreview( if (previews_decider && previews_decider->ShouldCommitPreview(
previews_data, navigation_handle, previews_data, navigation_handle,
previews::PreviewsType::NOSCRIPT)) { previews::PreviewsType::NOSCRIPT)) {
LogCommittedPreview(previews_data, PreviewsType::NOSCRIPT);
return blink::PreviewsTypes::NOSCRIPT_ON; return blink::PreviewsTypes::NOSCRIPT_ON;
} }
// Remove NOSCRIPT_ON from |previews_state| since we decided not to // Remove NOSCRIPT_ON from |previews_state| since we decided not to
......
...@@ -255,7 +255,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) { ...@@ -255,7 +255,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) {
std::string()); std::string());
PreviewsUserData user_data(1); PreviewsUserData user_data(1);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G); user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
base::HistogramTester histogram_tester;
// DeferAllScript has precedence over NoScript and ResourceLoadingHints. // DeferAllScript has precedence over NoScript and ResourceLoadingHints.
EXPECT_EQ(blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON, EXPECT_EQ(blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON,
...@@ -265,11 +264,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) { ...@@ -265,11 +264,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) {
blink::PreviewsTypes::NOSCRIPT_ON | blink::PreviewsTypes::NOSCRIPT_ON |
blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON, blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON,
enabled_previews_decider(), nullptr)); 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", 1);
// RESOURCE_LOADING_HINTS has precedence over NoScript. // RESOURCE_LOADING_HINTS has precedence over NoScript.
EXPECT_EQ(blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON, EXPECT_EQ(blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON,
...@@ -278,11 +272,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) { ...@@ -278,11 +272,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsState) {
blink::PreviewsTypes::NOSCRIPT_ON | blink::PreviewsTypes::NOSCRIPT_ON |
blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON, blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON,
enabled_previews_decider(), nullptr)); enabled_previews_decider(), nullptr));
histogram_tester.ExpectBucketCount(
"Previews.Triggered.EffectiveConnectionType2.ResourceLoadingHints",
static_cast<int>(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G), 1);
histogram_tester.ExpectTotalCount(
"Previews.Triggered.EffectiveConnectionType2", 2);
// Only NoScript: // Only NoScript:
EXPECT_EQ(blink::PreviewsTypes::NOSCRIPT_ON, EXPECT_EQ(blink::PreviewsTypes::NOSCRIPT_ON,
...@@ -299,7 +288,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsStateForHttp) { ...@@ -299,7 +288,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsStateForHttp) {
std::string()); std::string());
PreviewsUserData user_data(1); PreviewsUserData user_data(1);
user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G); user_data.set_navigation_ect(net::EFFECTIVE_CONNECTION_TYPE_2G);
base::HistogramTester histogram_tester;
// Verify that these previews do now commit on HTTP. // Verify that these previews do now commit on HTTP.
EXPECT_EQ(blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON, EXPECT_EQ(blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON,
...@@ -309,8 +297,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsStateForHttp) { ...@@ -309,8 +297,6 @@ TEST_F(PreviewsContentUtilTest, DetermineCommittedClientPreviewsStateForHttp) {
blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON | blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON |
blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON, blink::PreviewsTypes::DEFER_ALL_SCRIPT_ON,
enabled_previews_decider(), nullptr)); enabled_previews_decider(), nullptr));
histogram_tester.ExpectTotalCount(
"Previews.Triggered.EffectiveConnectionType2", 1);
EXPECT_EQ(blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON, EXPECT_EQ(blink::PreviewsTypes::RESOURCE_LOADING_HINTS_ON,
previews::DetermineCommittedClientPreviewsState( previews::DetermineCommittedClientPreviewsState(
......
...@@ -10552,10 +10552,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -10552,10 +10552,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</histogram> </histogram>
<histogram name="Previews.CacheControlNoTransform.BlockedPreview" <histogram name="Previews.CacheControlNoTransform.BlockedPreview"
enum="PreviewsType" expires_after="2020-12-12"> enum="PreviewsType" expires_after="2021-03-31">
<owner>dougarnett@chromium.org</owner> <owner>sophiechang@chromium.org</owner>
<owner>mcrouse@chromium.org</owner> <owner>mcrouse@chromium.org</owner>
<owner>src/components/data_reduction_proxy/OWNERS</owner> <owner>src/components/previews/OWNERS</owner>
<summary> <summary>
Blocked previews due to Cache-Control:no-transform directive. Blocked previews due to Cache-Control:no-transform directive.
</summary> </summary>
...@@ -10696,9 +10696,12 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -10696,9 +10696,12 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
<histogram name="Previews.Triggered.EffectiveConnectionType2" <histogram name="Previews.Triggered.EffectiveConnectionType2"
enum="NQEEffectiveConnectionType" expires_after="2020-12-12"> enum="NQEEffectiveConnectionType" expires_after="2020-12-12">
<obsolete>
Obsolete as of 11/2020.
</obsolete>
<owner>dougarnett@chromium.org</owner> <owner>dougarnett@chromium.org</owner>
<owner>mcrouse@chromium.org</owner> <owner>mcrouse@chromium.org</owner>
<owner>src/components/data_reduction_proxy/OWNERS</owner> <owner>src/components/previews/OWNERS</owner>
<summary> <summary>
Records the effective connection type of a navigation that triggers a Records the effective connection type of a navigation that triggers a
preview. This is captured at commit time but uses the effective connection preview. This is captured at commit time but uses the effective connection
......
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