Cleanup JS() expressions. New Constructor for JsObject that preserves the correct prototype. Type c…

Cleanup JS() expressions.
New Constructor for JsObject that preserves the correct prototype.
Type check user-supplied values that pass through to JS().
Don't call _convertToJs(this)

Based on: https://chromiumcodereview.appspot.com/26092003/

R=ahe@google.com, alexandre.ardhuin@gmail.com

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

[ahe, 2013-10-11 12:40:51]

lgtm

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:71: // the wrapped JS object
Please convert to proper sentence, that is, start with uppercase letter and end
with period.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:125: dynamic operator[](dynamic key) {
Remove dynamic (twice).

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:167: if (type is! JsFunction) throw new
ArgumentError("type is not a JsFunction");
Long line.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:247: dynamic _convertToDart(dynamic o) {
Remove dynamic (twice).

[alexandre.ardhuin, 2013-10-11 14:07:13]

The same kind of object construction need to be changed in interop.js ( see
function construct(args) )

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:95: JS('void', 'console.log(#)',
factoryFunction);
log to remove ?

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:138: operator[]=(String key, value) {
Change "key" type to dynamic to be consistant with bellow test ( if (key is!
String && key is! int) )

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:150: bool hasProperty(String name) {
Change "name" type to dynamic.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:157: void deleteProperty(String name) {
Change "name" type to dynamic.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:166: bool instanceof(JsFunction type) {
Remove JsFunction otherwise Serializable<JsFunction> could not be used as
parameter.

[justinfagnani, 2013-10-12 21:16:22]

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:71: // the wrapped JS object
On 2013/10/11 12:40:51, ahe wrote:
> Please convert to proper sentence, that is, start with uppercase letter and
end
> with period.

Done.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:95: JS('void', 'console.log(#)',
factoryFunction);
On 2013/10/11 14:07:13, alexandre.ardhuin wrote:
> log to remove ?

There's a very strange heisenbug here. Without the log statement, I get this
error:
function apply() { [native code] } is not a constructor

This doesn't happen in native JS. I've replaced it with allocating a string for
now, but this might be a compiler issue.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:125: dynamic operator[](dynamic key) {
On 2013/10/11 12:40:51, ahe wrote:
> Remove dynamic (twice).

Done.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:138: operator[]=(String key, value) {
On 2013/10/11 14:07:13, alexandre.ardhuin wrote:
> Change "key" type to dynamic to be consistant with bellow test ( if (key is!
> String && key is! int) ) 

Done.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:150: bool hasProperty(String name) {
On 2013/10/11 14:07:13, alexandre.ardhuin wrote:
> Change "name" type to dynamic.

Done.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:157: void deleteProperty(String name) {
On 2013/10/11 14:07:13, alexandre.ardhuin wrote:
> Change "name" type to dynamic.

Done.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:166: bool instanceof(JsFunction type) {
On 2013/10/11 14:07:13, alexandre.ardhuin wrote:
> Remove JsFunction otherwise Serializable<JsFunction> could not be used as
> parameter.

Done.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:167: if (type is! JsFunction) throw new
ArgumentError("type is not a JsFunction");
On 2013/10/11 12:40:51, ahe wrote:
> Long line.

Done.

https://chromiumcodereview.appspot.com/26270003/diff/11001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:247: dynamic _convertToDart(dynamic o) {
On 2013/10/11 12:40:51, ahe wrote:
> Remove dynamic (twice).

Done, but changed the return type to Object to indicate that the function does
return a value.

[alexandre.ardhuin, 2013-10-13 18:49:41]

https://chromiumcodereview.appspot.com/26270003/diff/16001/pkg/browser/lib/in...
File pkg/browser/lib/interop.js (right):

https://chromiumcodereview.appspot.com/26270003/diff/16001/pkg/browser/lib/in...
pkg/browser/lib/interop.js:475: // The first argument to bind() is the binding
of 'this', make it 'null'
A line has been dropped compared to js_dart2js.dart.

https://chromiumcodereview.appspot.com/26270003/diff/16001/sdk/lib/js/dart2js...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://chromiumcodereview.appspot.com/26270003/diff/16001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:126: Object operator[](key) {
Returning Object will generate warnings in existing code and will force to use a
lot of casts (you should see those warnings in js_test.dart). I would prefer to
use dynamic.
If you really want to use Object. Shouldn't js_dartium.dart be updated to be
consistant on the method signatures ?

https://chromiumcodereview.appspot.com/26270003/diff/16001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:179: Object callMethod(String name, [List
args]) {
Same as operator[](key) about returning Object.

[justinfagnani, 2013-10-15 22:18:08]

https://chromiumcodereview.appspot.com/26270003/diff/16001/pkg/browser/lib/in...
File pkg/browser/lib/interop.js (right):

https://chromiumcodereview.appspot.com/26270003/diff/16001/pkg/browser/lib/in...
pkg/browser/lib/interop.js:475: // The first argument to bind() is the binding
of 'this', make it 'null'
On 2013/10/13 18:49:41, alexandre.ardhuin wrote:
> A line has been dropped compared to js_dart2js.dart.

It's slightly different, since this args list already has an extra slot before
the argument to hold the constructor. Since we created the list, I just set the
first slot to null.

https://chromiumcodereview.appspot.com/26270003/diff/16001/sdk/lib/js/dart2js...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://chromiumcodereview.appspot.com/26270003/diff/16001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:126: Object operator[](key) {
On 2013/10/13 18:49:41, alexandre.ardhuin wrote:
> Returning Object will generate warnings in existing code and will force to use
a
> lot of casts (you should see those warnings in js_test.dart). I would prefer
to
> use dynamic.
> If you really want to use Object. Shouldn't js_dartium.dart be updated to be
> consistant on the method signatures ?

I prefer dynamic too, forgot about the warnings. I'll switch it back.

https://chromiumcodereview.appspot.com/26270003/diff/16001/sdk/lib/js/dart2js...
sdk/lib/js/dart2js/js_dart2js.dart:179: Object callMethod(String name, [List
args]) {
On 2013/10/13 18:49:41, alexandre.ardhuin wrote:
> Same as operator[](key) about returning Object.

Done.

[alexandre.ardhuin, 2013-10-16 08:12:01]

lgtm

[justinfagnani, 2013-10-17 18:15:13]

Committed patchset #4 manually as r28813 (presubmit successful).