All users were logged out of Bugzilla on October 13th, 2018

test_MediaSource.html incorrectly assumes video element's preload is not "none"

RESOLVED FIXED in mozilla27

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

(Blocks: 1 bug)

Trunk
mozilla27
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Comment hidden (empty)
(Assignee)

Comment 2

5 years ago
Try push for debugging: https://tbpl.mozilla.org/?tree=Try&rev=5305c4b7ba83
Assignee: nobody → kinetik
Unrelated, but SetBoolPref() doesn't work on B2G. You should use the async pushPrefEnv (see test_texttrack.html for an example) or enable the pref unconditionally for the test run (see testing/profiles/prefs_general.js) in expectation of enabling MediaSource tests there.
(Assignee)

Comment 4

5 years ago
Thanks for pointing that out, I'll fix the test. :-)
(Assignee)

Comment 5

5 years ago
Had a quick look at this today after wasting a bunch of time setting up a local Android debug environment only to discover the problem is fairly obvious.  Media elements default to preload=none on Android (and B2G, but I guess the test never ran there to reveal the problem?).  The MediaSource object is attached to the media element in HTMLMediaElement::LoadResource, but HTMLMediaElement::SelectResource early-exits before calling LoadResource if preload is "none".  Calling HTMLMediaElement::Play should result in playback, but the code to attach the MediaSource is missing from this code path.

So the short-term fix to get some test coverage of this code is to change the test to use preload=auto.  I'll morph this to cover the root cause, which I'll return to fix once I've finished up bug 905513 and friends.

Test fix: https://tbpl.mozilla.org/?tree=Try&rev=8d1b2824d48a
(Assignee)

Comment 6

5 years ago
Progress, it now fails in a different place:

24468 INFO TEST-PASS | /tests/content/media/mediasource/test/test_MediaSource.html | Create a SourceBuffer
24469 INFO TEST-PASS | /tests/content/media/mediasource/test/test_MediaSource.html | MediaSource.sourceBuffers is expected length
244
command timed out: 2400 seconds without output, attempting to kill
(Assignee)

Comment 7

5 years ago
Try, try again: https://tbpl.mozilla.org/?tree=Try&rev=b6f3999a9184 (now with more logging)
(Assignee)

Comment 8

5 years ago
This is passing with the latest patch in bug 905513: https://tbpl.mozilla.org/?tree=Try&rev=b6f3999a9184
(Assignee)

Updated

5 years ago
Depends on: 905513
(Assignee)

Updated

5 years ago
Summary: Fix timeout in test_MediaSource.html on Android → MediaSource attach doesn't run for preload=none media elements
(Assignee)

Comment 9

5 years ago
Looking at this with fresh eyes, it is the test itself that is wrong.  The test creates a Video and a MediaSource, sets the Video src to the MediaSource object URL, then waits for a sourceopen event, which runs the rest of the test.  The steps that would fire "sourceopen" run before the "perform a potentially CORS-enabled fetch" step of the media element's resource fetch algorithm.  The preceeding step of that algorithm handles preload="none", and stops the algorithm until an implementation-defined event occurs.  So the current behaviour is correct, and calling play() on the video element correctly results in "sourceopen" firing.  This is the same behaviour as Chrome has, too.

So this was fixed by the test fix landed with bug 905513.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Summary: MediaSource attach doesn't run for preload=none media elements → test_MediaSource.html incorrectly assumes video element's preload is not "none"
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.