Add implementations for Headers, Request, Response and dart:io/dart:html clients to http_base

Add implementations for Headers, Request, Response and dart:io/dart:html clients to http_base

R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=38984

[kustermann, 2014-08-07 11:46:00]

I want to emphasize again, that I strongly dislike having two http client
implementations.

[Søren Gjesse, 2014-08-07 12:16:23]

lgtm

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
File dart/pkg/http_base/lib/http_base.dart (right):

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:135: _m[key] = value;
Shouldn't this be a copy of the list?

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:195: /// An implementation of [Request].
Please add comment to say that this is immutable, except through the use of read
(and replace which calls read).

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:215: /// An implementation of [Response].
ditto.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:227: /// of the body stream to the
returned object.
Add this comment above as well.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
File dart/pkg/http_base/lib/http_base_html.dart (right):

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base_html.dart:11: /// The following headers will be
blocked by browsers.
Add the reference to http://www.w3.org/TR/XMLHttpRequest/.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base_html.dart:28: return new ResponseImpl(
Fits one line?

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/test/...
File dart/pkg/http_base/test/http_base_io_test.dart (right):

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/test/...
dart/pkg/http_base/test/http_base_io_test.dart:37: equals(['foo','bar']));
Also test that response.headers['foo'] is 'foo,bar'.

[kustermann, 2014-08-07 12:50:38]

You can take another look if you want to.

One question is whether getMultiple() should split on ';' for Cookies?!

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
File dart/pkg/http_base/lib/http_base.dart (right):

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:135: _m[key] = value;
On 2014/08/07 12:16:22, Søren Gjesse wrote:
> Shouldn't this be a copy of the list?

Yes.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:195: /// An implementation of [Request].
On 2014/08/07 12:16:22, Søren Gjesse wrote:
> Please add comment to say that this is immutable, except through the use of
read
> (and replace which calls read).

Done. But the documentation on the stream ownership is at the replace method.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:215: /// An implementation of [Response].
On 2014/08/07 12:16:22, Søren Gjesse wrote:
> ditto.

Done.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base.dart:227: /// of the body stream to the
returned object.
On 2014/08/07 12:16:22, Søren Gjesse wrote:
> Add this comment above as well.

Done.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
File dart/pkg/http_base/lib/http_base_html.dart (right):

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base_html.dart:11: /// The following headers will be
blocked by browsers.
On 2014/08/07 12:16:22, Søren Gjesse wrote:
> Add the reference to http://www.w3.org/TR/XMLHttpRequest/.

Currently I don't use this list, but it is here for documentation anyway. Since
it's private, it doesn't hurt anyone.

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/lib/h...
dart/pkg/http_base/lib/http_base_html.dart:28: return new ResponseImpl(
On 2014/08/07 12:16:22, Søren Gjesse wrote:
> Fits one line?

Yes :)

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/test/...
File dart/pkg/http_base/test/http_base_io_test.dart (right):

https://codereview.chromium.org/445933004/diff/40001/dart/pkg/http_base/test/...
dart/pkg/http_base/test/http_base_io_test.dart:37: equals(['foo','bar']));
On 2014/08/07 12:16:22, Søren Gjesse wrote:
> Also test that response.headers['foo'] is 'foo,bar'.

This should be covered by the tests for HeaderImpl, but I can add it.

[Søren Gjesse, 2014-08-07 12:57:20]

lgtm

[kustermann, 2014-08-07 13:02:15]

Committed patchset #3 manually as r38984 (presubmit successful).