Use WeakMap to support Expando if available.

Use WeakMap to support Expando if available.

Retains the existing code as fallback for settings without WeakMap for now.

Fixes issue #5144

BUG= http://dartbug.com/5144
R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/75d1366dbd76566b562248a9d1ba3fbf5f339b41

[Lasse Reichstein Nielsen, 2016-01-19 09:29:06]

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

[Lasse Reichstein Nielsen, 2016-01-19 09:29:28]

Description was changed from

==========
Use WeakMap to support Expando if available.

Retains the existing code as fallback for settings without WeakMap for now.
==========

to

==========
Use WeakMap to support Expando if available.

Retains the existing code as fallback for settings without WeakMap for now.

Fixes issue #5144

BUG= http://dartbug.com/5144
==========

[sra1, 2016-01-19 16:53:40]

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

[sra1, 2016-01-19 16:53:40]

DBC

https://codereview.chromium.org/1599393003/diff/1/sdk/lib/_internal/js_runtim...
File sdk/lib/_internal/js_runtime/lib/core_patch.dart (right):

https://codereview.chromium.org/1599393003/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/core_patch.dart:121: ? JS('Object|Null', 'new
WeakMap()')
Use =Object or an empty 'type'.
'Object' means everything, so disables tree shaking.

https://codereview.chromium.org/1599393003/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/core_patch.dart:126: if (_jsWeakMapOrKey is!
String) {
You can avoid these checks by making class Expando abstract and make Expando() a
factory constructor that creates ExpandoByWeakMap or ExpandoByProperty.

[Lasse Reichstein Nielsen, 2016-01-19 21:05:03]

https://codereview.chromium.org/1599393003/diff/1/sdk/lib/_internal/js_runtim...
File sdk/lib/_internal/js_runtime/lib/core_patch.dart (right):

https://codereview.chromium.org/1599393003/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/core_patch.dart:121: ? JS('Object|Null', 'new
WeakMap()')
On 2016/01/19 16:53:40, sra1 wrote:
> Use =Object or an empty 'type'.
> 'Object' means everything, so disables tree shaking. 

Acknowledged.

https://codereview.chromium.org/1599393003/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/core_patch.dart:126: if (_jsWeakMapOrKey is!
String) {
Making the constructor a factory constructor would break anybody who extends
Expando (which they shouldn't be doing, but technically it's a breaking change).

[Siggi Cherem (dart-lang), 2016-01-20 23:37:43]

lgtm

[Lasse Reichstein Nielsen, 2016-01-25 11:06:10]

Description was changed from

==========
Use WeakMap to support Expando if available.

Retains the existing code as fallback for settings without WeakMap for now.

Fixes issue #5144

BUG= http://dartbug.com/5144
==========

to

==========
Use WeakMap to support Expando if available.

Retains the existing code as fallback for settings without WeakMap for now.

Fixes issue #5144

BUG= http://dartbug.com/5144
R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/75d1366dbd76566b562248a9d1ba3fbf5f339b41
==========

[Lasse Reichstein Nielsen, 2016-01-25 11:06:11]

Committed patchset #2 (id:20001) manually as
75d1366dbd76566b562248a9d1ba3fbf5f339b41 (presubmit successful).