Refactoring of the HTTP library

Refactoring of the HTTP library

Better state handling: On both server and client the state of the
connection is tracked. This goes for both the HTTP
request/response state (idle, active, done) and the socket
state (open, read closed, write closed).

Simplified the socket event handling: All socket onData, onClosed
and onError events go directly to the HTTP parser. The HTTP
connection only receives events from the HTTP parser now.

Better close handling: A close queue have been added where
sockets which should be closed are added. When in the close queue
the socket will be closed when either all data from the output
stream are written or an error occours.

Changed the HTTP parser to parse either requests or responses.

Removed the null-checks in the HTTP parser for whether the
callbacks where set. The HTTP parser is only used internally and
we always set all callbacks.

Whether it fixes the bugs listed below still needs to be verified
through thorough testing on all platforms. However the code should be
in much better shape from this refactoring.

R=ager@google.com

BUG=dart:6393, dart:6521, dart:6594
Committed: https://code.google.com/p/dart/source/detail?r=14840

[Mads Ager (google), 2012-11-12 09:03:29]

This looks a lot cleaner!

One question below about potential socket double-close.

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

https://codereview.chromium.org/11410003/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:611: // peer.  closed.
Finish comment.

https://codereview.chromium.org/11410003/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:625: void checkFullyClosed() {
Maybe move the local function declaration to the top of the function so we have
first declarations then code?

Since the function only checks for write closed, maybe we should rename this to
closeIfDone or something?

https://codereview.chromium.org/11410003/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:628: // fully closing the socket. There seems to be
not need for that.
not -> no

What happens if we get the write close first and then a read close. Then we will
call checkFullyClosed twice and call close on the socket twice? Or can't we get
read close after write close?

[Søren Gjesse, 2012-11-12 12:36:37]

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

https://codereview.chromium.org/11410003/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:611: // peer.  closed.
On 2012/11/12 09:03:29, Mads Ager wrote:
> Finish comment.

Done.

https://codereview.chromium.org/11410003/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:625: void checkFullyClosed() {
On 2012/11/12 09:03:29, Mads Ager wrote:
> Maybe move the local function declaration to the top of the function so we
have
> first declarations then code?
> 
> Since the function only checks for write closed, maybe we should rename this
to
> closeIfDone or something?

Done.

https://codereview.chromium.org/11410003/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:628: // fully closing the socket. There seems to be
not need for that.
On 2012/11/12 09:03:29, Mads Ager wrote:
> not -> no
> 
> What happens if we get the write close first and then a read close. Then we
will
> call checkFullyClosed twice and call close on the socket twice? Or can't we
get
> read close after write close?

I have turned the read closed handler into a nop. As far as I can see we don't
care about write close. For both server and client connections the inbound
message have been read to completion when the socket enters the close queue. If
read closed have not been seen already we do expect it, but will still close
when writing is done.

[Søren Gjesse, 2012-11-13 10:23:41]

After having looked into the closing I think that the current implementation is
correct. When the output stream closes a socket shutdown is called before the
socket close is called. This should ensure that all data is sent.

There should be no need to wait for a socket shutdown from the other end.

The different closing cases is already tested in
tests/standalone/io/http_connection_close_test.dart.

[Mads Ager (google), 2012-11-13 10:31:21]

LGTM