New issue
Advanced search Search tips

Issue 845155 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: MultiColumnFragmentainerGroupTest.LotsOfContent



Sign in to add a comment

MultiColumnFragmentainerGroupTest.LotsOfContent is Flaky

Project Member Reported by Findit, May 21 2018

Issue description

Findit has detected a flake at test MultiColumnFragmentainerGroupTest.LotsOfContent.

Culprit (100.0% confidence): https://chromium-review.googlesource.com/q/Ia45d52b23f1246331d4b193839d34d7d9527151b
Regression range: None

Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVyrwELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJ5Y2hyb21pdW0ud2luL1dpbjcgVGVzdHMgKGRiZykoMSkvNjkxNTAvd2Via2l0X3VuaXRfdGVzdHMvVFhWc2RHbERiMngxYlc1R2NtRm5iV1Z1ZEdGcGJtVnlSM0p2ZFhCVVpYTjBMa3h2ZEhOUFprTnZiblJsYm5RPQwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAEM

If this result was incorrect, apply the label Test-Findit-Wrong, mark the bug as Untriaged and the component Tools>Test>Findit>Flakiness.
 
Project Member

Comment 1 by Findit, May 21 2018

Findit identified the culprit r560088 with confidence 100.0% in the config "chromium.win / Win7 Tests (dbg)(1)"
based on the flakiness trend:

https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVyrwELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJ5Y2hyb21pdW0ud2luL1dpbjcgVGVzdHMgKGRiZykoMSkvNjkxNTAvd2Via2l0X3VuaXRfdGVzdHMvVFhWc2RHbERiMngxYlc1R2NtRm5iV1Z1ZEdGcGJtVnlSM0p2ZFhCVVpYTjBMa3h2ZEhOUFprTnZiblJsYm5RPQwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAEM


Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
Feedback is welcome! Please use component Tools>Test>FindIt>Flakiness
Owner: mstensho@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, May 22 2018

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

commit 215f05199a9205ed75f728ff6343e895c05b4025
Author: calamity <calamity@chromium.org>
Date: Tue May 22 06:11:13 2018

Revert "Improve used column count allowance calculation."

This reverts commit 1c7424a87fff7f5c4351719597eae8051e96fba3.

Reason for revert: Causing flakes on MultiColumnFragmentainerGroupTest.LotsOfContent.
See  https://crbug.com/845155 

Original change's description:
> Improve used column count allowance calculation.
> 
> Better detection of "legitimate" reasons to use many columns. Let the
> allowance be based on the amount of "content". The more content (lines
> or boxes) inside a multicol container, the more columns will be allowed.
> We used to base it on the column height, but it turned out that it
> wasn't good enough. Also, there are legitimate reasons to use more than
> 500 columns in some cases, so increase the limit to 2000. With the
> current implementation, this may get very slow, because of quadratic
> performance complexity, presumably somewhere in paint code.
> 
> One multicol regressed in LayoutNG. Not sure why. It shouldn't have
> passed in the first place.
> 
> Bug:  808189 
> Change-Id: Ia45d52b23f1246331d4b193839d34d7d9527151b
> Reviewed-on: https://chromium-review.googlesource.com/1057629
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#560088}

TBR=chrishtr@chromium.org,eae@chromium.org,mstensho@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  808189 ,  845155 
Change-Id: I28bf3894c4b65a75c7c533a362ca58f3c7e29973
Reviewed-on: https://chromium-review.googlesource.com/1068587
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560496}
[modify] https://crrev.com/215f05199a9205ed75f728ff6343e895c05b4025/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/215f05199a9205ed75f728ff6343e895c05b4025/third_party/WebKit/LayoutTests/fast/multicol/scrollable-basic.html
[modify] https://crrev.com/215f05199a9205ed75f728ff6343e895c05b4025/third_party/blink/renderer/core/layout/layout_multi_column_flow_thread.cc
[modify] https://crrev.com/215f05199a9205ed75f728ff6343e895c05b4025/third_party/blink/renderer/core/layout/layout_multi_column_flow_thread.h
[modify] https://crrev.com/215f05199a9205ed75f728ff6343e895c05b4025/third_party/blink/renderer/core/layout/multi_column_fragmentainer_group.cc
[modify] https://crrev.com/215f05199a9205ed75f728ff6343e895c05b4025/third_party/blink/renderer/core/layout/multi_column_fragmentainer_group.h
[modify] https://crrev.com/215f05199a9205ed75f728ff6343e895c05b4025/third_party/blink/renderer/core/layout/multi_column_fragmentainer_group_test.cc

Labels: -Sheriff-Chromium
Project Member

Comment 5 by bugdroid1@chromium.org, May 22 2018

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

commit d123c14a79589cb40efc367a178213a0d246f06a
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue May 22 22:47:14 2018

Reland: Improve used column count allowance calculation.

Better detection of "legitimate" reasons to use many columns. Let the
allowance be based on the amount of "content". The more content (lines
or boxes) inside a multicol container, the more columns will be allowed.
We used to base it on the column height, but it turned out that it
wasn't good enough. Also, there are legitimate reasons to use more than
500 columns in some cases, so increase the limit to 2000. With the
current implementation, this may get very slow, because of quadratic
performance complexity, presumably somewhere in paint code.

One multicol regressed in LayoutNG. Not sure why. It shouldn't have
passed in the first place.

This CL was previously landed as
https://chromium-review.googlesource.com/c/chromium/src/+/1057629 and
then reverted because of test flakiness. This new CL has reduced the
number of columns in those tests, so that the tests don't occasionally
time out on slow bots.

Bug:  808189 , 845155 
Change-Id: I55d560faba73128518a689ae38f628ee89e58568
Reviewed-on: https://chromium-review.googlesource.com/1068929
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560820}
[modify] https://crrev.com/d123c14a79589cb40efc367a178213a0d246f06a/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/d123c14a79589cb40efc367a178213a0d246f06a/third_party/WebKit/LayoutTests/fast/multicol/scrollable-basic.html
[modify] https://crrev.com/d123c14a79589cb40efc367a178213a0d246f06a/third_party/blink/renderer/core/layout/layout_multi_column_flow_thread.cc
[modify] https://crrev.com/d123c14a79589cb40efc367a178213a0d246f06a/third_party/blink/renderer/core/layout/layout_multi_column_flow_thread.h
[modify] https://crrev.com/d123c14a79589cb40efc367a178213a0d246f06a/third_party/blink/renderer/core/layout/multi_column_fragmentainer_group.cc
[modify] https://crrev.com/d123c14a79589cb40efc367a178213a0d246f06a/third_party/blink/renderer/core/layout/multi_column_fragmentainer_group.h
[modify] https://crrev.com/d123c14a79589cb40efc367a178213a0d246f06a/third_party/blink/renderer/core/layout/multi_column_fragmentainer_group_test.cc

Status: Fixed (was: Assigned)
New attempt, with friendlier tests. Should be no flakiness now, unless the bot is a 386SX. :)
Cc: calamity@chromium.org mstensho@chromium.org
 Issue 845022  has been merged into this issue.

Sign in to add a comment