Commit f8f15ef8 authored by Luna Lu's avatar Luna Lu Committed by Commit Bot

Update max-image policy

Currently the policy uses a image element's naturalWidth/naturalHeight
as the image's size, but this is incorrect for cases:
  1. intrinsicSize attribute overrides naturalWidth/naturalHeight;
  2. srcset w descriptor sizes will determine
     naturalWidth/naturalHeight;

So, instead, we should be using the actual size of the Image object.

Bug: 888578
Change-Id: I2abc9854ffd8c0d942651085d510885a82884ad8
Reviewed-on: https://chromium-review.googlesource.com/1240497Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Commit-Queue: Luna Lu <loonybear@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593665}
parent 87a32276
<!DOCTYPE html>
<body>
<iframe id="simple" src="resources/frame-with-max-downscaling-image-responsive-images-expected.html" width="600" height="500">
</iframe>
</body>
<!DOCTYPE html>
<link rel="match" href="feature-policy-max-downscaling-image-responsive-image-expected.html">
<body>
<iframe id="simple" src="resources/frame-with-max-downscaling-image-responsive-images.html" allow="max-downscaling-image 'none'" width="600" height="500">
</iframe>
</body>
<!DOCTYPE html>
<body>
<img src="green-256x256.jpg" style="filter:invert(1);" width="127" height="127">
<img src="green-256x256.jpg" style="filter:invert(1);" width="127" height="127">
<img src="green-256x256.jpg" width="128" height="128">
<img src="green-256x256.jpg" width="129" height="129">
</body>
<!DOCTYPE html>
<body>
<img src="green-256x256.jpg" intrinsicsize="100 x 100" width="127" height="127">
<img srcset="green-256x256.jpg 256w" sizes="100px" width="127" height="127">
<img srcset="green-256x256.jpg 256w" sizes="100px" width="128" height="128">
<img srcset="green-256x256.jpg 256w" sizes="100px" width="129" height="129">
</body>
...@@ -75,25 +75,29 @@ bool CheckForOptimizedImagePolicy(const LocalFrame& frame, ...@@ -75,25 +75,29 @@ bool CheckForOptimizedImagePolicy(const LocalFrame& frame,
} }
bool CheckForMaxDownscalingImagePolicy(const LocalFrame& frame, bool CheckForMaxDownscalingImagePolicy(const LocalFrame& frame,
HTMLImageElement* element, ImageResourceContent* new_image,
LayoutImage* layout_image) { LayoutImage* layout_image) {
DCHECK(new_image);
if (!RuntimeEnabledFeatures::ExperimentalProductivityFeaturesEnabled() || if (!RuntimeEnabledFeatures::ExperimentalProductivityFeaturesEnabled() ||
frame.IsFeatureEnabled(mojom::FeaturePolicyFeature::kMaxDownscalingImage)) frame.IsFeatureEnabled(mojom::FeaturePolicyFeature::kMaxDownscalingImage))
return false; return false;
// Invert the image if the image's size is more than 2 times bigger than the if (auto* image = new_image->GetImage()) {
// size it is being laid-out by. // Invert the image if the image's size is more than 2 times bigger than the
LayoutUnit layout_width = layout_image->ContentWidth(); // size it is being laid-out by.
LayoutUnit layout_height = layout_image->ContentHeight(); LayoutUnit layout_width = layout_image->ContentWidth();
auto image_width = element->naturalWidth(); LayoutUnit layout_height = layout_image->ContentHeight();
auto image_height = element->naturalHeight(); int image_width = image->width();
if (layout_width > 0 && layout_height > 0 && image_width > 0 && int image_height = image->height();
image_height > 0) {
double device_pixel_ratio = frame.DevicePixelRatio(); if (layout_width > 0 && layout_height > 0 && image_width > 0 &&
if (LayoutUnit(image_width / (kmax_downscaling_ratio * image_height > 0) {
device_pixel_ratio)) > layout_width || double device_pixel_ratio = frame.DevicePixelRatio();
LayoutUnit(image_height / (kmax_downscaling_ratio * if (LayoutUnit(image_width / (kmax_downscaling_ratio *
device_pixel_ratio)) > layout_height) device_pixel_ratio)) > layout_width ||
return true; LayoutUnit(image_height / (kmax_downscaling_ratio *
device_pixel_ratio)) > layout_height)
return true;
}
} }
return false; return false;
} }
...@@ -274,7 +278,7 @@ void LayoutImage::ImageNotifyFinished(ImageResourceContent* new_image) { ...@@ -274,7 +278,7 @@ void LayoutImage::ImageNotifyFinished(ImageResourceContent* new_image) {
CheckForOptimizedImagePolicy(frame, this, new_image); CheckForOptimizedImagePolicy(frame, this, new_image);
if (auto* image_element = ToHTMLImageElementOrNull(GetNode())) { if (auto* image_element = ToHTMLImageElementOrNull(GetNode())) {
is_downscaled_image_ = is_downscaled_image_ =
CheckForMaxDownscalingImagePolicy(frame, image_element, this); CheckForMaxDownscalingImagePolicy(frame, new_image, this);
} }
if (old_flag != ShouldInvertColor()) if (old_flag != ShouldInvertColor())
UpdateShouldInvertColor(); UpdateShouldInvertColor();
...@@ -502,12 +506,14 @@ void LayoutImage::UpdateAfterLayout() { ...@@ -502,12 +506,14 @@ void LayoutImage::UpdateAfterLayout() {
if (View() && View()->GetFrameView()) { if (View() && View()->GetFrameView()) {
const LocalFrame& frame = View()->GetFrameView()->GetFrame(); const LocalFrame& frame = View()->GetFrameView()->GetFrame();
// Check for optimized image policies. if (image_resource_ && image_resource_->CachedImage()) {
bool old_flag = ShouldInvertColor(); // Check for optimized image policies.
is_downscaled_image_ = bool old_flag = ShouldInvertColor();
CheckForMaxDownscalingImagePolicy(frame, image_element, this); is_downscaled_image_ = CheckForMaxDownscalingImagePolicy(
if (old_flag != ShouldInvertColor()) frame, image_resource_->CachedImage(), this);
UpdateShouldInvertColor(); if (old_flag != ShouldInvertColor())
UpdateShouldInvertColor();
}
} }
// Report violation of unsized-media policy. // Report violation of unsized-media policy.
......
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