|
|
Created:
4 years, 4 months ago by hiroshige Modified:
4 years, 1 month ago CC:
chromium-reviews, blink-reviews-html_chromium.org, blink-reviews-style_chromium.org, sof, eae+blinkwatch, blink-reviews-dom_chromium.org, dglazkov+blink, blink-reviews, rwlbuis, loading-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReland 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
Committed: https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9
Cr-Original-Commit-Position: refs/heads/master@{#430348}
Cr-Commit-Position: refs/heads/master@{#430572}
Patch Set 1 #Patch Set 2 : Block onload event #Patch Set 3 : Blocking #Patch Set 4 : not Blocking #Patch Set 5 : Update test expectations #Patch Set 6 : Rebase #Patch Set 7 : Use DOMManipulation task runner #Patch Set 8 : Rebase #Patch Set 9 : Test fixes #Patch Set 10 : Fix more tests. #Patch Set 11 : Call restoreScrollPositionAndViewState() in finishedParsing() #Patch Set 12 : Rebase #Patch Set 13 : Fix #Patch Set 14 : Fixes. #Patch Set 15 : Rebase. #Patch Set 16 : Rebase #Patch Set 17 : minor refinement #Patch Set 18 : Run test in onload() #Patch Set 19 : Rebase #Patch Set 20 : Rebase (First Commit) #Patch Set 21 : Deflake #
Total comments: 2
Patch Set 22 : remove t.step() #Messages
Total messages: 94 (74 generated)
Description was changed from ========== Remove EventSender from HTMLStyleElement BUG= ========== to ========== 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. BUG=624697 ==========
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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. BUG=624697 ========== to ========== 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. (after document onload -> before document onload) [B] Latter half of FrameLoader::finishedParsing(). [C] JavaScript function |f| scheduled as |setTimeout(f, 0)| in the <script> at the end of <body>. (after document onload -> before document onload) 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-after-onload.html: [A] Fixed in https://codereview.chromium.org/2450793002. fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ==========
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Description was changed from ========== 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. (after document onload -> before document onload) [B] Latter half of FrameLoader::finishedParsing(). [C] JavaScript function |f| scheduled as |setTimeout(f, 0)| in the <script> at the end of <body>. (after document onload -> before document onload) 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-after-onload.html: [A] Fixed in https://codereview.chromium.org/2450793002. fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ========== to ========== 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-after-onload.html: [A] Fixed in https://codereview.chromium.org/2450793002. fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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-after-onload.html: [A] Fixed in https://codereview.chromium.org/2450793002. fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ========== to ========== 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 in https://codereview.chromium.org/2450793002. fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ==========
Description was changed from ========== 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 in https://codereview.chromium.org/2450793002. fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ========== to ========== 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 in https://codereview.chromium.org/2450793002. virtual/rootlayerscrolls/fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ==========
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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 in https://codereview.chromium.org/2450793002. virtual/rootlayerscrolls/fast/history/scroll-restoration/scroll-restoration-fragment-navigation-crossdoc.html: [B] Before this CL, in FrameLoader::finishedParsing(): 1. document onload and restoreScrollPositionAndViewState() are called in checkCompleted(), and thus documentLoader()->initialScrollState().didRestoreFromHistory is set true. 2. FrameLoader::processFragment() is called and it doesn't cause scroll because |shouldScrollToFragment| is false because |didRestoreFromHistory| is true. After this CL: 1. FrameLoader::restoreScrollPositionAndViewState() is NOT called in checkCompleted() because this CL causes the document onload to be called after finishedParsing(). 2. In FrameLoader::processFragment(), scrolling occurs because |shouldScrollToFragment| is true because |didRestoreFromHistory| is false. This CL calls restoreScrollPositionAndViewState() before the processFragment() call in finishedParsing() to fix this test. 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 ========== to ========== 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. 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 ==========
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
hiroshige@chromium.org changed reviewers: + haraken@chromium.org, kojii@chromium.org
PTAL. kojii@ for LayoutTests/TestExpectations haraken@ for other files as a core/ OWNER.
On 2016/10/31 09:35:26, hiroshige wrote: > PTAL. > > kojii@ for LayoutTests/TestExpectations > haraken@ for other files as a core/ OWNER. I'm fine with this change but might want to have some loading expert check if the event-dispatching timing change is okay.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
hiroshige@chromium.org changed reviewers: + japhet@chromium.org, yhirano@chromium.org
+japhet@, yhirano@. Could you take a look? (although changes are not in core/fetch|loader, this CL is related to loading)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
LGTM
LayoutTests/TestExpectations lgtm
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hiroshige@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, kojii@chromium.org, yhirano@chromium.org Link to the patchset: https://codereview.chromium.org/2269043002/#ps360001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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. 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 ========== to ========== 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 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by hiroshige@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, kojii@chromium.org, yhirano@chromium.org Link to the patchset: https://codereview.chromium.org/2269043002/#ps380001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 ==========
Message was sent while issue was closed.
Committed patchset #20 (id:380001)
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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} ==========
Message was sent while issue was closed.
Patchset 20 (id:??) landed as https://crrev.com/52d00fd56184deadce99ff4fe599156ea1da3543 Cr-Commit-Position: refs/heads/master@{#430348}
Message was sent while issue was closed.
A revert of this CL (patchset #20 id:380001) has been created in https://codereview.chromium.org/2487433002/ by xlai@chromium.org. The reason for reverting is: This CL causes two layout tests to fail. See https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds....
Message was sent while issue was closed.
Description was changed from ========== 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} ========== to ========== 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} ==========
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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} ========== to ========== 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 Cr-Commit-Position: refs/heads/master@{#430348} ==========
Patchset #21 (id:400001) has been deleted
yhirano@, could you take a look again? Patch Set 21 deflakes the following tests: fast/forms/text/text-set-selection-crash.html fast/harness/error-in-async-test.html
https://codereview.chromium.org/2269043002/diff/420001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash.html (right): https://codereview.chromium.org/2269043002/diff/420001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash.html:8: t.step(function() { Can you tell me why this |t.step| is needed?
The CQ bit was checked by hiroshige@chromium.org to run a CQ dry run
https://codereview.chromium.org/2269043002/diff/420001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash.html (right): https://codereview.chromium.org/2269043002/diff/420001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/forms/text/text-set-selection-crash.html:8: t.step(function() { On 2016/11/08 06:25:54, yhirano wrote: > Can you tell me why this |t.step| is needed? Removing t.step() also works.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by hiroshige@chromium.org
The CQ bit was checked by hiroshige@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, kojii@chromium.org Link to the patchset: https://codereview.chromium.org/2269043002/#ps440001 (title: "remove t.step()")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== 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 Cr-Commit-Position: refs/heads/master@{#430348} ========== to ========== 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 Cr-Commit-Position: refs/heads/master@{#430348} ==========
Message was sent while issue was closed.
Committed patchset #22 (id:440001)
Message was sent while issue was closed.
Description was changed from ========== 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 Cr-Commit-Position: refs/heads/master@{#430348} ========== to ========== 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 Committed: https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9 Cr-Original-Commit-Position: refs/heads/master@{#430348} Cr-Commit-Position: refs/heads/master@{#430572} ==========
Message was sent while issue was closed.
Patchset 22 (id:??) landed as https://crrev.com/b8cb16f79a95c8bc639a9c58785a8484c197aca9 Cr-Commit-Position: refs/heads/master@{#430572}
Message was sent while issue was closed.
This CL exposes unfavorable timing dependencies to document onload() in some tests and thus might make the tests flaky. Please mark them (if any) in TestExpectations and refer to BUG=624697. I'll investigate and fix the tests (or revert this CL if needed). |