Add zone task support to http-requests.

Add zone task support to http-requests.

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

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

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

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

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

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

[Lasse Reichstein Nielsen, 2016-06-08 09:40:29]

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
File tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate (right):

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:16: final String
url;
Document fields (e.g., by referencing the XmlHttpRequest object or its method
parameters).

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:25: * executed as
part of the http-request task, but as an event callback.
I don't understand what it means to be "part of the http-request task" and how
an event callback is different. Try rewording.

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:45: * This
specification serves as signal to zones that an http request has been
http -> HTTP
acronym, dittos below.

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:48: * data
event-listeners that have the request object as event target are
commas in this sentence? I have a hard time parsing it.
"gets" -> "receives"?

What is the "request object"?

[floitsch, 2016-06-10 15:46:57]

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
File tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate (right):

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:16: final String
url;
On 2016/06/08 09:40:28, Lasse Reichstein Nielsen wrote:
> Document fields (e.g., by referencing the XmlHttpRequest object or its method
> parameters).

Done.

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:25: * executed as
part of the http-request task, but as an event callback.
On 2016/06/08 09:40:28, Lasse Reichstein Nielsen wrote:
> I don't understand what it means to be "part of the http-request task" and how
> an event callback is different. Try rewording.

Done.

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:45: * This
specification serves as signal to zones that an http request has been
On 2016/06/08 09:40:28, Lasse Reichstein Nielsen wrote:
> http -> HTTP
> acronym, dittos below.

Done.

https://codereview.chromium.org/2042033002/diff/1/tools/dom/templates/html/im...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:48: * data
event-listeners that have the request object as event target are
On 2016/06/08 09:40:28, Lasse Reichstein Nielsen wrote:
> commas in this sentence? I have a hard time parsing it.
> "gets" -> "receives"?
> 
> What is the "request object"?

Reworded.
Done.

[Jacob, 2016-06-13 23:41:00]

jacobr@google.com changed reviewers:
+ alanknight@google.com
- jacobr@google.com

[Jacob, 2016-06-13 23:41:20]

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

[Jacob, 2016-06-13 23:41:21]

Reassigning to Alan Knight as he owns dart:html

[Alan Knight, 2016-06-14 00:05:16]

lgtm

https://codereview.chromium.org/2042033002/diff/20001/tests/html/xhr_task2_te...
File tests/html/xhr_task2_test.dart (right):

https://codereview.chromium.org/2042033002/diff/20001/tests/html/xhr_task2_te...
tests/html/xhr_task2_test.dart:1: // Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
2012?

[floitsch, 2016-06-30 19:42:58]

https://chromiumcodereview.appspot.com/2042033002/diff/20001/tests/html/xhr_t...
File tests/html/xhr_task2_test.dart (right):

https://chromiumcodereview.appspot.com/2042033002/diff/20001/tests/html/xhr_t...
tests/html/xhr_task2_test.dart:1: // Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
On 2016/06/14 00:05:16, Alan Knight wrote:
> 2012?

Done.

[Lasse Reichstein Nielsen, 2016-07-01 12:41:30]

lgtm

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
File tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate (right):

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:8: * A task
specification for http requests.
http -> HTTP 
(lots more below).

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:13: * A task
created by this specification is a `Future<HttpRequest>`.
by -> from.
It's created by the `create` function from the specification, not directly by
the specification.

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:21: /// The
method used to do the request.
"method" here needs to be put in context. Maybe "request method" or even "HTTP
request method".

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:36: /// - `""`:
(same as `"text"`),
It seems a little odd to have `""` be equal to `"text"` and null equal to `""`.
Why not just make `"text"` the default if the value is `null` or `""`?

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:43: /// By
default (when `null`) the desired format is `""`.
This says that if the desired response format is null, the desired format is
`""`. That confuses me.

Maybe just say: If the value is `null`, it should be treated as `""`.
Or, alternatively/preferably, just prevent it from being `null` in the
constructor.

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:46: /// The
desired mime type.
mime -> MIME

I think "media type" or "content type" would be preferable, for the field too.

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:48: /// This
overrides the default mime-type which is set up to transfer textual
no dash in "MIME type".

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:48: /// This
overrides the default mime-type which is set up to transfer textual
which is set up to transfer textual data -> which is `text/plain`.
(I assume). The original is too vague.

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:57: /// When
provided (not `null`) must be a [ByteBuffer],
When data is provided (the value is not `null`), it must ...

https://chromiumcodereview.appspot.com/2042033002/diff/40001/tools/dom/templa...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:85: * no callback
is ever executed in this task.
Doesn't that make it a non-task? If a task is something which runs a callback as
an event in the event loop, not having a callback means that this
isn't/shouldn't be a task?

[floitsch, 2016-07-02 01:53:55]

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
File tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate (right):

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:8: * A task
specification for http requests.
On 2016/07/01 12:41:29, Lasse Reichstein Nielsen wrote:
> http -> HTTP 
> (lots more below).

Done.

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:13: * A task
created by this specification is a `Future<HttpRequest>`.
On 2016/07/01 12:41:29, Lasse Reichstein Nielsen wrote:
> by -> from.
> It's created by the `create` function from the specification, not directly by
> the specification.

Done.

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:21: /// The
method used to do the request.
On 2016/07/01 12:41:29, Lasse Reichstein Nielsen wrote:
> "method" here needs to be put in context. Maybe "request method" or even "HTTP
> request method".

changed to 'The HTTP request method.'

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:36: /// - `""`:
(same as `"text"`),
On 2016/07/01 12:41:30, Lasse Reichstein Nielsen wrote:
> It seems a little odd to have `""` be equal to `"text"` and null equal to
`""`.
> Why not just make `"text"` the default if the value is `null` or `""`?

not my choice. This is just documentation copied from the http-request method.

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:43: /// By
default (when `null`) the desired format is `""`.
On 2016/07/01 12:41:30, Lasse Reichstein Nielsen wrote:
> This says that if the desired response format is null, the desired format is
> `""`. That confuses me.
> 
> Maybe just say: If the value is `null`, it should be treated as `""`.
> Or, alternatively/preferably, just prevent it from being `null` in the
> constructor.
> 

Changed to: "When no value is provided (when equal to `null`) defaults to `""`.

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:46: /// The
desired mime type.
On 2016/07/01 12:41:29, Lasse Reichstein Nielsen wrote:
> mime -> MIME
> 
> I think "media type" or "content type" would be preferable, for the field too.

I'm just copying/documenting the values from the `request` function.
Not changing the name.

done (for mime->MIME).

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:48: /// This
overrides the default mime-type which is set up to transfer textual
On 2016/07/01 12:41:30, Lasse Reichstein Nielsen wrote:
> no dash in "MIME type".

Done.

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:48: /// This
overrides the default mime-type which is set up to transfer textual
On 2016/07/01 12:41:30, Lasse Reichstein Nielsen wrote:
> which is set up to transfer textual data -> which is `text/plain`.
> (I assume). The original is too vague.

I even went into the spec, but couldn't find the MIME type for `null`.
"textual data" is what others used.
Left as is.

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:57: /// When
provided (not `null`) must be a [ByteBuffer],
On 2016/07/01 12:41:29, Lasse Reichstein Nielsen wrote:
> When data is provided (the value is not `null`), it must ...

Done.

https://codereview.chromium.org/2042033002/diff/40001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_XMLHttpRequest.darttemplate:85: * no callback
is ever executed in this task.
On 2016/07/01 12:41:29, Lasse Reichstein Nielsen wrote:
> Doesn't that make it a non-task? If a task is something which runs a callback
as
> an event in the event loop, not having a callback means that this
> isn't/shouldn't be a task?

Somehow yes.

I added it:
- because it makes http-request a task, independently on how it is started.
- the event-listeners (tasks) on xml-http-request aren't really scheduled as
long as the http-request isn't sent. In some way you have to look at
event-listener+send to make it work. I found it nicer to group it.
- http-request was the main reason people wanted tasks.

[floitsch, 2016-07-02 02:44:21]

Description was changed from

==========
Add zone task support to http-requests.
==========

to

==========
Add zone task support to http-requests.

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

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

[floitsch, 2016-07-02 02:44:22]

Committed patchset #4 (id:60001) manually as
b40cfcb57fa8d1890358f026d2c3f014909b2240 (presubmit successful).