1123189 - Queue "durationchange" instead of dispatching synchronously

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P2)

Description

[Karl Tomlinson (:karlt)]

Queuing durationchange from MediaDecoder::DurationChanged() is particularly
important for MediaSourceDecoder, which calls the function synchronously from
content-exposed methods, such as during the MediaSource duration setter for
example, but is also required for all media elements:

http://www.w3.org/TR/2014/REC-html5-20141028/embedded-content-0.html#offsets-into-the-media-resource

"When the length of the media resource changes to a known value (e.g. from being unknown to known, or from a previously established length to a new length) the user agent must queue a task to fire a simple event named durationchange at the media element."

This would seem consistent with efforts to queue timeupdate, abort, and
emptied events.

https://hg.mozilla.org/mozilla-central/rev/a6d06ecdfde1
https://hg.mozilla.org/mozilla-central/rev/e5ae3b72e03c

Comment 1

[Karl Tomlinson (:karlt)]

https://tbpl.mozilla.org/?tree=Try&rev=d5636b1bc4d7

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Queue "durationchange" instead of dispatching synchronously

Queue durationchange event instead of firing it simultaneously

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 8551080 [details] [diff] [review]
Queue "durationchange" instead of dispatching synchronously

Yes, this is a subset of my patch in comment 1 [1].
As this affects non-MSE media elements, I'd still like to ask cpearce to look at it, in case he is aware of any unexpected side-effects.
I think we can also keep the removal of the now unused MediaDecoderOwner::DispatchEvent().

[1] https://hg.mozilla.org/try/rev/cf705732766f

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: Queue "durationchange" instead of dispatching synchronously

https://tbpl.mozilla.org/?tree=Try&rev=e15803754990

Comment 5

[Jean-Yves Avenard [:jya]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=31d8bdcacc1a

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: Fix mochitest

Fix mochitest test_DurationChanged.html

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Fix mochitest

Fix mochitest test_DurationChanged.html. Patch wasn't properly refreshed.

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Fix mochitest

sorry for that, queue being messed up

Comment 9

[Karl Tomlinson (:karlt)]

Comment on attachment 8551548 [details] [diff] [review]
Fix mochitest

Yes, looking at http://www.w3.org/TR/2014/CR-media-source-20140717/#h4_sourcebuffer-coded-frame-processing duration should be queued after loadeddata.

Comment 10

[Jean-Yves Avenard [:jya]]

Attachment: Fix mochitest

As per IRC discussion, using sourceended event trigger, as we don't have to wait for the video to play

Comment 11

[Jean-Yves Avenard [:jya]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=49070b2cbebb

looks good so far

Comment 12

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bddb69ed8cae
https://hg.mozilla.org/integration/mozilla-inbound/rev/077140ba65d0

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bddb69ed8cae
https://hg.mozilla.org/mozilla-central/rev/077140ba65d0

Comment 14

[Ralph Giles (:rillian)]

Comment on attachment 8551502 [details] [diff] [review]
Queue "durationchange" instead of dispatching synchronously

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing. Sites more likely to serve flash video.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: Change is straightforward but affects all media playback. This improves our spec compliance, but is a minor behaviour change from before.
[String/UUID change made/needed]: None.

Comment 15

[Ralph Giles (:rillian)]

Approval request is for both patches. We need the test update as well.

Comment 16

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-beta/rev/09df37258699
https://hg.mozilla.org/releases/mozilla-beta/rev/677c75e4d519

Comment 17

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c32a06cb18a5
https://hg.mozilla.org/releases/mozilla-aurora/rev/044f52a1b4e0