Open Bug 986455 Opened 6 years ago Updated 9 months ago

Support implementing part of C++ WebIDL interfaces in JS

Categories

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

x86
macOS
defect

Tracking

()

People

(Reporter: ehsan, Unassigned)

References

(Blocks 1 open bug)

Details

For the payment API, we need to support doing something like:

$ git diff
diff --git a/dom/webidl/Navigator.webidl b/dom/webidl/Navigator.webidl
index f98c3e6..443a5dc 100644
--- a/dom/webidl/Navigator.webidl
+++ b/dom/webidl/Navigator.webidl
@@ -354,3 +354,12 @@ partial interface Navigator {
   boolean sendBeacon(DOMString url,
                      optional (ArrayBufferView or Blob or DOMString or FormData)? data = null);
 };
+
+[JSImplementation="@mozilla.org/payment/content-helper;1",
+ Pref="dom.mozPay.enabled",
+ NoInterfaceObject]
+interface NavigatorPayment {
+  DOMRequest mozPay(DOMString jwt);
+  DOMRequest mozPay(sequence<DOMString> jwts);
+};
+Navigator implements NavigatorPayment;

Currently the JSImplementation bit is ignored and the generated binding code would try to call Navigator::MozPay().
So... we've thought about this on and off, but it's not a trivial problem.  :(  Let me summarize the considerations as I see them:

1)  We want to have this not require manual changes to Navigator, if possible.
2)  The bindings code currently has a single C++ object to work with.  For the Navigator
    interface, that's the Navigator object.
3)  That implies that the Navigator object or some superclass needs to have those methods. 
    This requires C++ changes no matter what.  And if it's a superclass it needs something
    where a Navigator has some object with very nontrivial construct/init behavior (e.g. a
    codegenned js-implemented webidl object) as the superclass.  This means changes to how
    a Navigator is constructed, which is more nontrivial C++.
4)  Alternately, we come up with some sort of new binding method that does not just call
    into the C++ object for the binding.  We have some support for calling self-hosted
    code, for example, and could try something similar here.  This needs various
    infrastructure work (e.g. for places where the code would live) and some decisions
    about which compartment and hence which principal should be associated with that
    code...
5)  Also alternately, if this sort of thing will be limited to Navigator, we could do some
    C++ work once to create a codegenned superclass of Navigator that lazily instantiates
    the various JS contracts needed for the various methods and forwards the calls along.

Any other ideas for how this could work are welcome, by the way.
Also, I could have sworn we had an existing bug on this, filed back when I saw <https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/XXyHw-X841A>, but I can't find it.
Hmm, I was thinking about some kind of a tear-off object that gets constructed by the binding code on demand when such a method is called, and is then attached to the native object for later retrieval.  This means that the binding code needs to be aware of the kind of interface a WebIDL method is declared on (being able to determine whether it's a normal interface, or an "implements" interface which has a [JSImplementation]), and in the case of a JS interface, it would query the native object for the tear-off object implementing that part.  The C++ tear-off object would be codegened similar to the way that the current C++ wrapper objects for JS implemented stuff are codegened.  This means that we can have a base-class sketched like the below:

template<class JSImplWrapper>
class JSTearOffHolder {
  T* mTearOff;
public:
  T* GetTearOff() const { return mTearOff; }
  void CacheTearOff(T* p) { mTearOff = p; }
};

This way the construction of the tear-off class can be handled by the bindings layer, and the only change to the C++ class would be to make it inherit from JSTearOffHolder<NavigatorPaymentBinding>, and have the tear-off participate in CC etc.  We also won't need to change anything in the construction of the native object itself.

We can add something similar to __DOM_IMPL__ on the chrome JS object for accessing the real object from the JS side of things, so that if that code wants to call navigator.foo, it can do:

this.__NATIVE_THIS__.foo();

This is of course lacking tons of details, I'm sure.  But does this seem realistic?
Flags: needinfo?(bzbarsky)
Hmm.  I guess that's true; we could hack up the "self" stuff to get some member off the C++ object.  It would be a bit ugly in the codegen, but we could do it.

We would still need C++ changes the first time we added a JS-implemented method to the object; I'd been slightly hoping we could avoid that.

> This means that the binding code needs to be aware of the kind of interface a WebIDL
> method is declared on

This would be fairly simple to do.  In fact, we could just allow annotating individual methods directly with a contractid, as long as it was the same contractid for everything on the interface, which I think your proposal assumes.

> We can add something similar to __DOM_IMPL__ on the chrome JS object

Not similar, just the exact same thing.  We'd just need to set it to the Navigator object in this case.  We should consider renaming __DOM_IMPL__ to __NATIVE_THIS__!  I like that name.
Flags: needinfo?(bzbarsky)
(In reply to comment #4)
> Hmm.  I guess that's true; we could hack up the "self" stuff to get some member
> off the C++ object.  It would be a bit ugly in the codegen, but we could do it.
> 
> We would still need C++ changes the first time we added a JS-implemented method
> to the object; I'd been slightly hoping we could avoid that.

That is impossible to avoid, I think...

> > This means that the binding code needs to be aware of the kind of interface a WebIDL
> > method is declared on
> 
> This would be fairly simple to do.  In fact, we could just allow annotating
> individual methods directly with a contractid, as long as it was the same
> contractid for everything on the interface, which I think your proposal
> assumes.

Yes, I think the two would be identical in terms of the usability of this.  But is there a good reason to support putting the contractid on the methods?  Perhaps for partial interfaces you mean?

> > We can add something similar to __DOM_IMPL__ on the chrome JS object
> 
> Not similar, just the exact same thing.  We'd just need to set it to the
> Navigator object in this case.  We should consider renaming __DOM_IMPL__ to
> __NATIVE_THIS__!  I like that name.

Haha yes, you're right!  FWIW __NATIVE_THIS__ is a much better name, see how __DOM_IMPL__ just confused me?  :-)
> But is there a good reason to support putting the contractid on the methods? 

It seems simple to use.  In practice, having the methods come via "implements" from an interface flagged with the contract would just desugar to flagging the contract on the methods themselves...
Duplicate of this bug: 1019777
Blocks: 1027734
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.