Suppress generation of JsGlobal and JsType code and use window as the default value instead of {} f…

Suppress generation of JsGlobal and JsType code and use window as the default value instead of {} for libraries that define JsGlobal fields.

BUG=
R=jmesserly@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/35f0d4142e46784cb956528f5847ec16589142a4

[Jacob, 2015-03-17 00:03:18]

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

[Siggi Cherem (dart-lang), 2015-03-17 01:25:35]

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

[Siggi Cherem (dart-lang), 2015-03-17 01:25:36]

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:977: var result = <JS.Expression>[
DBC - you might need to run pub-ugprade, seems like an old version of the
formatter.

I just noticed this earlier today so I'm sending a CL to pin the formatter to a
precise version to avoid running into this in the future (see
https://codereview.chromium.org/1011003002/)

[Jennifer Messerly, 2015-03-17 16:39:19]

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:53: bool _containsJsGlobals = false;
should this be explicit on the library tag? e.g. library tag declares that it is
describing APIs on JS "window" object:

@JsGlobal("window")
library dom;

That way we aren't hardcoding "window", and we don't need to infer if the code
had a @JsGlobal inside. Like if we removed the "document" accessor, it would be
nice if the dom.js file still worked

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:89: var defaultValue = _containsJsGlobals ?
'window' : '{}';
normally we'd do this as an interpolated expression:

var defaultValue = js.call(_containsJsGlobals ? 'window' : '{}');

js.call makes an expression. It'll cache the resulting AST. slightly faster than
interpolating this as a string below

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:93: "(function($_EXPORTS) { 'use strict'; #;
})(# || (# = $defaultValue));",
this should be:
"(function($_EXPORTS) { 'use strict'; #; })(# || (# = #));"

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:94: [body, name, name])
[body, name, name, defaultValue]

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:928: _containsJsGlobals = true;
per suggestion above, this won't be needed

https://codereview.chromium.org/1013653004/diff/1/test/codegen/expect/dom.js
File test/codegen/expect/dom.js (left):

https://codereview.chromium.org/1013653004/diff/1/test/codegen/expect/dom.js#...
test/codegen/expect/dom.js:51: class CanvasDrawingStyles extends dart.Object {
do we need to fix these too?

[Jennifer Messerly, 2015-03-17 16:41:38]

Another thought on dom.dart ... does the JsGlobal/JsType distinction matter? it
seems that it can be known from context. All we really need to know is that it's
a JS interface.

another option here is to use `external` ... it can't be on classes, but it
could be on methods of the classes, which would imply the class itself is
external.

[Jacob, 2015-03-17 21:22:44]

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:53: bool _containsJsGlobals = false;
On 2015/03/17 16:39:18, John Messerly wrote:
> should this be explicit on the library tag? e.g. library tag declares that it
is
> describing APIs on JS "window" object:
> 
> @JsGlobal("window")
> library dom;
> 
> That way we aren't hardcoding "window", and we don't need to infer if the code
> had a @JsGlobal inside. Like if we removed the "document" accessor, it would
be
> nice if the dom.js file still worked

Discussed offline. Switched to @JsName here and for JsType and removed JsGlobal.

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:89: var defaultValue = _containsJsGlobals ?
'window' : '{}';
On 2015/03/17 16:39:18, John Messerly wrote:
> normally we'd do this as an interpolated expression:
> 
> var defaultValue = js.call(_containsJsGlobals ? 'window' : '{}');
> 
> js.call makes an expression. It'll cache the resulting AST. slightly faster
than
> interpolating this as a string below

Done.

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:93: "(function($_EXPORTS) { 'use strict'; #;
})(# || (# = $defaultValue));",
On 2015/03/17 16:39:18, John Messerly wrote:
> this should be:
> "(function($_EXPORTS) { 'use strict'; #; })(# || (# = #));"

Done.

https://codereview.chromium.org/1013653004/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:94: [body, name, name])
On 2015/03/17 16:39:18, John Messerly wrote:
> [body, name, name, defaultValue]

Done.

[Jennifer Messerly, 2015-03-17 21:30:38]

lgtm

[Jacob, 2015-03-17 21:42:46]

Committed patchset #2 (id:20001) manually as
35f0d4142e46784cb956528f5847ec16589142a4 (presubmit successful).