Do not add secondary interfaces implemented by parents.

Do not emit members of parent EventTarget interface.

Currently WebKit mixes EventTarget in.

R=podivilov@chromium.org,sra@google.com

Committed: https://code.google.com/p/dart/source/detail?r=14288

[Anton Muhin, 2012-10-29 18:21:06]

A bit of background: v8 do not generate clone bindings in this case I was hit by
that with the most recent Chrome roll (see
https://codereview.chromium.org/11301022/)

[Anton Muhin, 2012-10-30 13:34:06]

ptal

[podivilov, 2012-10-30 13:43:49]

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
File lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
lib/html/scripts/systemhtml.py:572: while
self._database.HasInterface(parent.type.id):
EventTarget is an extended attribute in idls, not an implemented interface. We
currently add it to interface hierarchy, which is a hack and should be fixed.
Given that, adding EventTarget to parents only if interface is EventTarget and
its parent is not an EventTarget seems like a better solution.

[Anton Muhin, 2012-10-30 13:45:08]

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
File lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
lib/html/scripts/systemhtml.py:572: while
self._database.HasInterface(parent.type.id):
I am not sure it's a hack.  And even though currently it only helps for
EventTargets, I still think it's superior approach---we shouldn't emit
additional members.

On 2012/10/30 13:43:49, podivilov wrote:
> EventTarget is an extended attribute in idls, not an implemented interface. We
> currently add it to interface hierarchy, which is a hack and should be fixed.
> Given that, adding EventTarget to parents only if interface is EventTarget and
> its parent is not an EventTarget seems like a better solution.

[podivilov, 2012-10-30 13:55:42]

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
File lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
lib/html/scripts/systemhtml.py:572: while
self._database.HasInterface(parent.type.id):
On 2012/10/30 13:45:09, Anton Muhin wrote:
> I am not sure it's a hack.  And even though currently it only helps for
> EventTargets, I still think it's superior approach---we shouldn't emit
> additional members.
> 
It's a hack because we agreed that IDL AST should represent what's written in
idls. I'm not sure we shouldn't emit additional members, as it will break
bindings for non-virtual methods.

[Anton Muhin, 2012-10-30 14:05:01]

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
File lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
lib/html/scripts/systemhtml.py:572: while
self._database.HasInterface(parent.type.id):
WebKit IDLs has many peculiarities, for example, according to
https://developer.mozilla.org/en-US/docs/IndexedDB/IDBRequest IDBRequest should
inherit from EventTarget.

So it's rather irregular way in which information about EventTarget is
represented in WebKit IDLs than a hack.

In the similar way one can always make subinterface extend an interface which
parent implements.

I remember I was concerned about non-virtual methods too, but thinking more
about it I am not sure if it should be treated as another peculiar thing about
WebKit or not---I believe in the cases where methods are overridden by IDLs,
they should be virtual in C++ as per Web IDL semantics.

On 2012/10/30 13:55:42, podivilov wrote:
> On 2012/10/30 13:45:09, Anton Muhin wrote:
> > I am not sure it's a hack.  And even though currently it only helps for
> > EventTargets, I still think it's superior approach---we shouldn't emit
> > additional members.
> > 
> It's a hack because we agreed that IDL AST should represent what's written in
> idls. I'm not sure we shouldn't emit additional members, as it will break
> bindings for non-virtual methods.

[podivilov, 2012-10-30 14:12:15]

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
File lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11340013/diff/4001/lib/html/scripts/systemhtm...
lib/html/scripts/systemhtml.py:572: while
self._database.HasInterface(parent.type.id):
I think the rule of thumb is we shouldn't be more clever than
CodeGeneratorV8.pm, otherwise we'll be beaten one day. Adding non-trivial logic
in places such as this only make future debugging more painful. If you
explicitly state that you are adding an exception only for EventTarget, this
code will be easier to find, understand and modify.

[Anton Muhin, 2012-10-30 15:00:09]

PTAL

That also removes dispatchEvent from ScriptProcessorNode which is probably an
error in WebKit IDLs.

[podivilov, 2012-10-30 15:01:14]

lgtm

[sra1, 2012-10-30 20:12:29]

I think this is a step in the right direction, but why does it remove only 4 of
the 30+ ?

What I would like to see is only one Impl class (_EventTargetImpl) with an
implementation of addEventListener etc.

My understanding is that our friends working on WebKit would like EventTarget to
be a real base class, as that is the way it is in w3 IDL.

This would have a nice size benefit since there are still 30x3 redundant
methods.
Removing 90 redundant methods will surely benefit snapshot size and I estimate
it will take another 20k off the size of swarm compiled with dart2js.

Is this something we can do now?

Even if it is not possible to remove all the extra EventTarget methods for
Dartium I want to remove all from dart2js.