New issue
Advanced search Search tips

Issue 920899 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

LazyLoad: Fix deferred loading of CSS background images

Project Member Reported by rajendrant@chromium.org, Jan 11

Issue description

There is a bug preventing lazyloading of CSS background images.

There should be tests for the CSS background image case.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 17 (6 days ago)

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

commit 884d05298198751cbf2fc527181a98404f45884b
Author: rajendrant <rajendrant@chromium.org>
Date: Thu Jan 17 04:32:02 2019

LazyLoad: Fix deferred loading of CSS background images

kDeferImageLoad bit is not set in the FetchParameters when CSS background
images need to be deferred. This CL fixes that, and also adds a browser
test.

Bug:  920899 
Change-Id: I4d96d63c1bba2d2a0892e1b84ae364f29f53c152
Reviewed-on: https://chromium-review.googlesource.com/c/1405592
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623575}
[add] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/chrome/browser/previews/lazyload_browsertest.cc
[modify] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/chrome/test/BUILD.gn
[add] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/chrome/test/data/lazyload/css-background-image.html
[add] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/chrome/test/data/lazyload/images/fruit1.jpg
[add] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/chrome/test/data/lazyload/images/fruit2.jpg
[modify] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/third_party/blink/renderer/core/BUILD.gn
[modify] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/third_party/blink/renderer/core/css/css_image_value.cc
[add] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc
[modify] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc
[modify] https://crrev.com/884d05298198751cbf2fc527181a98404f45884b/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 17 (6 days ago)

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

commit 1d1fee8b5059cbfc044d6028cff20fb6a3020511
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Thu Jan 17 05:55:09 2019

Revert "LazyLoad: Fix deferred loading of CSS background images"

This reverts commit 884d05298198751cbf2fc527181a98404f45884b.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 623575 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzg4NGQwNTI5ODE5ODc1MWNiZjJmYzUyNzE4MWE5ODQwNGY0NTg4NGIM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/47077

Sample Failed Step: browser_tests

Original change's description:
> LazyLoad: Fix deferred loading of CSS background images
> 
> kDeferImageLoad bit is not set in the FetchParameters when CSS background
> images need to be deferred. This CL fixes that, and also adds a browser
> test.
> 
> Bug:  920899 
> Change-Id: I4d96d63c1bba2d2a0892e1b84ae364f29f53c152
> Reviewed-on: https://chromium-review.googlesource.com/c/1405592
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Reviewed-by: Rune Lillesveen <futhark@chromium.org>
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623575}

Change-Id: Ib129c2f7b13b742a10a48e3b6deaa5e8de707780
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  920899 
Reviewed-on: https://chromium-review.googlesource.com/c/1416878
Cr-Commit-Position: refs/heads/master@{#623603}
[delete] https://crrev.com/abfd257c64e3052a02a4263b96f0c6ba49579e02/chrome/browser/previews/lazyload_browsertest.cc
[modify] https://crrev.com/1d1fee8b5059cbfc044d6028cff20fb6a3020511/chrome/test/BUILD.gn
[delete] https://crrev.com/abfd257c64e3052a02a4263b96f0c6ba49579e02/chrome/test/data/lazyload/css-background-image.html
[delete] https://crrev.com/abfd257c64e3052a02a4263b96f0c6ba49579e02/chrome/test/data/lazyload/images/fruit1.jpg
[delete] https://crrev.com/abfd257c64e3052a02a4263b96f0c6ba49579e02/chrome/test/data/lazyload/images/fruit2.jpg
[modify] https://crrev.com/1d1fee8b5059cbfc044d6028cff20fb6a3020511/third_party/blink/renderer/core/BUILD.gn
[modify] https://crrev.com/1d1fee8b5059cbfc044d6028cff20fb6a3020511/third_party/blink/renderer/core/css/css_image_value.cc
[delete] https://crrev.com/abfd257c64e3052a02a4263b96f0c6ba49579e02/third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc
[modify] https://crrev.com/1d1fee8b5059cbfc044d6028cff20fb6a3020511/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc
[modify] https://crrev.com/1d1fee8b5059cbfc044d6028cff20fb6a3020511/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit a395226e3bdb1640c5b8b30b3a5f89d38395643a
Author: rajendrant <rajendrant@chromium.org>
Date: Fri Jan 18 11:22:45 2019

Reland "LazyLoad: Fix deferred loading of CSS background images"

kDeferImageLoad bit is not set in the FetchParameters when CSS background
images need to be deferred. This CL fixes that, and also adds browser
test and unit test.

Reland of https://chromium-review.googlesource.com/c/1416878 after
disabling one browser test in windows, chrome OS.

TBR=japhet@chromium.org,futhark@chromium.org

Bug:  920899 
Change-Id: Ide0462c7cc09891761759383adf50ff738929ecb
Reviewed-on: https://chromium-review.googlesource.com/c/1420298
Reviewed-by: rajendrant <rajendrant@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624073}
[add] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/chrome/browser/previews/lazyload_browsertest.cc
[modify] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/chrome/test/BUILD.gn
[add] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/chrome/test/data/lazyload/css-background-image.html
[add] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/chrome/test/data/lazyload/images/fruit1.jpg
[add] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/chrome/test/data/lazyload/images/fruit2.jpg
[modify] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/third_party/blink/renderer/core/BUILD.gn
[modify] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/third_party/blink/renderer/core/css/css_image_value.cc
[add] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/third_party/blink/renderer/core/html/lazy_load_image_observer_test.cc
[modify] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc
[modify] https://crrev.com/a395226e3bdb1640c5b8b30b3a5f89d38395643a/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.h

Comment 4 by rajendrant@chromium.org, Jan 18 (4 days ago)

Status: Fixed (was: Assigned)

Sign in to add a comment