Layout Test http/tests/loading/preload-picture-sizes-2x.html is flaky |
|||
Issue descriptionThe following layout test is flaky http/tests/loading/preload-picture-sizes-2x.html Probable cause: I suspect it got flaky due to my change: https://chromium.googlesource.com/chromium/src/+/bb924e1897f5b4c67325dbfbdf2441e7f0686d53 The change will be reverted soon, so I expect the flakiness will recover.
,
May 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da70e32d811fffe4e15728ba858dbdfa3180ed59 commit da70e32d811fffe4e15728ba858dbdfa3180ed59 Author: hiroshige <hiroshige@chromium.org> Date: Tue May 23 22:31:28 2017 Remove EventSender from ImageLoader Instead of using EventSender, this CL schedules ImageLoader's load and error events using PostCancellableTask. has_pending_load_event_ and has_pending_error_event_ booleans are replaced with TaskHandle to the cancellable tasks and IsActive() calls. This CL removes the dependencies between Document's load event and <img> load events via EventSender. This CL removes EventSender.h as ImageLoader was the last user of that. Test changes: stop-loading.html: testharness.js waits for the window load event before finishing. Previously, we call window.stop() in <img> load event, but window load event is fired, because the <img> load event is fired from Document::ImplicitClose() via EventSender. At that time window.stop() doesn't stop the window load event. After this CL, <img> load event is fired separately from Document::ImplicitClose(), and thus window.stop() stops the window load event. To work around this, this CL triggers window load event explicitly just to signal testharness.js to finish. http/tests/loading/preload-image-sizes-2x.html and http/tests/loading/preload-picture-sizes-2x.html: The tests uses srcset-helper.js that reloads the page with empty cache in the window load event. Because this CL changes the timing of the window load event, this causes some asynchronous tasks that cause new image loading to be scheduled before window load event and executed after that, causing some images left on the cache before reloading starts. This CL works around the issue by waiting a little bit for those tasks to be executed, and then clearing the cache and reloading the page. BUG= 624697 , 720268 Review-Url: https://codereview.chromium.org/2863763003 Cr-Commit-Position: refs/heads/master@{#474086} [modify] https://crrev.com/da70e32d811fffe4e15728ba858dbdfa3180ed59/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading.html [modify] https://crrev.com/da70e32d811fffe4e15728ba858dbdfa3180ed59/third_party/WebKit/LayoutTests/http/tests/resources/srcset-helper.js [modify] https://crrev.com/da70e32d811fffe4e15728ba858dbdfa3180ed59/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/da70e32d811fffe4e15728ba858dbdfa3180ed59/third_party/WebKit/Source/core/events/BUILD.gn [delete] https://crrev.com/789f8d1d41b5dbba8a252d8b69390f9bd67c83db/third_party/WebKit/Source/core/events/EventSender.h [modify] https://crrev.com/da70e32d811fffe4e15728ba858dbdfa3180ed59/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/da70e32d811fffe4e15728ba858dbdfa3180ed59/third_party/WebKit/Source/core/loader/ImageLoader.h
,
May 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5 commit 8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5 Author: yosin <yosin@chromium.org> Date: Wed May 24 04:07:29 2017 Revert of Remove EventSender from ImageLoader (patchset #10 id:180001 of https://codereview.chromium.org/2863763003/ ) Reason for revert: This patch probably causes object leaks: See details in "webkit_tests" of https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/4925 Original issue's description: > Remove EventSender from ImageLoader > > Instead of using EventSender, this CL schedules ImageLoader's > load and error events using PostCancellableTask. > has_pending_load_event_ and has_pending_error_event_ booleans are > replaced with TaskHandle to the cancellable tasks and IsActive() calls. > > This CL removes the dependencies between Document's load event and > <img> load events via EventSender. > > This CL removes EventSender.h as ImageLoader was the last user of that. > > Test changes: > stop-loading.html: > testharness.js waits for the window load event before finishing. > Previously, we call window.stop() in <img> load event, but window > load event is fired, because the <img> load event is fired from > Document::ImplicitClose() via EventSender. At that time window.stop() > doesn't stop the window load event. > After this CL, <img> load event is fired separately from > Document::ImplicitClose(), and thus window.stop() stops the window > load event. > To work around this, this CL triggers window load event explicitly > just to signal testharness.js to finish. > > http/tests/loading/preload-image-sizes-2x.html and > http/tests/loading/preload-picture-sizes-2x.html: > The tests uses srcset-helper.js that reloads the page with empty > cache in the window load event. > Because this CL changes the timing of the window load event, this > causes some asynchronous tasks that cause new image loading to be > scheduled before window load event and executed after that, causing > some images left on the cache before reloading starts. > This CL works around the issue by waiting a little bit for those > tasks to be executed, and then clearing the cache and reloading the > page. > > BUG= 624697 , 720268 > > Review-Url: https://codereview.chromium.org/2863763003 > Cr-Commit-Position: refs/heads/master@{#474086} > Committed: https://chromium.googlesource.com/chromium/src/+/da70e32d811fffe4e15728ba858dbdfa3180ed59 TBR=japhet@chromium.org,yhirano@chromium.org,kinuko@chromium.org,kouhei@chromium.org,tzik@chromium.org,hiroshige@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 624697 , 720268 Review-Url: https://codereview.chromium.org/2903773002 Cr-Commit-Position: refs/heads/master@{#474154} [modify] https://crrev.com/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading.html [modify] https://crrev.com/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5/third_party/WebKit/LayoutTests/http/tests/resources/srcset-helper.js [modify] https://crrev.com/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5/third_party/WebKit/Source/core/events/BUILD.gn [add] https://crrev.com/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5/third_party/WebKit/Source/core/events/EventSender.h [modify] https://crrev.com/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5/third_party/WebKit/Source/core/loader/ImageLoader.h
,
May 30 2017
Sorry, I relanded the CL, and forgot about this issue. It seems hiroshige@ has a fix: https://codereview.chromium.org/2905233002/
,
May 30 2017
,
May 30 2017
> #4 https://codereview.chromium.org/2906063003 might resolve the flakiness, but not sure.
,
May 30 2017
> #6 Oops, thank you for the correction.
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67e066b84ec7953ec06ecf252f466c2c2c063f33 commit 67e066b84ec7953ec06ecf252f466c2c2c063f33 Author: hiroshige <hiroshige@chromium.org> Date: Tue May 30 15:09:31 2017 Reland of remove EventSender from ImageLoader (patchset #1 id:1 of https://codereview.chromium.org/2903773002/ ) Reason for reland: https://codereview.chromium.org/2905233002 will fix the leak, because ImageLoader isn't kept alive after the context is destroyed. Original issue's description: > Revert of Remove EventSender from ImageLoader (patchset #10 id:180001 of https://codereview.chromium.org/2863763003/ ) > > Reason for revert: > This patch probably causes object leaks: > > See details in "webkit_tests" of > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/4925 > > > Original issue's description: > > Remove EventSender from ImageLoader > > > > Instead of using EventSender, this CL schedules ImageLoader's > > load and error events using PostCancellableTask. > > has_pending_load_event_ and has_pending_error_event_ booleans are > > replaced with TaskHandle to the cancellable tasks and IsActive() calls. > > > > This CL removes the dependencies between Document's load event and > > <img> load events via EventSender. > > > > This CL removes EventSender.h as ImageLoader was the last user of that. > > > > Test changes: > > stop-loading.html: > > testharness.js waits for the window load event before finishing. > > Previously, we call window.stop() in <img> load event, but window > > load event is fired, because the <img> load event is fired from > > Document::ImplicitClose() via EventSender. At that time window.stop() > > doesn't stop the window load event. > > After this CL, <img> load event is fired separately from > > Document::ImplicitClose(), and thus window.stop() stops the window > > load event. > > To work around this, this CL triggers window load event explicitly > > just to signal testharness.js to finish. > > > > http/tests/loading/preload-image-sizes-2x.html and > > http/tests/loading/preload-picture-sizes-2x.html: > > The tests uses srcset-helper.js that reloads the page with empty > > cache in the window load event. > > Because this CL changes the timing of the window load event, this > > causes some asynchronous tasks that cause new image loading to be > > scheduled before window load event and executed after that, causing > > some images left on the cache before reloading starts. > > This CL works around the issue by waiting a little bit for those > > tasks to be executed, and then clearing the cache and reloading the > > page. > > > > BUG= 624697 , 720268 > > > > Review-Url: https://codereview.chromium.org/2863763003 > > Cr-Commit-Position: refs/heads/master@{#474086} > > Committed: https://chromium.googlesource.com/chromium/src/+/da70e32d811fffe4e15728ba858dbdfa3180ed59 > > TBR=japhet@chromium.org,yhirano@chromium.org,kinuko@chromium.org,kouhei@chromium.org,tzik@chromium.org,hiroshige@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 624697 , 720268 > > Review-Url: https://codereview.chromium.org/2903773002 > Cr-Commit-Position: refs/heads/master@{#474154} > Committed: https://chromium.googlesource.com/chromium/src/+/8e7dc7edbcacdd495eb18ff33fe88d1fb89a15d5 BUG= 624697 , 720268 , 726091 Review-Url: https://codereview.chromium.org/2906063003 Cr-Commit-Position: refs/heads/master@{#475528} [modify] https://crrev.com/67e066b84ec7953ec06ecf252f466c2c2c063f33/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading.html [modify] https://crrev.com/67e066b84ec7953ec06ecf252f466c2c2c063f33/third_party/WebKit/LayoutTests/http/tests/resources/srcset-helper.js [modify] https://crrev.com/67e066b84ec7953ec06ecf252f466c2c2c063f33/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/67e066b84ec7953ec06ecf252f466c2c2c063f33/third_party/WebKit/Source/core/events/BUILD.gn [delete] https://crrev.com/2a48181837832ca432f87369fddfa092f0450768/third_party/WebKit/Source/core/events/EventSender.h [modify] https://crrev.com/67e066b84ec7953ec06ecf252f466c2c2c063f33/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/67e066b84ec7953ec06ecf252f466c2c2c063f33/third_party/WebKit/Source/core/loader/ImageLoader.h
,
Jun 1 2017
Apparently fixed. Thank you! |
|||
►
Sign in to add a comment |
|||
Comment 1 by hirosh...@chromium.org
, May 12 2017