Bug 198195

Summary: REGRESSION (r244353) [Mac WK2] Layout Test fast/css/sticky/sticky-left-percentage.html is a flaky ImageOnlyFailure
Product: WebKit Reporter: Shawn Roberts <sroberts>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: NEW ---    
Severity: Normal CC: drousso, ews, jbedard, jlewis3, rniwa, ryanhaddad, simon.fraser, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=198200
Attachments:
Description Flags
Patch
ews: commit-queue-
Archive of layout-test-results from ews102 for mac-highsierra
none
Archive of layout-test-results from ews215 for win-future
none
Archive of layout-test-results from ews116 for mac-highsierra
none
Archive of layout-test-results from ews107 for mac-highsierra-wk2 none

Description Shawn Roberts 2019-05-23 14:08:50 PDT
The following layout test is flaky on Mac Release WK2

fast/css/sticky/sticky-left-percentage.html

Probable cause:

Test wad modified in https://trac.webkit.org/changeset/244353/webkit and started to become a flaky failure

Reproduced locally with:

run-webkit-tests fast/css/sticky/sticky-left-percentage.html --iter 100 -f --exit-after-n-failures=2

Usually hits 2 failures within 10 iterations. 

stdio shows:

Error: test and reference images have different sizes. Test image is 800x700, reference image is 800x600
ImageDiff crashed

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fcss%2Fsticky%2Fsticky-left-percentage.html
Comment 1 Radar WebKit Bug Importer 2019-05-23 14:09:26 PDT
<rdar://problem/51081794>
Comment 2 Shawn Roberts 2019-05-23 14:22:54 PDT
Updated expectations in https://trac.webkit.org/changeset/245718/webkit while waiting for a fix.
Comment 3 Jonathan Bedard 2019-05-30 09:33:32 PDT
Created attachment 370947 [details]
Patch
Comment 4 Jonathan Bedard 2019-05-30 09:38:00 PDT
I'd like to find a better approach than waiting, but I'm not really sure what that would be.

It seems that this is only a problem in the reference (I've never seen the actual test hit the race condition).

A list of approaches I've tried:
- UIScriptController, but that timed out.
- Listening for resize events on the body, but the race condition was still present.
- Modifying resizeTo in WebCore so that it blocked until the window reported the new size.

Only thing that seems to work is some variation of waiting. Not quite sure what we're waiting for, we do trigger a repaint before snapshotting, so what ever it is, it's not obvious.
Comment 5 Simon Fraser (smfr) 2019-05-30 10:00:29 PDT
Comment on attachment 370947 [details]
Patch

This seems like a bad hack, and people will continue to add new tests that break. I think we need a WTR solution that is reliable.
Comment 6 Build Bot 2019-05-30 10:37:25 PDT
Comment on attachment 370947 [details]
Patch

Attachment 370947 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12329342

New failing tests:
fast/css/sticky/sticky-left-percentage.html
Comment 7 Build Bot 2019-05-30 10:37:27 PDT
Created attachment 370953 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 8 Build Bot 2019-05-30 11:02:02 PDT
Comment on attachment 370947 [details]
Patch

Attachment 370947 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12329395

New failing tests:
fast/css/sticky/sticky-left-percentage.html
Comment 9 Build Bot 2019-05-30 11:02:04 PDT
Created attachment 370956 [details]
Archive of layout-test-results from ews215 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews215  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 10 Build Bot 2019-05-30 11:20:34 PDT
Comment on attachment 370947 [details]
Patch

Attachment 370947 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12329389

New failing tests:
fast/css/sticky/sticky-left-percentage.html
Comment 11 Build Bot 2019-05-30 11:20:36 PDT
Created attachment 370958 [details]
Archive of layout-test-results from ews116 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 12 Jonathan Bedard 2019-05-30 12:03:20 PDT
(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 370947 [details]
> Patch
> 
> This seems like a bad hack, and people will continue to add new tests that
> break. I think we need a WTR solution that is reliable.

What would the WTR solution look like, though?

The only thing I can think of, at this point, would be to provide a sort of WebKitTestRunner resizeTo JS function which is smart enough to the wait itself. But I still think that we will ultimately be stuck with a wait. The other option would be a wait before actually taking the snapshot, but that seems like a bad idea.

Other possible options are that we're triggering the onresize before the actual resize has occurred. But looking at the VisualViewport code, that doesn't look to be the case.  We might be able to change Mac snapshot function so that it lives in WebKitTestRunner (like it does for iOS) instead of using the CG API. But that seems like the sort of change that could break tons of layout tests.

Ultimately, I think that we're either going to end up with something like the proposed change or just futuring this bug. Other than modifications to the test, the fixes I can think of are all bigger problems than the test just being flakey.
Comment 13 Build Bot 2019-05-30 13:29:09 PDT
Comment on attachment 370947 [details]
Patch

Attachment 370947 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12330723

New failing tests:
http/wpt/service-workers/useragent.https.html
Comment 14 Build Bot 2019-05-30 13:29:11 PDT
Created attachment 370974 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 15 Simon Fraser (smfr) 2019-05-30 13:44:56 PDT
fast/css-grid-layout/flex-content-sized-columns-resize.html is another ref tests that does resizeTo.
Comment 16 Simon Fraser (smfr) 2019-05-30 13:49:54 PDT
(In reply to Jonathan Bedard from comment #12)
> (In reply to Simon Fraser (smfr) from comment #5)
> > Comment on attachment 370947 [details]
> > Patch
> > 
> > This seems like a bad hack, and people will continue to add new tests that
> > break. I think we need a WTR solution that is reliable.
> 
> What would the WTR solution look like, though?

UIScriptController::resizeWindow. UIScriptController knows how to wait for async things.