w3c / media-source

Media Source Extensions
https://w3c.github.io/media-source/
Other
268 stars 58 forks source link

Editorial? Prepare append algorithm's caller needs to abort if the algorithm aborted #288

Open wolenetz opened 3 years ago

wolenetz commented 3 years ago

Currently, the last step in the "Prepare Append" algorithm is (bold for emphasis in this copy of it):

If the buffer full flag equals true, then throw a QuotaExceededError exception and abort these steps.

However, the caller (SourceBuffer's appendBuffer() method) does not acknowledge that the Prepare Append algorithm might have thrown and aborted.

If the Prepare Append algorithm had aborted, the remainder of the steps in appendBuffer() should *not be run. e.g., |data| should not be appended to the input buffer, updating shouldn't become true, updatestart event shouldn't be enqueued, and no asynchronous run of the buffer append algorithm should happen.

I believe this is likely editorial, but will see if I can find behavior to the contrary in either Mozilla or Safari MSE implementations (assuming I can get them to decide that an append is too much and give QuotaExceededError).

If truly editorial, the next step after "Run the prepare append algorithm" in appendBuffer() needs to abort if the prepare append algorithm aborted.

wolenetz commented 3 years ago

Speculative wpt change, from Chromium/Blink, to help understand Mozilla & Safari behavior around this scenario is https://chromium-review.googlesource.com/c/chromium/src/+/3083226

wolenetz commented 3 years ago

Fortunately, the strengthened wpt test for this behavior appears [1] to indicate that recent Chrome, Firefox, Edge, Safari all abort the appendBuffer steps if prepare append aborted with QuotaExceededError, making this fix more editorial (though there could still be other implementations that strictly interpreted the REC spec text and would thus fail the strengthened wpt test).

[1] https://wpt.fyi/results/media-source/mediasource-appendbuffer-quota-exceeded.html?run_id=5652876606046208&run_id=5668596790329344&run_id=5710437891964928&run_id=5719534498480128