Closed Bug 1253762 Opened 4 years ago Closed 4 years ago

[webvtt] Adapt chrome tests to work with e10s

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox47 --- affected
firefox48 --- fixed

People

(Reporter: rillian, Assigned: rillian)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

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.
Here's a start. Use SpecialPowers.wrap() and unwrap() to access higher-privilege members. Thanks to mccr8 and mrbrap for help working this out.
e10s -> p1
Priority: -- → P1
Blocks: e10s-tests
tracking-e10s: --- → +
Attachment #8726967 - Flags: review?(kinetik)
Attachment #8728097 - Flags: review?(kinetik)
Attachment #8728099 - Flags: review?(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?
Attachment #8726967 - Flags: review?(kinetik) → review+
Attachment #8728097 - Flags: review?(kinetik) → review+
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?
Attachment #8728099 - Flags: review?(kinetik) → review+
(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.
> 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
Attachment #8728563 - Flags: review?(kinetik)
Blocks: 1130751
Attachment #8728563 - Flags: review?(kinetik) → review+
You need to log in before you can comment on or make changes to this bug.