1294358 - Add test for suspended videos still fire 'ended' event

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[u480271]

There was a bug where suspending video decode causes videos not to fire ended event until they became visible. Add a test to ensure the desire behaviour still works.

Comment 1

[u480271]

Attachment: Bug 1294358 - P1: Change test files to specify mime type.

Review commit: https://reviewboard.mozilla.org/r/70868/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70868/

Comment 2

[u480271]

Attachment: Bug 1294358 - P2: Add test for 'ended' event firing for suspended video.

This exercises the decoder update logic path that doesn't rely on
AudioSink to advance time.

Review commit: https://reviewboard.mozilla.org/r/70870/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70870/

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review68252

::: dom/media/test/test_background_video_suspend_ends.html:35
(Diff revision 1)
> +    // This test checks that 'ended' event is received for videos with
> +    // suspended video decoding. This is important for looping video logic
> +    // handling in HTMLMediaElement.
> +    waitUntilPlaying(v)
> +      .then(() => testVideoSuspendsWhenHidden(v))
> +      .then(() => waitUntilEnded(v))

Might be worth checking currentTime approximates the duration when ended.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review68254

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8780032 [details]
Bug 1294358 - P2: Add test for 'ended' event firing for suspended video.

https://reviewboard.mozilla.org/r/70870/#review68256

Comment 6

[u480271]

Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70868/diff/1-2/

Comment 7

[u480271]

Comment on attachment 8780032 [details]
Bug 1294358 - P2: Add test for 'ended' event firing for suspended video.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70870/diff/1-2/

Comment 8

[u480271]

Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70868/diff/2-3/

Comment 9

[u480271]

Comment on attachment 8780032 [details]
Bug 1294358 - P2: Add test for 'ended' event firing for suspended video.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70870/diff/2-3/

Comment 10

[u480271]

Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review68252

> Might be worth checking currentTime approximates the duration when ended.

OK. Will do.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/848141457f25
P1: Add test for 'ended' event firing for suspended video. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/886441076b5f
P2: Added test video with no audio. r=jwwang

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for xp media test failures: https://treeherder.mozilla.org/logviewer.html#?job_id=2057985&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/cc222be2b97b

Comment 13

[u480271]

(In reply to Wes Kocher (:KWierso) from comment #12)
> Backed out for xp media test failures:
> https://treeherder.mozilla.org/logviewer.html#?job_id=2057985&repo=autoland
> 
> https://hg.mozilla.org/integration/autoland/rev/cc222be2b97b

Thanks Wes, I'll take a look.

Comment 14

[u480271]

Attachment: Bug 1294358 - P3: Added test video with no audio.

Review commit: https://reviewboard.mozilla.org/r/73238/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73238/

Comment 15

[u480271]

Attachment: Bug 1294358 - P4: Shrink video size.

Make the test videos smaller so they are visible in the mochitest
iframe.

Review commit: https://reviewboard.mozilla.org/r/73240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73240/

Comment 16

[u480271]

Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70868/diff/3-4/

Comment 17

[u480271]

Comment on attachment 8780032 [details]
Bug 1294358 - P2: Add test for 'ended' event firing for suspended video.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70870/diff/3-4/

Comment 18

[Jean-Yves Avenard [:jya]]

Comment on attachment 8783430 [details]
Bug 1294358 - P4: Shrink video size.

https://reviewboard.mozilla.org/r/73240/#review70998

technically you aren't making them smaller, you just scaling them.

Comment 19

[Jean-Yves Avenard [:jya]]

Comment on attachment 8780031 [details]
Bug 1294358 - P1: Change test files to specify mime type.

https://reviewboard.mozilla.org/r/70868/#review71000

assuming you got them all this time :)

Comment 20

[u480271]

Comment on attachment 8783429 [details]
Bug 1294358 - P3: Added test video with no audio.

r=me, carry r+ from jwwang.

Comment 21

[Jean-Yves Avenard [:jya]]

Comment on attachment 8783429 [details]
Bug 1294358 - P3: Added test video with no audio.

https://reviewboard.mozilla.org/r/73238/#review71252

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15de17e6f1e1
P1: Change test files to specify mime type. r=jwwang,jya
https://hg.mozilla.org/integration/autoland/rev/24f0a2f224c4
P2: Add test for 'ended' event firing for suspended video. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/25b8cda49700
P3: Added test video with no audio. r=jya
https://hg.mozilla.org/integration/autoland/rev/65cc7b37cf3f
P4: Shrink video size. r=jya

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/15de17e6f1e1
https://hg.mozilla.org/mozilla-central/rev/24f0a2f224c4
https://hg.mozilla.org/mozilla-central/rev/25b8cda49700
https://hg.mozilla.org/mozilla-central/rev/65cc7b37cf3f