Move all EventSenders to WebFrameScheduler |
|||||||||||||||||||||||
Issue descriptionWe should move EventSenders to WebFrameScheduler. Blink has 12 EventSenders. EventSender is a broken scheduling primitive that sends events in a batch which is not correct per the spec and creates event listener floods (ex. tons of loading events all at once). Since these events are inherently asynchronous, we can be much smarter by posting them to WebFrameScheduler. However, the tricky part is that some of the EventSenders are using dispatchPendingEvents to synchronously dispatch pending events before a specific event is fired (e.g., ensure that images are loaded before document.onload is fired). This should be fixed but it will be a substantial amount of work in loading code to get rid of the dependency on dispatchPendingEvents. On the other hand, it would be straightforward to move other EventSenders to WebFrameScheduler. hiroshige's design doc: https://docs.google.com/document/d/152fm7sPadgyPivZwjRbyagvOiNyQAOLdWIPnmpEjqgg/edit ⛆ |
|
|
,
Aug 24 2016
,
Sep 2 2016
,
Sep 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aaf5ece46ff3b24c2f5ff0054600900f60c70ce4 commit aaf5ece46ff3b24c2f5ff0054600900f60c70ce4 Author: hiroshige <hiroshige@chromium.org> Date: Thu Sep 15 11:18:21 2016 Remove EventSender from HTMLLinkElement This CL replaces EventSender in HTMLLinkElement with postTask(): Instead of calling HTMLLinkElement::dispatchPendingLoadEvents() in Document::implicitClose() to enforce <link>'s onload events to be executed before the document's onload event, this CL blocks document's onload until <link>'s onload event using IncrementLoadEventDelayCount. This CL also modifies SimTest: This CL potentially makes onload events of <link> and documents async and delayed, and thus SimTest can be destructed before document's onload event, which cause assertion failure. This CL calls testing::runPendingTasks() in ~SimTest() to flush scheduled onload events to make Document to be loaded before SimTest's destruction. BUG= 624697 Review-Url: https://codereview.chromium.org/2275493002 Cr-Commit-Position: refs/heads/master@{#418835} [modify] https://crrev.com/aaf5ece46ff3b24c2f5ff0054600900f60c70ce4/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/aaf5ece46ff3b24c2f5ff0054600900f60c70ce4/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp [modify] https://crrev.com/aaf5ece46ff3b24c2f5ff0054600900f60c70ce4/third_party/WebKit/Source/core/html/HTMLLinkElement.h [modify] https://crrev.com/aaf5ece46ff3b24c2f5ff0054600900f60c70ce4/third_party/WebKit/Source/web/tests/sim/SimTest.cpp
,
Sep 29 2016
Hi pdr@, fs@, ericwilligers@, Do you know which task runners / |TaskType|s should be used for dispatching asynchronously the following events? - SVGUseElement's load event - SVGStyleElement's error event - SVGSMILElement's beginEvent, endEvent, repeatEvent and repeatn List of |TaskType|s: https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/TaskRunnerHelper.h?sq=package:chromium&dr=C&rcl=1475104848&l=20 Currently |EventSender|s are used, and I'd like to assign specific |TaskType| if e.g. specified in the specs. In the case of HTMLLinkElement, |DOMManipulation| TaskType is used, according to the spec: https://html.spec.whatwg.org/multipage/semantics.html#the-link-element > The task source for these tasks is the DOM manipulation task source. But I couldn't find references to task source/task runner/etc. in the SVG/SMIL specs.
,
Sep 29 2016
I think I would suggest DOMManipulation for all of these. Motivations follow. SVGUseElement[1]: Seems to match similar cases SVGStyleElement: Like HTMLStyleElement SMIL events: Web Animations post their tasks on DOM manipulation [1] Note: It would probably make sense to "post a task" for 'error' here as well.
,
Sep 29 2016
,
Sep 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4bc45550a35c63d7e400733e9bdf00114aa13e10 commit 4bc45550a35c63d7e400733e9bdf00114aa13e10 Author: hiroshige <hiroshige@chromium.org> Date: Fri Sep 30 05:13:10 2016 Remove timing dependency to document onload in dialog-close-event.html A follow-up of https://codereview.chromium.org/2355743005. Previously, the layout test fast/dom/HTMLDialogElement/dialog-close-event.html expected dialog's close event is not called before test completed, because the dialog's close event should be called asynchronously. However, if the timing of document's onload event is changed (e.g. by Issue 624697 ), the dialog's close event might be called before document's onload event, i.e. before test completes. This CL makes the test to wait until two dialog's close events are called, no matter when document's onload is called. BUG= 380087 , 624697 Review-Url: https://codereview.chromium.org/2375413002 Cr-Commit-Position: refs/heads/master@{#422033} [modify] https://crrev.com/4bc45550a35c63d7e400733e9bdf00114aa13e10/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/dialog-close-event-expected.txt [modify] https://crrev.com/4bc45550a35c63d7e400733e9bdf00114aa13e10/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/dialog-close-event.html
,
Sep 30 2016
> #6 fs@, thanks for sugesstions! I updated the draft CLs to use DOMManipulation.
,
Sep 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/759a69172d0558751a9623f535920e14429f7a9b commit 759a69172d0558751a9623f535920e14429f7a9b Author: hiroshige <hiroshige@chromium.org> Date: Fri Sep 30 11:47:18 2016 Remove EventSender from SVGSMILElement BUG= 624697 Review-Url: https://codereview.chromium.org/2096253002 Cr-Commit-Position: refs/heads/master@{#422083} [modify] https://crrev.com/759a69172d0558751a9623f535920e14429f7a9b/third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp [modify] https://crrev.com/759a69172d0558751a9623f535920e14429f7a9b/third_party/WebKit/Source/core/svg/animation/SVGSMILElement.h
,
Sep 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/637dbfc42bf553b2e40b6090e25ec00a339979e4 commit 637dbfc42bf553b2e40b6090e25ec00a339979e4 Author: hiroshige <hiroshige@chromium.org> Date: Fri Sep 30 12:17:19 2016 Remove EventSender from SVGStyleElement BUG= 624697 Review-Url: https://codereview.chromium.org/2095243002 Cr-Commit-Position: refs/heads/master@{#422086} [modify] https://crrev.com/637dbfc42bf553b2e40b6090e25ec00a339979e4/third_party/WebKit/Source/core/svg/SVGStyleElement.cpp [modify] https://crrev.com/637dbfc42bf553b2e40b6090e25ec00a339979e4/third_party/WebKit/Source/core/svg/SVGStyleElement.h
,
Oct 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79dc95f170313734d2e0e61733e7064bb34f0c6b commit 79dc95f170313734d2e0e61733e7064bb34f0c6b Author: hiroshige <hiroshige@chromium.org> Date: Mon Oct 03 05:56:43 2016 Remove EventSender from SVGUseElement BUG= 624697 Review-Url: https://codereview.chromium.org/2095253002 Cr-Commit-Position: refs/heads/master@{#422375} [modify] https://crrev.com/79dc95f170313734d2e0e61733e7064bb34f0c6b/third_party/WebKit/Source/core/svg/SVGUseElement.cpp [modify] https://crrev.com/79dc95f170313734d2e0e61733e7064bb34f0c6b/third_party/WebKit/Source/core/svg/SVGUseElement.h
,
Oct 3 2016
,
Oct 26 2016
,
Nov 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be commit f768d0e471877996cf0dd06f2a3b1cb9ba49c6be Author: hiroshige <hiroshige@chromium.org> Date: Fri Nov 04 09:33:28 2016 Call invalidatePaintForTickmarks() in Internals::addTextMatchMarker() Callers of DocumentMarkerController::addTextMatchMarker() must invalidate tickmarks (according a comment in addTextMatchMarker()) but Internals::addTextMatchMarker() doesn't invalidate the tickmark. This causes the fast/scrolling/scrollbar-tickmarks-styled.html test to fail if internals.addTextMatchMarker() is called after the first layout. This CL makes addTextMatchMarker() to do paint invalidation, just like TextFinder does in TextFinder::invalidateIfNecessary and TextFinder::finishCurrentScopingEffort(). This CL adds fast/scrolling/scrollbar-tickmarks-styled-after-onload.html that tests internals.addTextMatchMarker() after the first layout and document onload. (the test is marked as failing in rootlayerscrolls because the original scrollbar-tickmarks-styled.html is already marked as failing.) This is preparation for https://codereview.chromium.org/2269043002 that modifies the timing of document onload event and moves document onload after first layout in the test above. BUG= 624697 TEST=fast/scrolling/scrollbar-tickmarks-styled.html fast/scrolling/scrollbar-tickmarks-styled-after-onload.html Review-Url: https://codereview.chromium.org/2450793002 Cr-Commit-Position: refs/heads/master@{#429841} [modify] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls [modify] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-styled-after-onload-expected.txt [add] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-styled-after-onload.html [add] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/platform/mac/fast/scrolling/scrollbar-tickmarks-styled-after-onload-expected.png [add] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/platform/mac/virtual/scroll_customization/fast/scrolling/scrollbar-tickmarks-styled-after-onload-expected.png [add] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/platform/win/fast/scrolling/scrollbar-tickmarks-styled-after-onload-expected.png [add] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/LayoutTests/platform/win/virtual/scroll_customization/fast/scrolling/scrollbar-tickmarks-styled-after-onload-expected.png [modify] https://crrev.com/f768d0e471877996cf0dd06f2a3b1cb9ba49c6be/third_party/WebKit/Source/core/testing/Internals.cpp
,
Nov 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41ca771082edfcdbe304abbce42fce6de58c0657 commit 41ca771082edfcdbe304abbce42fce6de58c0657 Author: hiroshige <hiroshige@chromium.org> Date: Mon Nov 07 09:06:24 2016 Check scrollRestorationType explicitly in FrameLoader::processFragment() This is preparation for [1], fixing test failure of virtual/rootlayerscrolls/fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html with [1]. Before [1], the test is passing because, in FrameLoader::finishedParsing(), restoreScrollPositionAndViewState() is called (in checkCompleted()) before processFragment(), and thus processFragment() doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is set to true by restoreScrollPositionAndViewState(). However, after [1], restoreScrollPositionAndViewState() is NOT called in checkCompleted() because [1] causes the document onload to be called after finishedParsing(), and thus processFragment() causes scrolling unexpectedly, not reflecting history.scrollRestoration = 'manual'. This CL makes processFragment() to check scrollRestorationType explicitly, not depending on previous restoreScrollPositionAndViewState() call. [1] https://codereview.chromium.org/2269043002/. BUG= 624697 , 417782 TEST=virtual/rootlayerscrolls/fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html Review-Url: https://codereview.chromium.org/2467433002 Cr-Commit-Position: refs/heads/master@{#430237} [modify] https://crrev.com/41ca771082edfcdbe304abbce42fce6de58c0657/third_party/WebKit/Source/core/loader/FrameLoader.cpp
,
Nov 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6253bb13131f4afaba8140133b1b865487b82006 commit 6253bb13131f4afaba8140133b1b865487b82006 Author: hiroshige <hiroshige@chromium.org> Date: Mon Nov 07 16:13:49 2016 Do not call internals.settings.setOverlayScrollbarsEnabled() in onload internals.settings.setOverlayScrollbarsEnabled() doesn't work well when it is called after ScrollbarTheme is initialized in the first layout. The CL [1] modifies the timing of document load event and makes onload() in fast/scrolling/scrollbar-tickmarks-hittest.html to be called after the first layout, and thus breaks the test. This CL moves setOverlayScrollbarsEnabled() to outside onload() such that it is called before ScrollbarTheme is initialized. [1] https://codereview.chromium.org/2269043002/ BUG= 624697 TEST=fast/scrolling/scrollbar-tickmarks-hittest.html Review-Url: https://codereview.chromium.org/2484613003 Cr-Commit-Position: refs/heads/master@{#430284} [modify] https://crrev.com/6253bb13131f4afaba8140133b1b865487b82006/third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-hittest.html
,
Nov 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/52d00fd56184deadce99ff4fe599156ea1da3543 commit 52d00fd56184deadce99ff4fe599156ea1da3543 Author: hiroshige <hiroshige@chromium.org> Date: Mon Nov 07 19:36:50 2016 Remove EventSender from HTMLStyleElement This CL replaces EventSender in HTMLLinkElement with postTask(): Instead of calling HTMLStyleElement::dispatchPendingLoadEvents() in Document::implicitClose() to enforce <style>'s onload events to be executed before the document's onload event, this CL blocks document's onload until <style>'s onload event using IncrementLoadEventDelayCount. This CL changes the timing of body onload, particularly causes some of the following events that were previously called AFTER document onload to be called BEFORE document onload: [A] The first layout. [B] Latter half of FrameLoader::finishedParsing(). [C] JavaScript function |f| scheduled as |setTimeout(f, 0)| in the <script> at the end of <body>. Notable test fixes (marked with the causes of failures): fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html: [A] Marked as flaky, because it depends on whether body onload is called before (Pass) or after (Failure) the first layout (crbug/651343). fast/scrolling/scrollbar-tickmarks-styled.html: [A] Fixed by https://codereview.chromium.org/2450793002. fast/scrolling/scrollbar-tickmarks-hittest.html: [A] Fixed by https://codereview.chromium.org/2484613003. virtual/rootlayerscrolls/fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Fixed by https://codereview.chromium.org/2467433002. svg/animations/animate-end-attribute-numeric-precision.html: [C] After this CL, calling executeTest() in svg/animations/script-tests/animate-end-attribute-numeric-precision.js causes sampleAnimation() to be called before document onload and test to fail. We set |animationStartsImmediately| true instead, in order to ensure the test is started in document onload. accessibility/inline-text-changes.html: [Race between the accessibility notification and document onload] Ignore the second call of the notification handler. Even before this CL, the notification is called multiple times, but the first notification terminated the test immediately by finishJSTest() because it is called after document onload. This CL makes the notification calls before document onload, and thus the test continues until document onload and we have to ignore the second call explicitly. fast/block/float/marquee-shrink-to-avoid-floats.html: The layout tree changes slightly, but it looks like acceptable because the image expectation matches. BUG= 624697 , 651343 Review-Url: https://codereview.chromium.org/2269043002 Cr-Commit-Position: refs/heads/master@{#430348} [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/accessibility/inline-text-changes.html [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/animations/animation-duration-infinite.html [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/fast/block/float/marquee-shrink-to-avoid-floats-expected.txt [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/inert-node-is-unfocusable-expected.txt [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash-expected.txt [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/fast/harness/error-in-async-test-expected.txt [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/http/tests/loading/preload-img-test-expected.txt [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/LayoutTests/svg/animations/script-tests/animate-end-attribute-numeric-precision.js [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp [modify] https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543/third_party/WebKit/Source/core/html/HTMLStyleElement.h
,
Nov 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/98116e47e7660b5ad3a71949efea694f96c1104e commit 98116e47e7660b5ad3a71949efea694f96c1104e Author: xlai <xlai@chromium.org> Date: Mon Nov 07 20:43:21 2016 Revert of Remove EventSender from HTMLStyleElement (patchset #20 id:380001 of https://codereview.chromium.org/2269043002/ ) Reason for revert: This CL causes two layout tests to fail. See https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/39126 Original issue's description: > Remove EventSender from HTMLStyleElement > > This CL replaces EventSender in HTMLLinkElement with postTask(): > Instead of calling HTMLStyleElement::dispatchPendingLoadEvents() in > Document::implicitClose() to enforce <style>'s onload events to be executed > before the document's onload event, this CL blocks document's onload until > <style>'s onload event using IncrementLoadEventDelayCount. > > This CL changes the timing of body onload, particularly causes some of the > following events that were previously called AFTER document onload > to be called BEFORE document onload: > [A] The first layout. > [B] Latter half of FrameLoader::finishedParsing(). > [C] JavaScript function |f| scheduled as |setTimeout(f, 0)| in the <script> > at the end of <body>. > > Notable test fixes (marked with the causes of failures): > > fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html: > [A] Marked as flaky, because it depends on whether body onload is called > before (Pass) or after (Failure) the first layout (crbug/651343). > > fast/scrolling/scrollbar-tickmarks-styled.html: > [A] Fixed by https://codereview.chromium.org/2450793002. > > fast/scrolling/scrollbar-tickmarks-hittest.html: > [A] Fixed by https://codereview.chromium.org/2484613003. > > virtual/rootlayerscrolls/fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: > [B] Fixed by https://codereview.chromium.org/2467433002. > > svg/animations/animate-end-attribute-numeric-precision.html: > [C] After this CL, calling executeTest() in > svg/animations/script-tests/animate-end-attribute-numeric-precision.js > causes sampleAnimation() to be called before document onload and test to fail. > We set |animationStartsImmediately| true instead, in order to ensure the test > is started in document onload. > > accessibility/inline-text-changes.html: > [Race between the accessibility notification and document onload] > Ignore the second call of the notification handler. > Even before this CL, the notification is called multiple times, but the first > notification terminated the test immediately by finishJSTest() because it is > called after document onload. > This CL makes the notification calls before document onload, and thus the test > continues until document onload and we have to ignore the second call > explicitly. > > fast/block/float/marquee-shrink-to-avoid-floats.html: > The layout tree changes slightly, but it looks like acceptable because the > image expectation matches. > > BUG= 624697 , 651343 > > Committed: https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543 > Cr-Commit-Position: refs/heads/master@{#430348} TBR=haraken@chromium.org,kojii@chromium.org,japhet@chromium.org,yhirano@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 , 651343 Review-Url: https://codereview.chromium.org/2487433002 Cr-Commit-Position: refs/heads/master@{#430366} [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/accessibility/inline-text-changes.html [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/animations/animation-duration-infinite.html [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/fast/block/float/marquee-shrink-to-avoid-floats-expected.txt [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/inert-node-is-unfocusable-expected.txt [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash-expected.txt [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/fast/harness/error-in-async-test-expected.txt [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/http/tests/loading/preload-img-test-expected.txt [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/LayoutTests/svg/animations/script-tests/animate-end-attribute-numeric-precision.js [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp [modify] https://crrev.com/98116e47e7660b5ad3a71949efea694f96c1104e/third_party/WebKit/Source/core/html/HTMLStyleElement.h
,
Nov 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8cb16f79a95c8bc639a9c58785a8484c197aca9 commit b8cb16f79a95c8bc639a9c58785a8484c197aca9 Author: hiroshige <hiroshige@chromium.org> Date: Tue Nov 08 10:53:43 2016 Reland of Remove EventSender from HTMLStyleElement This CL replaces EventSender in HTMLLinkElement with postTask(): Instead of calling HTMLStyleElement::dispatchPendingLoadEvents() in Document::implicitClose() to enforce <style>'s onload events to be executed before the document's onload event, this CL blocks document's onload until <style>'s onload event using IncrementLoadEventDelayCount. This CL changes the timing of body onload, particularly causes some of the following events that were previously called AFTER document onload to be called BEFORE document onload: [A] The first layout. [B] Latter half of FrameLoader::finishedParsing(). [C] JavaScript function |f| scheduled as |setTimeout(f, 0)| in the <script> at the end of <body>. Notable test fixes (marked with the causes of failures): fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html: [A] Marked as flaky, because it depends on whether body onload is called before (Pass) or after (Failure) the first layout (crbug/651343). fast/scrolling/scrollbar-tickmarks-styled.html: [A] Fixed by https://codereview.chromium.org/2450793002. fast/scrolling/scrollbar-tickmarks-hittest.html: [A] Fixed by https://codereview.chromium.org/2484613003. fast/forms/text/text-set-selection-crash.html: [A] There is race between "Blink Test Plugin: initializing" console log (which is asynchronously triggered by the first layout) and document onload (=test finish). Because the test should pass if not crashes, this CL make the test to ignore the log, by converting the test to use testharness.js and removing the -expected.txt. virtual/rootlayerscrolls/fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Fixed by https://codereview.chromium.org/2467433002. svg/animations/animate-end-attribute-numeric-precision.html: [C] After this CL, calling executeTest() in svg/animations/script-tests/animate-end-attribute-numeric-precision.js causes sampleAnimation() to be called before document onload and test to fail. We set |animationStartsImmediately| true instead, in order to ensure the test is started in document onload. fast/harness/error-in-async-test.html: [C] Whether "Uncaught Exception" console log is shown depends on whether buggyAsyncCode() is executed before or after document onload. This CL deflakes the test by ensuring it is executed after document onload. accessibility/inline-text-changes.html: [Race between the accessibility notification and document onload] Ignore the second call of the notification handler. Even before this CL, the notification is called multiple times, but the first notification terminated the test immediately by finishJSTest() because it is called after document onload. This CL makes the notification calls before document onload, and thus the test continues until document onload and we have to ignore the second call explicitly. fast/block/float/marquee-shrink-to-avoid-floats.html: The layout tree changes slightly, but it looks like acceptable because the image expectation matches. BUG= 624697 , 651343 Committed: https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543 Review-Url: https://codereview.chromium.org/2269043002 Cr-Original-Commit-Position: refs/heads/master@{#430348} Cr-Commit-Position: refs/heads/master@{#430572} [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/accessibility/inline-text-changes.html [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/animations/animation-duration-infinite.html [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/fast/block/float/marquee-shrink-to-avoid-floats-expected.txt [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/inert-node-is-unfocusable-expected.txt [delete] https://crrev.com/5db5553160affb9ea2464666320c3855e5d203d2/third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash-expected.txt [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash.html [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/fast/harness/error-in-async-test.html [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/http/tests/loading/preload-img-test-expected.txt [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/LayoutTests/svg/animations/script-tests/animate-end-attribute-numeric-precision.js [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp [modify] https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9/third_party/WebKit/Source/core/html/HTMLStyleElement.h
,
Nov 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dbfafb1dbbe4b520e2d42de5a991fe90ae4dbf7d commit dbfafb1dbbe4b520e2d42de5a991fe90ae4dbf7d Author: hiroshige <hiroshige@chromium.org> Date: Tue Nov 08 12:34:04 2016 Fix scroll-position-restored-on-back-at-load-event.html Follow-up of [1]. The test doesn't work if the |window.location| change occurs after document onload, and [1] makes the function in setTimeout() in DOMContentLoaded in the test to be sometimes executed (especially on debug bots) after document onload. This CL fixes the test by ensuring |window.location| change is always executed after document onload. [1] https://codereview.chromium.org/2269043002/ BUG= 624697 Review-Url: https://codereview.chromium.org/2489523002 Cr-Commit-Position: refs/heads/master@{#430591} [modify] https://crrev.com/dbfafb1dbbe4b520e2d42de5a991fe90ae4dbf7d/third_party/WebKit/LayoutTests/fast/loader/scroll-position-restored-on-back-at-load-event.html
,
Nov 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77c635eea1e4f6a9b23f111e0b146463cd485498 commit 77c635eea1e4f6a9b23f111e0b146463cd485498 Author: hiroshige <hiroshige@chromium.org> Date: Wed Nov 09 06:04:22 2016 Remove EventSender in HTMLDetailsElement BUG= 624697 Review-Url: https://codereview.chromium.org/2478603003 Cr-Commit-Position: refs/heads/master@{#430877} [modify] https://crrev.com/77c635eea1e4f6a9b23f111e0b146463cd485498/third_party/WebKit/Source/core/html/HTMLDetailsElement.cpp [modify] https://crrev.com/77c635eea1e4f6a9b23f111e0b146463cd485498/third_party/WebKit/Source/core/html/HTMLDetailsElement.h
,
Nov 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f34e7c74ea016495fe3772da1a33062ffce3221 commit 6f34e7c74ea016495fe3772da1a33062ffce3221 Author: hiroshige <hiroshige@chromium.org> Date: Wed Nov 09 07:54:06 2016 Remove EventSender from HTMLSourceElement BUG= 624697 Review-Url: https://codereview.chromium.org/2480523003 Cr-Commit-Position: refs/heads/master@{#430885} [modify] https://crrev.com/6f34e7c74ea016495fe3772da1a33062ffce3221/third_party/WebKit/Source/core/html/HTMLSourceElement.cpp [modify] https://crrev.com/6f34e7c74ea016495fe3772da1a33062ffce3221/third_party/WebKit/Source/core/html/HTMLSourceElement.h
,
Nov 17 2016
,
Nov 17 2016
,
Nov 22 2016
,
Dec 22 2016
,
Jan 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/857998f6024697334675af683c89e39d804f45a9 commit 857998f6024697334675af683c89e39d804f45a9 Author: hiroshige <hiroshige@chromium.org> Date: Wed Jan 18 23:58:35 2017 Introduce IncrementLoadEventDelayCount::clearAndCheckLoadEvent() This is to enable checkCompleted() to be called synchronously when IncrementLoadEventDelayCount unblocks Document load event (for performance). Particularly, this will be used in https://codereview.chromium.org/2605173002 and https://codereview.chromium.org/2391923002. BUG= 624697 Review-Url: https://codereview.chromium.org/2605383002 Cr-Commit-Position: refs/heads/master@{#444552} [modify] https://crrev.com/857998f6024697334675af683c89e39d804f45a9/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/857998f6024697334675af683c89e39d804f45a9/third_party/WebKit/Source/core/dom/Document.h [modify] https://crrev.com/857998f6024697334675af683c89e39d804f45a9/third_party/WebKit/Source/core/dom/IncrementLoadEventDelayCount.cpp [modify] https://crrev.com/857998f6024697334675af683c89e39d804f45a9/third_party/WebKit/Source/core/dom/IncrementLoadEventDelayCount.h
,
Jan 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0712ce38d1a05590e731d07f9aad4b46870f53ae commit 0712ce38d1a05590e731d07f9aad4b46870f53ae Author: hiroshige <hiroshige@chromium.org> Date: Thu Jan 19 23:23:09 2017 Call checkCompleted() synchronously in HTMLStyleElement::dispatchPendingEvent() Previously, there was an asynchronous hop from <style>'s load event to document's load event, because HTMLStyleElement::dispatchPendingEvent() scheduled checkCompleted() asynchronously via IncrementLoadEventDelayCount. This CL makes HTMLStyleElement::dispatchPendingEvent() to call checkCompleted() synchronously, by using IncrementLoadEventDelayCount::clearAndCheckLoadEvent(). Because checkCompleted() is called at the end of asynchronously called HTMLStyleElement::dispatchPendingEvent(), this CL doesn't increase synchronous timing dependencies around Document load events. This fixes performance (memory consumption) regression in memory.top_10_mobile_stress caused by https://codereview.chromium.org/2275493002/. BUG= 663823 , 624697 TEST=https_www_google_co_uk_hl_en_q_science in memory:chrome:all_processes:reported_by_chrome:v8:effective_size in memory.top_10_mobile_stress Review-Url: https://codereview.chromium.org/2605173002 Cr-Commit-Position: refs/heads/master@{#444886} [modify] https://crrev.com/0712ce38d1a05590e731d07f9aad4b46870f53ae/third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/inert-node-is-unfocusable-expected.txt [modify] https://crrev.com/0712ce38d1a05590e731d07f9aad4b46870f53ae/third_party/WebKit/LayoutTests/http/tests/loading/preload-img-test-expected.txt [modify] https://crrev.com/0712ce38d1a05590e731d07f9aad4b46870f53ae/third_party/WebKit/LayoutTests/plugins/overlay-scrollbar-mouse-capture-expected.txt [modify] https://crrev.com/0712ce38d1a05590e731d07f9aad4b46870f53ae/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp [modify] https://crrev.com/0712ce38d1a05590e731d07f9aad4b46870f53ae/third_party/WebKit/Source/core/html/HTMLStyleElement.h
,
Jan 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/46649a2f0bf126330d95f47b6c8773823451d23f commit 46649a2f0bf126330d95f47b6c8773823451d23f Author: hiroshige <hiroshige@chromium.org> Date: Fri Jan 20 03:18:25 2017 Call checkCompleted() synchronously in HTMLLinkElement::dispatchPendingEvent() Previously, there was an asynchronous hop from <link>'s load event to document's load event, because HTMLLinkElement::dispatchPendingEvent() scheduled checkCompleted() asynchronously via IncrementLoadEventDelayCount. This CL makes HTMLLinkElement::dispatchPendingEvent() to call checkCompleted() synchronously, by using IncrementLoadEventDelayCount::clearAndCheckLoadEvent(). Because checkCompleted() is called at the end of asynchronously called HTMLLinkElement::dispatchPendingEvent(), this CL doesn't increase synchronous timing dependencies around Document load events. This CL fixes the performance regression of smoothness.top_25_smooth on Mac 10.10 caused by https://codereview.chromium.org/2275493002/. BUG= 648887 , 624697 Review-Url: https://codereview.chromium.org/2391923002 Cr-Commit-Position: refs/heads/master@{#444965} [modify] https://crrev.com/46649a2f0bf126330d95f47b6c8773823451d23f/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp [modify] https://crrev.com/46649a2f0bf126330d95f47b6c8773823451d23f/third_party/WebKit/Source/core/html/HTMLLinkElement.h
,
Apr 4 2017
,
May 5 2017
As Issue 667641 is progressing, I'm restarting the effort to remove EventSenders in ImageLoader.
,
May 8 2017
Draft CLs and plan: [1] https://codereview.chromium.org/2859383002/: Add CHECK()s [2] https://codereview.chromium.org/2859093003/: has_pending_load_event_ refactoring [3] https://codereview.chromium.org/2863763003/: main CL I'm landing [1] now. I'll land [2] a couple of days later to confirm CHECK()s in [1] are not failing. I'll land [3] after https://codereview.chromium.org/2613853002/.
,
May 9 2017
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d5af742666d2acde551ac33e82f3d8662ffa0f15 commit d5af742666d2acde551ac33e82f3d8662ffa0f15 Author: hiroshige <hiroshige@chromium.org> Date: Fri May 12 01:56:02 2017 Split ImageLoader::SetImage() and set flags consistently in tests Previously, ImageLoader::SetImage() is used in three ways: 1. SetImage(nullptr), 2. SetImage(non-null) for ImageDocument, and 3. SetImage(non-null) for unit tests for non-ImageDocument. and SetImage() sets has_pending_load_event_ = false and image_complete_ = true for all of these. This flag setting is consistent for 1., but not for 3., and causes assertion failures when we apply stronger assertions in [1]. This CL fixes this by splitting SetImage() for separate methods: 1. ClearImage(), 2. SetImageForImageDocument(), and 3. SetImageForTest(), and introducing UpdateImageState() that sets: - image_ - has_pending_load_event_ - image_complete_ This CL - Doesn't change non-test behavior, i.e. keeps the behavior for Cases 1 and 2. Particularly, this leaves the flag values that are apparently inconsistent but needed for the current implementation in Case 2 in SetImageForImageDocument(). - Changes the test-only behavior of Case 3, to set has_pending_load_event_ = true and image_complete_ = false in SetImageForTest(). This doesn't affect the tested behavior but fixes the assertion failures in [1]. [1] https://codereview.chromium.org/2859383002 BUG= 624697 , 719759 Review-Url: https://codereview.chromium.org/2864253003 Cr-Commit-Position: refs/heads/master@{#471177} [modify] https://crrev.com/d5af742666d2acde551ac33e82f3d8662ffa0f15/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp [modify] https://crrev.com/d5af742666d2acde551ac33e82f3d8662ffa0f15/third_party/WebKit/Source/core/html/HTMLImageElement.h [modify] https://crrev.com/d5af742666d2acde551ac33e82f3d8662ffa0f15/third_party/WebKit/Source/core/html/HTMLObjectElement.cpp [modify] https://crrev.com/d5af742666d2acde551ac33e82f3d8662ffa0f15/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/d5af742666d2acde551ac33e82f3d8662ffa0f15/third_party/WebKit/Source/core/loader/ImageLoader.h
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f33a3ba6de05eaed0115636c6dd552b9f8c80333 commit f33a3ba6de05eaed0115636c6dd552b9f8c80333 Author: hiroshige <hiroshige@chromium.org> Date: Fri May 12 19:18:35 2017 Add layout tests for ImageLoader::UpdateFromElement(kUpdateForcedReload) HTMLImageElement::ForceReload() and ImageLoader::UpdateFromElement(kUpdateForcedReload) were introduced by [1] but are not covered by any tests. This CL adds layout tests for this code path. This is also confirming the correctness of [2] and [3]. [1] https://codereview.chromium.org/1112513005/ [2] https://codereview.chromium.org/2877583002/ [3] https://codereview.chromium.org/2859383002/ BUG=485295, 624697 , 719759 Review-Url: https://codereview.chromium.org/2874073003 Cr-Commit-Position: refs/heads/master@{#471401} [add] https://crrev.com/f33a3ba6de05eaed0115636c6dd552b9f8c80333/third_party/WebKit/LayoutTests/http/tests/cache/resources/random-cached-image.php [add] https://crrev.com/f33a3ba6de05eaed0115636c6dd552b9f8c80333/third_party/WebKit/LayoutTests/http/tests/images/force-reload-image-document.html [add] https://crrev.com/f33a3ba6de05eaed0115636c6dd552b9f8c80333/third_party/WebKit/LayoutTests/http/tests/images/force-reload.html [modify] https://crrev.com/f33a3ba6de05eaed0115636c6dd552b9f8c80333/third_party/WebKit/Source/core/testing/Internals.cpp [modify] https://crrev.com/f33a3ba6de05eaed0115636c6dd552b9f8c80333/third_party/WebKit/Source/core/testing/Internals.h [modify] https://crrev.com/f33a3ba6de05eaed0115636c6dd552b9f8c80333/third_party/WebKit/Source/core/testing/Internals.idl
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e263cd1373b7f330054bc1b252c19abf1ac9ba09 commit e263cd1373b7f330054bc1b252c19abf1ac9ba09 Author: hiroshige <hiroshige@chromium.org> Date: Fri May 12 21:52:18 2017 Refactor ImageLoader::UpdateFromElement(kUpdateForcedReload) Previously, there were two cases when |loading_image_document_| is true: Case 1. Initial ImageDocument loading. |image_| is not associated with ResourceLoader, and the response/data are supplied by ImageDocument instead. Case 2. Reloading. |image_| is created normally via ResourceFetcher and thus is associated with ResourceLoader. This is same as non-ImageDocument image loading. This CL clears |loading_image_document_| and |image_resource_for_image_document_| in the second case, to - Make |image_| always under control of ImageDocument if |loading_image_document_| is true. - Avoid |image_| from being controlled by both of ImageDocument and ResourceLoader when |loading_image_document_| is false. - Simplify the invariants and make the assertion in [1] hold. [1] https://codereview.chromium.org/2859383002/ BUG= 624697 , 719759 , 485295 Review-Url: https://codereview.chromium.org/2877583002 Cr-Commit-Position: refs/heads/master@{#471455} [modify] https://crrev.com/e263cd1373b7f330054bc1b252c19abf1ac9ba09/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/e263cd1373b7f330054bc1b252c19abf1ac9ba09/third_party/WebKit/Source/core/loader/ImageLoader.h
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b070387595668ec438ffec65185e61017300e3c commit 1b070387595668ec438ffec65185e61017300e3c Author: hiroshige <hiroshige@chromium.org> Date: Fri May 12 22:21:22 2017 Add CHECK()s about ImageLoader::has_pending_load_event_ As preparation for [1], this CL checks some assumptions to hold that are helpful for proving the equivalences in [1]. [1] https://codereview.chromium.org/2859093003/ BUG= 624697 , 719759 Review-Url: https://codereview.chromium.org/2859383002 Cr-Commit-Position: refs/heads/master@{#471464} [modify] https://crrev.com/1b070387595668ec438ffec65185e61017300e3c/third_party/WebKit/Source/core/loader/ImageLoader.cpp
,
May 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d5bcf58d9932d151f12231a6544b67903780fed commit 9d5bcf58d9932d151f12231a6544b67903780fed Author: hiroshige <hiroshige@chromium.org> Date: Sat May 13 18:03:28 2017 Revert of Add CHECK()s about ImageLoader::has_pending_load_event_ (patchset #12 id:220001 of https://codereview.chromium.org/2859383002/ ) Reason for revert: Assertion failures in the wild and on clusterfuzz. BUG= 721994 Original issue's description: > Add CHECK()s about ImageLoader::has_pending_load_event_ > > As preparation for [1], this CL checks some assumptions to hold that > are helpful for proving the equivalences in [1]. > > [1] https://codereview.chromium.org/2859093003/ > > BUG= 624697 , 719759 > > Review-Url: https://codereview.chromium.org/2859383002 > Cr-Commit-Position: refs/heads/master@{#471464} > Committed: https://chromium.googlesource.com/chromium/src/+/1b070387595668ec438ffec65185e61017300e3c TBR=japhet@chromium.org,yhirano@chromium.org,kinuko@chromium.org,kouhei@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 624697 , 719759 Review-Url: https://codereview.chromium.org/2878263002 Cr-Commit-Position: refs/heads/master@{#471598} [modify] https://crrev.com/9d5bcf58d9932d151f12231a6544b67903780fed/third_party/WebKit/Source/core/loader/ImageLoader.cpp
,
May 13 2017
,
May 15 2017
,
May 15 2017
,
May 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c5d3f11a80d88d65f5fae8398ed97a7b5ea1508 commit 1c5d3f11a80d88d65f5fae8398ed97a7b5ea1508 Author: hiroshige <hiroshige@chromium.org> Date: Mon May 15 22:28:51 2017 Partial reland of "Add CHECK()s about ImageLoader::has_pending_load_event_" Original CL: https://codereview.chromium.org/2859383002/#ps220001 This CL relands the CHECK()s except for the failing one in DispatchPendingErrorEvent(). Issue 722500 tracks the issue around DispatchPendingErrorEvent() and relanding of the remaining CHECK(). Original CL description: > As preparation for [1], this CL checks some assumptions to hold that > are helpful for proving the equivalences in [1]. > > [1] https://codereview.chromium.org/2859093003/ > > BUG= 624697 , 719759 > > Review-Url: https://codereview.chromium.org/2859383002 > Cr-Commit-Position: refs/heads/master@{#471464} > Committed: https://chromium.googlesource.com/chromium/src/+/1b070387595668ec438ffec65185e61017300e3c BUG= 624697 , 719759 , 722500 Review-Url: https://codereview.chromium.org/2884773002 Cr-Commit-Position: refs/heads/master@{#471925} [modify] https://crrev.com/1c5d3f11a80d88d65f5fae8398ed97a7b5ea1508/third_party/WebKit/Source/core/loader/ImageLoader.cpp
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/84fb9e0a3e168b81e538a5ff8ce79e3690396a10 commit 84fb9e0a3e168b81e538a5ff8ce79e3690396a10 Author: hiroshige <hiroshige@chromium.org> Date: Fri May 19 16:53:20 2017 Make ImageLoader have at most one pending error event on EventSender Relanding the CHECK() that asserts ImageLoader::DispatchPendingErrorEvent() should always called when has_pending_error_event_ is true. Previously, we sometimes schedule multiple error events on EventSender: 1. DispatchErrorEvent() is called. 2. DispatchErrorEvent() is called. 3. DispatchPendingErrorEvent() is called (corresponding to Step 1) and actual processing is done. 4. DispatchPendingErrorEvent() is called (corresponding to Step 2) but ignored because has_pending_error_event_ is already false. This CL makes DispatchErrorEvent() cancel previous scheduled error events if any, ensures there can be at most scheduled error event on EventSender, and makes the assertion hold. The scenario above will be processed as follows, and thus this CL shouldn't change any observable behavior. 1. DispatchErrorEvent() is called. 2. DispatchErrorEvent() is called. The error event scheduled by Step 1 is cancelled. 3. DispatchPendingErrorEvent() is called (corresponding to Step 2) and actual processing is done. [1] https://codereview.chromium.org/2859383002/ [2] https://codereview.chromium.org/2884773002/ BUG= 624697 , 719759 , 722500 Review-Url: https://codereview.chromium.org/2893993002 Cr-Commit-Position: refs/heads/master@{#473214} [add] https://crrev.com/84fb9e0a3e168b81e538a5ff8ce79e3690396a10/third_party/WebKit/LayoutTests/images/multiple-inflight-error-event-crash.html [modify] https://crrev.com/84fb9e0a3e168b81e538a5ff8ce79e3690396a10/third_party/WebKit/Source/core/loader/ImageLoader.cpp
,
May 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e6f72c80e0106f348fd9437536edc2e9f3b7acc commit 2e6f72c80e0106f348fd9437536edc2e9f3b7acc Author: hiroshige <hiroshige@chromium.org> Date: Mon May 22 20:49:24 2017 Change the semantics of ImageLoader::has_pending_load_event_ ImageLoader works like: (1) Image loading is started (AddObserver()), (2) Image loading is finished and the image element's load event is scheduled (ImageNotifyFinished()), and then (3) The load event is fired. Previously, has_pending_load_event_ was true from (1) to (3). This CL makes has_pending_load_event_ true only from (2) to (3), i.e. only while there is a pending task on EventSender. Because "image_ && !image_complete_" is true from (1) to (2), The previous "has_pending_load_event_" is equivalent to the current "(image_ && !image_complete_) || has_pending_load_event_". This CL, however, leaves most of "has_pending_load_event_" usage unchanged, because in such cases they are equivalent, given the assumptions around image_ and image_complete_, some of which are confirmed by [1]. This is preparation for [2] that replaces has_pending_load_event_ with a reference to a task that corresponds to the pending load event. [1] https://codereview.chromium.org/2859383002/ [2] https://codereview.chromium.org/2863763003/ BUG= 624697 , 719759 Review-Url: https://codereview.chromium.org/2859093003 Cr-Commit-Position: refs/heads/master@{#473684} [modify] https://crrev.com/2e6f72c80e0106f348fd9437536edc2e9f3b7acc/third_party/WebKit/Source/core/loader/ImageLoader.cpp [modify] https://crrev.com/2e6f72c80e0106f348fd9437536edc2e9f3b7acc/third_party/WebKit/Source/core/loader/ImageLoader.h
,
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 24 2017
,
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
,
Jun 2 2017
,
Nov 30 2017
All EventSender were removed! Closing. |
||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by hirosh...@chromium.org
, Aug 23 2016