Skip to content

Use fetch rather than XMLHttpRequest for downloading Wasm files #22015

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.

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

Merged
merged 20 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -600,3 +600,4 @@ a license to everyone to use it as detailed in LICENSE.)
* YAMAMOTO Takashi <[email protected]>
* Artur Gatin <[email protected]> (copyright owned by Teladoc Health, Inc.)
* Christian Lloyd <[email protected]> (copyright owned by Teladoc Health, Inc.)
* Sean Morris <[email protected]>
80 changes: 80 additions & 0 deletions src/polyfill/fetch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Fetch polyfill from https://github.com/developit/unfetch
// License:
//==============================================================================
// Copyright (c) 2017 Jason Miller
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//==============================================================================

#if !POLYFILL
#error "this file should never be included unless POLYFILL is set"
#endif

if (typeof globalThis.fetch == 'undefined') {
globalThis.fetch = function (url, options) {
options = options || {};
return new Promise((resolve, reject) => {
const request = new XMLHttpRequest();
const keys = [];
const headers = {};

request.responseType = 'arraybuffer';

const response = () => ({
ok: ((request.status / 100) | 0) == 2, // 200-299
statusText: request.statusText,
status: request.status,
url: request.responseURL,
text: () => Promise.resolve(request.responseText),
json: () => Promise.resolve(request.responseText).then(JSON.parse),
blob: () => Promise.resolve(new Blob([request.response])),
arrayBuffer: () => Promise.resolve(request.response),
clone: response,
headers: {
keys: () => keys,
entries: () => keys.map((n) => [n, request.getResponseHeader(n)]),
get: (n) => request.getResponseHeader(n),
has: (n) => request.getResponseHeader(n) != null,
},
});

request.open(options.method || "get", url, true);

request.onload = () => {
request
.getAllResponseHeaders()
.toLowerCase()
.replace(/^(.+?):/gm, (m, key) => {
headers[key] || keys.push((headers[key] = key));
});
resolve(response());
};

request.onerror = reject;

request.withCredentials = options.credentials == "include";

for (const i in options.headers) {
request.setRequestHeader(i, options.headers[i]);
}

request.send(options.body || null);
});
}
}
5 changes: 5 additions & 0 deletions src/shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ var Module = typeof {{{ EXPORT_NAME }}} != 'undefined' ? {{{ EXPORT_NAME }}} : {
// See https://caniuse.com/mdn-javascript_builtins_bigint64array
#include "polyfill/bigint64array.js"
#endif

#if MIN_CHROME_VERSION < 40 || MIN_FIREFOX_VERSION < 39 || MIN_SAFARI_VERSION < 103000
// See https://caniuse.com/fetch
#include "polyfill/fetch.js"
#endif
#endif // POLYFILL

#if MODULARIZE
Expand Down
21 changes: 8 additions & 13 deletions src/web_or_worker_shell_read.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,12 @@
}

readAsync = (url, onload, onerror) => {
var xhr = new XMLHttpRequest();
xhr.open('GET', url, true);
xhr.responseType = 'arraybuffer';
xhr.onload = () => {
if (xhr.status == 200 || (xhr.status == 0 && xhr.response)) { // file URLs can return 0
onload(xhr.response);
return;
fetch(url)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also update the above two uses of XMLHttpRequest ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the two earlier calls are synchronous, it might take some more re-engineering to get those converted.

.then((response) => {
if (response.ok) {
return response.arrayBuffer();
}
onerror();
};
xhr.onerror = onerror;
xhr.send(null);
}

return Promise.reject(new Error(response.statusText + ' : ' + response.url));
})
.then(onload, onerror)
};
2 changes: 1 addition & 1 deletion test/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2272,7 +2272,7 @@ def compile_btest(self, filename, args, reporting=Reporting.FULL):
# also include report_result.c and force-include report_result.h
self.run_process([EMCC, '-c', '-I' + TEST_ROOT,
'-DEMTEST_PORT_NUMBER=%d' % self.port,
test_file('report_result.c')] + self.get_emcc_args(compile_only=True))
test_file('report_result.c')] + self.get_emcc_args(compile_only=True) + (['-fPIC'] if '-fPIC' in args else []))
args += ['report_result.o', '-include', test_file('report_result.h')]
if EMTEST_BROWSER == 'node':
args.append('-DEMTEST_NODE')
Expand Down
39 changes: 38 additions & 1 deletion test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,9 @@ def setup(assetLocalization):
<center><canvas id='canvas' width='256' height='256'></canvas></center>
<hr><div id='output'></div><hr>
<script type='text/javascript'>
window.onerror = (error) => {
const handler = (event) => {
event.stopImmediatePropagation();
const error = String(event instanceof ErrorEvent ? event.message : (event.reason || event));
window.disableErrorReporting = true;
window.onerror = null;
var result = error.includes("test.data") ? 1 : 0;
Expand All @@ -709,6 +711,8 @@ def setup(assetLocalization):
xhr.send();
setTimeout(function() { window.close() }, 1000);
}
window.addEventListener('error', handler);
window.addEventListener('unhandledrejection', handler);
var Module = {
locateFile: function (path, prefix) {if (path.endsWith(".wasm")) {return prefix + path;} else {return "''' + assetLocalization + r'''" + path;}},
print: (function() {
Expand Down Expand Up @@ -5545,6 +5549,39 @@ def test_webpack(self):
shutil.copyfile('webpack/src/hello.wasm', 'webpack/dist/hello.wasm')
self.run_browser('webpack/dist/index.html', '/report_result?exit:0')

def test_fetch_polyfill_shared_lib(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we even need to involve shared libraries here since the polyfill is needed even to load the base wasm file (I think).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm either way though since you've already gone to the trouble to writing the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its good to have this test either way, that stuff could be changed in the future as the code becomes more unified. We should make sure the code works in all cases.

create_file('library.c', r'''
#include <stdio.h>
int library_func() {
return 42;
}
''')
create_file('main.c', r'''
#include <dlfcn.h>
#include <stdio.h>
int main() {
void *lib_handle = dlopen("/library.so", RTLD_NOW);
typedef int (*voidfunc)();
voidfunc x = (voidfunc)dlsym(lib_handle, "library_func");
return x();
}
''')

self.run_process([EMCC, 'library.c', '-sSIDE_MODULE', '-O2', '-o', 'library.so'])

def test(args, expect_fail):
self.compile_btest('main.c', ['-fPIC', 'library.so', '-sMAIN_MODULE=2', '-sEXIT_RUNTIME', '-o', 'a.out.html'] + args)
if expect_fail:
js = read_file('a.out.js')
create_file('a.out.js', 'fetch = undefined;\n' + js)
return self.run_browser('a.out.html', '/report_result?abort:TypeError')
else:
return self.run_browser('a.out.html', '/report_result?exit:42')

test([], expect_fail=True)
test(['-sLEGACY_VM_SUPPORT'], expect_fail=False)
test(['-sLEGACY_VM_SUPPORT', '-sNO_POLYFILL'], expect_fail=True)


class emrun(RunnerCore):
def test_emrun_info(self):
Expand Down
Loading