Skip to content

Remove fallback to BlobBuilder if Blob constructor is missing #19277

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

Merged
merged 1 commit into from
May 2, 2023
Merged

Remove fallback to BlobBuilder if Blob constructor is missing #19277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ See docs/process.md for more on how version tagging works.

3.1.38 (in development)
-----------------------
- Remove extra code for falling back to long-deprecated BlobBuilder browser API
when Blob constructor is missing. This was a fix for an issue that has long
been fixed. (#19277)

3.1.37 - 04/26/23
-----------------
Expand Down
4 changes: 0 additions & 4 deletions src/closure-externs/closure-externs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@ var wakaUnknownAfter;
* @suppress {undefinedVars}
*/
var wakaUnknownBefore;
/**
* @suppress {undefinedVars}
*/
var MozBlobBuilder;

// Module loaders externs, for AMD etc.

Expand Down
13 changes: 0 additions & 13 deletions src/headless.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,19 +295,6 @@ if (typeof console == "undefined") {
}
};
}
var MozBlobBuilder = () => {
this.data = new Uint8Array(0);
this.append = function(buffer) {
var data = new Uint8Array(buffer);
var combined = new Uint8Array(this.data.length + data.length);
combined.set(this.data);
combined.set(data, this.data.length);
this.data = combined;
};
this.getBlob = function() {
return this.data.buffer; // return the buffer as a "blob". XXX We might need to change this if it is not opaque
};
};

// additional setup
if (!Module['canvas']) {
Expand Down
113 changes: 42 additions & 71 deletions src/library_browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,6 @@ var LibraryBrowser = {

if (Browser.initted) return;
Browser.initted = true;

try {
new Blob();
Browser.hasBlobConstructor = true;
} catch(e) {
Browser.hasBlobConstructor = false;
err("warning: no blob constructor, cannot create blobs with mimetypes");
}
Browser.BlobBuilder = typeof MozBlobBuilder != "undefined" ? MozBlobBuilder : (typeof WebKitBlobBuilder != "undefined" ? WebKitBlobBuilder : (!Browser.hasBlobConstructor ? err("warning: no BlobBuilder") : null));
Browser.URLObject = typeof window != "undefined" ? (window.URL ? window.URL : window.webkitURL) : undefined;
if (!Module.noImageDecoding && typeof Browser.URLObject == 'undefined') {
err("warning: Browser does not support creating object URLs. Built-in browser image decoding will not be available.");
Expand All @@ -133,22 +124,10 @@ var LibraryBrowser = {
return !Module.noImageDecoding && /\.(jpg|jpeg|png|bmp)$/i.test(name);
};
imagePlugin['handle'] = function imagePlugin_handle(byteArray, name, onload, onerror) {
var b = null;
if (Browser.hasBlobConstructor) {
try {
b = new Blob([byteArray], { type: Browser.getMimetype(name) });
if (b.size !== byteArray.length) { // Safari bug #118630
// Safari's Blob can only take an ArrayBuffer
b = new Blob([(new Uint8Array(byteArray)).buffer], { type: Browser.getMimetype(name) });
}
} catch(e) {
warnOnce('Blob constructor present but fails: ' + e + '; falling back to blob builder');
}
}
if (!b) {
var bb = new Browser.BlobBuilder();
bb.append((new Uint8Array(byteArray)).buffer); // we need to pass a buffer, and must copy the array to get the right data range
b = bb.getBlob();
var b = new Blob([byteArray], { type: Browser.getMimetype(name) });
if (b.size !== byteArray.length) { // Safari bug #118630
// Safari's Blob can only take an ArrayBuffer
b = new Blob([(new Uint8Array(byteArray)).buffer], { type: Browser.getMimetype(name) });
}
var url = Browser.URLObject.createObjectURL(b);
#if ASSERTIONS
Expand Down Expand Up @@ -192,56 +171,48 @@ var LibraryBrowser = {
preloadedAudios[name] = new Audio(); // empty shim
if (onerror) onerror();
}
if (Browser.hasBlobConstructor) {
try {
var b = new Blob([byteArray], { type: Browser.getMimetype(name) });
} catch(e) {
return fail();
}
var url = Browser.URLObject.createObjectURL(b); // XXX we never revoke this!
var b = new Blob([byteArray], { type: Browser.getMimetype(name) });
var url = Browser.URLObject.createObjectURL(b); // XXX we never revoke this!
#if ASSERTIONS
assert(typeof url == 'string', 'createObjectURL must return a url as a string');
assert(typeof url == 'string', 'createObjectURL must return a url as a string');
#endif
var audio = new Audio();
audio.addEventListener('canplaythrough', () => finish(audio), false); // use addEventListener due to chromium bug 124926
audio.onerror = function audio_onerror(event) {
if (done) return;
err('warning: browser could not fully decode audio ' + name + ', trying slower base64 approach');
function encode64(data) {
var BASE = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/';
var PAD = '=';
var ret = '';
var leftchar = 0;
var leftbits = 0;
for (var i = 0; i < data.length; i++) {
leftchar = (leftchar << 8) | data[i];
leftbits += 8;
while (leftbits >= 6) {
var curr = (leftchar >> (leftbits-6)) & 0x3f;
leftbits -= 6;
ret += BASE[curr];
}
}
if (leftbits == 2) {
ret += BASE[(leftchar&3) << 4];
ret += PAD + PAD;
} else if (leftbits == 4) {
ret += BASE[(leftchar&0xf) << 2];
ret += PAD;
var audio = new Audio();
audio.addEventListener('canplaythrough', () => finish(audio), false); // use addEventListener due to chromium bug 124926
audio.onerror = function audio_onerror(event) {
if (done) return;
err('warning: browser could not fully decode audio ' + name + ', trying slower base64 approach');
function encode64(data) {
var BASE = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/';
var PAD = '=';
var ret = '';
var leftchar = 0;
var leftbits = 0;
for (var i = 0; i < data.length; i++) {
leftchar = (leftchar << 8) | data[i];
leftbits += 8;
while (leftbits >= 6) {
var curr = (leftchar >> (leftbits-6)) & 0x3f;
leftbits -= 6;
ret += BASE[curr];
}
return ret;
}
audio.src = 'data:audio/x-' + name.substr(-3) + ';base64,' + encode64(byteArray);
finish(audio); // we don't wait for confirmation this worked - but it's worth trying
};
audio.src = url;
// workaround for chrome bug 124926 - we do not always get oncanplaythrough or onerror
safeSetTimeout(() => {
finish(audio); // try to use it even though it is not necessarily ready to play
}, 10000);
} else {
return fail();
}
if (leftbits == 2) {
ret += BASE[(leftchar&3) << 4];
ret += PAD + PAD;
} else if (leftbits == 4) {
ret += BASE[(leftchar&0xf) << 2];
ret += PAD;
}
return ret;
}
audio.src = 'data:audio/x-' + name.substr(-3) + ';base64,' + encode64(byteArray);
finish(audio); // we don't wait for confirmation this worked - but it's worth trying
};
audio.src = url;
// workaround for chrome bug 124926 - we do not always get oncanplaythrough or onerror
safeSetTimeout(() => {
finish(audio); // try to use it even though it is not necessarily ready to play
}, 10000);
};
Module['preloadPlugins'].push(audioPlugin);

Expand Down
6 changes: 1 addition & 5 deletions tools/file_packager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,11 +635,7 @@ def generate_js(data_target, data_files, metadata):
Module['FS_createPreloadedFile'](this.name, null, byteArray, true, true, function() {
Module['removeRunDependency']('fp ' + that.name);
}, function() {
if (that.audio) {
Module['removeRunDependency']('fp ' + that.name); // workaround for chromium bug 124926 (still no audio with this, but at least we don't hang)
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this? Don't we need the workaround if the bug is WONTFIX?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The WONTFIX was for broken BlobBuilder. According to the bug:

Well, BlobBuilder appears to be deprecated (probably for quite a while now) and is undefined in current Linux Chrome dev M35. See comment #16 and http://updates.html5rocks.com/2012/06/Don-t-Build-Blobs-Construct-Them.

As attached, I modified the script in page.html to use the new Blob constructor instead, and the ogg file loads successfully and two sounds play, including when I replay.

And since we are using Blob constructor now there should no longer be an issue.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Yeah, that's probably right, then, though an earlier comment says

it happens with the Blob constructor too, not just the (deprecated) BlobBuilder
https://bugs.chromium.org/p/chromium/issues/detail?id=124926#c16

But the later comment says it works, and I do see it work locally when I try that modified page.html, so looks good.

err('Preloading file ' + that.name + ' failed');
}
err('Preloading file ' + that.name + ' failed');
}, false, true); // canOwn this data in the filesystem, it is a slide into the heap that will never change\n'''
create_data = '''// canOwn this data in the filesystem, it is a slide into the heap that will never change
Module['FS_createDataFile'](this.name, null, byteArray, true, true, true);
Expand Down