[webkit-dev] Request for comments on some bindings generator patches

[Nikos Andronikos]

Hey all,

We’ve got some web animations related patches that are stalled waiting on feedback about options to update the bindings generator.

There’s currently three patches waiting to be reviewed, one Web Animations, two binding generator fixes.

Patch 1. Web Animations: https://bugs.webkit.org/show_bug.cgi?id=156096
Dean already gave this one r+. But it ended up causing a build issue when Web Animations was disabled, and was rolled back.
So it’s now waiting on the resolution of one of the other patches below.

Patch 2. Our preferred solution to the build issue that occurs with patch 1 is to add support for the Conditional extended attribute on the implements WebIDL statement. Then we can put a conditional on the implements statement that adds the Animatable interface to Element.
My colleague Rawinder did this in the following patch:
https://bugs.webkit.org/show_bug.cgi?id=158830
This patch stops the IDL preprocessor from looking for the IDL file at all, and all is well.

Patch 3: Another option to address the build issue is to move the Animatable.idl file out of the conditional in the CMake file list, so that the IDL preprocessor can find it and include it as a supplemental file.
This _should_ be ok, because the internals of that file are protected by conditionals.
But there is another bug where the preprocessor is trying to include the file that defines the return type for Element.getAnimations(), which is sequence<WebAnimations>. The IDL file that defines the WebAnimations type is still hidden by a conditional in the CMake file list.
Rawinder submitted a patch to fix that as well: https://bugs.webkit.org/show_bug.cgi?id=158975
We’re ok with this solution, but don’t feel it’s as elegant as enabling conditionals on the implement statement.
This option copies Animatable.idl to DerivedSources, but at least it doesn’t generate any unused bindings code.

Another option is to move all the Web Animations IDL files out of the conditional in the CMake file list.
This seems really clunky and inefficient to me as it involves unnecessary work by the IDL preprocessor and generates unused bindings code. There are also problems with this solution, caused by the GObject bindings, that would need to be investigated and fix.
We could look at this, but because it’s not our preferred solution, are hesitant to get started. There’s a history of this sort of investigation being a deep rabbit hole that sucks up lots of time. I think we have enough to do with Web Animations by itself at this point.

It would be great to get some input on these bugs so we can continue with the web animations implementation.

Thanks!
Nikos The information contained in this email message and any attachments may be confidential and may also be the subject to legal professional privilege. If you are not the intended recipient, any use, interference with, disclosure or copying of this material is unauthorised and prohibited. If you have received this email in error, please immediately advise the sender by return email and delete the information from your system.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20160720/0c8fda6c/attachment.html>