Bug 182370

Summary: Fix race-condition in fast/forms/ios/ipad/select-form-run-twice.html
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: lforschler, megan_gardner, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=182062
Attachments:
Description Flags
Patch
none
Patch thorton: review+

Description Jonathan Bedard 2018-01-31 18:24:58 PST
fast/forms/ios/ipad/select-form-run-twice.html should wait for it's promise to be fulfilled before finishing the test. If it does not, the subsequent test may crash.
Comment 1 Jonathan Bedard 2018-01-31 18:34:03 PST
Created attachment 332830 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2018-01-31 18:34:21 PST
<rdar://problem/37098169>
Comment 3 Jonathan Bedard 2018-02-01 08:10:39 PST
Comment on attachment 332830 [details]
Patch

We need to go through and audit layout tests for outstanding promises when the test ends. I will not have time to do this thoroughly in the next few weeks, before I do the audit, I've asked the bot watchers to send me these sorts of test failures as they are found. This is another one of them.
Comment 4 Wenson Hsieh 2018-02-01 09:04:36 PST
Comment on attachment 332830 [details]
Patch

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

> LayoutTests/fast/forms/ios/ipad/select-form-run-twice.html:37
> +            Promise.all(promises).then(function() {

`promises` is empty when we Promise.all here, so wouldn't this just resolve on the next runloop?
Comment 5 Jonathan Bedard 2018-02-01 09:10:26 PST
Comment on attachment 332830 [details]
Patch

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

>> LayoutTests/fast/forms/ios/ipad/select-form-run-twice.html:37
>> +            Promise.all(promises).then(function() {
> 
> `promises` is empty when we Promise.all here, so wouldn't this just resolve on the next runloop?

For some reason I had thought that the other js functions had run already, but you are correct. I'll re-work this change.
Comment 6 Megan Gardner 2018-02-01 16:46:58 PST
Created attachment 332924 [details]
Patch
Comment 7 Tim Horton 2018-02-01 17:30:08 PST
Comment on attachment 332924 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        Multiple potential button clicked, due to looping function calls can cause threads to be running in the next 

I think you grammared here.

Threads?
Comment 8 Megan Gardner 2018-02-01 18:14:09 PST
https://trac.webkit.org/r227992