Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Unified Diff: components/data_reduction_proxy/content/browser/content_lofi_decider.cc

Issue 2802843003: Update CPAT protocol to send lite-page transform acceptance with ect
Patch Set: Fixed testLitePageNoFallback integration test (fixed forcing ECT) 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
Download patch
Index: components/data_reduction_proxy/content/browser/content_lofi_decider.cc
diff --git a/components/data_reduction_proxy/content/browser/content_lofi_decider.cc b/components/data_reduction_proxy/content/browser/content_lofi_decider.cc
index f600474209e79d6a53f1f31e3462e89a25855216..b67424c3063079bb84847d41ab130b4ee55ec9ac 100644
--- a/components/data_reduction_proxy/content/browser/content_lofi_decider.cc
+++ b/components/data_reduction_proxy/content/browser/content_lofi_decider.cc
@@ -43,7 +43,6 @@ bool ContentLoFiDecider::IsUsingLoFi(const net::URLRequest& request) const {
void ContentLoFiDecider::MaybeSetAcceptTransformHeader(
const net::URLRequest& request,
- bool are_previews_disabled,
net::HttpRequestHeaders* headers) const {
const content::ResourceRequestInfo* request_info =
content::ResourceRequestInfo::ForRequest(&request);
@@ -67,69 +66,37 @@ void ContentLoFiDecider::MaybeSetAcceptTransformHeader(
return;
}
- // The Lo-Fi and Lite Page directives should not be added for users in the
- // Lo-Fi field trial "Control" group.
- bool lofi_enabled_via_flags_or_field_trial =
- params::IsLoFiOnViaFlags() || params::IsIncludedInLoFiEnabledFieldTrial();
-
- bool lite_page_enabled_via_flags_or_field_trial =
- (params::IsLoFiOnViaFlags() && params::AreLitePagesEnabledViaFlags()) ||
- params::IsIncludedInLitePageFieldTrial();
-
- // User does not have previews enabled.
- if (!lofi_enabled_via_flags_or_field_trial &&
- !lite_page_enabled_via_flags_or_field_trial) {
- return;
- }
-
- // Previews has been disabled.
- if (are_previews_disabled)
+ // Do not add the Chrome-Proxy-Accept-Transform header if the page load
+ // has previews turned off.
megjablon 2017/04/25 23:10:36 s/if the page load/if the request
dougarnett 2017/04/26 19:50:48 Done.
+ content::PreviewsState previews_state = request_info->GetPreviewsState();
+ if (previews_state & content::PREVIEWS_OFF)
return;
+ // TODO(dougarnett): Remove once blink uses Off instead of NoTransform.
// Do not add the Chrome-Proxy-Accept-Transform header when the page load
// explicitly forbids previews transformations.
- if (request_info->GetPreviewsState() & content::PREVIEWS_NO_TRANSFORM)
+ if (previews_state & content::PREVIEWS_NO_TRANSFORM)
return;
- // Lo-Fi is not allowed on the main frame, stylesheet, script, font resource,
- // media, service worker, or CSP report.
- bool resource_type_supports_empty_image =
- !(resource_type == content::RESOURCE_TYPE_MAIN_FRAME ||
- resource_type == content::RESOURCE_TYPE_STYLESHEET ||
- resource_type == content::RESOURCE_TYPE_SCRIPT ||
- resource_type == content::RESOURCE_TYPE_FONT_RESOURCE ||
- resource_type == content::RESOURCE_TYPE_MEDIA ||
- resource_type == content::RESOURCE_TYPE_CSP_REPORT);
-
// If the Lite Page field trial or flag is enabled, only add the "lite-page"
// directive on main frame requests. Only add "empty-image" directives to
// other requests when Lite Page previews are not enabled after the main
- // frame. Add the "if-heavy" qualifier to allow the server to provide a
- // preview when the page is data heavy on if a preview was not otherwise
- // triggered.
+ // frame.
std::string accept_transform_value;
- if (lite_page_enabled_via_flags_or_field_trial &&
+ if (previews_state & content::SERVER_LITE_PAGE_ON &&
resource_type == content::RESOURCE_TYPE_MAIN_FRAME) {
accept_transform_value = lite_page_directive();
-
- // Since a Lite Page was not triggered client side, ask the server to
- // provide a Lite Page only if the page is otherwise data-heavy.
- if (!(request_info->GetPreviewsState() & content::SERVER_LITE_PAGE_ON))
- accept_transform_value += base::StringPrintf(";%s", if_heavy_qualifier());
- } else if (lofi_enabled_via_flags_or_field_trial &&
- // Only use Lo-Fi if Lite Pages aren't enabled or fallback from
- // Lite Pages to Lo-Fi is enabled.
- (!lite_page_enabled_via_flags_or_field_trial ||
- params::IsLitePageFallbackEnabled()) &&
- resource_type_supports_empty_image &&
- !(request_info->GetPreviewsState() &
- content::SERVER_LITE_PAGE_ON)) {
- accept_transform_value = empty_image_directive();
-
- // Since Lo-Fi was not triggered client side, ask the server to provide
- // Lo-Fi only if the page is otherwise data-heavy.
- if (!(request_info->GetPreviewsState() & content::SERVER_LOFI_ON))
- accept_transform_value += base::StringPrintf(";%s", if_heavy_qualifier());
+ } else if (previews_state & content::SERVER_LOFI_ON) {
megjablon 2017/04/25 23:10:36 I'd keep resource_type_supports_empty_image and us
dougarnett 2017/04/26 19:50:48 Done.
+ // Lo-Fi is not allowed on the main frame, stylesheet, script, font
+ // resource, media, service worker, or CSP report.
+ if (!(resource_type == content::RESOURCE_TYPE_MAIN_FRAME ||
+ resource_type == content::RESOURCE_TYPE_STYLESHEET ||
+ resource_type == content::RESOURCE_TYPE_SCRIPT ||
+ resource_type == content::RESOURCE_TYPE_FONT_RESOURCE ||
+ resource_type == content::RESOURCE_TYPE_MEDIA ||
+ resource_type == content::RESOURCE_TYPE_CSP_REPORT)) {
+ accept_transform_value = empty_image_directive();
+ }
}
if (accept_transform_value.empty())
return;
@@ -180,19 +147,23 @@ void ContentLoFiDecider::RemoveAcceptTransformHeader(
headers->RemoveHeader(chrome_proxy_accept_transform_header());
}
-void ContentLoFiDecider::MaybeSetIgnorePreviewsBlacklistDirective(
+void ContentLoFiDecider::MaybeSetForceLitePageDirective(
net::HttpRequestHeaders* headers) const {
- if (!headers || !params::AreLitePagesEnabledViaFlags() ||
- !IsLitePagePreviewRequested(*headers)) {
+ // Ensure lite page is requested in headers.
+ if (!headers || !IsLitePagePreviewRequested(*headers))
return;
- }
+
+ // Ensure either command line flag for lite pages or for LoFi on cellular.
+ if (!params::AreLitePagesEnabledViaFlags() &&
megjablon 2017/04/25 23:10:36 Why do we care about cellular only here?
dougarnett 2017/04/26 19:50:48 Yeah, not sure, reverting.
+ !params::IsLoFiCellularOnlyViaFlags())
megjablon 2017/04/25 23:10:36 Use brackets
dougarnett 2017/04/26 19:50:48 Done.
+ return;
+
std::string chrome_proxy_header_value;
headers->GetHeader(chrome_proxy_header(), &chrome_proxy_header_value);
headers->RemoveHeader(chrome_proxy_header());
if (!chrome_proxy_header_value.empty())
chrome_proxy_header_value += ", ";
- chrome_proxy_header_value +=
- chrome_proxy_lite_page_ignore_blacklist_directive();
+ chrome_proxy_header_value += chrome_proxy_force_lite_page_directive();
headers->SetHeader(chrome_proxy_header(), chrome_proxy_header_value);
}

Powered by Google App Engine
This is Rietveld 408576698