New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 874576 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

image-compression is flaky

Project Member Reported by ericbidelman@chromium.org, Aug 15

Issue description

Chrome Version: 70.0.3523.0
OS: macos x

What steps will reproduce the problem?
(1) Visit https://feature-policy-demos.appspot.com/image-compression.html?on

What is the expected result?

The image on the left should always have inverted colors. The policy on the frame is image-compression 'none' when that page loads.

What happens instead?

The image sometimes has inverted colors.


The flakiness seems related to the browser caching the image. Opening the devtools and disabled the browser cache fixes the issue (the image is always inverted). Hard refreshing also seem to work.
 
Cc: -lunalu@chromium.org loonyb...@chromium.org iclell...@chromium.org
Owner: loonyb...@chromium.org
Status: Assigned (was: Untriaged)
Reproduced it locally. Will look into it. 
Status: Started (was: Assigned)
The problem was that we use the http response to tell the file size but when the image is cached there's no response. So the policy will get bypassed. The fix is to use image data size (if available) when response is missing.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/931192325d10e124470b235abac1d98f7ffd4a64

commit 931192325d10e124470b235abac1d98f7ffd4a64
Author: Luna Lu <loonybear@chromium.org>
Date: Thu Oct 25 14:51:39 2018

Feature Policy image-compression policy use image data size as resource legnth

The policy was flaky with cached image since the for cached images the
encodedbody and decodedbody will be 0. Instead, we should use image
data size to fix the problem.

Bug:  874576 
Change-Id: I8776e202d325353c9bf58b6a46c6b3fe0ee94eed
Reviewed-on: https://chromium-review.googlesource.com/c/1298236
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Luna Lu <loonybear@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602706}
[add] https://crrev.com/931192325d10e124470b235abac1d98f7ffd4a64/third_party/WebKit/LayoutTests/http/tests/images/feature-policy-image-compression-cached-image-expected.png
[add] https://crrev.com/931192325d10e124470b235abac1d98f7ffd4a64/third_party/WebKit/LayoutTests/http/tests/images/feature-policy-image-compression-cached-image.html
[modify] https://crrev.com/931192325d10e124470b235abac1d98f7ffd4a64/third_party/blink/renderer/core/loader/resource/image_resource_content.cc

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a3aaa38203e9fbb058280bf4e279b7a3f85c539

commit 7a3aaa38203e9fbb058280bf4e279b7a3f85c539
Author: Luna Lu <loonybear@chromium.org>
Date: Thu Oct 25 15:46:13 2018

Update test expect for image policies.

With the change in this CL: https://chromium-review.googlesource.com/c/chromium/src/+/1244111
The new test introduced in this CL https://chromium-review.googlesource.com/c/chromium/src/+/1298236
should update its test expect.

Bug:  874576 
Change-Id: I9dfa7e055e62656c291d960a197aa24eee7f5fd5
Reviewed-on: https://chromium-review.googlesource.com/c/1299338
Commit-Queue: Luna Lu <loonybear@chromium.org>
Reviewed-by: Luna Lu <loonybear@chromium.org>
Reviewed-by: Adithya Srinivasan <adithyas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602718}
[modify] https://crrev.com/7a3aaa38203e9fbb058280bf4e279b7a3f85c539/third_party/WebKit/LayoutTests/http/tests/images/feature-policy-image-compression-cached-image-expected.png

Status: Assigned (was: Fixed)
This is now failing consistently after the previous patch.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f037de8d335a1724a679e13ba05393d322fb40c

commit 6f037de8d335a1724a679e13ba05393d322fb40c
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Oct 25 16:55:19 2018

Update test expectations to skip failing test.

BUG= 874576 
TEST=none
TBR=loonybear

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I85294c0bd3004a4344ced457aae8029f8b239698
Reviewed-on: https://chromium-review.googlesource.com/c/1299544
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602759}
[modify] https://crrev.com/6f037de8d335a1724a679e13ba05393d322fb40c/third_party/WebKit/LayoutTests/TestExpectations

Status: Fixed (was: Assigned)
Tests re-enabled in https://chromium-review.googlesource.com/c/chromium/src/+/1318372

Sign in to add a comment