Unfork html events generation in dart2js and dartium.

Unfork html events generation in dart2js and dartium.


R=antonm@google.com,sra@google.com

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

[Anton Muhin, 2012-07-02 10:49:47]

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:882: if self._backend.HasImplementation():
is it the only place we have this sequence of statements?  should we factor it
away or it's idiomatic enough?

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:1038: "  EventListenerList get $NAME() =>
this['$DOM_NAME'];\n",
nit: do we need trailing \n?

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:1054: '\n  $TYPE get on() =>\n    new
$TYPE(this);\n',
nit: two more spaces to make it four indent for new expression, please

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
File lib/dom/scripts/systemnative.py (right):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:203: def AddMergedMembers(self,
merged_interface):
why it doesn't need any implementation?

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
File lib/html/dartium/html_dartium.dart (left):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
lib/html/dartium/html_dartium.dart:70: void $dom_addEventListener(type,
listener, [useCapture = _null]) {
those methods are apparently can be inherited from _EventTargetImpl right now. 
Can it be fixed in this CL?

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
File lib/html/dartium/html_dartium.dart (right):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
lib/html/dartium/html_dartium.dart:5002: +  EventListenerList get
contentLoaded() => this['DOMContentLoaded'];
why we got this added?  Do we really need it given our isolation model?

[podivilov, 2012-07-02 17:08:18]

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:882: if self._backend.HasImplementation():
On 2012/07/02 10:49:48, antonmuhin wrote:
> is it the only place we have this sequence of statements?  should we factor it
> away or it's idiomatic enough?

Let me fix minor nits at the end when the dust settles.

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:1038: "  EventListenerList get $NAME() =>
this['$DOM_NAME'];\n",
On 2012/07/02 10:49:48, antonmuhin wrote:
> nit: do we need trailing \n?

I'm trying to minimize the noise in generated files. This code is moved from
HtmlFrogClassGenerator, and I don't want to introduce additional whitespace
changes in html_frog lib in this cl.

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:1054: '\n  $TYPE get on() =>\n    new
$TYPE(this);\n',
On 2012/07/02 10:49:48, antonmuhin wrote:
> nit: two more spaces to make it four indent for new expression, please

ditto, let me fix this in a separate change.

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
File lib/dom/scripts/systemnative.py (right):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:203: def AddMergedMembers(self,
merged_interface):
On 2012/07/02 10:49:48, antonmuhin wrote:
> why it doesn't need any implementation?

Underlying c++ object doesn't implement merged methods.  Added comment
explaining this

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
File lib/html/dartium/html_dartium.dart (left):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
lib/html/dartium/html_dartium.dart:70: void $dom_addEventListener(type,
listener, [useCapture = _null]) {
On 2012/07/02 10:49:48, antonmuhin wrote:
> those methods are apparently can be inherited from _EventTargetImpl right now.

> Can it be fixed in this CL?

Why should we bother eliminating inherited methods? Sometimes they are not even
virtual in c++. AFAIK in V8 bindings such methods aren't eliminated.

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
File lib/html/dartium/html_dartium.dart (right):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
lib/html/dartium/html_dartium.dart:5002: +  EventListenerList get
contentLoaded() => this['DOMContentLoaded'];
On 2012/07/02 10:49:48, antonmuhin wrote:
> why we got this added?  Do we really need it given our isolation model?

It's a result of unforking. Dart2js has them, so should we. At least for now.

[Anton Muhin, 2012-07-02 17:14:09]

lgtm

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
File lib/html/dartium/html_dartium.dart (left):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
lib/html/dartium/html_dartium.dart:70: void $dom_addEventListener(type,
listener, [useCapture = _null]) {
To reduce the size of the snapshot?

Do you know examples of non-virtual methods?  That smells pretty much like a bug
to me.

On 2012/07/02 17:08:18, podivilov wrote:
> On 2012/07/02 10:49:48, antonmuhin wrote:
> > those methods are apparently can be inherited from _EventTargetImpl right
now.
> 
> > Can it be fixed in this CL?
> 
> Why should we bother eliminating inherited methods? Sometimes they are not
even
> virtual in c++. AFAIK in V8 bindings such methods aren't eliminated.

[podivilov, 2012-07-03 11:40:45]

Thanks!

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
File lib/html/dartium/html_dartium.dart (left):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
lib/html/dartium/html_dartium.dart:70: void $dom_addEventListener(type,
listener, [useCapture = _null]) {
On 2012/07/02 17:14:10, antonmuhin wrote:
> To reduce the size of the snapshot?
> 
> Do you know examples of non-virtual methods?  That smells pretty much like a
bug
> to me.

Uint8Array vs. Uint8ClampedArray :)

> 
> On 2012/07/02 17:08:18, podivilov wrote:
> > On 2012/07/02 10:49:48, antonmuhin wrote:
> > > those methods are apparently can be inherited from _EventTargetImpl right
> now.
> > 
> > > Can it be fixed in this CL?
> > 
> > Why should we bother eliminating inherited methods? Sometimes they are not
> even
> > virtual in c++. AFAIK in V8 bindings such methods aren't eliminated.
>

[Anton Muhin, 2012-07-03 12:20:51]

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
File lib/html/dartium/html_dartium.dart (left):

https://chromiumcodereview.appspot.com/10698049/diff/1/lib/html/dartium/html_...
lib/html/dartium/html_dartium.dart:70: void $dom_addEventListener(type,
listener, [useCapture = _null]) {
On 2012/07/03 11:40:45, podivilov wrote:
> On 2012/07/02 17:14:10, antonmuhin wrote:
> > To reduce the size of the snapshot?
> > 
> > Do you know examples of non-virtual methods?  That smells pretty much like a
> bug
> > to me.
> 
> Uint8Array vs. Uint8ClampedArray :)
> 
> > 
> > On 2012/07/02 17:08:18, podivilov wrote:
> > > On 2012/07/02 10:49:48, antonmuhin wrote:
> > > > those methods are apparently can be inherited from _EventTargetImpl
right
> > now.
> > > 
> > > > Can it be fixed in this CL?
> > > 
> > > Why should we bother eliminating inherited methods? Sometimes they are not
> > even
> > > virtual in c++. AFAIK in V8 bindings such methods aren't eliminated.
> > 
> 

Yes, but I am not sure this applies to EventTarget methods as those apparently
come from C++ multiple inheritance.