Execute callbacks from JS in the correct zone

Execute callbacks from JS in the correct zone

BUG=https://code.google.com/p/dart/issues/detail?id=17301

[justinfagnani, 2014-04-02 23:44:44]

This is just the fix for the dart2js side, still need to work on the Dartium
side.

[floitsch, 2014-04-03 12:52:30]

LGTM.

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

https://codereview.chromium.org/222203009/diff/1/sdk/lib/js/dart2js/js_dart2j...
sdk/lib/js/dart2js/js_dart2js.dart:131: return
Zone.current.bindUnaryCallback(callDart, runGuarded: true);
are you sure you want to run guarded?
Running guarded means, that a thrown exception is considered unhandled and
reported as such.

If you expect JS to be able to deal with thrown exceptions you want `runGuarded`
to be false.

[Jacob, 2014-04-09 16:54:58]

lgtm

[justinfagnani, 2014-04-10 00:26:59]

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

https://codereview.chromium.org/222203009/diff/1/sdk/lib/js/dart2js/js_dart2j...
sdk/lib/js/dart2js/js_dart2js.dart:131: return
Zone.current.bindUnaryCallback(callDart, runGuarded: true);
On 2014/04/03 12:52:30, floitsch wrote:
> are you sure you want to run guarded?
> Running guarded means, that a thrown exception is considered unhandled and
> reported as such.
> 
> If you expect JS to be able to deal with thrown exceptions you want
`runGuarded`
> to be false.

I'll assume that JS should have a chance to handle the exception. Removing
runGuarded.

[justinfagnani, 2014-04-10 00:27:38]

Jacob PTAL. I added the Dartium support to js_dartium.dart, and now this CL
depends on https://codereview.chromium.org/231743006/

[Jacob, 2014-04-10 00:51:53]

lgtm

https://codereview.chromium.org/222203009/diff/20001/sdk/lib/js/dartium/js_da...
File sdk/lib/js/dartium/js_dartium.dart (right):

https://codereview.chromium.org/222203009/diff/20001/sdk/lib/js/dartium/js_da...
sdk/lib/js/dartium/js_dartium.dart:369: if (value is Function) return
_applyZoned(value);
nit: the following is cleaner.
_maybeWrapFunction(value) => value is Function ? _applyZoned(value) : value;

[justinfagnani, 2014-04-10 03:55:05]

Added the test for round trip functions that fails in Dartium. I have an
attempted fix in the other CL, but it's timing out.

https://codereview.chromium.org/222203009/diff/20001/sdk/lib/js/dartium/js_da...
File sdk/lib/js/dartium/js_dartium.dart (right):

https://codereview.chromium.org/222203009/diff/20001/sdk/lib/js/dartium/js_da...
sdk/lib/js/dartium/js_dartium.dart:369: if (value is Function) return
_applyZoned(value);
On 2014/04/10 00:51:53, Jacob wrote:
> nit: the following is cleaner.
> _maybeWrapFunction(value) => value is Function ? _applyZoned(value) : value;

Done.

[Jennifer Messerly, 2014-05-28 18:29:34]

ping! How's this patch doing?

Spark is hitting this issue a *lot* because they use JS interop to talk to
Chrome app APIs. Here's one example but there were several others:
https://github.com/dart-lang/spark/pull/1971

[sergeygs, 2014-09-16 00:29:07]

On 2014/05/28 18:29:34, John Messerly wrote:
> ping! How's this patch doing?
> 
> Spark is hitting this issue a *lot* because they use JS interop to talk to
> Chrome app APIs. Here's one example but there were several others:
> https://github.com/dart-lang/spark/pull/1971

Ping #2!

[Jennifer Messerly, 2014-09-16 00:35:25]

jmesserly@google.com changed reviewers:
+ jakemac@google.com, sigmund@google.com

[Jennifer Messerly, 2014-09-16 00:35:26]

+Siggi & Jake ... would one of you be interested in taking this CL over? (Just
guessing that Justin isn't going to get to it...)

[Jennifer Messerly, 2014-09-16 00:35:55]

On 2014/09/16 00:35:26, John Messerly wrote:
> +Siggi & Jake ... would one of you be interested in taking this CL over? (Just
> guessing that Justin isn't going to get to it...)

relevant:
https://github.com/dart-lang/chromedeveditor/issues/2252#issuecomment-55679745

[justinfagnani, 2014-09-16 00:36:42]

On 2014/09/16 00:35:26, John Messerly wrote:
> +Siggi & Jake ... would one of you be interested in taking this CL over? (Just
> guessing that Justin isn't going to get to it...)

This CL isn't the issue, it's ready to go. The Dartium CL needs to be submitted
together with it.

[Jennifer Messerly, 2014-09-16 00:39:03]

On 2014/09/16 00:36:42, justinfagnani wrote:
> On 2014/09/16 00:35:26, John Messerly wrote:
> > +Siggi & Jake ... would one of you be interested in taking this CL over?
(Just
> > guessing that Justin isn't going to get to it...)
> 
> This CL isn't the issue, it's ready to go. The Dartium CL needs to be
submitted
> together with it.

was it this one? https://codereview.chromium.org/231743006/

it looks like it's been sitting for a while, just wondering how we can get the
ball rolling on this again. Maybe someone experienced with Dartium could help
out (Jacob? Siggi?)

[justinfagnani, 2014-09-16 00:39:23]

On 2014/09/16 00:36:42, justinfagnani wrote:
> On 2014/09/16 00:35:26, John Messerly wrote:
> > +Siggi & Jake ... would one of you be interested in taking this CL over?
(Just
> > guessing that Justin isn't going to get to it...)
> 
> This CL isn't the issue, it's ready to go. The Dartium CL needs to be
submitted
> together with it.

FYI, my attempt at the Dartium fix is here:
https://codereview.chromium.org/231743006/
It wasn't fully working, so Jacob has a fix to that.

[Jennifer Messerly, 2015-05-11 16:51:31]

jmesserly@google.com changed reviewers:
- jmesserly@google.com