Removing interfaces from dart:html

Removing interfaces.

This converts all interfaces in dart:html to abstract classes and changes the factory providers from factory methods to static methods.

This CL represents the minimal initial change to remove interfaces, but there's quite a bit of further cleanup which can be done after this (eliminating the factory providers in favor of redirecting factory constructors).

Also I'll be splitting the dartdoc changes out to a separate CL since that's a fairly large change in itself.

BUG=4525
Committed: https://code.google.com/p/dart/source/detail?r=12580

[Anton Muhin, 2012-09-17 10:11:28]

Pretty neat, thanks a lot!

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
File lib/html/scripts/generator.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:369: if parameter_count is None:
please, do not duplicate common logic.  Instead, something like:
 if parameter_count is None:
  parameter_count = len(self.param_infos)

or even in the decl parameter_count = sys.maxint

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:403: def ConstructorFullName(self):
with your change, are there any other references to this method?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:410: type_name = rename_type(self.type_name)
why you rename type here, but it's not renamed in ConstructorFullName above?

Overall cannot ConstructorFactoryName be something like: 'create' +
ConstructorFullName() ?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:417: has_optional = False
has_optional = any(param_info.is_optional for param_info in self.param_infos) ?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:438: '    if (!?$(OPT_PARAM_NAME)) {\n'
nit: why OPT_PARAM_NAME is in parentheses?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:446: emitter.Emit(
I'd rather do something like:

dispatcher_emitter = emitter.Emit(
    '\n'
    '     factory $CTOR($PARAMS) {\n'
    ' $!DISPATCHER
    '       return $FACTORY.$CTOR_FACTORY_NAME($FACTORY_PARAMS);\n',
    '     }\n',
   ...)

and then alter EmitOptionalParameterInvocation to use dispatcher emitter

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:452: for param_info in self.param_infos:
for index, param_info in enumerate(self.param_infos) ?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/sy...
File lib/html/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/sy...
lib/html/scripts/systemhtml.py:491: extends = []
extends -> implements?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/sy...
lib/html/scripts/systemhtml.py:629: if read_only:
nit: I'd prefer not to duplicate the logic here too (even though it might be
longer), something like

if read_only:
  template = '\n   abstract $TYPE get $NAME'
else:
  template = '\n $TYPE $NAME'
self._members_emitter.Emit(template, NAME=html_name, TYPE=....)

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/templates/...
File lib/html/templates/html/dart2js/factoryprovider_OptionElement.darttemplate
(right):

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/templates/...
lib/html/templates/html/dart2js/factoryprovider_OptionElement.darttemplate:7:
[String data = null, String value = null, bool defaultSelected = null,
as you edit this code anyway, may you drop = null, please?

[blois, 2012-09-17 19:48:22]

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
File lib/html/scripts/generator.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:369: if parameter_count is None:
On 2012/09/17 10:11:28, Anton Muhin wrote:
> please, do not duplicate common logic.  Instead, something like:
>  if parameter_count is None:
>   parameter_count = len(self.param_infos)
> 
> or even in the decl parameter_count = sys.maxint

Done.

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:403: def ConstructorFullName(self):
On 2012/09/17 10:11:28, Anton Muhin wrote:
> with your change, are there any other references to this method?

Only within this file, made it private.

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:410: type_name = rename_type(self.type_name)
The rename_type here converts between DOM names and Dart names
(WebKitCSSMatrix->CSSMatrix), this is handled from the callsites elsewhere.

I've converted this method to call ConstructorFullName and more closely follow
the others.

On 2012/09/17 10:11:28, Anton Muhin wrote:
> why you rename type here, but it's not renamed in ConstructorFullName above?
> 
> Overall cannot ConstructorFactoryName be something like: 'create' +
> ConstructorFullName() ?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:417: has_optional = False
On 2012/09/17 10:11:28, Anton Muhin wrote:
> has_optional = any(param_info.is_optional for param_info in self.param_infos)
?

Done.

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:438: '    if (!?$(OPT_PARAM_NAME)) {\n'
Probably leftover from something else. Converted.

On 2012/09/17 10:11:28, Anton Muhin wrote:
> nit: why OPT_PARAM_NAME is in parentheses?

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:446: emitter.Emit(
Done. Wasn't aware of that trick!

On 2012/09/17 10:11:28, Anton Muhin wrote:
> I'd rather do something like:
> 
> dispatcher_emitter = emitter.Emit(
>     '\n'
>     '     factory $CTOR($PARAMS) {\n'
>     ' $!DISPATCHER
>     '       return $FACTORY.$CTOR_FACTORY_NAME($FACTORY_PARAMS);\n',
>     '     }\n',
>    ...)
> 
> and then alter EmitOptionalParameterInvocation to use dispatcher emitter
>

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/ge...
lib/html/scripts/generator.py:452: for param_info in self.param_infos:
On 2012/09/17 10:11:28, Anton Muhin wrote:
> for index, param_info in enumerate(self.param_infos) ?

Done.

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/sy...
File lib/html/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/sy...
lib/html/scripts/systemhtml.py:491: extends = []
On 2012/09/17 10:11:29, Anton Muhin wrote:
> extends -> implements?

Done.

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/scripts/sy...
lib/html/scripts/systemhtml.py:629: if read_only:
On 2012/09/17 10:11:29, Anton Muhin wrote:
> nit: I'd prefer not to duplicate the logic here too (even though it might be
> longer), something like
> 
> if read_only:
>   template = '\n   abstract $TYPE get $NAME'
> else:
>   template = '\n $TYPE $NAME'
> self._members_emitter.Emit(template, NAME=html_name, TYPE=....)

Done.

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/templates/...
File lib/html/templates/html/dart2js/factoryprovider_OptionElement.darttemplate
(right):

https://chromiumcodereview.appspot.com/10915245/diff/5002/lib/html/templates/...
lib/html/templates/html/dart2js/factoryprovider_OptionElement.darttemplate:7:
[String data = null, String value = null, bool defaultSelected = null,
On 2012/09/17 10:11:29, Anton Muhin wrote:
> as you edit this code anyway, may you drop = null, please?

Done.

[Anton Muhin, 2012-09-18 07:45:34]

STV!

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/ge...
File lib/html/scripts/generator.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/ge...
lib/html/scripts/generator.py:430: def EmitOptionalParameterInvocation(index,
emitter):
up to you, another option would be to put definition of this closure below
definition of dispatcher_emitter and do no pass the argument at all.

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/sy...
File lib/html/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/sy...
lib/html/scripts/systemhtml.py:492: suppressed_extends = []
suppressed_implements :) ?

[Lasse Reichstein Nielsen, 2012-09-18 08:19:47]

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/src/dart2j...
File lib/html/src/dart2js_FactoryProviders.dart (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/src/dart2j...
lib/html/src/dart2js_FactoryProviders.dart:16: }
Can you make the original constructor do this directly?
I.e.,
  class Point {
    factory Point(num x, num y) native 'return new WebKitPoint(x,y);';
    ...
?
There shouldn't be any need for the provider classes when you don't use
interfaces.

[Anton Muhin, 2012-09-18 08:20:54]

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/src/dart2j...
File lib/html/src/dart2js_FactoryProviders.dart (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/src/dart2j...
lib/html/src/dart2js_FactoryProviders.dart:16: }
Lasse, that is planned, but Peter prefers to do small steps.  And for what it
matters I am really with him here.

On 2012/09/18 08:19:47, Lasse Reichstein Nielsen wrote:
> Can you make the original constructor do this directly?
> I.e.,
>   class Point {
>     factory Point(num x, num y) native 'return new WebKitPoint(x,y);';
>     ...
> ?
> There shouldn't be any need for the provider classes when you don't use
> interfaces.

[vsm, 2012-09-18 15:57:03]

LGTM!

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/ge...
File lib/html/scripts/generator.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/ge...
lib/html/scripts/generator.py:420: '  factory $CTOR($PARAMS) =>
$FACTORY.$CTOR_FACTORY_NAME($FACTORY_PARAMS);\n',
nit: line length

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/templates/...
File lib/html/templates/html/interface/interface.darttemplate (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/templates/...
lib/html/templates/html/interface/interface.darttemplate:8: abstract class
$ID$EXTENDS {
These $EXTENDS are now really implements, right?

[blois, 2012-09-18 18:12:25]

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/ge...
File lib/html/scripts/generator.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/ge...
lib/html/scripts/generator.py:420: '  factory $CTOR($PARAMS) =>
$FACTORY.$CTOR_FACTORY_NAME($FACTORY_PARAMS);\n',
On 2012/09/18 15:57:03, vsm wrote:
> nit: line length

Done.

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/ge...
lib/html/scripts/generator.py:430: def EmitOptionalParameterInvocation(index,
emitter):
On 2012/09/18 07:45:34, Anton Muhin wrote:
> up to you, another option would be to put definition of this closure below
> definition of dispatcher_emitter and do no pass the argument at all.

Done.

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/sy...
File lib/html/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/scripts/sy...
lib/html/scripts/systemhtml.py:492: suppressed_extends = []
On 2012/09/18 07:45:34, Anton Muhin wrote:
> suppressed_implements :) ?

Done. Note that the template still uses $IMPLEMENTS because it is shared
elsewhere where it actually is implements.

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/templates/...
File lib/html/templates/html/interface/interface.darttemplate (right):

https://chromiumcodereview.appspot.com/10915245/diff/5007/lib/html/templates/...
lib/html/templates/html/interface/interface.darttemplate:8: abstract class
$ID$EXTENDS {
On 2012/09/18 15:57:03, vsm wrote:
> These $EXTENDS are now really implements, right?

Only sometimes (confusingly enough)- this template is interchangeable with ones
which do have a separate implements. Also, moving forward I'd like to make this
an explicit 'extends' (as a separate CL).