Prepare Append Algorithm should return QuotaExceededError when exceeding buffer size

RESOLVED FIXED in Firefox 37

Status

()

P2
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla38
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox37 fixed, firefox38 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

4 years ago
As per spec:
http://w3c.github.io/media-source/#sourcebuffer-prepare-append
"If the buffer full flag equals true, then throw a QuotaExceededError exception and abort these step."

If after evicting data the size of our TrackBuffer still exceeds the threshold size, we should return a QuotaExceededError error.
(Assignee)

Comment 1

4 years ago
Created attachment 8565251 [details] [diff] [review]
Fix mochitest so it doesn't append data forever

This mochitest triggered a quota exceeded error. Turned out, it was appending 190306 bytes in a loop, which on my machine would reach 75MB before loadedmetadata got fired.
Attachment #8565251 - Flags: review?(cajbir.bugzilla)
(Assignee)

Updated

4 years ago
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
(Assignee)

Comment 3

4 years ago
Created attachment 8565256 [details] [diff] [review]
Return an error when attempting to append too much data

Implement step 6 of the Prepare Append Algorithm, if our buffer is full, throw a quota exceeded error
Attachment #8565256 - Flags: review?(cajbir.bugzilla)

Updated

4 years ago
Priority: -- → P2

Updated

4 years ago
Attachment #8565251 - Flags: review?(cajbir.bugzilla) → review+

Updated

4 years ago
Attachment #8565256 - Flags: review?(cajbir.bugzilla) → review+
(Assignee)

Comment 4

4 years ago
Created attachment 8566208 [details] [diff] [review]
Part2. Don't accept buffer exceeding our threshold

Shouldn't allow an append greater than our threshold. However, YouTube attempts to load data in excess of 8MB when close to the end of the
video, and never attempts to re-append should it error. As such,
the sourcebuffer threshold can't be set to lower than 8MB with this change.
Attachment #8566208 - Flags: review?(cajbir.bugzilla)

Updated

4 years ago
Attachment #8566208 - Flags: review?(cajbir.bugzilla) → review+
https://hg.mozilla.org/mozilla-central/rev/661dcf41cef9
https://hg.mozilla.org/mozilla-central/rev/af39fc4994cc
https://hg.mozilla.org/mozilla-central/rev/74014ad8e1f8
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment on attachment 8565256 [details] [diff] [review]
Return an error when attempting to append too much data

Requestion retroactive 37 uplift approval for all patches on this bug.

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Spec compliance, less consistent testing.
[Describe test coverage new/current, TreeHerder]: Landed on m-c. Mochitests.
[Risks and why]: MSE-specific, so low.
[String/UUID change made/needed]: None.
Attachment #8565256 - Flags: approval-mozilla-aurora?
Comment on attachment 8565256 [details] [diff] [review]
Return an error when attempting to append too much data

As stated, this bug was pre-approved to land with a set of changes for MSE. Marking the approval after the fact.
Attachment #8565256 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8565251 - Flags: approval-mozilla-aurora+
Attachment #8566208 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.