fix #569, cache constants defined in methods

fix #569, cache constants defined in methods

R=leafp@google.com, sra@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/9f978ebfc6be88f04ff46b2110394bd7af45b15c

[Jennifer Messerly, 2016-05-14 00:22:56]

jmesserly@google.com changed reviewers:
+ leafp@google.com, sra@google.com

[sra1, 2016-05-14 19:58:28]

lgtm

https://codereview.chromium.org/1979013003/diff/1/lib/runtime/dart_sdk.js
File lib/runtime/dart_sdk.js (right):

https://codereview.chromium.org/1979013003/diff/1/lib/runtime/dart_sdk.js#new...
lib/runtime/dart_sdk.js:72070: +      allowedAttributes = allowedAttributes !=
null ? allowedAttributes : const$57 || (const$57 = dart.const(dart.list([],
core.String)));
Maybe map the constant value to a variable to reuse the variable.

https://codereview.chromium.org/1979013003/diff/1/test/browser/language_tests.js
File test/browser/language_tests.js (right):

https://codereview.chromium.org/1979013003/diff/1/test/browser/language_tests...
test/browser/language_tests.js:474: 'html_escape_test': skip_fail,
maybe just fail so we know when it is fixed.

[Leaf, 2016-05-16 18:13:57]

lgtm!

[Jennifer Messerly, 2016-05-17 18:04:14]

Thanks guys!

https://codereview.chromium.org/1979013003/diff/1/lib/runtime/dart_sdk.js
File lib/runtime/dart_sdk.js (right):

https://codereview.chromium.org/1979013003/diff/1/lib/runtime/dart_sdk.js#new...
lib/runtime/dart_sdk.js:72070: +      allowedAttributes = allowedAttributes !=
null ? allowedAttributes : const$57 || (const$57 = dart.const(dart.list([],
core.String)));
On 2016/05/14 19:58:27, sra1 wrote:
> Maybe map the constant value to a variable to reuse the variable.

Nice catch. Yeah that's an interesting idea.

So it looks like this code is part of a constructor. Each time an instance is
constructed, it will use the "allowedAttributes" you passed in, but otherwise it
will use a constant.

In that case, it seems like we benefit from having the "const$57" here, because
it will be constructed and canonicalized once ever, instead of once per instance
creation.

In other words, it looks to me like one variable is caching it at module level,
while the other is caching it only for an instance of this class. I could be
missing something though.

https://codereview.chromium.org/1979013003/diff/1/test/browser/language_tests.js
File test/browser/language_tests.js (right):

https://codereview.chromium.org/1979013003/diff/1/test/browser/language_tests...
test/browser/language_tests.js:474: 'html_escape_test': skip_fail,
On 2016/05/14 19:58:27, sra1 wrote:
> maybe just fail so we know when it is fixed.

The problem here is it's failing inconsistently. Using an old SDK it will pass
(e.g. Travis bot). If using a new SDK, it will fail. Hence skip_fail

[Jennifer Messerly, 2016-05-17 18:10:04]

Description was changed from

==========
fix #569, cache constants defined in methods
==========

to

==========
fix #569, cache constants defined in methods

R=leafp@google.com, sra@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/9f978ebfc6be88f04ff46b211039...
==========

[Jennifer Messerly, 2016-05-17 18:10:05]

Committed patchset #2 (id:20001) manually as
9f978ebfc6be88f04ff46b2110394bd7af45b15c (presubmit successful).