(Single) image loading in Chrome 52+ perceived slower due to white boxes with grey border (+ JPEGs not progressive)
Reported by
kaihollb...@googlemail.com,
Jul 28 2016
|
||||||||||||
Issue descriptionChrome Version : 52+ - '52.0.2743.82'+, including current canary '54.0.2810.2 canary (64-bit)' URLs (if applicable) : E.g. http://img.zeit.de/entdecken/reisen/2016-07/wales-coast-path1/original (Any image, preferably progressive JPEG) Other browsers tested: Not applicable What steps will reproduce the problem? (1) Use Chrome 52 or newer (2) Load an image, e.g. http://img.zeit.de/entdecken/reisen/2016-07/wales-coast-path1/original (3) See small and large white box with border instead of the image loading. Esp. for progressive JPEG, this is perceived way slower. What is the expected result? - Loading behaviour of Chrome < 52, e.g. 51 What happens instead? - White boxes, image paints when loaed (perceived way slower) Please provide any additional information below. Attach a screenshot if possible. - For easier reproduction use a large image and disable cache and throttle the network to, e.g. 'DSL' preset or slower, using DevTools - I recorded a video of the previous behaviour and the buggy one which is attached, also viewable here: http://webmshare.com/07A7m - Screenshot of the loading-state is also attached - Doesnt appear to occur and regular page load, but still very annoying, esp. when debugging
,
Jul 29 2016
Able to reproduce the issue on win8.1 chrome version 54.0.2810.2 - Loadind teh page takes some time with white box displayed at the left most corner of the page. This is working fine in M51 and hence a regression Manual Bisect: Good Build:52.0.2718.0 Bad Build:52.0.2719.0 Bisect Tool Info: https://chromium.googlesource.com/chromium/src/+log/f9a87728113bccfac096b340f59a03f877327359..33a081977fa4f03d96265f6ada2d3347d9f0db46 Possible suspect : https://codereview.chromium.org/1879793003 Issue seen in mac and Linux as well
,
Jul 30 2016
,
Aug 3 2016
,
Aug 4 2016
Confirmed https://codereview.chromium.org/1879793003 was the first bad commit. This was because: After |setImage(ImageResource::create(imageSourceToKURL(m_element->imageSourceURL())));| in ImageLoader::updateFromElement(), m_hasPendingLoadEvent is false (set in ImageLoader::setImageWithoutConsideringPendingLoadEvent()). So ImageLoader is considered as finished, but ImageResource doesn't have image, so considered as broken.
,
Aug 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/576c7a048bfc30faf510eeda07f303fa9dd12898 commit 576c7a048bfc30faf510eeda07f303fa9dd12898 Author: hiroshige <hiroshige@chromium.org> Date: Wed Aug 24 07:45:22 2016 Fix ImageLoader::m_hasPendingLoadEvent/m_imageComplete in ImageDocument Case 1: When a image is loaded as a subresource, loading is stared by ImageResource::fetch() in ImageLoader::doUpdateFromElement(). Case 2: When a image is loaded as a main document, the start of the loading is emulated by setImage() in ImageLoader::updateFromElement() and ImageDocumentParser will do the subsequent loading steps. However in Case 2, |m_hasPendingLoadEvent| is set to false (in setImageWithoutConsideringPendingLoadEvent()), causing |imageStillLoading| to be false and ensureFallbackContent() to be called in HTMLImageElement::selectSourceURL(), and thus a box indicating a broken image is displayed during loading instead of the image displayed progressively. This is regression since https://codereview.chromium.org/1879793003. This CL makes the behavior of Case 1 and 2 consistent, by moving what is done in Case 1 to setImagePending(), and makes Case 1 and 2 both to call setImagePending(). This CL also adds a layout test to confirm that an image is displayed progressively when loaded as a main document. BUG= 632495 Review-Url: https://codereview.chromium.org/2208073004 Cr-Commit-Position: refs/heads/master@{#414008} [add] https://crrev.com/576c7a048bfc30faf510eeda07f303fa9dd12898/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.png [add] https://crrev.com/576c7a048bfc30faf510eeda07f303fa9dd12898/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt [add] https://crrev.com/576c7a048bfc30faf510eeda07f303fa9dd12898/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html [modify] https://crrev.com/576c7a048bfc30faf510eeda07f303fa9dd12898/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/576c7a048bfc30faf510eeda07f303fa9dd12898/third_party/WebKit/Source/core/loader/ImageLoader.h
,
Aug 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b3f5df1a65187e662fcf2fd5ead1ccb89226f31 commit 8b3f5df1a65187e662fcf2fd5ead1ccb89226f31 Author: johnme <johnme@chromium.org> Date: Thu Aug 25 15:30:16 2016 Revert of Fix ImageLoader::m_hasPendingLoadEvent/m_imageComplete in ImageDocument (patchset #4 id:60001 of https://codereview.chromium.org/2208073004/ ) Reason for revert: Reverting since http/tests/images/png-partial-load-as-document.html consistently leaks on WebKit Linux Leak and LeakExpectations says "Sheriff is expected to revert culprit CLs instead of suppressing the leaks" https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=http%2Ftests%2Fimages%2Fpng-partial-load-as-document.html&testType=webkit_tests Original issue's description: > Fix ImageLoader::m_hasPendingLoadEvent/m_imageComplete in ImageDocument > > Case 1: When a image is loaded as a subresource, loading is stared by > ImageResource::fetch() in ImageLoader::doUpdateFromElement(). > Case 2: When a image is loaded as a main document, the start of the loading is > emulated by setImage() in ImageLoader::updateFromElement() and > ImageDocumentParser will do the subsequent loading steps. > > However in Case 2, |m_hasPendingLoadEvent| is set to false > (in setImageWithoutConsideringPendingLoadEvent()), causing |imageStillLoading| > to be false and ensureFallbackContent() to be called in > HTMLImageElement::selectSourceURL(), and thus a box indicating a broken image > is displayed during loading instead of the image displayed progressively. > This is regression since https://codereview.chromium.org/1879793003. > > This CL makes the behavior of Case 1 and 2 consistent, by moving what is done > in Case 1 to setImagePending(), and makes Case 1 and 2 both to call > setImagePending(). > > This CL also adds a layout test to confirm that an image is displayed > progressively when loaded as a main document. > > BUG= 632495 > > Committed: https://crrev.com/576c7a048bfc30faf510eeda07f303fa9dd12898 > Cr-Commit-Position: refs/heads/master@{#414008} TBR=japhet@chromium.org,yhirano@chromium.org,hiroshige@chromium.org NOTRY=true NOTREECHECKS=true BUG= 632495 Review-Url: https://codereview.chromium.org/2278953002 Cr-Commit-Position: refs/heads/master@{#414441} [delete] https://crrev.com/19ad77688a08480a7641d621787525ad336cbe69/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.png [delete] https://crrev.com/19ad77688a08480a7641d621787525ad336cbe69/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt [delete] https://crrev.com/19ad77688a08480a7641d621787525ad336cbe69/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html [modify] https://crrev.com/8b3f5df1a65187e662fcf2fd5ead1ccb89226f31/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/8b3f5df1a65187e662fcf2fd5ead1ccb89226f31/third_party/WebKit/Source/core/loader/ImageLoader.h
,
Aug 25 2016
The fix was reverted due to leak ( Issue 640886 ). Probably the CL makes ImageLoader::m_hasPendingLoadEvent true and thus might prevent something from being garbage collected. Particularly, ImageLoader should be notified when the loading of the image document is cancelled, but it seems not.
,
Sep 8 2016
Related, with animated capture: https://bugs.chromium.org/p/chromium/issues/detail?id=635215
,
Sep 8 2016
,
Sep 9 2016
Issue 645247 has been merged into this issue.
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5e43f7edf4e55155fcda51be0285f37b20c2e76 commit c5e43f7edf4e55155fcda51be0285f37b20c2e76 Author: hiroshige <hiroshige@chromium.org> Date: Thu Sep 22 19:52:41 2016 HTMLImageElement: do not use fallback content for ImageDocument After https://codereview.chromium.org/1879793003, ImageLoader::hasPendingActivity() became false for ImageDocument and thus HTMLImageElement::selectSourceURL() didn't consider the image as still loading during load (|imageStillLoading| became false). This caused ImageDocument not to be displayed progressively. This CL makes HTMLImageElement to use primary content for ImageDocument. This causes ImageDocument not to be replaced with fallback content, but I expect that is more acceptable than images not displayed progressively. BUG= 632495 Review-Url: https://codereview.chromium.org/2343613002 Cr-Commit-Position: refs/heads/master@{#420432} [add] https://crrev.com/c5e43f7edf4e55155fcda51be0285f37b20c2e76/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.png [add] https://crrev.com/c5e43f7edf4e55155fcda51be0285f37b20c2e76/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt [add] https://crrev.com/c5e43f7edf4e55155fcda51be0285f37b20c2e76/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html [modify] https://crrev.com/c5e43f7edf4e55155fcda51be0285f37b20c2e76/third_party/WebKit/Source/core/html/HTMLImageElement.cpp [modify] https://crrev.com/c5e43f7edf4e55155fcda51be0285f37b20c2e76/third_party/WebKit/Source/core/loader/ImageLoader.h
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/70e44d10fc52150a6af5a7de760cb9181587fbb4 commit 70e44d10fc52150a6af5a7de760cb9181587fbb4 Author: gcasto <gcasto@chromium.org> Date: Thu Sep 22 23:27:34 2016 Revert of HTMLImageElement: do not use fallback content for ImageDocument (patchset #2 id:20001 of https://codereview.chromium.org/2343613002/ ) Reason for revert: Speculatively reverting to fix broken layout tests on Windows (https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Win10, https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Win7) Tests are fast/backgrounds/border-radius-split-background-image.html fast/backgrounds/border-radius-split-background.html fast/borders/border-styles-split.html paint/invalidation/background-resize-height.html Original issue's description: > HTMLImageElement: do not use fallback content for ImageDocument > > After https://codereview.chromium.org/1879793003, > ImageLoader::hasPendingActivity() became false for ImageDocument and thus > HTMLImageElement::selectSourceURL() didn't consider the image as still loading > during load (|imageStillLoading| became false). > This caused ImageDocument not to be displayed progressively. > > This CL makes HTMLImageElement to use primary content for ImageDocument. > This causes ImageDocument not to be replaced with fallback content, but I > expect that is more acceptable than images not displayed progressively. > > BUG= 632495 > > Committed: https://crrev.com/c5e43f7edf4e55155fcda51be0285f37b20c2e76 > Cr-Commit-Position: refs/heads/master@{#420432} TBR=yhirano@chromium.org,japhet@chromium.org,ellyjones@chromium.org,hiroshige@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 632495 Review-Url: https://codereview.chromium.org/2365523003 Cr-Commit-Position: refs/heads/master@{#420499} [delete] https://crrev.com/04b1826356a37de2a322e7e6e0e80f07d05f210f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.png [delete] https://crrev.com/04b1826356a37de2a322e7e6e0e80f07d05f210f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt [delete] https://crrev.com/04b1826356a37de2a322e7e6e0e80f07d05f210f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html [modify] https://crrev.com/70e44d10fc52150a6af5a7de760cb9181587fbb4/third_party/WebKit/Source/core/html/HTMLImageElement.cpp [modify] https://crrev.com/70e44d10fc52150a6af5a7de760cb9181587fbb4/third_party/WebKit/Source/core/loader/ImageLoader.h
,
Sep 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc960a01f9c5d39ead364b1cb929c999323d3dc6 commit cc960a01f9c5d39ead364b1cb929c999323d3dc6 Author: hiroshige <hiroshige@chromium.org> Date: Fri Sep 23 15:48:08 2016 Reland of HTMLImageElement: do not use fallback content for ImageDocument (patchset #1 id:1 of https://codereview.chromium.org/2365523003/ ) Reason for revert: As per comment #8, this CL wasn't the cause of bot failures. https://codereview.chromium.org/2365523003/#msg8 Original issue's description: > Revert of HTMLImageElement: do not use fallback content for ImageDocument (patchset #2 id:20001 of https://codereview.chromium.org/2343613002/ ) > > Reason for revert: > Speculatively reverting to fix broken layout tests on Windows (https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Win10, https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Win7) > > Tests are > > fast/backgrounds/border-radius-split-background-image.html > fast/backgrounds/border-radius-split-background.html > fast/borders/border-styles-split.html > paint/invalidation/background-resize-height.html > > Original issue's description: > > HTMLImageElement: do not use fallback content for ImageDocument > > > > After https://codereview.chromium.org/1879793003, > > ImageLoader::hasPendingActivity() became false for ImageDocument and thus > > HTMLImageElement::selectSourceURL() didn't consider the image as still loading > > during load (|imageStillLoading| became false). > > This caused ImageDocument not to be displayed progressively. > > > > This CL makes HTMLImageElement to use primary content for ImageDocument. > > This causes ImageDocument not to be replaced with fallback content, but I > > expect that is more acceptable than images not displayed progressively. > > > > BUG= 632495 > > > > Committed: https://crrev.com/c5e43f7edf4e55155fcda51be0285f37b20c2e76 > > Cr-Commit-Position: refs/heads/master@{#420432} > > TBR=yhirano@chromium.org,japhet@chromium.org,ellyjones@chromium.org,hiroshige@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 632495 > > Committed: https://crrev.com/70e44d10fc52150a6af5a7de760cb9181587fbb4 > Cr-Commit-Position: refs/heads/master@{#420499} TBR=yhirano@chromium.org,japhet@chromium.org,ellyjones@chromium.org,gcasto@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 632495 Review-Url: https://codereview.chromium.org/2364073002 Cr-Commit-Position: refs/heads/master@{#420616} [add] https://crrev.com/cc960a01f9c5d39ead364b1cb929c999323d3dc6/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.png [add] https://crrev.com/cc960a01f9c5d39ead364b1cb929c999323d3dc6/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt [add] https://crrev.com/cc960a01f9c5d39ead364b1cb929c999323d3dc6/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html [modify] https://crrev.com/cc960a01f9c5d39ead364b1cb929c999323d3dc6/third_party/WebKit/Source/core/html/HTMLImageElement.cpp [modify] https://crrev.com/cc960a01f9c5d39ead364b1cb929c999323d3dc6/third_party/WebKit/Source/core/loader/ImageLoader.h
,
Sep 26 2016
The fix (#12, relanded #14) was landed on 55.0.2870.0 and I confirmed the large image (Comment #0) was shown progressively on 55.0.2871.0 on Windows 7. Requesting merge to M-54 beta (the fix stayed on canary for > one day).
,
Sep 26 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Sep 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f commit 01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Mon Sep 26 05:12:36 2016 HTMLImageElement: do not use fallback content for ImageDocument After https://codereview.chromium.org/1879793003, ImageLoader::hasPendingActivity() became false for ImageDocument and thus HTMLImageElement::selectSourceURL() didn't consider the image as still loading during load (|imageStillLoading| became false). This caused ImageDocument not to be displayed progressively. This CL makes HTMLImageElement to use primary content for ImageDocument. This causes ImageDocument not to be replaced with fallback content, but I expect that is more acceptable than images not displayed progressively. BUG= 632495 Review-Url: https://codereview.chromium.org/2343613002 Cr-Commit-Position: refs/heads/master@{#420432} (cherry picked from commit c5e43f7edf4e55155fcda51be0285f37b20c2e76) Review URL: https://codereview.chromium.org/2371643002 . Cr-Commit-Position: refs/branch-heads/2840@{#522} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [add] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.png [add] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt [add] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html [modify] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/Source/core/html/HTMLImageElement.cpp [modify] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/Source/core/loader/ImageLoader.h
,
Sep 28 2016
Verified the issue on Mac 10.11.6, Win 10 and Ubuntu 14.04 using Beta # 54.0.2840.41 and its working fine now. Please find the attached screen shot for the same. hiroshige@ : Please mark the issue to Fixed if there is no further work to be done on it.
,
Oct 3 2016
Requesting merge to M-53 if there was a further M-53 stable update. If the next stable update is M-54 then we don't need merge.
,
Oct 3 2016
[Automated comment] Request affecting a post-stable build (M53), manual review required.
,
Oct 3 2016
Dropping the request as Comment #20 says M-53 is post-stable. Closing as fixed.
,
Oct 3 2016
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f commit 01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Mon Sep 26 05:12:36 2016 HTMLImageElement: do not use fallback content for ImageDocument After https://codereview.chromium.org/1879793003, ImageLoader::hasPendingActivity() became false for ImageDocument and thus HTMLImageElement::selectSourceURL() didn't consider the image as still loading during load (|imageStillLoading| became false). This caused ImageDocument not to be displayed progressively. This CL makes HTMLImageElement to use primary content for ImageDocument. This causes ImageDocument not to be replaced with fallback content, but I expect that is more acceptable than images not displayed progressively. BUG= 632495 Review-Url: https://codereview.chromium.org/2343613002 Cr-Commit-Position: refs/heads/master@{#420432} (cherry picked from commit c5e43f7edf4e55155fcda51be0285f37b20c2e76) Review URL: https://codereview.chromium.org/2371643002 . Cr-Commit-Position: refs/branch-heads/2840@{#522} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [add] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.png [add] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt [add] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html [modify] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/Source/core/html/HTMLImageElement.cpp [modify] https://crrev.com/01e1c1571cd16ec4f3ff0342f8f1e0f943cd942f/third_party/WebKit/Source/core/loader/ImageLoader.h |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by kaihollb...@googlemail.com
, Jul 28 2016