Add JSNative utility class with static methods methods to efficiently manipulate typed JSInterop ob…

Add JSNative utility class with static methods methods to efficiently manipulate typed JSInterop objects in cases where the member name is not known statically. These methods would be extension methods on JSObject if Dart supported extension methods. Update package js to export these methods. Implement in Dart2JS. Implement JS$ in dart2js.

BUG=
R=alanknight@google.com, sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/96ca5db7e5ad4a7c259376b7ce1bf2d694ccf479

[Jacob, 2016-07-16 00:25:07]

jacobr@google.com changed reviewers:
+ sra@google.com

[Jacob, 2016-07-16 00:25:07]

Add JSNative utility class with static methods methods to efficiently manipulate
typed JSInterop objects in cases
where method names aren't known statically or are not valid
Dart identifiers.

Support JS$ for exposing JS classes and members with names that are not allowed
in Dart such as class or delete.

Add significant extra test coverage.

[Alan Knight, 2016-07-19 16:56:03]

alanknight@google.com changed reviewers:
+ alanknight@google.com

[Alan Knight, 2016-07-19 16:56:04]

lgtm

https://codereview.chromium.org/2150313003/diff/1/sdk/lib/js/dart2js/js_dart2...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://codereview.chromium.org/2150313003/diff/1/sdk/lib/js/dart2js/js_dart2...
sdk/lib/js/dart2js/js_dart2js.dart:788: static callMethod(o, String method, List
args) => JS('Object', '#[#].apply(#, #)', o, method, o, args);
nit: line length

https://codereview.chromium.org/2150313003/diff/1/sdk/lib/js/dart2js/js_dart2...
sdk/lib/js/dart2js/js_dart2js.dart:822: return JS('Object', 'new #(#, #, #, #)',
constr, arg0, arg1, arg2, arg3);
Also line length

[Jacob, 2016-07-19 17:42:19]

https://codereview.chromium.org/2150313003/diff/1/sdk/lib/js/dart2js/js_dart2...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://codereview.chromium.org/2150313003/diff/1/sdk/lib/js/dart2js/js_dart2...
sdk/lib/js/dart2js/js_dart2js.dart:788: static callMethod(o, String method, List
args) => JS('Object', '#[#].apply(#, #)', o, method, o, args);
On 2016/07/19 16:56:04, Alan Knight wrote:
> nit: line length

Done. Ran dartfmt.

https://codereview.chromium.org/2150313003/diff/1/sdk/lib/js/dart2js/js_dart2...
sdk/lib/js/dart2js/js_dart2js.dart:822: return JS('Object', 'new #(#, #, #, #)',
constr, arg0, arg1, arg2, arg3);
On 2016/07/19 16:56:04, Alan Knight wrote:
> Also line length

Done.

[sra1, 2016-07-20 18:19:20]

LGTM if we make it a library.

https://codereview.chromium.org/2150313003/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/native_data.dart (right):

https://codereview.chromium.org/2150313003/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/native_data.dart:226: String
getUnescapedJSInteropName(String name) {
Some tests should use JS$JS$ to ensure this is called once.

https://codereview.chromium.org/2150313003/diff/20001/sdk/lib/js/dart2js/js_d...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://codereview.chromium.org/2150313003/diff/20001/sdk/lib/js/dart2js/js_d...
sdk/lib/js/dart2js/js_dart2js.dart:735: class JSNative {
Maybe call JSUtils ? JavaScriptUtils ?
Or a dart:library

https://codereview.chromium.org/2150313003/diff/20001/sdk/lib/js/dart2js/js_d...
sdk/lib/js/dart2js/js_dart2js.dart:791: static instanceof(o, Function type) =>
JS('bool', '# instanceof #', o, type);
instanceOf ?

[Jacob, 2016-07-22 19:28:24]

Added all requested tests, made dart_util a library, fixed an extra escaping bug
present in dartium if a JS class name had $ in the name by implementing a simple
Dart string literal escaping method.

https://codereview.chromium.org/2150313003/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/native_data.dart (right):

https://codereview.chromium.org/2150313003/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/native_data.dart:226: String
getUnescapedJSInteropName(String name) {
On 2016/07/20 18:19:20, sra1 wrote:
> Some tests should use JS$JS$ to ensure this is called once.

JS$JS$done

https://codereview.chromium.org/2150313003/diff/20001/sdk/lib/js/dart2js/js_d...
File sdk/lib/js/dart2js/js_dart2js.dart (right):

https://codereview.chromium.org/2150313003/diff/20001/sdk/lib/js/dart2js/js_d...
sdk/lib/js/dart2js/js_dart2js.dart:735: class JSNative {
On 2016/07/20 18:19:20, sra1 wrote:
> Maybe call JSUtils ? JavaScriptUtils ?
> Or a dart:library

added
dart:js_util

re-export it as 
package:js/js_util.dart
as we don't really want to be adding new dart:libraries

https://codereview.chromium.org/2150313003/diff/20001/sdk/lib/js/dart2js/js_d...
sdk/lib/js/dart2js/js_dart2js.dart:791: static instanceof(o, Function type) =>
JS('bool', '# instanceof #', o, type);
On 2016/07/20 18:19:20, sra1 wrote:
> instanceOf ?

ran it by Bob and he thinks it is reasonable to stay consistent with the JS
names rather than dartifying the names as this is a low level JS interop utility
library.

[sra1, 2016-07-22 20:16:33]

LGTM with some cleanup of the string literal formatting code.

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
File sdk/lib/js/dartium/js_dartium.dart (right):

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:180: // Identical to JSON string lieral
escaping except that we also escape $.
lieral?

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:214: } else if (charCode == QUOTE || charCode
== BACKSLASH || charCode == CHAR_$) {
line length

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:232: * If a value is both a [List] and a
[Map], it's serialized as a [List].
This comment is wrong.

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:234: bool writeStringLiteral(String str) {
Doesn't return anything.

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:244: return output.toString();
Could you use the JSON codec rather than copy & paste the code?

[Jacob, 2016-07-25 16:31:59]

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
File sdk/lib/js/dartium/js_dartium.dart (right):

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:180: // Identical to JSON string lieral
escaping except that we also escape $.
On 2016/07/22 20:16:32, sra1 wrote:
> lieral?

Done.

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:214: } else if (charCode == QUOTE || charCode
== BACKSLASH || charCode == CHAR_$) {
On 2016/07/22 20:16:32, sra1 wrote:
> line length

done. ran dartfmt

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:234: bool writeStringLiteral(String str) {
On 2016/07/22 20:16:32, sra1 wrote:
> Doesn't return anything.

Done.

https://codereview.chromium.org/2150313003/diff/60001/sdk/lib/js/dartium/js_d...
sdk/lib/js/dartium/js_dartium.dart:244: return output.toString();
On 2016/07/22 20:16:32, sra1 wrote:
> Could you use the JSON codec rather than copy & paste the code?

Can't easily because of $
JSON doesn't need to escape $ but Dart string literals do.
I could do a two pass approach where I JSON encode and then find and replace $
with \$ but that would be a bit slower and feels a little fragile. 
This code should not live that long so I'm not as worried about a little copy
and pasting than I would be otherwise.

[Jacob, 2016-07-25 16:59:06]

Description was changed from

==========
Add JSNative utility class with static methods methods to efficiently manipulate
typed JSInterop objects in cases where the member name is not known statically.
These methods would be extension methods on JSObject if Dart supported extension
methods. Update package js to export these methods. Implement in Dart2JS.
Implement JS$ in dart2js.

BUG=
==========

to

==========
Add JSNative utility class with static methods methods to efficiently manipulate
typed JSInterop objects in cases where the member name is not known statically.
These methods would be extension methods on JSObject if Dart supported extension
methods. Update package js to export these methods. Implement in Dart2JS.
Implement JS$ in dart2js.

BUG=
R=alanknight@google.com, sra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/96ca5db7e5ad4a7c259376b7ce1bf2d694ccf479
==========

[Jacob, 2016-07-25 16:59:08]

Committed patchset #5 (id:80001) manually as
96ca5db7e5ad4a7c259376b7ce1bf2d694ccf479 (presubmit successful).

[kevmoo, 2016-07-25 17:34:40]

kevmoo@google.com changed reviewers:
+ kevmoo@google.com

[kevmoo, 2016-07-25 17:34:41]

Late DBC

https://codereview.chromium.org/2150313003/diff/80001/pkg/js/CHANGELOG.md
File pkg/js/CHANGELOG.md (right):

https://codereview.chromium.org/2150313003/diff/80001/pkg/js/CHANGELOG.md#new...
pkg/js/CHANGELOG.md:4: statically. These methods would be extension methods on
JSObject if
Remove this last "These methods would be..." sentence.

Not helpful. :-)

https://codereview.chromium.org/2150313003/diff/80001/pkg/js/pubspec.yaml
File pkg/js/pubspec.yaml (right):

https://codereview.chromium.org/2150313003/diff/80001/pkg/js/pubspec.yaml#new...
pkg/js/pubspec.yaml:8: sdk: '>=1.13.0 <2.0.0'
Would this require a min SDK bump? Is it wiring together features you've added
to the SDK?

[sra1, 2016-07-26 00:49:41]

https://codereview.chromium.org/2150313003/diff/100001/tests/compiler/dart2js...
File tests/compiler/dart2js_native/dart2js_native.status (left):

https://codereview.chromium.org/2150313003/diff/100001/tests/compiler/dart2js...
tests/compiler/dart2js_native/dart2js_native.status:8: [ $compiler == dart2js ]
Restore