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

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

Issue 2911673002: New CPAT support in ContentLoFiDecider guarded by feature flag. (Closed)
Patch Set: Backed out some debug code Created 3 years, 7 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
« no previous file with comments | « no previous file | components/data_reduction_proxy/content/browser/content_lofi_decider_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 4f98e4eea1b944c2b0e38617618007e909bf3065..2261b4cfd87a050e076035184faf6b76c8b2c028 100644
--- a/components/data_reduction_proxy/content/browser/content_lofi_decider.cc
+++ b/components/data_reduction_proxy/content/browser/content_lofi_decider.cc
@@ -6,9 +6,11 @@
#include <string>
+#include "base/feature_list.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
#include "content/public/browser/resource_request_info.h"
@@ -67,29 +69,47 @@ 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)
- return;
+ content::PreviewsState previews_state = request_info->GetPreviewsState();
// 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;
+ }
+
+ // For the proxy-decides-transform feature, the header is set based only
+ // on the request's PreviewsState (no checking of previews flags).
+ // This is for a newer version of the proxy server protocol where the
+ // server makes the transform decision and client simply advertises
+ // if is accepts transforms for the resource type (determined here from
+ // previously set previews state bits). The previous logic in this method
+ // that checks various flags will be deprecated.
+ bool check_previews_flags = !base::FeatureList::IsEnabled(
+ features::kDataReductionProxyDecidesTransform);
+
+ bool lofi_enabled_via_flags_or_field_trial = false;
+ bool lite_page_enabled_via_flags_or_field_trial = false;
+ if (check_previews_flags) {
+ // The Lo-Fi and Lite Page directives should not be added for users in the
+ // Lo-Fi field trial "Control" group.
+ lofi_enabled_via_flags_or_field_trial =
+ params::IsLoFiOnViaFlags() ||
+ params::IsIncludedInLoFiEnabledFieldTrial();
+
+ lite_page_enabled_via_flags_or_field_trial =
+ (params::IsLoFiOnViaFlags() && params::AreLitePagesEnabledViaFlags()) ||
+ params::IsIncludedInLitePageFieldTrial();
+
+ // Previews have been disabled.
+ if (are_previews_disabled)
+ return;
+
+ // User does not have previews enabled.
+ if (!lofi_enabled_via_flags_or_field_trial &&
+ !lite_page_enabled_via_flags_or_field_trial) {
+ return;
+ }
+ }
// Lo-Fi is not allowed on the main frame, stylesheet, script, font resource,
// media, service worker, or CSP report.
@@ -101,31 +121,43 @@ void ContentLoFiDecider::MaybeSetAcceptTransformHeader(
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.
std::string accept_transform_value;
- if (lite_page_enabled_via_flags_or_field_trial &&
- resource_type == content::RESOURCE_TYPE_MAIN_FRAME) {
+ if (!check_previews_flags) {
+ // For the proxy-decides-transform feature, check PreviewsState bits and
+ // type of resource to determine which, if any, transformation to accept.
+ if ((previews_state & content::SERVER_LITE_PAGE_ON) &&
+ resource_type == content::RESOURCE_TYPE_MAIN_FRAME) {
+ accept_transform_value = lite_page_directive();
+ } else if ((previews_state & content::SERVER_LOFI_ON) &&
+ resource_type_supports_empty_image) {
+ // Note that for subresource requests, the Lo-Fi bit should only be set
+ // if the main frame response provided the "empty-image" directive (for
+ // the client to echo back to the server here for any image resources).
+ accept_transform_value = empty_image_directive();
+ }
+ } else if (lite_page_enabled_via_flags_or_field_trial &&
+ resource_type == content::RESOURCE_TYPE_MAIN_FRAME) {
+ // 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.
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))
+ if (!(previews_state & content::SERVER_LITE_PAGE_ON))
accept_transform_value += base::StringPrintf(";%s", if_heavy_qualifier());
} else if (lofi_enabled_via_flags_or_field_trial &&
!lite_page_enabled_via_flags_or_field_trial &&
resource_type_supports_empty_image &&
- !(request_info->GetPreviewsState() &
- content::SERVER_LITE_PAGE_ON)) {
+ !(previews_state & 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))
+ if (!(previews_state & content::SERVER_LOFI_ON))
accept_transform_value += base::StringPrintf(";%s", if_heavy_qualifier());
}
if (accept_transform_value.empty())
« no previous file with comments | « no previous file | components/data_reduction_proxy/content/browser/content_lofi_decider_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698