1253762 - [webvtt] Adapt chrome tests to work with e10s

Status: RESOLVED FIXED

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

Description

[Ralph Giles (:rillian)]

We have three tests listed in dom/media/test/chrome.ini because they use chrome-only API features of the TextTrack API. Running these in the chrome space doesn't work with e10s because they don't have access to the web content in that context.

Adapt these so they work and we can keep them enabled. See https://wiki.mozilla.org/Electrolysis/e10s_test_tips for tips.

Comment 1

[Ralph Giles (:rillian)]

Attachment: Adapt test_texttrack_chrome.html

Here's a start. Use SpecialPowers.wrap() and unwrap() to access higher-privilege members. Thanks to mccr8 and mrbrap for help working this out.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

e10s -> p1

Comment 3

[Ralph Giles (:rillian)]

Attachment: Adapt test_texttrackcue_chrome.html

Comment 4

[Ralph Giles (:rillian)]

Attachment: Adapt test_texttracklist_chrome.html

Comment 5

[Ralph Giles (:rillian)]

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

Comment 6

[Matthew Gregan [:kinetik]]

Comment on attachment 8726967 [details] [diff] [review]
Adapt test_texttrack_chrome.html

Review of attachment 8726967 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/test/test_texttrack_moz.html
@@ +46,5 @@
> +      video.addEventListener("loadedmetadata", function run_tests() {
> +        // Re-que run_tests() at the end of the event loop until the track
> +        // element has loaded its data.
> +        if (trackElement.readyState == HTMLTrackElement.LOADING) {
> +          setTimeout(run_tests, 0);

(I know this is from the old code.)

I'm guessing this is required because there's no more-explicit event to wait on for the state change the test is waiting for?

Comment 7

[Matthew Gregan [:kinetik]]

Comment on attachment 8728099 [details] [diff] [review]
Adapt test_texttracklist_chrome.html

Review of attachment 8728099 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/test/mochitest.ini
@@ +851,4 @@
>  [test_texttrackcue_moz.html]
>  [test_texttrackevents_video.html]
>  [test_texttracklist.html]
> +skip-if = os=='mac' && debug # bug 1130751

Is it worth confirming this still applies now it's a regular mochitest?

Comment 8

[Ralph Giles (:rillian)]

(In reply to Matthew Gregan [:kinetik] from comment #6)

> > +        // Re-que run_tests() at the end of the event loop until the track
> > +        // element has loaded its data.

> I'm guessing this is required because there's no more-explicit event to wait
> on for the state change the test is waiting for?

IIRC that's correct. There's no onstatechange on track elements. It's possible the spec has changed to gate the parent media element's play events on the TextTrack ready state, but (a) I didn't try to determine that and (b) that's testing two things at once.

Comment 9

[Ralph Giles (:rillian)]

Attachment: Enable test_texttracklist_moz on Mac debug

> Is it worth confirming this still applies now it's a regular mochitest?

Good point. Bug 1130751 was specific to MacOS X 10.8. We no longer run tests on that version, so it should be fine. Looks ok on try too. https://treeherder.mozilla.org/#/jobs?repo=try&revision=0202850abaf4

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/af346b5bd5a6
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c22c66781d3
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0ca84de2add
https://hg.mozilla.org/integration/mozilla-inbound/rev/a99eb961c111

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/af346b5bd5a6
https://hg.mozilla.org/mozilla-central/rev/6c22c66781d3
https://hg.mozilla.org/mozilla-central/rev/e0ca84de2add
https://hg.mozilla.org/mozilla-central/rev/a99eb961c111