Inline workers broken in Firefox 8

VERIFIED FIXED in Firefox 9

Status

()

Core
DOM
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: Jussi Kalliokoski, Assigned: Ben Turner (not reading bugmail, use the needinfo flag!))

Tracking

(5 keywords)

8 Branch
mozilla10
x86_64
Linux
dev-doc-complete, qawanted, relnote, verified-aurora, verified-beta
Points:
---

Firefox Tracking Flags

(firefox8- affected, firefox9+ fixed)

Details

(Whiteboard: [qa!])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Build ID: 20111026191032

Steps to reproduce:

Go to http://labs.avd.io/sink.js/tests/callback.html (turn your volume down, if not using Firefox 8)

The site uses sink.js that is a library that provides a consistent API for audio output across Firefox and Chrome, and because of the timers clamping in Firefox 5+, I had to devise a mechanism that allowed me time accurate workers in background tabs as well, but didn't include the cruft of the end-developer having to include another JS file that the worker is loaded from. So, there is Sink.doInterval(callback, timeout) that attempts to create an inline worker using blob urls. It's try-catching everything, so if there's an error, it just goes with the regular timers. This worked in Firefox 6-7, however it no longer seems to work on Firefox 8. You can find out what doInterval does from https://github.com/jussi-kalliokoski/sink.js/blob/master/sink.js#L517 . This is related to an earlier bug related to the timers clamping change https://bugzilla.mozilla.org/show_bug.cgi?id=665000 . And just like that time, there's even no way to feature detect this.

I cannot stress enough the importance of this feature. If any application that uses the Audio Data API to play back some audio wants to do that also when in a background tab (any music streaming application, you don't keep them in the open tab, do you?), this is needed to work without making the codebase a horrible cruft. Just look at http://jsmad.org for example.

</rant> :)


Actual results:

[00:57:15.160] Failed to load script: moz-filedata:207126df-bbc2-46e1-8754-fa3ae9c0014c (nsresult = 0x80004002)


Expected results:

You should hear some terrible generated noise.

Comment 1

6 years ago
bent, could you please look at this.
Assignee: nobody → bent.mozilla
tracking-firefox10: --- → ?
tracking-firefox8: --- → ?
tracking-firefox9: --- → ?

Updated

6 years ago
Component: General → DOM
Product: Firefox → Core
QA Contact: general → general

Comment 2

6 years ago
I mean, this *looks like* a major worker problem, and workers were re-written in FF8.
(Reporter)

Comment 3

6 years ago
Yep, to me it looks like you can't create workers from blob urls anymore.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Created attachment 571891 [details] [diff] [review]
Patch, v1

This fixes us.
Attachment #571891 - Flags: review?(jonas)
(In reply to Jussi Kalliokoski from comment #3)
> Yep, to me it looks like you can't create workers from blob urls anymore.

Who knew this worked! Yikes.

The script loading part was almost totally unchanged from the last implementation, but in the old implementation I think we were a little more lax when calculating stuff for the location object.
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

We need tests too. You can create a Blob using the MozBlobBuilder interface.
Attachment #571891 - Flags: review?(jonas) → review+
(Reporter)

Comment 7

6 years ago
Created attachment 571941 [details]
A test for workers from blob URLs

I don't have the time to download the sources on my laptop right now and validate that the test works, but I think it should do the trick.
(Reporter)

Comment 8

6 years ago
Meh, I was sloppy and that test doesn't actually work, will put up a better one a bit later.
(Reporter)

Comment 9

6 years ago
Created attachment 571948 [details]
A test for workers from blob URLs, v2

Ok, now it works.
Attachment #571941 - Attachment is obsolete: true
(In reply to Jussi Kalliokoski from comment #9)
> Created attachment 571948 [details]
> A test for workers from blob URLs, v2

Thanks! I'll incorporate that test into the patch.
https://hg.mozilla.org/integration/mozilla-inbound/rev/936643625d71
Status: NEW → ASSIGNED
Whiteboard: [inbound]
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

Should we get this to branches.
It would be rather unfortunate to break all sorts of
audio web apps.
Attachment #571891 - Flags: approval-mozilla-beta?
Attachment #571891 - Flags: approval-mozilla-aurora?
Yes, I was just waiting to get it on m-c first.
Jussi, I wonder if we could get more tests for your audio libraries to prevent this kinds of
regressions. If you think you could write such tests, please file a new bug and attach the patch there.
(Reporter)

Comment 15

6 years ago
Sure a thing, Olli, it's actually kinda a part of my job these days. :)
https://hg.mozilla.org/mozilla-central/rev/936643625d71
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla10

Comment 17

6 years ago
[triage comment]

We'll definitely take this in aurora / Fx9. Fx 8 is shipping tomorrow. What's the fallout of shipping with this? Do you have any idea as to the extent of sites using workers with binary blobs?
status-firefox8: --- → affected
status-firefox9: --- → affected
tracking-firefox10: ? → ---
tracking-firefox8: ? → +
tracking-firefox9: ? → +

Comment 18

6 years ago
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

[triage comment]
Please land on mozilla-aurora today.
Attachment #571891 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 19

6 years ago
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

[triage comment]

We will not be respinning Firefox 8 for this based on the information we have. We have not heard about any high-volume web apps this is breaking, QA's tests are passing, and we haven't got feedback from our testing community saying we broke this. If you have examples that we should consider, please renominate. Denied for beta/Firefox 8.
Attachment #571891 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Updated

6 years ago
status-firefox8: affected → wontfix
(Reporter)

Comment 20

6 years ago
Nothing high volume I guess, but basically any audio app that relies on sink.js or audiolib.js (jsmad, the proof of js performance, though)... However, turns out html5rocks has a tutorial on the subject, so I have no idea if it's actually more widespread: http://www.html5rocks.com/en/tutorials/workers/basics/#toc-inlineworkers-bloburis .
(Reporter)

Comment 21

6 years ago
Oh yeah, also, anything using Audiolet ( https://github.com/oampo/Audiolet ) will be broken because it's using sink.js as well. It's breaking basicly most of the complex audio demos out there.
https://hg.mozilla.org/releases/mozilla-aurora/rev/9addd25bb730
status-firefox9: affected → fixed
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

In light of that, renominating.
Attachment #571891 - Flags: approval-mozilla-beta- → approval-mozilla-beta?
status-firefox8: wontfix → affected
We're shipping in hours, so you shouldn't be nominating for approval, you should be emailing release-drivers convincing them to pull the train's emergency brake.
Duplicate of this bug: 700626

Comment 26

6 years ago
Created attachment 572780 [details]
A test for workers from dataURI

Updated

6 years ago
Attachment #572780 - Attachment mime type: text/plain → text/html

Comment 27

6 years ago
Please try attachment: A test for workers from daraURI.html

Opera 11.5 - pass

Updated

6 years ago
Attachment #572780 - Attachment description: A test for workers from daraURI → A test for workers from dataURI
@bugzilla33 : That's bug 699857. This bug is about blob URLs which are created by createObjectURL.
Woops, I duped his bug to the wrong one :-/

Updated

6 years ago
Keywords: relnote

Updated

6 years ago
Keywords: dev-doc-needed
Duplicate of this bug: 701345
We want to take this as a ride-along for an 8.0.1 update.  We need to get some testing to make sure that the fix that we're taking works with the audio sites in question.  Can we please test with nightlies and once we have an 8.0.1 test build?
Keywords: qawanted
Jussi: if you download a build from http://nightly.mozilla.org, does everything work as it should there?

If you can verify that it does work, then we might be able to fix this for Firefox 8.0.1
Blocks: 701537
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

[Triage Comment]
Approved for beta. Please land ASAP. We're considering this for an 8.0.1 and want to make sure that it lands elsewhere first, to prevent surprises.
Attachment #571891 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Alex, this was landed on aurora before the merge on tuesday, so it's already on beta.

Do you want me to land it on mozilla-release?
(Reporter)

Comment 35

6 years ago
Jonas: I tried it and it appears to fix at least my case. :)
Awesome, thanks Jussi!
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

per comment #34.
Attachment #571891 - Flags: approval-mozilla-release?
No longer blocks: 701537

Updated

6 years ago
tracking-firefox8: + → -
Comment on attachment 571891 [details] [diff] [review]
Patch, v1

[Triage Comment]
Denying for release as we are no longer considering shipping 8.0.1 to all Firefox users - it will be a targeted release for those who are, or could potentially be, affected by bug 700835 or bug 691271.
Attachment #571891 - Flags: approval-mozilla-release? → approval-mozilla-release-
Whiteboard: [qa+]

Comment 39

6 years ago
Verfied as fixed on:
Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0 (20111116091359)
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111122 Firefox/11.0a1

The test case from comment 0 was used.

The automated test case (dom/workers/test/test_blobWorkers.html	) is also passing on al OSs:
https://tbpl.mozilla.org/php/getParsedLog.php?id=7486436&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=7485704&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=7485705&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=7484699&full=1&branch=mozilla-beta

https://tbpl.mozilla.org/php/getParsedLog.php?id=7516724&full=1&branch=mozilla-aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=7515744&full=1&branch=mozilla-aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=7516005&full=1&branch=mozilla-aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=7514311&full=1&branch=mozilla-aurora

https://tbpl.mozilla.org/php/getParsedLog.php?id=7528338&full=1&branch=mozilla-inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=7528321&full=1&branch=mozilla-inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=7529122&full=1&branch=mozilla-inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=7528050&full=1&branch=mozilla-inbound
Status: RESOLVED → VERIFIED
Keywords: verified-aurora, verified-beta
Whiteboard: [qa+] → [qa!]
Added a note to Firefox 9 for developers that these are working again.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.