Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
wolenetz opened this issue Aug 4, 2021 · 2 comments
Open

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

wolenetz opened this issue Aug 4, 2021 · 2 comments
Assignees
@wolenetz
Milestone
V3

Comments

@wolenetz
Copy link
Member

wolenetz commented Aug 4, 2021

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 wolenetz added this to the V2 milestone Aug 4, 2021
@wolenetz wolenetz self-assigned this Aug 4, 2021
@wolenetz
Copy link
Member Author

wolenetz commented Aug 9, 2021

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

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 9, 2021
@wolenetz @chromium-wpt-export-bot
MSE: Strengthen QuotaExceededError validation for fixing spec #288
f49ce8e 
This change strengthens the checks that when an appendBuffer()'s
internal steps are aborted due to QuotaExceededError exception within
the "prepare append algorithm", the rest of the appendBuffer()'s steps
are also aborted. Note that verifying the state of the implementations
internal "input buffer" is not included in this change due to
complexity of that verification.

See w3c/media-source#288. This change will
help inform whether that issues' fix would be a breaking change for
implementations that report results on https://wpt.fyi

Change-Id: Iaba91eb739ca40d4633b15326871bd3635e16fbd
Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 10, 2021
@wolenetz @chromium-wpt-export-bot
MSE: Strengthen QuotaExceededError validation for fixing spec #288
12e799b 
This change strengthens the checks that when an appendBuffer()'s
internal steps are aborted due to QuotaExceededError exception within
the "prepare append algorithm", the rest of the appendBuffer()'s steps
are also aborted. Note that verifying the state of the implementations
internal "input buffer" is not included in this change due to
complexity of that verification.

See w3c/media-source#288. This change will
help inform whether that issues' fix would be a breaking change for
implementations that report results on https://wpt.fyi

Change-Id: Iaba91eb739ca40d4633b15326871bd3635e16fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3083226
Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: Will Cassella <cassew@google.com>
Cr-Commit-Position: refs/heads/master@{#910333}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 10, 2021
@wolenetz @chromium-wpt-export-bot
MSE: Strengthen QuotaExceededError validation for fixing spec #288
60bc589 
This change strengthens the checks that when an appendBuffer()'s
internal steps are aborted due to QuotaExceededError exception within
the "prepare append algorithm", the rest of the appendBuffer()'s steps
are also aborted. Note that verifying the state of the implementations
internal "input buffer" is not included in this change due to
complexity of that verification.

See w3c/media-source#288. This change will
help inform whether that issues' fix would be a breaking change for
implementations that report results on https://wpt.fyi

Change-Id: Iaba91eb739ca40d4633b15326871bd3635e16fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3083226
Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: Will Cassella <cassew@google.com>
Cr-Commit-Position: refs/heads/master@{#910333}
pull bot pushed a commit to FreddyZeng/chromium that referenced this issue Aug 10, 2021
@wolenetz
MSE: Strengthen QuotaExceededError validation for fixing spec #288
52ad81b 
This change strengthens the checks that when an appendBuffer()'s
internal steps are aborted due to QuotaExceededError exception within
the "prepare append algorithm", the rest of the appendBuffer()'s steps
are also aborted. Note that verifying the state of the implementations
internal "input buffer" is not included in this change due to
complexity of that verification.

See w3c/media-source#288. This change will
help inform whether that issues' fix would be a breaking change for
implementations that report results on https://wpt.fyi

Change-Id: Iaba91eb739ca40d4633b15326871bd3635e16fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3083226
Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: Will Cassella <cassew@google.com>
Cr-Commit-Position: refs/heads/master@{#910333}
@wolenetz
Copy link
Member Author

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

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 14, 2021
@wolenetz @moz-wptsync-bot
Bug 1724813 [wpt PR 29958] - MSE: Strengthen QuotaExceededError valid…
6722b32 
…ation for fixing spec #288, a=testonly

Automatic update from web-platform-tests
MSE: Strengthen QuotaExceededError validation for fixing spec #288

This change strengthens the checks that when an appendBuffer()'s
internal steps are aborted due to QuotaExceededError exception within
the "prepare append algorithm", the rest of the appendBuffer()'s steps
are also aborted. Note that verifying the state of the implementations
internal "input buffer" is not included in this change due to
complexity of that verification.

See w3c/media-source#288. This change will
help inform whether that issues' fix would be a breaking change for
implementations that report results on https://wpt.fyi

Change-Id: Iaba91eb739ca40d4633b15326871bd3635e16fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3083226
Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: Will Cassella <cassew@google.com>
Cr-Commit-Position: refs/heads/master@{#910333}

--

wpt-commits: 60bc5896a5ce5586303378fe7f92ff8c756970a2
wpt-pr: 29958
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Aug 20, 2021
@wolenetz @moz-wptsync-bot
Bug 1724813 [wpt PR 29958] - MSE: Strengthen QuotaExceededError valid…
728eddc 
…ation for fixing spec #288, a=testonly

Automatic update from web-platform-tests
MSE: Strengthen QuotaExceededError validation for fixing spec #288

This change strengthens the checks that when an appendBuffer()'s
internal steps are aborted due to QuotaExceededError exception within
the "prepare append algorithm", the rest of the appendBuffer()'s steps
are also aborted. Note that verifying the state of the implementations
internal "input buffer" is not included in this change due to
complexity of that verification.

See w3c/media-source#288. This change will
help inform whether that issues' fix would be a breaking change for
implementations that report results on https://wpt.fyi

Change-Id: Iaba91eb739ca40d4633b15326871bd3635e16fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3083226
Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: Will Cassella <cassew@google.com>
Cr-Commit-Position: refs/heads/master@{#910333}

--

wpt-commits: 60bc5896a5ce5586303378fe7f92ff8c756970a2
wpt-pr: 29958
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@wolenetz
MSE: Strengthen QuotaExceededError validation for fixing spec #288
a8d835a 
This change strengthens the checks that when an appendBuffer()'s
internal steps are aborted due to QuotaExceededError exception within
the "prepare append algorithm", the rest of the appendBuffer()'s steps
are also aborted. Note that verifying the state of the implementations
internal "input buffer" is not included in this change due to
complexity of that verification.

See w3c/media-source#288. This change will
help inform whether that issues' fix would be a breaking change for
implementations that report results on https://wpt.fyi

Change-Id: Iaba91eb739ca40d4633b15326871bd3635e16fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3083226
Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: Will Cassella <cassew@google.com>
Cr-Commit-Position: refs/heads/master@{#910333}
NOKEYCHECK=True
GitOrigin-RevId: 52ad81b7cf3d8fdceffdf5c9072a58ba88592afc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wolenetz wolenetz

Labels
None yet
Projects
None yet
Milestone
V3
Development

No branches or pull requests
1 participant
@wolenetz