Bug 216394 – REGRESSION (r266634): fast/selectors/text-field-selection-stroke-color.html and fast/selectors/text-field-selection-window-inactive-stroke-color.html are flaky failures with pixel noise

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Hector Lopez]

fast/selectors/text-field-selection-stroke-color.html

Test is a flaky failure according to history on macOS wk2 Release. First occurrence of failure is at r266388 but could not determine change was cause.

History:
https://results.webkit.org/?suite=layout-tests&test=fast%2Fselectors%2Ftext-field-selection-stroke-color.html&limit=50000&platform=mac

Diff:
https://build.webkit.org/results/Apple-Catalina-Release-WK2-Tests/r266899%20(8458)/fast/selectors/text-field-selection-stroke-color-diffs.html

or see attachments

8 pixel difference

[Hector Lopez]

Created attachment 408504 [details]
Expected Image

[Hector Lopez]

Created attachment 408505 [details]
Actual Image

[Hector Lopez]

Created attachment 408506 [details]
Diff Image

[Hector Lopez]

Test expectation while investigated:

https://trac.webkit.org/changeset/266905/webkit

[Radar WebKit Bug Importer]

<rdar://problem/68679551>

[Alexey Proskuryakov]

There were failures with r266388 and r266392, then a string of success, then it started again with r266662. 

This makes me suspect bug 216012.

Internal bots tell a similar story, although there were some failures on macOS Mojave in early August too. Bug 216012 may not be the root cause, but it looks like it exacerbated the problem.

[Alexey Proskuryakov]

This is very easy to reproduce, and I confirmed that this is a regression from r266634.

run-webkit-tests fast/selectors/text-field-selection-stroke-color.html --repeat 100 --child-processes=1 --no-retry

This affects more tests that this one. I only checked fast/selectors, and found these:

fast/selectors/selection-window-inactive-stroke-color.html
fast/selectors/selection-window-inactive.html
fast/selectors/text-field-selection-stroke-color.html
fast/selectors/text-field-selection-window-inactive-stroke-color.html

[Alexey Proskuryakov]

That test was with r266634 archive. Follow-ups made a difference, it's only two tests on trunk:

fast/selectors/text-field-selection-stroke-color.html
fast/selectors/text-field-selection-window-inactive-stroke-color.html

[Hector Lopez]

Correction for test expectation:

https://trac.webkit.org/changeset/267012/webkit

[Sihui Liu]

(In reply to Alexey Proskuryakov from comment #8)
> That test was with r266634 archive. Follow-ups made a difference, it's only
> two tests on trunk:
> 
> fast/selectors/text-field-selection-stroke-color.html
> fast/selectors/text-field-selection-window-inactive-stroke-color.html

What config/revision did you use for reproducing the failure?
I used your command on both release and debug build with r267017 and cannot reproduce: 
run-webkit-tests fast/selectors/text-field-selection-stroke-color.html --repeat 100 --child-processes=1 --no-retry

Are you on macOS Catalina?

[Sihui Liu]

(In reply to Sihui Liu from comment #10)
> (In reply to Alexey Proskuryakov from comment #8)
> > That test was with r266634 archive. Follow-ups made a difference, it's only
> > two tests on trunk:
> > 
> > fast/selectors/text-field-selection-stroke-color.html
> > fast/selectors/text-field-selection-window-inactive-stroke-color.html
> 
> What config/revision did you use for reproducing the failure?
> I used your command on both release and debug build with r267017 and cannot
> reproduce: 
> run-webkit-tests fast/selectors/text-field-selection-stroke-color.html
> --repeat 100 --child-processes=1 --no-retry
> 
> Are you on macOS Catalina?

Never mind. I didn't update the test expectations before running. Now I can reproduce.

[Hector Lopez]

fast/selectors/selection-window-inactive-stroke-color.html

Test exhibits the same behavior. Adding to bug and changing test expectations

test expectation for fast/selectors/selection-window-inactive-stroke-color.html:

https://trac.webkit.org/changeset/267178/webkit

[Sihui Liu]

Created attachment 409141 [details]
Patch

[Alexey Proskuryakov]

Impressive analysis!

Makes me wonder if it would make sense for layout tests to dump tile size, so that we could catch such issues faster and more directly than through pixel noise.

[EWS]

Committed r267284: <https://trac.webkit.org/changeset/267284>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409141 [details].

[Sihui Liu]

(In reply to Alexey Proskuryakov from comment #14)
> Impressive analysis!
> 
> Makes me wonder if it would make sense for layout tests to dump tile size,
> so that we could catch such issues faster and more directly than through
> pixel noise.

Yes, that may be useful! This kind of small flaky pixel difference could be hard to reason about. If we know the noise comes with with different tiling, at least we have some hint about what may go wrong. (It took me quite a while before noticing more PlatformCALayers were created when test failed :|)