Bug 33900 - media/video-play-pause-exception.html is flaky (on GTK at least)
Summary: media/video-play-pause-exception.html is flaky (on GTK at least)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-20 07:10 PST by Philippe Normand
Modified: 2010-01-20 09:31 PST (History)
1 user (show)

See Also:


Attachments
Refactored the test to use the pause event instead of a timer. (2.54 KB, patch)
2010-01-20 07:12 PST, Philippe Normand
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2010-01-20 07:10:32 PST
In that test we try to play an empty <video> element. A timer is started after pause() has been called. I think it would be better to wait the pause event instead.
Comment 1 Philippe Normand 2010-01-20 07:12:46 PST
Created attachment 47024 [details]
Refactored the test to use the pause event instead of a timer.
Comment 2 Darin Adler 2010-01-20 08:29:44 PST
Comment on attachment 47024 [details]
Refactored the test to use the pause event instead of a timer.

> +<html>
> +<body>

This seems like a pointless change to me; including explicit HTML and body elements does not make our tests better. I assume it has no effect on the test.

> +    waitForEvent("pause", onpause);

It seems to me we should call waitForEvent("pause") before calling video.play() and video.pause().

I would not have preserved the blank line at the end of the test created with consoleWrite.

I would probably have used waitForEventTestAndEnd.

Seems OK as is. r=me
Comment 3 Philippe Normand 2010-01-20 08:49:37 PST
Oh I didn't know about waitForEventTestAndEnd. Perfect fit for this test indeed ;)
Comment 4 Philippe Normand 2010-01-20 09:31:38 PST
Landed as r53550. Thanks for the review Darin!