Bug 55414 – Fix race condition in 4 media tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Gentilcore]

Fix race condition in 4 media tests

[Tony Gentilcore]

Created attachment 84134 [details]
Patch

[Darin Adler]

Comment on attachment 84134 [details]
Patch

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

Looks OK as long as the waitForEvent function is only ever used on events that are unique enough.

> LayoutTests/media/video-test.js:3
> -var mediaElement = null;
> +var mediaElement = document;

This is pretty strange; expedient but pretty unclear! Probably needs a comment.

[Tony Gentilcore]

(In reply to comment #2)
> (From update of attachment 84134 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84134&action=review
> 
> Looks OK as long as the waitForEvent function is only ever used on events that are unique enough.

They are all unique to media elements except for 'error.' I don't think this is a big problem as other errors on the page should be unexpected. But if you think it is a good idea, I could also check the target of the event and only dispatch the callback if it is a media element.

> 
> > LayoutTests/media/video-test.js:3
> > -var mediaElement = null;
> > +var mediaElement = document;
> 
> This is pretty strange; expedient but pretty unclear! Probably needs a comment.

It ends up being strange because of these special "video" and "mediaElement" global variables. Would be much more clear if they were arguments to the method, but that is more refactoring that I want to do here. Will add a comment.

[Tony Gentilcore]

Created attachment 84136 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 84136 [details]
Patch for landing

Clearing flags on attachment: 84136

Committed r79949: <http://trac.webkit.org/changeset/79949>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/79949 might have broken SnowLeopard Intel Release (Build)