Closed Bug 785193 Opened 9 years ago Closed 9 years ago

Autogenerate glue to allow XPIDL to implement basic WebIDL interfaces


(Core :: DOM: Core & HTML, defect)

17 Branch
Not set





(Reporter: mccr8, Assigned: mccr8)



(3 files, 1 obsolete file)

- Generate an XPIDL interface from a WebIDL interface.

- Generate a glue class that holds a reference to an implementation of the XPIDL interface and implements the WebIDL interface using inline methods.

This is one part of allowing a WebIDL interface to be implemented by JS, while having XPCOM implement all of the gross and dangerous stuff.

For now, ignore more complex WebIDL things like unions.
I implemented a very primitive WebIDL-to-XPIDL translator that sort of works for ClientRectList
Attached patch example diffs (obsolete) — Splinter Review
This patch shows the diffs between what is in the tree and what my scripts generate, for two simple classes, ClientRectList and TreeWalker. The latter doesn't seem to be built and wasn't quite working, so I manually patched up the WebIDL.
I added a crude RTCPeerConnection.webidl (though I didn't try to do callbacks) that is now parseable into a reasonable XPIDL.
Attachment #677850 - Attachment is obsolete: true
Comment on attachment 677930 [details] [diff] [review]
sort of added support for RTCPeerConnection.webidl

Is there any spec webidl you can use here?

Depending on how they're used the jsval arguments should either be "any" or dictionaries...

Includes are a bit of a pain.  What will happen when your xpidl needs to take a WebIDL type?  We'll probably need a type mapping from WebIDL typenames to xpcom ones or something?

webidl_files is alphabetically sorted.
> Is there any spec webidl you can use here?

Yes, there's one here:

It differs slightly from the existing XPIDL version in ways I don't understand. Maybe I am not looking at the right version of the spec.

> Depending on how they're used the jsval arguments should either be "any" or dictionaries...

Ah, great. It looked like these were being used as mappings from strings to values, which I'm assuming is a dictionary.
Hrm.  That spec is not all that useful; e.g. it never defines MediaConstraints.

It's the current editor's version of the spec, though, so the latest thing.

> which I'm assuming is a dictionary.

Yes, if the set of strings is not open-ended.
I started manually looking at writing the C++ wrapper class.  What do you think should be done if the XPIDL class has a failure in an accessor that is infallible in WebIDL?
Either whatever happened in the XPIDL should cause an exception to be thrown to the thing calling into WebIDL (in which case it shouldn't be infallible; since we are the ones deciding whether something is infallible, and we base it on our implementation, we can set it as needed), or the XPIDL failure gets swallowed somehow (e.g. ignoring an rv, possibly reporting any pending exceptions if that's needed).
Cc:ing Anant who may be able to help sort out issues with the WebIDL in the WebRTC spec.
Attachment #678971 - Attachment description: manually edited simple implementation → manually edited simple implementation, header
I used the example generator to generate a bindings for this simple webidl class:

interface Simple
  attribute boolean field;

Then I manually edited it to turn it into a wrapper class, by adding a field nsCOMPtr<nsIDOMSimple>. The getter/setter methods call the corresponding getter/setter on the nsIDOMSimple, passing around errors as needed. I also made it WRAPPER_CACHE_1 with the new field.
Just so you know, example gen will get more reusable in bug 779048.  At that point we might be able to subclass it and use it here as needed...
Great. I was planning to use it somehow, as it seems to be just the thing.
The patches in that bug should apply directly to inbound at this point, so if you want to just import them and experiment, go for it...  Or I can publish my mq somewhere if you prefer.  Let me know!
I've started looking at this again. Sorry for the delay.

I don't really know much about proper Python OO style. I was going to add methods  makeGetter(self, descriptor, m) and makeSetter in CGExampleClass that just calls the underlying CGExampleGetter(descriptor, m) or Setter, that can be overloaded in subclasses to change how the body is generated. Does that sound reasonable?

What's a good way to add the nsCOMPtr field to the object? Right now, I'm stuffing it into extra declarations, which is a little sad because it is added to the start rather than the end of the class.

Next I'm going to try to understand how all the error munging code works, to try to figure out how to actually generate the body of the getter/setters.
The makeGetter/etc sounds fine to me, I think.  I'd have to see what it actually ends up looking like.

> What's a good way to add the nsCOMPtr field to the object?

Pass members=[ClassMember("mFoo", "nsCOMPtr<nsIWhatever>")] to the CGClass init method.
> Next I'm going to try to understand how all the error munging code works

Oh, which error munging code?
Boris has some kind of alternate plan to utilize the callback generator code, instead of using an XPIDL intermediate layer, which has the advantage that it will be able to deal with the various WebIDL stuff that XPIDL doesn't have.
Closed: 9 years ago
Resolution: --- → WONTFIX
So even with that approach we'll still need a bit of work on top of bug 822470 to do that.  In particular, we'd need to do callback-like codegen for non-callback interfaces that we want to implement in JS.  And possibly do codegen for the actual implementation class...

Not sure whether you want to do that here or in a new bug, but we should have a bug tracking it.
Yes, I was thinking a new bug would be better.

I'll be sure to set aside a few days a week for this when I get back from vacation so it doesn't end up getting ignored...
Sounds good.  With any luck, doing this on top of callbacks will be a lot simpler than doing it on top of xpidl...
Filed bug 827486 for that. I'll start working on understanding the callback codegen stuff this week.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.