With: user_pref("media.webaudio.enabled", true); The testcase causes a crash during shutdown.

On Windows: bp-7fa18288-bcdc-4686-be9a-c5fe92121104

Oh, cute. In that testcase, we enter AudioBuffer::InitializeBuffers, call NS_HOLD_JS_OBJECTS, then don't add anything to mChannels. So our destructor never calls NS_DROP_JS_OBJECTS. What we should probably do is hold only after we add the first element to the array or something...

Shouldn't we throw in this case? It makes no sense to create a buffer with 0 channels.

I agree it makes no sense. Spec says nothing about throwing... But more importantly, we'd get the same failure mode if we fail to SetCapacity or to insert the first object, no? It wouldn't matter that we threw, since we would have done the HOLD already...

There are a bunch of edge cases here for the current DROP condition, that maybe can't occur with the current usage of this class. What do you do if somebody initializes an AudioBuffer more than once? That could eliminate all of the channels, so we'll miss the drop. What happens if inserting the first object succeeds, but a later one fails? If we HOLD at the end of the function, then we'll have a dangling pointer (though with the current usage, that is okay because we don't use the object after failure).

Can we just HOLD unconditionally in the constructor and DROP unconditionally in the destructor? Looking at the code, it seems like in practice what usually happens is that we call the constructor, then initialize it with a non-zero number of buffers, so we'll basically always need to call HOLD. I wonder if I should change the behavior of DROP to allow failure in XPCJSRuntime in the case where something isn't in the table, then only update the global state in the nsContentUtils function. I've seen bloat logs where the HOLD-DROP count has underflowed, which would happen if we DROP too much.

> then only update the global state in the nsContentUtils function ...only update the global state if the XPCJSR drop succeeds, that is.

I'll raise a spec issue about the 0-channel madness.

Comment on attachment 678391 [details] [diff] [review] Patch (v1) Review of attachment 678391 [details] [diff] [review]: ----------------------------------------------------------------- woot!

Hmm. Okay, maybe this isn't quite right. The JS pointers need to be nulled out.

Comment on attachment 678391 [details] [diff] [review] Patch (v1) The problem that I was concerned about was that we could trigger Unlink without nulling out all of the pointers to JS, so we'd end up keeping JS alive, which maybe could cause us to leak. But the |NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMARRAY(mChannels)| deletes all the JS pointers so it should be okay! My apologies.

(well, the unlink plus the wrapper cache magic)

Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.

I got this error by just clicking on a link (to reliable site) in a electronic mail. And it is not non-reproducible, totally random: https://crash-stats.mozilla.com/report/index/600b3122-91be-4fed-8769-5398a2140512

zxspectrum3579, sometimes we can trace down crashes based on stacks, but that tends not to be the case for crashes in the JS GC. Sorry. (In the future, please file new bugs instead of commenting on old FIXED bugs.)