[MSE] Preload argument must be ignored with MSE

RESOLVED FIXED in Firefox 51

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

(URL)

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Per new spec, the preload argument should be ignored.

This cause http://www.w3c-test.org/media-source/mediasource-preload.html to fail
(Assignee)

Comment 1

3 years ago
Created attachment 8778022 [details]
Bug 1292157: Ignore preload value when dealing with MediaSource originated URI.

Review commit: https://reviewboard.mozilla.org/r/69398/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69398/
Attachment #8778022 - Flags: review?(jwwang)
(Assignee)

Comment 2

3 years ago
I think only the first test with preload=none is valid. The URI provided is a real one, its mediasource origin can be tracked.

For the corrupted URI, the test simply add the character "0" to the URI. This URI doesn't have mediasource origin as such, and preload shouldn't be ignored as such.

Same for the revoked URI. Its mediasource origin has been lost, it's no longer a mediasource URI, and so preload can't be ignored either.

In those last two cases, we couldn't determine the original point of origin even if we wanted it (at least certainly for the corrupted URI, for the revoked one, we would need to keep a permanent origin member in the URI object).

JW, what do you think?
Flags: needinfo?(jwwang)
(Assignee)

Updated

3 years ago
Assignee: nobody → jyavenard
Comment on attachment 8778022 [details]
Bug 1292157: Ignore preload value when dealing with MediaSource originated URI.

https://reviewboard.mozilla.org/r/69398/#review66572

LGTM. I wonder if it makes a difference to test IsMediaSourceURI(mLoadingSrc) which is more consistent with IsMediaStreamURI(mLoadingSrc).
Attachment #8778022 - Flags: review?(jwwang) → review+
(In reply to Jean-Yves Avenard [:jya] from comment #2)
> I think only the first test with preload=none is valid. The URI provided is
> a real one, its mediasource origin can be tracked.
> 
> For the corrupted URI, the test simply add the character "0" to the URI.
> This URI doesn't have mediasource origin as such, and preload shouldn't be
> ignored as such.
> 
> Same for the revoked URI. Its mediasource origin has been lost, it's no
> longer a mediasource URI, and so preload can't be ignored either.
> 
> In those last two cases, we couldn't determine the original point of origin
> even if we wanted it (at least certainly for the corrupted URI, for the
> revoked one, we would need to keep a permanent origin member in the URI
> object).
> 
> JW, what do you think?

SGTM. preload=none should be ignored only when we have a valid mediasource URI. Btw, does the spec say to ignore preload=none or preload=any value?
Flags: needinfo?(jwwang)
(Assignee)

Comment 5

3 years ago
the specs state "If the resource fetch algorithm absolute URL matches the MediaSource object URL, ignore any preload attribute of the media element, skip any optional steps for when preload equals none"

what about for the other tests failing though? (corrupted or revoked), do you agree with my assessment?
(Assignee)

Comment 6

3 years ago
Created attachment 8778107 [details]
Bug 1292157: P2. Adjust webref expected results.

Review commit: https://reviewboard.mozilla.org/r/69486/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69486/
Attachment #8778107 - Flags: review?(jwwang)
Attachment #8778107 - Flags: review?(jwwang) → review+
(In reply to Jean-Yves Avenard [:jya] from comment #5)
> what about for the other tests failing though? (corrupted or revoked), do
> you agree with my assessment?
Sure.

Somehow I have test timeouts on Chrome:
Timeout	sourceopen occurs with element preload=none
Timeout	error occurs with bogus blob URL (revoked MediaSource object URL) and element preload=none
Timeout	error occurs with bogus blob URL (corrupted MediaSource object URL) and element preload=none

And Firefox47 also exhibits the same results.
(Assignee)

Comment 9

3 years ago
(In reply to JW Wang [:jwwang] from comment #8)
> (In reply to Jean-Yves Avenard [:jya] from comment #5)
> > what about for the other tests failing though? (corrupted or revoked), do
> > you agree with my assessment?
> Sure.
> 
> Somehow I have test timeouts on Chrome:
> Timeout	sourceopen occurs with element preload=none
> Timeout	error occurs with bogus blob URL (revoked MediaSource object URL)
> and element preload=none
> Timeout	error occurs with bogus blob URL (corrupted MediaSource object URL)
> and element preload=none
> 

The MSE spec related to preload was updated last June. My guess is that you would have to try Chrome canary.

The test timeout because they wait for the error event to be fired. But as we are in preload=none, there's not been any attempts to use the source element yet ; so the error event isn't fired => timeout

> And Firefox47 also exhibits the same results.

This is what this bug was about, fixing our own implementation. won't in in 47, it's not even in central yet :)

Comment 10

3 years ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf3c1a4f7f30
Ignore preload value when dealing with MediaSource originated URI. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/9c05a4633b91
P2. Adjust webref expected results. r=jwwang

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bf3c1a4f7f30
https://hg.mozilla.org/mozilla-central/rev/9c05a4633b91
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.