Rewrite parts of http-parser, to better handle connection errors and pausing.

Rewrite parts of http-parser, to better handle connection errors and pausing.

We now correctly catch up and report 'dead' connections in the
HttpClient.

BUG=
R=sgjesse@google.com

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

[Søren Gjesse, 2013-05-17 07:09:28]

LGTM!

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:1074: bool closed = false;
Private? (also key above).

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:1093: // We assume the response is not here, until we
have send the request.
Shouldn't this be an error instead of an assert? A server not conforming to the
protocol should not hit an assert.

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:1094: assert(_nextResponseCompleter != null);
Do we still need this temp variable after the completion changed to be always
async?

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_parser.dart
File sdk/lib/io/http_parser.dart (right):

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_parser.dart#n...
sdk/lib/io/http_parser.dart:284: if (_bodyPaused) {
Don't we need to check for both _paused and _bodyPaused? Without we just go
ahead and parse the whole header evne though the stream of incoming is paused -
maybe that's fine - but could deserve a comment.

[Anders Johnsen, 2013-05-17 08:27:59]

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:1074: bool closed = false;
On 2013/05/17 07:09:28, Søren Gjesse wrote:
> Private? (also key above).

This is used from another class (HttpClient). Also, this class is never exposed
to the user. Same with key.

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:1093: // We assume the response is not here, until we
have send the request.
On 2013/05/17 07:09:28, Søren Gjesse wrote:
> Shouldn't this be an error instead of an assert? A server not conforming to
the
> protocol should not hit an assert.

Done.

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:1094: assert(_nextResponseCompleter != null);
On 2013/05/17 07:09:28, Søren Gjesse wrote:
> Do we still need this temp variable after the completion changed to be always
> async?

Done.

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_parser.dart
File sdk/lib/io/http_parser.dart (right):

https://codereview.chromium.org/15256002/diff/1/sdk/lib/io/http_parser.dart#n...
sdk/lib/io/http_parser.dart:284: if (_bodyPaused) {
On 2013/05/17 07:09:28, Søren Gjesse wrote:
> Don't we need to check for both _paused and _bodyPaused? Without we just go
> ahead and parse the whole header evne though the stream of incoming is paused
-
> maybe that's fine - but could deserve a comment.

Done.

[Anders Johnsen, 2013-05-17 08:28:29]

Committed patchset #2 manually as r22834 (presubmit successful).