Alternative fix for the js-interop crash.

Alternative fix for the js-interop crash.

R=sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/3fa0cae192dce46aef1c9040e8782702aa1e2d9c

[Siggi Cherem (dart-lang), 2015-11-20 01:47:41]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2015-11-20 01:47:46]

Patchset #1 (id:20001) has been deleted

[Siggi Cherem (dart-lang), 2015-11-20 01:48:26]

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

[sra1, 2015-11-20 19:06:24]

https://chromiumcodereview.appspot.com/1457383003/diff/40001/pkg/compiler/lib...
File pkg/compiler/lib/src/native/behavior.dart (right):

https://chromiumcodereview.appspot.com/1457383003/diff/40001/pkg/compiler/lib...
pkg/compiler/lib/src/native/behavior.dart:656:
backend.helpers.jsJavaScriptObjectClass.thisType);
It does seem crazy to do this for 'int'.

Perhaps we should do this instead of adding the declared type for types like
'dynamic' and 'Object', which otherwise will pull in all of dart:html and
friends.

[Siggi Cherem (dart-lang), 2015-11-25 21:09:43]

Patchset #2 (id:60001) has been deleted

[Siggi Cherem (dart-lang), 2015-11-25 21:15:26]

PTAL

I think we should still go the conservative route for now (this CL). The current
side-cast behavior is weird, but it is what dartium does, I'd like to avoid
making things inconsistent. We should consider that in more detail when Jacob
returns and decide whether to change the behavior in both places.

I updated this CL to continue with the conservative approach, but also to add a
flag to experiment with trusting the js-interop type annotations.

https://codereview.chromium.org/1457383003/diff/40001/pkg/compiler/lib/src/na...
File pkg/compiler/lib/src/native/behavior.dart (right):

https://codereview.chromium.org/1457383003/diff/40001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/behavior.dart:656:
backend.helpers.jsJavaScriptObjectClass.thisType);
On 2015/11/20 19:06:24, sra1 wrote:
> It does seem crazy to do this for 'int'.
> 
> Perhaps we should do this instead of adding the declared type for types like
> 'dynamic' and 'Object', which otherwise will pull in all of dart:html and
> friends.

Agree - I can't do anything for int to be sound, but I added a flag to enable
trust-js-interop-types, and in that case, I made the change you mentioned to be
more precise when it comes to dynamic and object.

[Siggi Cherem (dart-lang), 2015-11-26 04:02:34]

Patchset #3 (id:100001) has been deleted

[Siggi Cherem (dart-lang), 2015-11-26 04:09:10]

Finally ready for another look.

The new behavior matches more closely what Stephen suggested for dynamic return
types.

There is one test that is failing that I haven't had a chance to investigate in
detail. The test seems to return a DOM type from a js-interop call, so it's
quite possible that the type is not allocated anywhere else.

Do we want to support that? Thoughts?

[Siggi Cherem (dart-lang), 2015-12-01 00:19:45]

Patchset #4 (id:140001) has been deleted

[Siggi Cherem (dart-lang), 2015-12-01 00:22:16]

PTAL: made the final changes to include native types. All tests are passing now.

https://codereview.chromium.org/1457383003/diff/160001/pkg/compiler/lib/src/s...
File pkg/compiler/lib/src/ssa/builder.dart (right):

https://codereview.chromium.org/1457383003/diff/160001/pkg/compiler/lib/src/s...
pkg/compiler/lib/src/ssa/builder.dart:5948: String code;
note this portion is the same as before, I just simplified it slightly

[sra1, 2015-12-01 00:38:07]

lgtm

https://codereview.chromium.org/1457383003/diff/160001/pkg/compiler/lib/src/n...
File pkg/compiler/lib/src/native/behavior.dart (right):

https://codereview.chromium.org/1457383003/diff/160001/pkg/compiler/lib/src/n...
pkg/compiler/lib/src/native/behavior.dart:666: behavior._capture(type,
compiler.resolution, isInterop: isInterop,
break before isInterop

[Siggi Cherem (dart-lang), 2015-12-01 00:43:54]

thanks

https://codereview.chromium.org/1457383003/diff/160001/pkg/compiler/lib/src/n...
File pkg/compiler/lib/src/native/behavior.dart (right):

https://codereview.chromium.org/1457383003/diff/160001/pkg/compiler/lib/src/n...
pkg/compiler/lib/src/native/behavior.dart:666: behavior._capture(type,
compiler.resolution, isInterop: isInterop,
On 2015/12/01 00:38:07, sra1 wrote:
> break before isInterop

Done.

[Siggi Cherem (dart-lang), 2015-12-01 00:46:18]

Description was changed from

==========
Alternative fix for the js-interop crash.
==========

to

==========
Alternative fix for the js-interop crash.

R=sra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/3fa0cae192dce46aef1c9040e8782702aa1e2d9c
==========

[Siggi Cherem (dart-lang), 2015-12-01 00:46:19]

Committed patchset #5 (id:180001) manually as
3fa0cae192dce46aef1c9040e8782702aa1e2d9c (presubmit successful).