Allowing renaming on DOM members on a per-method basis

Changing the IDL DartName= annotation to only operate on a per-member basis, allowing overloaded methods to be renamed independently.

This also includes a change to rename one overload of CanvasRenderingContext2D.createImageData as a test case.

The remaining changes are to address differences between old semantics of DartName and new semantics, as well as some ordering differences of generated code.


BUG=

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

[Andrei Mouravski, 2013-03-20 22:42:08]

It looks great!

https://codereview.chromium.org/12964003/diff/2001/sdk/lib/html/dartium/html_...
File sdk/lib/html/dartium/html_dartium.dart (left):

https://codereview.chromium.org/12964003/diff/2001/sdk/lib/html/dartium/html_...
sdk/lib/html/dartium/html_dartium.dart:3095: -  String _getPropertyValue(String
propertyName) native "CSSStyleDeclaration__getPropertyValue_Callback";
In this CL or the next, could you sort the operations so that they're always
generated in the right place?

https://codereview.chromium.org/12964003/diff/2001/sdk/lib/html/dartium/html_...
File sdk/lib/html/dartium/html_dartium.dart (right):

https://codereview.chromium.org/12964003/diff/2001/sdk/lib/html/dartium/html_...
sdk/lib/html/dartium/html_dartium.dart:1507: +  ImageData createImageData(num
sw, num sh) native "CanvasRenderingContext2D_createImageData_Callback";
This is awesome.

https://codereview.chromium.org/12964003/diff/2001/tests/html/canvasrendering...
File tests/html/canvasrenderingcontext2d_test.dart (right):

https://codereview.chromium.org/12964003/diff/2001/tests/html/canvasrendering...
tests/html/canvasrenderingcontext2d_test.dart:539: var other =
context.createImageDataFromData(imageData);
I'd love to see this actually checking the data.

[blois, 2013-03-20 22:47:56]

https://codereview.chromium.org/12964003/diff/2001/sdk/lib/html/dartium/html_...
File sdk/lib/html/dartium/html_dartium.dart (left):

https://codereview.chromium.org/12964003/diff/2001/sdk/lib/html/dartium/html_...
sdk/lib/html/dartium/html_dartium.dart:3095: -  String _getPropertyValue(String
propertyName) native "CSSStyleDeclaration__getPropertyValue_Callback";
On 2013/03/20 22:42:08, Andrei Mouravski wrote:
> In this CL or the next, could you sort the operations so that they're always
> generated in the right place?

It is sorting by name, but what we consider the name changed slightly in this
CL.

https://codereview.chromium.org/12964003/diff/2001/tests/html/canvasrendering...
File tests/html/canvasrenderingcontext2d_test.dart (right):

https://codereview.chromium.org/12964003/diff/2001/tests/html/canvasrendering...
tests/html/canvasrenderingcontext2d_test.dart:539: var other =
context.createImageDataFromData(imageData);
On 2013/03/20 22:42:08, Andrei Mouravski wrote:
> I'd love to see this actually checking the data.

The data should be all 0 in all cases- critical portion seems to be that the
size is correct.

[Andrei Mouravski, 2013-03-20 22:50:19]

https://codereview.chromium.org/12964003/diff/2001/tests/html/canvasrendering...
File tests/html/canvasrenderingcontext2d_test.dart (right):

https://codereview.chromium.org/12964003/diff/2001/tests/html/canvasrendering...
tests/html/canvasrenderingcontext2d_test.dart:539: var other =
context.createImageDataFromData(imageData);
On 2013/03/20 22:47:56, blois wrote:
> On 2013/03/20 22:42:08, Andrei Mouravski wrote:
> > I'd love to see this actually checking the data.
> 
> The data should be all 0 in all cases- critical portion seems to be that the
> size is correct.

I guess the part I'd like to see is make an imageData object by getImageData
from a colored section of canvas, then createImageDataFromData.

[sra1, 2013-03-21 00:26:35]

lgtm

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
File tools/dom/idl/dart/dart.idl (right):

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
tools/dom/idl/dart/dart.idl:36: [DartName=createImageDataFromImageData]
ImageData createImageData(in ImageData imagedata) raises (DOMException);
createImageDataCopy?

[sra1, 2013-03-21 00:31:35]

On 2013/03/21 00:26:35, sra1 wrote:
> lgtm
> 
>
https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
> File tools/dom/idl/dart/dart.idl (right):
> 
>
https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
> tools/dom/idl/dart/dart.idl:36: [DartName=createImageDataFromImageData]
> ImageData createImageData(in ImageData imagedata) raises (DOMException);
> createImageDataCopy?

I see, it is not a copy, just the size is used.

[blois, 2013-03-21 00:36:07]

On 2013/03/21 00:31:35, sra1 wrote:
> On 2013/03/21 00:26:35, sra1 wrote:
> > lgtm
> > 
> >
>
https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
> > File tools/dom/idl/dart/dart.idl (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
> > tools/dom/idl/dart/dart.idl:36: [DartName=createImageDataFromImageData]
> > ImageData createImageData(in ImageData imagedata) raises (DOMException);
> > createImageDataCopy?
> 
> I see, it is not a copy, just the size is used.

Yeah, the pixels are always transparent black. Seems like a useless API, imho,
since you could easily use the other. But it is part of the spec. Maybe some
scenario relies on it for better performance?

[Andrei Mouravski, 2013-03-21 01:19:22]

lgtm

[Anton Muhin, 2013-03-21 14:03:02]

lgtm

https://chromiumcodereview.appspot.com/12964003/diff/2001/sdk/lib/html/dart2j...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://chromiumcodereview.appspot.com/12964003/diff/2001/sdk/lib/html/dart2j...
sdk/lib/html/dart2js/html_dart2js.dart:2495: +  @JSName('getPropertyValue')
why methods below are reordered?

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
File tools/dom/idl/dart/dart.idl (left):

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
tools/dom/idl/dart/dart.idl:244: [Supplemental]
why also this change?

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
File tools/dom/scripts/htmldartgenerator.py (right):

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
tools/dom/scripts/htmldartgenerator.py:112: operationsByName =self.
_OperationsByName(parent_interface)
nit: unnecessary space after .

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
tools/dom/scripts/htmldartgenerator.py:126: if name not in operationsByName:
those three lines can be rewritten as:

operationsByName.get(name, []).append(operation)

[blois, 2013-03-21 16:55:43]

https://chromiumcodereview.appspot.com/12964003/diff/2001/sdk/lib/html/dart2j...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://chromiumcodereview.appspot.com/12964003/diff/2001/sdk/lib/html/dart2j...
sdk/lib/html/dart2js/html_dart2js.dart:2495: +  @JSName('getPropertyValue')
On 2013/03/21 14:03:02, Anton Muhin wrote:
> why methods below are reordered?

Re-ordered because it was previously ordering based on operation.id, now it's
using DartName if available. (MutationObserver._observe actually moved, not
disconnect).

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
File tools/dom/idl/dart/dart.idl (left):

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/idl/dart/...
tools/dom/idl/dart/dart.idl:244: [Supplemental]
On 2013/03/21 14:03:02, Anton Muhin wrote:
> why also this change?

IDBIndex has two overloads (same as IDBObjectStore), but only one had the
DartName (which was actually being hidden by the other). We then renamed
everything again in htmlrenamer.py. Had to fix the DartName issue, simplified it
by just using htmlrenamer.py.

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
File tools/dom/scripts/htmldartgenerator.py (right):

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
tools/dom/scripts/htmldartgenerator.py:112: operationsByName =self.
_OperationsByName(parent_interface)
On 2013/03/21 14:03:02, Anton Muhin wrote:
> nit: unnecessary space after .

Done.

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
tools/dom/scripts/htmldartgenerator.py:126: if name not in operationsByName:
On 2013/03/21 14:03:02, Anton Muhin wrote:
> those three lines can be rewritten as:
> 
> operationsByName.get(name, []).append(operation)

Think it has to be:
operationsByName.setdefault(name, []).append(operation)?

[Anton Muhin, 2013-03-21 16:56:51]

still lgtm

FYI: Dartium bbs are red for now, you may want to wait a bit before submitting

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
File tools/dom/scripts/htmldartgenerator.py (right):

https://chromiumcodereview.appspot.com/12964003/diff/2001/tools/dom/scripts/h...
tools/dom/scripts/htmldartgenerator.py:126: if name not in operationsByName:
My bad and you're right, sorry.

On 2013/03/21 16:55:43, blois wrote:
> On 2013/03/21 14:03:02, Anton Muhin wrote:
> > those three lines can be rewritten as:
> > 
> > operationsByName.get(name, []).append(operation)
> 
> Think it has to be:
> operationsByName.setdefault(name, []).append(operation)?

[blois, 2013-03-21 21:50:21]

Committed patchset #2 manually as r20355 (presubmit successful).