OK I decided that I'm going to implement decodeAudioData first: void decodeAudioData(in ArrayBuffer audioData, in [Callback] AudioBufferCallback successCallback, in [Optional, Callback] AudioBufferCallback errorCallback) raises(DOMException); I'm not sure how I'm supposed to represent AudioBufferCallback in WebIDL. Is it OK if I just use Function?

I think you can do something similar to EventListener.webidl.

You have two options. You can either use a callback interface or a callback. They differ slightly in that the latter is always just a function but the former is an object implementing some interface with possibly multiple functions hanging off of it. Since this spec is not using WebIDL (something it should _so_ fix), it's hard to tell which it means. EventListener is a callback interface, since it's defined that way in the spec. Function is a callback which tells you nothing about its behavior. Given the lack of interface definition for AudioBufferCallback, it sounds like the spec really means a callback, not a callback interface. But it doesn't define the arguments it takes or anything (which it should, and would have to if this were WebIDL). Do you want to send that feedback on the spec, or should I? If you use a callback, you'll just get the raw JSObject* for the callable. Unfortunately, actually _calling_ it safely is a bit of rocket science. This was OK for the one use case we've had for it so far (on* attributes) because those have all sort of machinery for it, but we should come up with a better setup for it. Maybe we need some C++ object for representing callbacks which will handle all the tracing and safe call machinery for us... You path of least resistance for now is probably to make this a callback interface, with some made-up method name and signature depending on what you plan to pass in. :(

(In reply to Boris Zbarsky (:bz) from comment #3) > If you use a callback, you'll just get the raw JSObject* for the callable. C++ implementation of some interface shouldn't need to deal with JS API > Unfortunately, actually _calling_ it safely is a bit of rocket science. > This was OK for the one use case we've had for it so far (on* attributes) > because those have all sort of machinery for it, but we should come up with > a better setup for it. Maybe we need some C++ object for representing > callbacks which will handle all the tracing and safe call machinery for us... ...so I think this would be good. Once bholley removes the need for context pushing, the rocket science part of callback handling should be gone :)

(In reply to comment #3) > You have two options. > > You can either use a callback interface or a callback. They differ slightly in > that the latter is always just a function but the former is an object > implementing some interface with possibly multiple functions hanging off of it. > > Since this spec is not using WebIDL (something it should _so_ fix), it's hard > to tell which it means. I'm planning to improve the situation here by submitting at least the simple Web IDL fixes to the spec (the ones that don't actually depend on the spec itself being fixed, such as this one.) > EventListener is a callback interface, since it's defined that way in the spec. > Function is a callback which tells you nothing about its behavior. > > Given the lack of interface definition for AudioBufferCallback, it sounds like > the spec really means a callback, not a callback interface. But it doesn't > define the arguments it takes or anything (which it should, and would have to > if this were WebIDL). Do you want to send that feedback on the spec, or should > I? I will raise the spec issue. But if there is only one method on the callback interface, is there any reason why one would choose one but not the other? > If you use a callback, you'll just get the raw JSObject* for the callable. > Unfortunately, actually _calling_ it safely is a bit of rocket science. This > was OK for the one use case we've had for it so far (on* attributes) because > those have all sort of machinery for it, but we should come up with a better > setup for it. Maybe we need some C++ object for representing callbacks which > will handle all the tracing and safe call machinery for us... Yeah I think that is a good idea. Where can I find out about what that rocket science is so that I can use it? Also, wouldn't it make sense for this magic to be handled by the bindings generator so that the C++ implementation could just accept a raw function pointer? > You path of least resistance for now is probably to make this a callback > interface, with some made-up method name and signature depending on what you > plan to pass in. :( That's sad. :(

> But if there is only one method on the callback interface, is there any reason why one > would choose one but not the other? Yes. It depends on whether there are use cases where an object implements multiple callback interfaces and is used as a central event-dispatching mechanism by the web page. There have been some bitter philosophical debates about this, with some people talking their book (e.g. some implementations have a hard time implementing one or the other, and the people working on those pretend like those problems are universal). That's in spec terms. I assume you were asking about those, not our implementation terms, right? > Where can I find out about what that rocket science is so that I can use it? Probably by talking to smaug is best... > wouldn't it make sense for this magic to be handled by the bindings generator so that > the C++ implementation could just accept a raw function pointer? You can't do a function pointer, because you have to have an ownership model for the underlying JS function. But you could have it be represented by some sort of interface with a virtual Run() method or whatnot. Could even use nsIRunnable. And yes, the "C++ object" thing I was talking about above would be handled by the bindings; the C++ consumer would just get an nsIFoo* of some sort.

(In reply to comment #6) > > But if there is only one method on the callback interface, is there any reason why one > > would choose one but not the other? > > Yes. It depends on whether there are use cases where an object implements > multiple callback interfaces and is used as a central event-dispatching > mechanism by the web page. > > There have been some bitter philosophical debates about this, with some people > talking their book (e.g. some implementations have a hard time implementing one > or the other, and the people working on those pretend like those problems are > universal). > > That's in spec terms. I assume you were asking about those, not our > implementation terms, right? Correct. Thanks! > > wouldn't it make sense for this magic to be handled by the bindings generator so that > > the C++ implementation could just accept a raw function pointer? > > You can't do a function pointer, because you have to have an ownership model > for the underlying JS function. But you could have it be represented by some > sort of interface with a virtual Run() method or whatnot. Could even use > nsIRunnable. And yes, the "C++ object" thing I was talking about above would > be handled by the bindings; the C++ consumer would just get an nsIFoo* of some > sort. OK, that makes sense to me. And since I want to make some progress here, I guess I'll just use something like EventListener for now.

Note to myself: need to set up an nsCxPusher before calling the JS function like here: <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMMutationObserver.cpp#657>

OK, for some reason I ran into a lot of problems with setting up the basics, but I think I have made most things work correctly now. I decided that I would repurpose this bug to only implement the mozAudioContext interface and nothing else. Here are the remaining problems: * I am experiencing intermittent crashes, mostly in unfamiliar code such as XPCIncrementalReleaseRunnable::ReleaseNow. This tells me that I have set up the DOM bindings incorrectly somehow, but I'm not quite sure why. * I have set up the object to be prefable, but |new mozAudioContext()| in the web console happily creates one such object for me! I think that is not expected. * The build system parts were tough to set up and I'm not sure if I got them all correctly. Now the patch builds and links, but I'd like to have Kyle's review on those parts (if he's not interested in the rest, that is!) * When I type in "mozAudioContext" in the web console, I don't get suggestions like I do for many other things. I'm not sure what I need to hook up in order to get those... Boris, can you please check over the DOM bindings bits to make sure I'm not doing something drastically stupid? Thanks!

Let's remove [games:p1] from the whiteboard. We know that web audio is a priority, and I'm planning to do the work piecemeal in small bugs, so no sense in the [games:p1] notation on individual bugs unless one of them is particularly important for some reason.

Sorry, wrong patch!

Comment on attachment 655212 [details] [diff] [review] Patch (v1) You should probably add nsISupports to your QI impl. It looks like we have other things in the tree that make the same mistake. :( Your crashes are probably because you don't addref the return value in AudioContext::Constructor. The "'prefable':True" just means the overall pref for disabling the prefable bindings will disable it. So the fact that |new mozAudioContext| works makes sense. There are some typos in the webidl header comment, both in the MPL ("obtaone") and in the origin of part ("origof"). The suggestion algorithm in the web console doesn't seem to see XMLHttpRequest either. Sounds like something specific to the new bindings, possibly worth filing.

(In reply to comment #12) > The suggestion algorithm in the web console doesn't seem to see XMLHttpRequest > either. Sounds like something specific to the new bindings, possibly worth > filing. Filed bug 786001.

Requesting review from Kyle on the build system parts, and from Boris on the rest.

Comment on attachment 655742 [details] [diff] [review] Patch (v2) r=me

Comment on attachment 655742 [details] [diff] [review] Patch (v2) Review of attachment 655742 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/Makefile.in @@ +13,5 @@ > +MODULE := content > +LIBRARY_NAME := gkconwebaudio_s > +LIBXUL_LIBRARY := 1 > + > + Double newline ::: dom/bindings/Makefile.in @@ +72,5 @@ > LOCAL_INCLUDES += -I$(topsrcdir)/js/xpconnect/src \ > -I$(topsrcdir)/js/xpconnect/wrappers \ > -I$(topsrcdir)/content/canvas/src \ > + -I$(topsrcdir)/content/html/content/src \ > + -I$(topsrcdir)/content/media/webaudio Boo. Put this in dom/dom-config.mk instead.

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