Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upImplement the arraybuffer API for Fetch bodies #20346
Closed
Labels
Comments
|
Not sure if I can make sound PR at once, but let me try it if you don't mind. Feel freely unassign / or close PR if needed. |
|
@highfive assign me |
|
Hey @kwonoj! Thanks for your interest in working on this issue. It's now assigned to you! |
bors-servo
added a commit
that referenced
this issue
Mar 24, 2018
feat(fetch): accept arraybuffer in consume_body <!-- Please describe your changes on the following line: --> Related to #20346. I realized I am not sufficiently knowledgeable about codebases and have high confidence this PR is not ready to be accepted. Raising it as PR early to possibly ask some suggestions around codebases. If this PR seems unrecoverable by code review, please feel freely close and unassign me from issue🙏 This PR tries to implement #20346, updating `Body` idl and implements corresponding implementation in `body.rs` for `fetch`. Criteria for changes may includes - does `run_array_buffer_data_algorithm` implementation is legit for allocating arraybuffer? (probably not) - does `run_array_buffer_data_algorithm` implementation is acceptable for handling error, by naively returning `Error::JSFailed`? - there are some number of wpt test started to PASS with this PR. Is this legit side effect, or something incorrect by current implementation? - etcs, vice versa --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20346 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - wpt test has changed in PR, need to be reviewed. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20406) <!-- Reviewable:end -->
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this issue
Mar 25, 2018
… (from kwonoj:feat-fetch-body-arraybuffer); r=jdm feat(fetch): accept arraybuffer in consume_body <!-- Please describe your changes on the following line: --> Related to servo/servo#20346. I realized I am not sufficiently knowledgeable about codebases and have high confidence this PR is not ready to be accepted. Raising it as PR early to possibly ask some suggestions around codebases. If this PR seems unrecoverable by code review, please feel freely close and unassign me from issue🙏 This PR tries to implement #20346, updating `Body` idl and implements corresponding implementation in `body.rs` for `fetch`. Criteria for changes may includes - does `run_array_buffer_data_algorithm` implementation is legit for allocating arraybuffer? (probably not) - does `run_array_buffer_data_algorithm` implementation is acceptable for handling error, by naively returning `Error::JSFailed`? - there are some number of wpt test started to PASS with this PR. Is this legit side effect, or something incorrect by current implementation? - etcs, vice versa --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20346 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - wpt test has changed in PR, need to be reviewed. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 23b2f42a368cdc68548310e79b31306f40f95553 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 3cf96067aca4fa0a3f7d31256758f472b3d1c169
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this issue
Oct 2, 2019
… (from kwonoj:feat-fetch-body-arraybuffer); r=jdm feat(fetch): accept arraybuffer in consume_body <!-- Please describe your changes on the following line: --> Related to servo/servo#20346. I realized I am not sufficiently knowledgeable about codebases and have high confidence this PR is not ready to be accepted. Raising it as PR early to possibly ask some suggestions around codebases. If this PR seems unrecoverable by code review, please feel freely close and unassign me from issue🙏 This PR tries to implement #20346, updating `Body` idl and implements corresponding implementation in `body.rs` for `fetch`. Criteria for changes may includes - does `run_array_buffer_data_algorithm` implementation is legit for allocating arraybuffer? (probably not) - does `run_array_buffer_data_algorithm` implementation is acceptable for handling error, by naively returning `Error::JSFailed`? - there are some number of wpt test started to PASS with this PR. Is this legit side effect, or something incorrect by current implementation? - etcs, vice versa --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20346 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - wpt test has changed in PR, need to be reviewed. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 23b2f42a368cdc68548310e79b31306f40f95553 UltraBlame original commit: c0dc6a1531f03bd155f509f317998c92f0a88cb0
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this issue
Oct 2, 2019
… (from kwonoj:feat-fetch-body-arraybuffer); r=jdm feat(fetch): accept arraybuffer in consume_body <!-- Please describe your changes on the following line: --> Related to servo/servo#20346. I realized I am not sufficiently knowledgeable about codebases and have high confidence this PR is not ready to be accepted. Raising it as PR early to possibly ask some suggestions around codebases. If this PR seems unrecoverable by code review, please feel freely close and unassign me from issue🙏 This PR tries to implement #20346, updating `Body` idl and implements corresponding implementation in `body.rs` for `fetch`. Criteria for changes may includes - does `run_array_buffer_data_algorithm` implementation is legit for allocating arraybuffer? (probably not) - does `run_array_buffer_data_algorithm` implementation is acceptable for handling error, by naively returning `Error::JSFailed`? - there are some number of wpt test started to PASS with this PR. Is this legit side effect, or something incorrect by current implementation? - etcs, vice versa --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20346 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - wpt test has changed in PR, need to be reviewed. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 23b2f42a368cdc68548310e79b31306f40f95553 UltraBlame original commit: c0dc6a1531f03bd155f509f317998c92f0a88cb0
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this issue
Oct 2, 2019
… (from kwonoj:feat-fetch-body-arraybuffer); r=jdm feat(fetch): accept arraybuffer in consume_body <!-- Please describe your changes on the following line: --> Related to servo/servo#20346. I realized I am not sufficiently knowledgeable about codebases and have high confidence this PR is not ready to be accepted. Raising it as PR early to possibly ask some suggestions around codebases. If this PR seems unrecoverable by code review, please feel freely close and unassign me from issue🙏 This PR tries to implement #20346, updating `Body` idl and implements corresponding implementation in `body.rs` for `fetch`. Criteria for changes may includes - does `run_array_buffer_data_algorithm` implementation is legit for allocating arraybuffer? (probably not) - does `run_array_buffer_data_algorithm` implementation is acceptable for handling error, by naively returning `Error::JSFailed`? - there are some number of wpt test started to PASS with this PR. Is this legit side effect, or something incorrect by current implementation? - etcs, vice versa --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20346 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - wpt test has changed in PR, need to be reviewed. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 23b2f42a368cdc68548310e79b31306f40f95553 UltraBlame original commit: c0dc6a1531f03bd155f509f317998c92f0a88cb0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Now that #20267 has merged, we can implement WebIDL APIs that involve typed arrays. This API needs to create an ArrayBuffer object that contains the bytes that make up the fetch response's body.
Code:
components/script/dom/webidls/Body.webidl,components/script/dom/fetch.rsTests:
./mach test-wpt tests/wpt/web-platform-tests/fetch/(this should result in new passing tests)