Commit 50c63e38 authored by Paul Meyer's avatar Paul Meyer Committed by Commit Bot

Change unoptimized-images feature policy to unoptimized-lossy-images.

This makes it more clear that this policy applies only to lossy image
types (for now, just JPEG). This patch also makes the implementation
more generic in order to make the policy compatible with parsed
policy values and to pave the way for implementing a
lossless-enforcing version of the unoptimized-images policy as well.

This is a reland of this reverted CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1531354

Bug: 942659
Change-Id: I7928de8a7f3e66841ec3792a4152a3570a709ca3
TBR: iclelland@chromium.org,pdr@chromium.org,kenrb@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1541579
Commit-Queue: Paul Meyer <paulmeyer@chromium.org>
Reviewed-by: default avatarPaul Meyer <paulmeyer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644832}
parent 2445a275
...@@ -378,9 +378,6 @@ const FeaturePolicy::FeatureList& FeaturePolicy::GetDefaultFeatureList() { ...@@ -378,9 +378,6 @@ const FeaturePolicy::FeatureList& FeaturePolicy::GetDefaultFeatureList() {
{mojom::FeaturePolicyFeature::kHid, {mojom::FeaturePolicyFeature::kHid,
FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForSelf, FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForSelf,
mojom::PolicyValueType::kBool)}, mojom::PolicyValueType::kBool)},
{mojom::FeaturePolicyFeature::kUnoptimizedImages,
FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForAll,
mojom::PolicyValueType::kBool)},
{mojom::FeaturePolicyFeature::kIdleDetection, {mojom::FeaturePolicyFeature::kIdleDetection,
FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForSelf, FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForSelf,
mojom::PolicyValueType::kBool)}, mojom::PolicyValueType::kBool)},
...@@ -444,6 +441,9 @@ const FeaturePolicy::FeatureList& FeaturePolicy::GetDefaultFeatureList() { ...@@ -444,6 +441,9 @@ const FeaturePolicy::FeatureList& FeaturePolicy::GetDefaultFeatureList() {
{mojom::FeaturePolicyFeature::kTopNavigation, {mojom::FeaturePolicyFeature::kTopNavigation,
FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForAll, FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForAll,
mojom::PolicyValueType::kBool)}, mojom::PolicyValueType::kBool)},
{mojom::FeaturePolicyFeature::kUnoptimizedLossyImages,
FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForAll,
mojom::PolicyValueType::kDecDouble)},
{mojom::FeaturePolicyFeature::kUnsizedMedia, {mojom::FeaturePolicyFeature::kUnsizedMedia,
FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForAll, FeatureDefaultValue(FeaturePolicy::FeatureDefault::EnableForAll,
mojom::PolicyValueType::kBool)}, mojom::PolicyValueType::kBool)},
......
...@@ -82,8 +82,6 @@ enum FeaturePolicyFeature { ...@@ -82,8 +82,6 @@ enum FeaturePolicyFeature {
kUnsizedMedia = 21, kUnsizedMedia = 21,
// Controls which image formats are allowed to be used in the document. // Controls which image formats are allowed to be used in the document.
kLegacyImageFormats = 22, kLegacyImageFormats = 22,
// When disallowed, requires images to have a reasonable byte-to-pixel ratio.
kUnoptimizedImages = 23,
// When disallowed, restricts source image size to be no more 2x larger than // When disallowed, restricts source image size to be no more 2x larger than
// the image's containing block. // the image's containing block.
kOversizedImages = 25, kOversizedImages = 25,
...@@ -125,6 +123,9 @@ enum FeaturePolicyFeature { ...@@ -125,6 +123,9 @@ enum FeaturePolicyFeature {
// Controls access to Idle Detection // Controls access to Idle Detection
kIdleDetection = 44, kIdleDetection = 44,
// When disallowed, these policies require images to have a reasonable byte-to-pixel ratio.
kUnoptimizedLossyImages = 45,
// Don't change assigned numbers of any item, and don't reuse removed slots. // Don't change assigned numbers of any item, and don't reuse removed slots.
// Also, run update_feature_policy_enum.py in // Also, run update_feature_policy_enum.py in
// chromium/src/tools/metrics/histograms/ to update the UMA mapping. // chromium/src/tools/metrics/histograms/ to update the UMA mapping.
......
...@@ -46,8 +46,13 @@ bool IsOriginTrialDisabled(const String& feature_name, ...@@ -46,8 +46,13 @@ bool IsOriginTrialDisabled(const String& feature_name,
// parse the policy value for each parameterized feature, and for non // parse the policy value for each parameterized feature, and for non
// parameterized feature (i.e. boolean-type policy value). // parameterized feature (i.e. boolean-type policy value).
PolicyValue GetFallbackValueForFeature(mojom::FeaturePolicyFeature feature) { PolicyValue GetFallbackValueForFeature(mojom::FeaturePolicyFeature feature) {
if (feature == mojom::FeaturePolicyFeature::kOversizedImages) if (feature == mojom::FeaturePolicyFeature::kOversizedImages) {
return PolicyValue(2.0, mojom::PolicyValueType::kDecDouble); return PolicyValue(2.0);
}
if (feature == mojom::FeaturePolicyFeature::kUnoptimizedLossyImages) {
return PolicyValue(0.5);
}
return PolicyValue(false); return PolicyValue(false);
} }
...@@ -317,9 +322,6 @@ const FeatureNameMap& GetDefaultFeatureNameMap() { ...@@ -317,9 +322,6 @@ const FeatureNameMap& GetDefaultFeatureNameMap() {
"document-domain", mojom::FeaturePolicyFeature::kDocumentDomain); "document-domain", mojom::FeaturePolicyFeature::kDocumentDomain);
default_feature_name_map.Set("font-display-late-swap", default_feature_name_map.Set("font-display-late-swap",
mojom::FeaturePolicyFeature::kFontDisplay); mojom::FeaturePolicyFeature::kFontDisplay);
default_feature_name_map.Set(
"unoptimized-images",
mojom::FeaturePolicyFeature::kUnoptimizedImages);
default_feature_name_map.Set("lazyload", default_feature_name_map.Set("lazyload",
mojom::FeaturePolicyFeature::kLazyLoad); mojom::FeaturePolicyFeature::kLazyLoad);
default_feature_name_map.Set( default_feature_name_map.Set(
...@@ -333,6 +335,9 @@ const FeatureNameMap& GetDefaultFeatureNameMap() { ...@@ -333,6 +335,9 @@ const FeatureNameMap& GetDefaultFeatureNameMap() {
"vertical-scroll", mojom::FeaturePolicyFeature::kVerticalScroll); "vertical-scroll", mojom::FeaturePolicyFeature::kVerticalScroll);
default_feature_name_map.Set("sync-script", default_feature_name_map.Set("sync-script",
mojom::FeaturePolicyFeature::kSyncScript); mojom::FeaturePolicyFeature::kSyncScript);
default_feature_name_map.Set(
"unoptimized-lossy-images",
mojom::FeaturePolicyFeature::kUnoptimizedLossyImages);
} }
if (RuntimeEnabledFeatures::FeaturePolicyForSandboxEnabled()) { if (RuntimeEnabledFeatures::FeaturePolicyForSandboxEnabled()) {
default_feature_name_map.Set( default_feature_name_map.Set(
......
...@@ -102,14 +102,13 @@ bool CheckForOptimizedImagePolicy(const Document& document, ...@@ -102,14 +102,13 @@ bool CheckForOptimizedImagePolicy(const Document& document,
ImageResourceContent* new_image) { ImageResourceContent* new_image) {
if (!new_image) if (!new_image)
return false; return false;
bool is_legacy_format_or_unoptimized_image = false;
// Render the image as a placeholder image if the document does not have the // Render the image as a placeholder image if the document does not have the
// 'legacy-image-formats' feature enabled, and the image is not one of the // 'legacy-image-formats' feature enabled, and the image is not one of the
// allowed formats. // allowed formats.
if (!new_image->IsAcceptableContentType()) { if (!new_image->IsAcceptableContentType()) {
// If the image violates the 'legacy-image-formats' policy, record violation // If the image violates the 'legacy-image-formats' policy, record violation
// Blink.UseCounter.FeaturePolicy.PotentialVioation. Note that violation // Blink.UseCounter.FeaturePolicy.PotentialVioation. Note that violation
// report is up to once per page load). // report is up to once per page load.
document.CountPotentialFeaturePolicyViolation( document.CountPotentialFeaturePolicyViolation(
mojom::FeaturePolicyFeature::kLegacyImageFormats); mojom::FeaturePolicyFeature::kLegacyImageFormats);
// Check if 'legacy-image-formats' policy is disallowed by feature policy. // Check if 'legacy-image-formats' policy is disallowed by feature policy.
...@@ -117,27 +116,19 @@ bool CheckForOptimizedImagePolicy(const Document& document, ...@@ -117,27 +116,19 @@ bool CheckForOptimizedImagePolicy(const Document& document,
!document.IsFeatureEnabled( !document.IsFeatureEnabled(
mojom::FeaturePolicyFeature::kLegacyImageFormats, mojom::FeaturePolicyFeature::kLegacyImageFormats,
ReportOptions::kReportOnFailure)) { ReportOptions::kReportOnFailure)) {
is_legacy_format_or_unoptimized_image = true; return true;
} }
} }
// Render the image as a placeholder image if the document does not have the
// 'unoptimized-images' feature enabled and the image is not // Render the image as a placeholder image if the image is not sufficiently
// sufficiently-well-compressed. // well-compressed, according to the unoptimized image feature policies on
if (!new_image->IsAcceptableCompressionRatio()) { // |document|.
// If the image violates the 'unoptimized-images' policy, record violation if (RuntimeEnabledFeatures::ExperimentalProductivityFeaturesEnabled() &&
// Blink.UseCounter.FeaturePolicy.PotentialVioation. Note that violation !new_image->IsAcceptableCompressionRatio(document)) {
// report is up to once per page load). return true;
document.CountPotentialFeaturePolicyViolation(
mojom::FeaturePolicyFeature::kUnoptimizedImages);
// Check if 'unoptimized-images' policy is disallowed by feature policy.
if (RuntimeEnabledFeatures::ExperimentalProductivityFeaturesEnabled() &&
!document.IsFeatureEnabled(
mojom::FeaturePolicyFeature::kUnoptimizedImages,
ReportOptions::kReportOnFailure)) {
is_legacy_format_or_unoptimized_image = true;
}
} }
return is_legacy_format_or_unoptimized_image;
return false;
} }
} // namespace } // namespace
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <memory> #include <memory>
#include "third_party/blink/renderer/core/execution_context/security_context.h"
#include "third_party/blink/renderer/core/loader/resource/image_resource.h" #include "third_party/blink/renderer/core/loader/resource/image_resource.h"
#include "third_party/blink/renderer/core/loader/resource/image_resource_info.h" #include "third_party/blink/renderer/core/loader/resource/image_resource_info.h"
#include "third_party/blink/renderer/core/loader/resource/image_resource_observer.h" #include "third_party/blink/renderer/core/loader/resource/image_resource_observer.h"
...@@ -502,11 +503,11 @@ bool ImageResourceContent::IsAcceptableContentType() { ...@@ -502,11 +503,11 @@ bool ImageResourceContent::IsAcceptableContentType() {
} }
// Return true if the image content is well-compressed (and not full of // Return true if the image content is well-compressed (and not full of
// extraneous metadata). This is currently defined as no using more than 0.5 // extraneous metadata). "well-compressed" is determined by comparing the
// byte per pixel of image data with approximate header size(1KB) removed. // image's compression ratio against a specific value that is defined by an
// TODO(crbug.com/838263): Support site-defined bit-per-pixel ratio through // unoptimized image feature policy on |context|.
// feature policy declarations. bool ImageResourceContent::IsAcceptableCompressionRatio(
bool ImageResourceContent::IsAcceptableCompressionRatio() { const SecurityContext& context) {
uint64_t pixels = IntrinsicSize(kDoNotRespectImageOrientation).Area(); uint64_t pixels = IntrinsicSize(kDoNotRespectImageOrientation).Area();
if (!pixels) if (!pixels)
return true; return true;
...@@ -517,8 +518,26 @@ bool ImageResourceContent::IsAcceptableCompressionRatio() { ...@@ -517,8 +518,26 @@ bool ImageResourceContent::IsAcceptableCompressionRatio() {
// WPT and LayoutTests server returns -1 or 0 for the content length. // WPT and LayoutTests server returns -1 or 0 for the content length.
resource_length = static_cast<double>(GetImage()->Data()->size()); resource_length = static_cast<double>(GetImage()->Data()->size());
} }
// Allow no more than 10 bits per compressed pixel
return (resource_length - 1024) / pixels <= 0.5; // Calculate the image's adjusted compression ratio (in bytes per pixel). A
// constant allowance (1024 bytes) is provided to allow room for headers and
// to account for small images (which are harder to compress).
double compression_ratio = (resource_length - 1024) / pixels;
// Note that this approach may not always correctly identify the image (for
// example, due to a misconfigured web server). This approach SHOULD work in
// all usual cases, but content sniffing could be used in the future to more
// confidently identify the image type.
// TODO(crbug.com/943203): Implement content sniffing.
AtomicString mime_type = GetResponse().HttpContentType();
if (MIMETypeRegistry::IsLossyImageMIMEType(mime_type)) {
// Enforce the lossy image policy.
return context.IsFeatureEnabled(
mojom::FeaturePolicyFeature::kUnoptimizedLossyImages,
PolicyValue(compression_ratio), ReportOptions::kReportOnFailure);
}
return true;
} }
void ImageResourceContent::DecodedSizeChangedTo(const blink::Image* image, void ImageResourceContent::DecodedSizeChangedTo(const blink::Image* image,
......
...@@ -28,6 +28,7 @@ class ImageResourceObserver; ...@@ -28,6 +28,7 @@ class ImageResourceObserver;
class ResourceError; class ResourceError;
class ResourceFetcher; class ResourceFetcher;
class ResourceResponse; class ResourceResponse;
class SecurityContext;
// ImageResourceContent is a container that holds fetch result of // ImageResourceContent is a container that holds fetch result of
// an ImageResource in a decoded form. // an ImageResource in a decoded form.
...@@ -178,7 +179,7 @@ class CORE_EXPORT ImageResourceContent final ...@@ -178,7 +179,7 @@ class CORE_EXPORT ImageResourceContent final
// image resource violates any of the image policies in effect on the current // image resource violates any of the image policies in effect on the current
// page. // page.
bool IsAcceptableContentType(); bool IsAcceptableContentType();
bool IsAcceptableCompressionRatio(); bool IsAcceptableCompressionRatio(const SecurityContext& context);
void LoadDeferredImage(ResourceFetcher* fetcher); void LoadDeferredImage(ResourceFetcher* fetcher);
......
...@@ -205,4 +205,10 @@ bool MIMETypeRegistry::IsSupportedTextTrackMIMEType(const String& mime_type) { ...@@ -205,4 +205,10 @@ bool MIMETypeRegistry::IsSupportedTextTrackMIMEType(const String& mime_type) {
return EqualIgnoringASCIICase(mime_type, "text/vtt"); return EqualIgnoringASCIICase(mime_type, "text/vtt");
} }
bool MIMETypeRegistry::IsLossyImageMIMEType(const String& mime_type) {
return EqualIgnoringASCIICase(mime_type, "image/jpeg") ||
EqualIgnoringASCIICase(mime_type, "image/jpg") ||
EqualIgnoringASCIICase(mime_type, "image/pjpeg");
}
} // namespace blink } // namespace blink
...@@ -103,6 +103,11 @@ class PLATFORM_EXPORT MIMETypeRegistry { ...@@ -103,6 +103,11 @@ class PLATFORM_EXPORT MIMETypeRegistry {
// Checks to see if a mime type is suitable for being loaded as a text track. // Checks to see if a mime type is suitable for being loaded as a text track.
static bool IsSupportedTextTrackMIMEType(const String& mime_type); static bool IsSupportedTextTrackMIMEType(const String& mime_type);
// Checks to see if a mime type is an image type with lossy compression, whose
// size will be restricted via the 'unoptimized-lossy-images' feature
// policy. (JPEG)
static bool IsLossyImageMIMEType(const String& mime_type);
}; };
} // namespace blink } // namespace blink
......
...@@ -13,7 +13,7 @@ var check_report_format = (reports, observer) => { ...@@ -13,7 +13,7 @@ var check_report_format = (reports, observer) => {
let report = reports[0]; let report = reports[0];
assert_equals(report.type, "feature-policy-violation"); assert_equals(report.type, "feature-policy-violation");
assert_equals(report.url, document.location.href); assert_equals(report.url, document.location.href);
assert_equals(report.body.featureId, "unoptimized-images"); assert_equals(report.body.featureId, "unoptimized-lossy-images");
assert_equals(report.body.disposition, "enforce"); assert_equals(report.body.disposition, "enforce");
}; };
......
...@@ -11,7 +11,7 @@ var check_report_format = (reports, observer) => { ...@@ -11,7 +11,7 @@ var check_report_format = (reports, observer) => {
let report = reports[0]; let report = reports[0];
assert_equals(report.type, "feature-policy-violation"); assert_equals(report.type, "feature-policy-violation");
assert_equals(report.url, document.location.href); assert_equals(report.url, document.location.href);
assert_equals(report.body.featureId, "unoptimized-images"); assert_equals(report.body.featureId, "unoptimized-lossy-images");
assert_equals(report.body.disposition, "enforce"); assert_equals(report.body.disposition, "enforce");
}; };
......
CONSOLE ERROR: Feature policy violation: unoptimized-images is not allowed in this document. CONSOLE ERROR: Feature policy violation: unoptimized-lossy-images is not allowed in this document.
CONSOLE ERROR: Feature policy violation: unoptimized-images is not allowed in this document.
CONSOLE ERROR: Feature policy violation: legacy-image-formats is not allowed in this document.
CONSOLE ERROR: Feature policy violation: legacy-image-formats is not allowed in this document. CONSOLE ERROR: Feature policy violation: legacy-image-formats is not allowed in this document.
...@@ -4,4 +4,4 @@ body { ...@@ -4,4 +4,4 @@ body {
font: 10px Ahem; font: 10px Ahem;
} }
</style> </style>
<body><iframe src="resources/frame-with-images-with-border-radius.html" allow="legacy-image-formats 'none'; unoptimized-images 'none'" width="440" height="180"></iframe></body> <body><iframe src="resources/frame-with-images-with-border-radius.html" allow="legacy-image-formats 'none'; unoptimized-lossy-images 'none'" width="440" height="180"></iframe></body>
CONSOLE ERROR: Feature policy violation: unoptimized-images is not allowed in this document. CONSOLE ERROR: Feature policy violation: unoptimized-lossy-images is not allowed in this document.
CONSOLE ERROR: Feature policy violation: unoptimized-images is not allowed in this document. CONSOLE ERROR: Feature policy violation: unoptimized-lossy-images is not allowed in this document.
CONSOLE ERROR: Feature policy violation: unoptimized-images is not allowed in this document. CONSOLE ERROR: Feature policy violation: unoptimized-lossy-images is not allowed in this document.
...@@ -5,7 +5,7 @@ body { ...@@ -5,7 +5,7 @@ body {
} }
</style> </style>
<body> <body>
<iframe src="resources/frame-with-compression-test-images.html" allow="unoptimized-images 'none'" width="440" height="180"></iframe> <iframe src="resources/frame-with-few-compression-test-images.html" allow="unoptimized-lossy-images 'none'" width="500" height="180"></iframe>
<iframe src="resources/frame-with-compression-test-images.html" allow="unoptimized-images 'none'" width="440" height="180"></iframe> <iframe src="resources/frame-with-few-compression-test-images.html" allow="unoptimized-lossy-images 'none'" width="500" height="180"></iframe>
<iframe src="resources/frame-with-compression-test-images.html" allow="unoptimized-images 'none'" width="440" height="180"></iframe> <iframe src="resources/frame-with-few-compression-test-images.html" allow="unoptimized-lossy-images 'none'" width="500" height="180"></iframe>
</body> </body>
CONSOLE ERROR: Feature policy violation: unoptimized-images is not allowed in this document.
CONSOLE ERROR: Feature policy violation: unoptimized-lossy-images is not allowed in this document.
...@@ -4,4 +4,4 @@ body { ...@@ -4,4 +4,4 @@ body {
font: 10px Ahem; font: 10px Ahem;
} }
</style> </style>
<body><iframe src="resources/frame-with-compression-test-images.html" allow="unoptimized-images 'none'" width="440" height="180"></iframe></body> <body><iframe src="resources/frame-with-compression-test-images.html" allow="unoptimized-lossy-images 'none'" width="700" height="500"></iframe></body>
...@@ -4,4 +4,10 @@ body { ...@@ -4,4 +4,10 @@ body {
font: 10px Ahem; font: 10px Ahem;
} }
</style> </style>
<body><img src="Fisher-large.jpg"/><img src="Fisher-small.jpg"/></body> <body>
<img src="Fisher-large.jpg" width="200"/>
<img src="Fisher-small.jpg" width="200"/>
<img src="pass-all.png" width="200"/>
<img src="fail-strict.png" width="200"/>
<img src="fail-all.png" width="200"/>
</body>
<!DOCTYPE html>
<style>
body {
font: 10px Ahem;
}
</style>
<body>
<img src="Fisher-large.jpg" width="200"/>
<img src="Fisher-small.jpg" width="200"/>
</body>
...@@ -35,7 +35,7 @@ speaker ...@@ -35,7 +35,7 @@ speaker
sync-script sync-script
sync-xhr sync-xhr
top-navigation top-navigation
unoptimized-images unoptimized-lossy-images
unsized-media unsized-media
usb usb
vertical-scroll vertical-scroll
......
...@@ -22660,7 +22660,7 @@ Called by update_net_error_codes.py.--> ...@@ -22660,7 +22660,7 @@ Called by update_net_error_codes.py.-->
<int value="20" label="Magnetometer"/> <int value="20" label="Magnetometer"/>
<int value="21" label="UnsizedMedia"/> <int value="21" label="UnsizedMedia"/>
<int value="22" label="LegacyImageFormats"/> <int value="22" label="LegacyImageFormats"/>
<int value="23" label="UnoptimizedImages"/> <int value="23" label="Deprecated: UnoptimizedImages"/>
<int value="24" label="Animations"/> <int value="24" label="Animations"/>
<int value="25" label="OversizedImages"/> <int value="25" label="OversizedImages"/>
<int value="26" label="PictureInPicture"/> <int value="26" label="PictureInPicture"/>
...@@ -22682,6 +22682,7 @@ Called by update_net_error_codes.py.--> ...@@ -22682,6 +22682,7 @@ Called by update_net_error_codes.py.-->
<int value="42" label="Serial"/> <int value="42" label="Serial"/>
<int value="43" label="Hid"/> <int value="43" label="Hid"/>
<int value="44" label="IdleDetection"/> <int value="44" label="IdleDetection"/>
<int value="45" label="UnoptimizedLossyImages"/>
</enum> </enum>
<enum name="FeedbackSource"> <enum name="FeedbackSource">
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