New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-land: Remove workaround [1] to add a placeholder for lazyload images. #22364
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review process for this patch is being conducted in the Chromium project.
1266d91
to
dd8cd1d
Compare
This is a re-land of [2]. It was reverted due to a flakiness introduce in [3]. The reason for the flakiness appears to have a couple of sources: (a) sometimes the image for one of the ref/non-ref will load a bit faster (b) scrollbars present in the ref case, because the image is not clipped by overflow:hidden (c) 8px margin (a) is fixed by using reftest-wait. (b) is fixed by adding overflow:hidden to the HTML element; (c) is fixed with margin:0. [1] https://chromium-review.googlesource.com/c/chromium/src/+/1773869 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2103708 [3] external/wpt/html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html Bug: 992765, 999019, 995119, 999209, 1045745 Change-Id: I6ad8c5564f65d131da9b04180499a4bd98a4c8d2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111906 Reviewed-by: Stefan Zager <szager@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#752209}
dd8cd1d
to
9d1d00e
Compare
@chrishtr this hasn't been exported automatically because image-loading-subpixel-clip.html is flaky in Chrome Dev, passing 5/10 times and timing out the other 5/10 times. Is there change in third_party/blink/renderer/core/loader/image_loader.cc needed to make this non-flaky? If so that hasn't reached the dev channel yet and we should just admin merge this. But if you expected the test to become non-flaky from the test changes alone, there may be something more going on. |
There should not be an additional fix needed. My re-land specifically de-flaked this test. Looking now... |
@stephenmcgruer a follow-up CL is created to deflake these tests. I think its safe to admin merge for now? |
Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] web-platform-tests/wpt#22364 TBR:szager@chromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#752511}
Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] #22364 TBR:szager@chromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#752511}
Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] #22364 TBR:szager@chromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#752511} Co-authored-by: Chris Harrelson <chrishtr@chromium.org>
…l., a=testonly Automatic update from web-platform-tests Simplify image-loading-subpixel-clip.html. (#22403) Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] web-platform-tests/wpt#22364 TBR:szager@chromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#752511} Co-authored-by: Chris Harrelson <chrishtr@chromium.org> -- wpt-commits: a82dbcb196f74ccaf2715ff9f2773ee9d8bafad8 wpt-pr: 22403
…l., a=testonly Automatic update from web-platform-tests Simplify image-loading-subpixel-clip.html. (#22403) Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] web-platform-tests/wpt#22364 TBR:szager@chromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#752511} Co-authored-by: Chris Harrelson <chrishtr@chromium.org> -- wpt-commits: a82dbcb196f74ccaf2715ff9f2773ee9d8bafad8 wpt-pr: 22403
…l., a=testonly Automatic update from web-platform-tests Simplify image-loading-subpixel-clip.html. (#22403) Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] web-platform-tests/wpt#22364 TBR:szagerchromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtrchromium.org> Reviewed-by: Stefan Zager <szagerchromium.org> Reviewed-by: Chris Harrelson <chrishtrchromium.org> Cr-Commit-Position: refs/heads/master{#752511} Co-authored-by: Chris Harrelson <chrishtrchromium.org> -- wpt-commits: a82dbcb196f74ccaf2715ff9f2773ee9d8bafad8 wpt-pr: 22403 UltraBlame original commit: 27a2ebfc7fc75a480e47ba385bff85a6946c9d39
…l., a=testonly Automatic update from web-platform-tests Simplify image-loading-subpixel-clip.html. (#22403) Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] web-platform-tests/wpt#22364 TBR:szagerchromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtrchromium.org> Reviewed-by: Stefan Zager <szagerchromium.org> Reviewed-by: Chris Harrelson <chrishtrchromium.org> Cr-Commit-Position: refs/heads/master{#752511} Co-authored-by: Chris Harrelson <chrishtrchromium.org> -- wpt-commits: a82dbcb196f74ccaf2715ff9f2773ee9d8bafad8 wpt-pr: 22403 UltraBlame original commit: 27a2ebfc7fc75a480e47ba385bff85a6946c9d39
…l., a=testonly Automatic update from web-platform-tests Simplify image-loading-subpixel-clip.html. (#22403) Verified that it still fails with [1] applied and [2] reverted. This may help with deflaking WPT [3] [1] https://chromium-review.googlesource.com/c/chromium/src/+/2111906 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2092698 [3] web-platform-tests/wpt#22364 TBR:szagerchromium.org Change-Id: Ie668c43c4d03ea442c37075c0e80be5ba1481fff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115932 Commit-Queue: Chris Harrelson <chrishtrchromium.org> Reviewed-by: Stefan Zager <szagerchromium.org> Reviewed-by: Chris Harrelson <chrishtrchromium.org> Cr-Commit-Position: refs/heads/master{#752511} Co-authored-by: Chris Harrelson <chrishtrchromium.org> -- wpt-commits: a82dbcb196f74ccaf2715ff9f2773ee9d8bafad8 wpt-pr: 22403 UltraBlame original commit: 27a2ebfc7fc75a480e47ba385bff85a6946c9d39
This is a re-land of [2]. It was reverted due to a flakiness introduce
in [3]. The reason for the flakiness appears to have a couple of sources:
(a) sometimes the image for one of the ref/non-ref will load a bit faster
(b) scrollbars present in the ref case, because the image is not clipped
by overflow:hidden
(c) 8px margin
(a) is fixed by using reftest-wait. (b) is fixed by adding overflow:hidden
to the HTML element; (c) is fixed with margin:0.
[1] https://chromium-review.googlesource.com/c/chromium/src/+/1773869
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2103708
[3] external/wpt/html/semantics/embedded-content/the-img-element/image-loading-subpixel-clip.html
Bug: 992765, 999019, 995119, 999209, 1045745
Change-Id: I6ad8c5564f65d131da9b04180499a4bd98a4c8d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111906
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752209}