947487 - Generate js::Class structs for DOM proxies.

Status: RESOLVED FIXED

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

Description

[Eric Faust [:efaust]]

At present, we have a way more complicated structure than we need to for WebIDL stuff implemented as proxies. In particular, the DOMJSClass and DOMClass structs hanging from different places. This is because prior to bug 924720, there was no way to create custom js::Class structs corresponding to proxies.

Now that this is no longer the case, we can do away with DOMClass and unify proxy and non-proxy objects under DOMJSClass.

Comment 1

[Eric Faust [:efaust]]

Attachment: Part 1: Convert DOMJSClass::mBase from JSClass to js::Class

In order to generate classes for proxies, we need to generate js::Classes, not JSClasses. Make the appropriate changes.

Comment 2

[Eric Faust [:efaust]]

Attachment: Part 1: Convert DOMJSClass::mBase from JSClass to js::Class

With name changes discussed on IRC

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8344937 [details] [diff] [review]
Part 1: Convert DOMJSClass::mBase from JSClass to js::Class

r=me, but maybe the FromJSClass functions should be fixed too?  That is, do the cast in the one that takes js::Class and have the other call it?

Comment 4

[Eric Faust [:efaust]]

Attachment: Part 2: Generate and use js::Class structs for DOM proxies

Comment 5

[Eric Faust [:efaust]]

Attachment: Part 2: Generate and use js::Class structs for DOM proxies

Now with more audited IsDOMClass callers.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8368443 [details] [diff] [review]
Part 2: Generate and use js::Class structs for DOM proxies

>+++ b/dom/bindings/BindingUtils.cpp

In NativeToString, can we now drop the IsDOMProxy branch?  Seems like we can as long as clasp->name is correct for our DOM proxy classes, since the IsDOMClass() branch will do the right thing there.

>+++ b/dom/bindings/BindingUtils.h
>@@ -524,7 +528,7 @@
>-  if (dom::IsDOMClass(clasp)) {
>+  if (dom::IsNonProxyDOMClass(clasp)) {

Actully, if you leave that as IsDOMClass(clasp) but check for clasp->isProxy() (just like we check for global) before entering that inner branch, you could get rid of the end-of-method IsDOMProxy() call in this method and just return false there, right?

If we get rid of these two IsDOMProxy() callsites, the only remaining IsDOMProxy() uses will be in DOMJSProxyHandler.h/cpp, I believe.  And the two-argument version of IsDOMProxy will be dead code.

>diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py
>--- a/dom/bindings/Codegen.py
>+++ b/dom/bindings/Codegen.py
>+class CGDOMProxyJSClass(CGThing):
>+static const DOMJSClass Class = {
>+    PROXY_CLASS_DEF("DOMProxy",

How about self.descriptor.interface.identifier.name for the name so we can make the NativeToString stuff above work?

Also, outdent this by two spaces, please.

>+    %s
>+};
>+""" % CGIndenter(CGGeneric(DOMClass(self.descriptor))).define()

Outdent the %s all the way, please; the CGIndenter will indent the text as needed.  The string replacing that %s is multiline, so the current setup just gives the first line a weird indent.

The rest looks great.  r=me with the above nits.

Comment 7

[Eric Faust [:efaust]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bedbbb50c106
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ed63a08515a4

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/bedbbb50c106
https://hg.mozilla.org/mozilla-central/rev/ed63a08515a4