WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
37003
timeupdate events stop firing after suspend event fires
https://bugs.webkit.org/show_bug.cgi?id=37003
Summary
timeupdate events stop firing after suspend event fires
Andrew Scherkus
Reported
2010-04-01 19:01:17 PDT
When an <audio> or <video> element returns to NETWORK_IDLE from NETWORK_LOADING (i.e., the user agent has intentionally paused downloading the media), we stop the progress event timer (good!) but also stop the playback event timer (bad!). This has the unfortunate consequence of stopping timeupdate events even the video is still playing and presumably has enough of an internal to keep playing until the user agent resumes downloading and goes back to NETWORK_LOADING.
Attachments
2010-04-01 Andrew Scherkus <scherkus@chromium.org>
(6.62 KB, patch)
2010-04-01 19:08 PDT
,
Andrew Scherkus
no flags
Details
Formatted Diff
Diff
2010-04-02 Andrew Scherkus <scherkus@chromium.org>
(7.59 KB, patch)
2010-04-02 00:10 PDT
,
Andrew Scherkus
no flags
Details
Formatted Diff
Diff
2010-04-02 Andrew Scherkus <scherkus@chromium.org>
(6.08 KB, patch)
2010-04-02 00:16 PDT
,
Andrew Scherkus
no flags
Details
Formatted Diff
Diff
2010-04-02 Andrew Scherkus <scherkus@chromium.org>
(6.36 KB, patch)
2010-04-02 11:58 PDT
,
Andrew Scherkus
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Andrew Scherkus
Comment 1
2010-04-01 19:08:39 PDT
Created
attachment 52367
[details]
2010-04-01 Andrew Scherkus <
scherkus@chromium.org
>
Eric Seidel (no email)
Comment 2
2010-04-01 23:47:47 PDT
Looks like you're using good old post-commits. :) You might want to try out "webkit-patch upload" it will do slightly smarter things with patch names at least.
Eric Seidel (no email)
Comment 3
2010-04-01 23:48:42 PDT
Comment on
attachment 52367
[details]
2010-04-01 Andrew Scherkus <
scherkus@chromium.org
> 63 # WebKit doesn't fire suspend events. Is a confusing comment in all the Skipped tests. You shoudl link to this bug or mention the platform name, or indicate if hte platofrm plans to fix that. :)
Andrew Scherkus
Comment 4
2010-04-02 00:10:13 PDT
Created
attachment 52387
[details]
2010-04-02 Andrew Scherkus <
scherkus@chromium.org
>
Andrew Scherkus
Comment 5
2010-04-02 00:12:54 PDT
Does webkit-patch upload work with git? It seems I need to have the changes uncomitted, which scares me a little :s
Eric Seidel (no email)
Comment 6
2010-04-02 00:12:55 PDT
Comment on
attachment 52387
[details]
2010-04-02 Andrew Scherkus <
scherkus@chromium.org
> Skipping in "mac/" will skip on all mac platforms. no need to skip for Leopard and Tiger individually.
Eric Seidel (no email)
Comment 7
2010-04-02 00:14:19 PDT
ojan is your man. He's making webkit-patch upload better. Just wait another day and it will be more git-friendly.
Andrew Scherkus
Comment 8
2010-04-02 00:16:41 PDT
Created
attachment 52391
[details]
2010-04-02 Andrew Scherkus <
scherkus@chromium.org
>
Andrew Scherkus
Comment 9
2010-04-02 00:18:26 PDT
(In reply to
comment #6
)
> (From update of
attachment 52387
[details]
) > Skipping in "mac/" will skip on all mac platforms. no need to skip for Leopard > and Tiger individually.
Perfect! I thought this might be the case, but not having access to a Tiger machine I wasn't really able to test out the theory. I noticed there were some duplicated Skipped entries between mac platforms, so I assumed that it might be per-platform. PS ojan if you're reading this... thank you :)
Eric Carlson
Comment 10
2010-04-02 09:26:49 PDT
Comment on
attachment 52391
[details]
2010-04-02 Andrew Scherkus <
scherkus@chromium.org
>
> +++ b/LayoutTests/http/tests/media/video-play-suspend.html > @@ -0,0 +1,26 @@ > +<video></video> > +<p>Test that timeupdate events are sent when media loading suspends itself.</p> > +<script src=../../../media/media-file.js></script> > +<script src=../../../media/video-test.js></script> > +<script> > + var suspendCount = 0; > + > + // Make sure we've at least reached NETWORK_LOADING before waiting for suspend. > + waitForEvent('loadstart', function() { > + // suspend event means the user agent has intentionally paused network usage, > + // however we should still receive timeupdate events. > + mediaElement.addEventListener('suspend', function () { > + // timeupdate events are fired as playback progresses so only verify that at least one > + // event is fired > + ++suspendCount; > + if (suspendCount == 1) { > + consoleWrite("EVENT(suspend)"); > + waitForEventAndEnd('timeupdate'); > + } > + } ); > + } ); > + > + var mediaFile = findMediaFile("video", "resources/test"); > + video.src = "
http://127.0.0.1:8000/media/
" + mediaFile; > + run("video.play()"); > +</script>
I really prefer to see tests as whole html documents rather than fragments - include <html> and <body>, put the <script> in a function in <head>, trigger the script on 'load', etc. I think it makes the tests *much* easier to read and understand for someone else down the line.
> diff --git a/LayoutTests/platform/gtk/Skipped b/LayoutTests/platform/gtk/Skipped > index d086452..4c4bd1f 100644 > --- a/LayoutTests/platform/gtk/Skipped > +++ b/LayoutTests/platform/gtk/Skipped > @@ -3424,6 +3424,7 @@ http/tests/local/send-sliced-dragged-file.html > http/tests/local/send-form-data.html > http/tests/media/video-play-stall-seek.html > http/tests/media/video-play-stall.html > +http/tests/media/video-play-suspend.html > http/tests/media/video-seekable-stall.html > http/tests/mime/standard-mode-loads-stylesheet-with-empty-content-type.html > http/tests/mime/standard-mode-loads-stylesheet-with-text-css-and-invalid-type.html
GTK port doesn't get a comment about why the test is skipped?
> diff --git a/LayoutTests/platform/mac/Skipped b/LayoutTests/platform/mac/Skipped > index 7a6a7f7..1eb9fc1 100644 > --- a/LayoutTests/platform/mac/Skipped > +++ b/LayoutTests/platform/mac/Skipped > @@ -60,6 +60,10 @@ svg/hixie/perf/002.xml > # <
rdar://problem/6710625
> Test media/video-error-abort.html doesn't work > http/tests/media/video-error-abort.html > > +# Mac uses QuickTime, which currently doesn't fire a suspend event for audio/video elements. > +#
https://bugs.webkit.org/show_bug.cgi?id=37003
> +http/tests/media/video-play-suspend.html > +
A bugzilla url in a skipped file should refer to the bug that must be fixed in order to un-skip the test. Please write bugs for each of the ports that don't fire 'suspend' and reference them in the skipped files, or omit the url. Are you certain that the state is not set to 'idle' because of QuickTime, or is it a bug in the WebCore code that uses QuickTime?
> +# Windows uses QuickTime, which currently doesn't fire a suspend event for audio/video elements. > +#
https://bugs.webkit.org/show_bug.cgi?id=37003
> +http/tests/media/video-play-suspend.html > +
Ditto. r=me with these minor changes.
Andrew Scherkus
Comment 11
2010-04-02 11:58:37 PDT
Created
attachment 52434
[details]
2010-04-02 Andrew Scherkus <
scherkus@chromium.org
>
Andrew Scherkus
Comment 12
2010-04-02 11:59:45 PDT
Thanks for the review Eric! I've created the following bugs tracking the NETWORK_IDLE/suspend work:
https://bugs.webkit.org/show_bug.cgi?id=37034
https://bugs.webkit.org/show_bug.cgi?id=37035
https://bugs.webkit.org/show_bug.cgi?id=37036
Andrew Scherkus
Comment 13
2010-04-02 14:09:26 PDT
Whoops I uploaded and clobbered the review+ Having addressed Eric's concerns I'll go ahead and land the patch.
Andrew Scherkus
Comment 14
2010-04-02 14:22:59 PDT
Committed as
http://trac.webkit.org/changeset/57023
Eric Seidel (no email)
Comment 15
2010-04-06 20:54:30 PDT
Comment on
attachment 52434
[details]
2010-04-02 Andrew Scherkus <
scherkus@chromium.org
> Clearing r? since this bug is closed.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug