900904 - Support webidl-only generated events

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

FYI, my current plan is to fix this on the week starting Aug 26

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This might allow removing nsIDOMBluetoothDevice.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Yes, and various other interfaces.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: TestEvent.webidl

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: TestEvent.h

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: TestEvent.cpp

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch v2

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v3 + BlobEvent as webidl only event

https://tbpl.mozilla.org/?tree=Try&rev=c1ed0a61bc24

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 799648 [details] [diff] [review]
v3 + BlobEvent as webidl only event

You can admire my beautiful python code.

Note, the codegen supports only rather basic types for now, since those
are the common ones. And no inheritance from other than Event interface, since 
that is also the common case.
And all the properties in dictionaries must have default values - since
that is the only reasonable thing for this kind of codegenerator.

I moved OwningNonNull to BindingDeclarations because bz asked to do that to
fix certain compilation problems.

I converted BlobEvent as an example.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 799648 [details] [diff] [review]
v3 + BlobEvent as webidl only event

A new patch with a small tweak coming.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v4

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 801490 [details] [diff] [review]
v4

Review of attachment 801490 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't review the actual code generator that hard.  I assumed that you knew what you were doing ;-)

::: dom/bindings/BindingDeclarations.h
@@ +611,5 @@
>    nsWrapperCache* const mWrapperCache;
>  };
>  
> +template<class T>
> +class OwningNonNull

Can you move this in a separate changeset?

::: dom/bindings/Codegen.py
@@ +10608,5 @@
> +                                                                   attr),
> +                                (attr.type, []),
> +                                ea)
> +
> +    def define(self, cgClass):

nit: put define at the end of the class.

@@ +10615,5 @@
> +        args = (', '.join([a.declare() for a in self.args]))
> +        body = self.getMethodBody()
> +        return ret + "\n" + methodName + '(' + args + ') const\n{\n' + body + "\n}\n"
> +
> +    def retval(self, type):

Why don't you just use CGNativeMember.getRetvalInfo (perhaps with some whitelisting of which types are allowed)?

@@ +10672,5 @@
> +            return "  return " + memberName + ";"
> +        if type.isDOMString() or type.isByteString():
> +            return "  aRetVal = " + memberName + ";";
> +        if type.isSpiderMonkeyInterface() or type.isObject():
> +            ret =  "  xpc_UnmarkGrayObject(%s);\n" % memberName

I removed xpc_UnmarkGrayObject last night.

@@ +10946,5 @@
> +
> +        # And now some include guards
> +        self.root = CGIncludeGuard(interfaceName, self.root)
> +
> +        # And our license block comes before everything else

Use AUTOGENERATED_WARNING_COMMENT too.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #12)
> I didn't review the actual code generator that hard.  I assumed that you
> knew what you were doing ;-)
oh dear.

> 
> ::: dom/bindings/BindingDeclarations.h
> @@ +611,5 @@
> >    nsWrapperCache* const mWrapperCache;
> >  };
> >  
> > +template<class T>
> > +class OwningNonNull
> 
> Can you move this in a separate changeset?
Not sure why.

> > +    def retval(self, type):
> 
> Why don't you just use CGNativeMember.getRetvalInfo (perhaps with some
> whitelisting of which types are allowed)?
Well, it is easier to see in one place what all the codegen supports.
I tried to use it at some point but it got messy

> I removed xpc_UnmarkGrayObject last night.
Yes. ugh.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

> > @@ +611,5 @@
> > >    nsWrapperCache* const mWrapperCache;
> > >  };
> > >  
> > > +template<class T>
> > > +class OwningNonNull
> > 
> > Can you move this in a separate changeset?
> Not sure why.
Looks like someone else did that change already.

Comment 15

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 801490 [details] [diff] [review]
v4

Review of attachment 801490 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/BindingGen.py
@@ +66,5 @@
>              file.close()
>          return contents
>      allWebIDLFiles = readFile(args[2]).split()
> +    generatedEventsWebIDLFiles = readFile(args[3]).split()
> +    changedDeps = readFile(args[4]).split()

Please update the usagestring variable above

::: dom/bindings/BindingUtils.h
@@ -1440,5 @@
>                         nsTArray<nsString>& names,
>                         bool shadowPrototypeProperties, JS::AutoIdVector& props);
>  
> -template<class T>
> -class OwningNonNull

This is in OwningNonNull.h now.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v5

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Attachment: handle generated_events_webidl_files

I think this builds. I tried to copy paste what is done with normal webidl files.

Comment 18

[Gregory Szorc [:gps]]

Comment on attachment 802478 [details] [diff] [review]
handle generated_events_webidl_files

Review of attachment 802478 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/frontend/sandbox_symbols.py
@@ +401,5 @@
>  
> +    'GENERATED_EVENTS_WEBIDL_FILES': (StrictOrderingOnAppendList, list, [],
> +        """WebIDL source files for generated events.
> +
> +        These will be parsed and converted to .cpp and .h files. 

Nit: Trailing whitespace.

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v6

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6db45ff38334

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6db45ff38334