New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847496 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 888542



Sign in to add a comment

[EventTiming] performance/event-timing/event-timing-bufferbeforeonload.html is flaky

Project Member Reported by maxlg@chromium.org, May 29 2018

Issue description

The test is observed to be flaky, but there is not enough information to tell which line cause the failure.
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests/70097
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 29 2018

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

commit 66d69d2841126c91f2bcf9660163e450fe217109
Author: Liquan(Max) Gu <maxlg@chromium.org>
Date: Tue May 29 17:47:13 2018

[EventTiming] Add error messages for layout tests

event-timing-bufferbeforeonload.html is observed to be flaky, but
the original test does not have enough information to capture the
flaky line. This patch is to add error messages for every asserts
in the related tests.

The flaky test will be marked as flaky to unblock the CQ, while
gathering more information before deciding on the fix.

Bug:  847496 
Change-Id: I12b10cc2ef8776e5e3081e613aa2fb7826347d73
Reviewed-on: https://chromium-review.googlesource.com/1076315
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Luna Lu <loonybear@chromium.org>
Commit-Queue: Liquan (Max) Gǔ <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562495}
[modify] https://crrev.com/66d69d2841126c91f2bcf9660163e450fe217109/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/66d69d2841126c91f2bcf9660163e450fe217109/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-bufferbeforeonload.html
[modify] https://crrev.com/66d69d2841126c91f2bcf9660163e450fe217109/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-crossiframe.html
[modify] https://crrev.com/66d69d2841126c91f2bcf9660163e450fe217109/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-observethenonload.html
[modify] https://crrev.com/66d69d2841126c91f2bcf9660163e450fe217109/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-onloadthenobserve.html
[modify] https://crrev.com/66d69d2841126c91f2bcf9660163e450fe217109/third_party/WebKit/LayoutTests/performance/event-timing/resources/event-timing-support.js

Comment 2 by maxlg@chromium.org, May 31 2018

Here's the failing message that I've gathered:
https://paste.googleplex.com/5117720496439296 

The test fails at this line:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-bufferbeforeonload.html?rcl=2e4fdc9e9e9656fce457e608645feac9bab9dbd5&l=28

The error msg indicates that entry.startTime < onload < entry.processingStart happened. The test design assumed that the order should be entry.startTime < entry.processingStart < onload.

Comment 3 by maxlg@chromium.org, May 31 2018

The bug arises when deciding whether it's time before onload. Having no access to performance timing doesn't necessarily means onload is not reached. The test would fail when onload has reached, but the performance object doesn't know that because it has no access to the frame or the document loader, so it has no access to the timing info.

Normally, the performance object track down this path to get the load event start (onload timestamp):
GetFrame()->Loader().GetDocumentLoader()->GetTiming().LoadEventStart()
Cc: npm@chromium.org
"The test would fail when onload has reached"

Should we just prevent onload from being reached? We could load a resource that doesn't finish loading for some amount of time.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 6 2018

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

commit ddcf12998995d87b6fed15c881ac79dc57ff9e43
Author: Liquan(Max) Gu <maxlg@chromium.org>
Date: Wed Jun 06 00:32:27 2018

[EventTiming] Remove the check of processing start being before onload

Due to the possible absence of frame or document loader from the
performance object, it's not guaranteed that processing start would be
always earlier than onload.

Reuse existing code to refactor the buffering-before-onload logic.

Bug:  847496 
Change-Id: If3fae54e90747572956610bedadd7963463a5dcc
Reviewed-on: https://chromium-review.googlesource.com/1080131
Commit-Queue: Liquan (Max) Gǔ <maxlg@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564736}
[modify] https://crrev.com/ddcf12998995d87b6fed15c881ac79dc57ff9e43/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-bufferbeforeonload.html
[modify] https://crrev.com/ddcf12998995d87b6fed15c881ac79dc57ff9e43/third_party/blink/renderer/core/timing/window_performance.cc

Comment 6 by npm@chromium.org, Jun 8 2018

Is this fixed?

Comment 7 by maxlg@chromium.org, Jun 8 2018

According to https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&showExpectations=true&tests=event-timing-bufferbeforeonload.html, it doesn't seem flaky anymore.

There are a few red boxes out there but not due to this test.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 9 2018

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

commit fbfb5b6a2e9b873c896e7190564bfef1713d033d
Author: Liquan(Max) Gu <maxlg@chromium.org>
Date: Sat Jun 09 04:29:16 2018

[EventTiming] Remove flakiness expectation

The layout test event-timing-bufferbeforeonload.html is not flaky any
more. Removing the flakiness expectation from TestExpectations.

Bug:  847496 
Change-Id: Ia6d819881d69dc8d3ab0d5886a7c5271f12ae418
Reviewed-on: https://chromium-review.googlesource.com/1093778
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Commit-Queue: Liquan (Max) Gǔ <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565857}
[modify] https://crrev.com/fbfb5b6a2e9b873c896e7190564bfef1713d033d/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 22 2018

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1afe3381c165b43d2dab958058318e04d5701414

commit 1afe3381c165b43d2dab958058318e04d5701414
Author: Liquan(Max) Gu <maxlg@chromium.org>
Date: Fri Jun 22 15:29:29 2018

[EventTiming] Add error messages for layout tests

event-timing-bufferbeforeonload.html is observed to be flaky, but
the original test does not have enough information to capture the
flaky line. This patch is to add error messages for every asserts
in the related tests.

The flaky test will be marked as flaky to unblock the CQ, while
gathering more information before deciding on the fix.

Bug:  847496 
Change-Id: I12b10cc2ef8776e5e3081e613aa2fb7826347d73
Reviewed-on: https://chromium-review.googlesource.com/1076315
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Luna Lu <loonybear@chromium.org>
Commit-Queue: Liquan (Max) Gǔ <maxlg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#562495}(cherry picked from commit 66d69d2841126c91f2bcf9660163e450fe217109)
Reviewed-on: https://chromium-review.googlesource.com/1112157
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#485}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/1afe3381c165b43d2dab958058318e04d5701414/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/1afe3381c165b43d2dab958058318e04d5701414/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-bufferbeforeonload.html
[modify] https://crrev.com/1afe3381c165b43d2dab958058318e04d5701414/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-crossiframe.html
[modify] https://crrev.com/1afe3381c165b43d2dab958058318e04d5701414/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-observethenonload.html
[modify] https://crrev.com/1afe3381c165b43d2dab958058318e04d5701414/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-onloadthenobserve.html
[modify] https://crrev.com/1afe3381c165b43d2dab958058318e04d5701414/third_party/WebKit/LayoutTests/performance/event-timing/resources/event-timing-support.js

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 22 2018

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

commit 90db034f3ecd784e6a3bffaa67a22650010edd5c
Author: Liquan(Max) Gu <maxlg@chromium.org>
Date: Fri Jun 22 15:30:38 2018

[EventTiming] Remove the check of processing start being before onload

Due to the possible absence of frame or document loader from the
performance object, it's not guaranteed that processing start would be
always earlier than onload.

Reuse existing code to refactor the buffering-before-onload logic.

Bug:  847496 
Change-Id: If3fae54e90747572956610bedadd7963463a5dcc
Reviewed-on: https://chromium-review.googlesource.com/1080131
Commit-Queue: Liquan (Max) Gǔ <maxlg@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564736}(cherry picked from commit ddcf12998995d87b6fed15c881ac79dc57ff9e43)
Reviewed-on: https://chromium-review.googlesource.com/1112158
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#486}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/90db034f3ecd784e6a3bffaa67a22650010edd5c/third_party/WebKit/LayoutTests/performance/event-timing/event-timing-bufferbeforeonload.html
[modify] https://crrev.com/90db034f3ecd784e6a3bffaa67a22650010edd5c/third_party/blink/renderer/core/timing/window_performance.cc

Blockedon: 888542
Status: Started (was: Assigned)
I intend to check flakiness via https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=event-timing-bufferbeforeonload.html, but the flakiness dashboard fails to load the   test.

Sign in to add a comment