Make Dom events run through zone tasks.

Make Dom events run through zone tasks.

R=jacobr@google.com, lrn@google.com

Committed: https://github.com/dart-lang/sdk/commit/6d1f6b2af65636cfe307f81e9d8ded997573c035

[floitsch, 2016-05-31 19:27:48]

floitsch@google.com changed reviewers:
+ jacobr@google.com, lrn@google.com

[floitsch, 2016-05-31 19:27:49]

Still missing the important tasks, like XHR, requestAnimationFrame, or
setTimeout.
I will do them in separate CLs (and only commit them together).

Depends on https://codereview.chromium.org/1848933002/

Will run perf tests before committing.

https://codereview.chromium.org/2022263002/diff/1/sdk/lib/html/dart2js/html_d...
File sdk/lib/html/dart2js/html_dart2js.dart (left):

https://codereview.chromium.org/2022263002/diff/1/sdk/lib/html/dart2js/html_d...
sdk/lib/html/dart2js/html_dart2js.dart:34628: -    return
_requestAnimationFrame(_wrapZone/*<num, dynamic>*/(callback));
This file is automatically generated.

https://codereview.chromium.org/2022263002/diff/1/sdk/lib/html/dartium/html_d...
File sdk/lib/html/dartium/html_dartium.dart (left):

https://codereview.chromium.org/2022263002/diff/1/sdk/lib/html/dartium/html_d...
sdk/lib/html/dartium/html_dartium.dart:37665: -    if ((blob_OR_source_OR_stream
is MediaStream)) {
This file is automatically generated.

[floitsch, 2016-06-06 20:25:09]

ping.

requestAnimationFrame and http-request support has been implemented here:
requestAnimationFrame:https://codereview.chromium.org/2039963003
http-request: https://codereview.chromium.org/2042033002

Still depends on https://codereview.chromium.org/1848933002/

[Lasse Reichstein Nielsen, 2016-06-07 12:21:45]

lgtm

https://codereview.chromium.org/2022263002/diff/60001/tests/html/event_subscr...
File tests/html/event_subscription_specification_test.dart (right):

https://codereview.chromium.org/2022263002/diff/60001/tests/html/event_subscr...
tests/html/event_subscription_specification_test.dart:30: for (var i = 0; i < 2;
i++) {
Just do: for (var useCapture in [true, false]) {
It's simpler.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
File tools/dom/src/EventStreamProvider.dart (right):

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:122: * Task specification for Dom Events.
Dom -> DOM

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:123: */
I noticed you used /// below. Feel free to use it here too.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:126: final String name;
Document the name. What is it? What values will it assume?

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:127: final bool isOneShot;
Document:
/// A one-shot event task is guaranteed to run at most once.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:130: final String eventType;
I assume this is the eventType of the event, so "click" for a click event?

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:169: _name = name ??
"dart.html.event.$eventType";
That looks potentially expensive if you have many event streams. Would it make
sense to cache the name?

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:237: }
This function is almost identical to listen above.
Maybe extract it into a helper function in the superclass that only takes onData
and useCapture as parameters? (I think that's the only difference between the
two functions).

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/shared_ht...
File tools/dom/src/shared_html.dart (right):

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/shared_ht...
tools/dom/src/shared_html.dart:10: if (zone == Zone.ROOT) return callback;
consider using `identical` for the test (or putting Zone.ROOT first to allow the
compiler to know receiver statically, for a tiny micro-optimization).

[floitsch, 2016-06-10 15:43:58]

https://codereview.chromium.org/2022263002/diff/60001/tests/html/event_subscr...
File tests/html/event_subscription_specification_test.dart (right):

https://codereview.chromium.org/2022263002/diff/60001/tests/html/event_subscr...
tests/html/event_subscription_specification_test.dart:30: for (var i = 0; i < 2;
i++) {
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> Just do: for (var useCapture in [true, false]) {
> It's simpler.

Thanks. That's nicer.
done.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
File tools/dom/src/EventStreamProvider.dart (right):

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:122: * Task specification for Dom Events.
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> Dom -> DOM

Done.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:123: */
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> I noticed you used /// below. Feel free to use it here too.

done.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:126: final String name;
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> Document the name. What is it? What values will it assume?

Should inherit the documentation from `TaskSpecification`.
I added a @override.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:127: final bool isOneShot;
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> Document:
> /// A one-shot event task is guaranteed to run at most once.

ditto.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:130: final String eventType;
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> I assume this is the eventType of the event, so "click" for a click event?

yes.
added a bit of documentation (not a lot, though).

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:169: _name = name ??
"dart.html.event.$eventType";
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> That looks potentially expensive if you have many event streams. Would it make
> sense to cache the name?

I think the EventStreamProvider (which is a `const` class for all the existing
events) should use the 'name' constructor for the important ones.
Currently the event-providers are generated as follows:

static const EventStreamProvider<Event> checkingEvent = const
EventStreamProvider<Event>('checking');

For the important ones we should just set the name there. (Probably also clean
the names a bit).

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/EventStre...
tools/dom/src/EventStreamProvider.dart:237: }
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> This function is almost identical to listen above.
> Maybe extract it into a helper function in the superclass that only takes
onData
> and useCapture as parameters? (I think that's the only difference between the
> two functions).

Done.

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/shared_ht...
File tools/dom/src/shared_html.dart (right):

https://codereview.chromium.org/2022263002/diff/60001/tools/dom/src/shared_ht...
tools/dom/src/shared_html.dart:10: if (zone == Zone.ROOT) return callback;
On 2016/06/07 12:21:44, Lasse Reichstein Nielsen wrote:
> consider using `identical` for the test (or putting Zone.ROOT first to allow
the
> compiler to know receiver statically, for a tiny micro-optimization).

Done.

[Jacob, 2016-06-29 16:00:08]

lgtm

[Lasse Reichstein Nielsen, 2016-07-01 12:23:43]

lgtm

[floitsch, 2016-07-02 02:43:08]

Description was changed from

==========
Make Dom events run through zone tasks.
==========

to

==========
Make Dom events run through zone tasks.

R=jacobr@google.com, lrn@google.com

Committed:
https://github.com/dart-lang/sdk/commit/6d1f6b2af65636cfe307f81e9d8ded997573c035
==========

[floitsch, 2016-07-02 02:43:10]

Committed patchset #7 (id:120001) manually as
6d1f6b2af65636cfe307f81e9d8ded997573c035 (presubmit successful).