Bug 179195

Summary: LayoutTest http/tests/security/cross-frame-access-put.html is a flaky failure
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ap, cdumez, commit-queue, darin, jbedard, koivisto, rniwa, sam, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Comment 1 Alexey Proskuryakov 2017-11-06 17:58:01 PST
It looks like the test pretty much works properly, but dumps the results as a render tree instead of text!

Very curious, I wonder if this reproduces locally.
Comment 2 Ryan Haddad 2017-11-07 08:40:27 PST
I can reproduce locally with: 
run-webkit-tests http/tests/security/cross-frame-access-put.html -fg --iter 10 --no-retry-failure
Comment 3 Chris Dumez 2017-11-07 09:54:36 PST
Created attachment 326218 [details]
Patch
Comment 4 Chris Dumez 2017-11-07 09:55:38 PST
(In reply to Ryan Haddad from comment #2)
> I can reproduce locally with: 
> run-webkit-tests http/tests/security/cross-frame-access-put.html -fg --iter
> 10 --no-retry-failure

Thanks for the repro steps Ryan, they worked.
Comment 5 Alexey Proskuryakov 2017-11-07 11:15:42 PST
Comment on attachment 326218 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326218&action=review

> LayoutTests/ChangeLog:9
> +        Fix flaky test by calling the testRunner functions as early as possible, not in
> +        the onload event handler.

Why does this matter?
Comment 6 WebKit Commit Bot 2017-11-07 11:29:35 PST
Comment on attachment 326218 [details]
Patch

Clearing flags on attachment: 326218

Committed r224538: <https://trac.webkit.org/changeset/224538>
Comment 7 WebKit Commit Bot 2017-11-07 11:29:36 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Chris Dumez 2017-11-07 11:58:11 PST
Comment on attachment 326218 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326218&action=review

>> LayoutTests/ChangeLog:9
>> +        the onload event handler.
> 
> Why does this matter?

Because the test can start running in the subframe *before* the onload event handler gets called in the top frame.
Comment 9 Alexey Proskuryakov 2017-11-07 13:58:23 PST
> Because the test can start running in the subframe *before* the onload event handler gets called in the top frame.

It definitely can, but how does this translate into the symptom? The test should still switch to text mode before any of the content is dumped in the end.
Comment 10 Chris Dumez 2017-11-07 14:04:05 PST
(In reply to Alexey Proskuryakov from comment #9)
> > Because the test can start running in the subframe *before* the onload event handler gets called in the top frame.
> 
> It definitely can, but how does this translate into the symptom? The test
> should still switch to text mode before any of the content is dumped in the
> end.

The test starts running in the subframe, then the subframes starts the test in the topFrame via a postMessage(). As far as I can tell, nothing prevented the test in the top frame to finish *before* the onload event handler was called in the main frame. Therefore, the test *could* finish *before* calling testRunner.dumpAsText() is called.
Comment 11 Alexey Proskuryakov 2017-11-07 14:25:41 PST
Any test that doesn't call waitUntilDone() finishes after onload is dispatched, so I still don't see how anything could go wrong.

I suspect that there is a deeper root cause, something with incorrect sequence of load event and API callbacks.
Comment 12 Radar WebKit Bug Importer 2017-11-15 12:26:25 PST
<rdar://problem/35567537>