Keep track of the request and response state in the HTTP client connection

sdk/lib/io/http_impl.dart

Index: sdk/lib/io/http_impl.dart
diff --git a/sdk/lib/io/http_impl.dart b/sdk/lib/io/http_impl.dart
index cb2c2e0a8c714948cc91a268ae6bbfca75fbeaf4..1ed6764e6f3efa23f32fbd2658ab9b7fcec8aada 100644
--- a/sdk/lib/io/http_impl.dart
+++ b/sdk/lib/io/http_impl.dart
@@ -1667,6 +1667,7 @@ class _HttpClientRequest
     _httpConnection._onNoPendingWrites = null;
     // Ensure that any trailing data is written.
     _writeDone();
+    _connection._requestDone();
   }
 
   void _streamSetNoPendingWriteHandler(callback()) {
@@ -1803,7 +1804,7 @@ class _HttpClientResponse
         // TODO(sgjesse): Support digest.
         if (cr.scheme == _AuthenticationScheme.BASIC) {
           inputStream.onData = inputStream.read;
-          inputStream.onClosed = _connection.retry;
+          _connection._retry();
           return;
         }
       }
@@ -1897,7 +1898,7 @@ class _HttpClientResponse
         }
         // Drain body and redirect.
         inputStream.onData = inputStream.read;
-        inputStream.onClosed = _connection.redirect;
+        _connection.redirect();
       } else {
         throw new RedirectLimitExceededException(_connection._redirects);
       }
@@ -1952,6 +1953,11 @@ class _HttpClientResponse
 
 class _HttpClientConnection
     extends _HttpConnectionBase implements HttpClientConnection {
+  static const int NONE = 0;
+  static const int REQUEST_DONE = 1;
+  static const int RESPONSE_DONE = 2;
+  static const int ALL_DONE = REQUEST_DONE | RESPONSE_DONE;
+
   _HttpClientConnection(_HttpClient this._client);
 
   void _connectionEstablished(_SocketConnection socketConn) {
@@ -1971,16 +1977,40 @@ class _HttpClientConnection
     _httpParser.error = (e) => _onError(e);
   }
 
+  bool get _isRequestDone => (_state & REQUEST_DONE) == REQUEST_DONE;

[Mads Ager (google), 2012/11/06 09:30:57]

Maybe remove 'is' from all of these?

if (requestDone) ...;

[Søren Gjesse, 2012/11/06 11:28:46]

I would have liked this as well. However there is already a method called
_requestDone.

+  bool get _isResponseDone => (_state & RESPONSE_DONE) == RESPONSE_DONE;
+  bool get _isAllDone => (_state & ALL_DONE) == ALL_DONE;
+
+  void _checkSocketDone() {
+    if (_isAllDone) {
+      if (!_closing) {
+        _client._returnSocketConnection(_socketConn);
+      }
+      _socket = null;
+      _socketConn = null;
+      if (_pendingRedirect != null) {
+        _doRedirect(_pendingRedirect);
+      } else if (_pendingRetry != null) {
+        _doRetry(_pendingRetry);
+      }
+      _pendingRetry = null;

[Mads Ager (google), 2012/11/06 09:30:57]

Should we move the resetting of these to null into the branches above? Or maybe
add an assert that we never have both _pendingRedirect and _pendingRetry?

[Søren Gjesse, 2012/11/06 11:28:46]

Done (both).

+      _pendingRedirect = null;
+    }
+  }
+
+  void _requestDone() {
+    _state |= REQUEST_DONE;
+    _checkSocketDone();
+  }
+
   void _responseDone() {
     if (_closing) {
       if (_socket != null) {
         _socket.close();
       }
-    } else {
-      _client._returnSocketConnection(_socketConn);
     }
-    _socket = null;
-    _socketConn = null;
+    _state |= RESPONSE_DONE;
+    _checkSocketDone();
   }
 
   HttpClientRequest open(String method, Uri uri) {
@@ -2057,32 +2087,52 @@ class _HttpClientConnection
     _onErrorCallback = callback;
   }
 
-  void retry() {
-    if (_socketConn != null) {
-      throw new HttpException("Cannot retry with body data pending");
-    }
+  void _doRetry(_RedirectInfo retry) {
+    assert(_socketConn == null);
+    _request = null;
+    _response = null;
+
     // Retry the URL using the same connection instance.
-    _client._openUrl(_method, _request._uri, this);
+    _state = NONE;
+    _client._openUrl(retry.method, retry.location, this);
   }
 
-  void redirect([String method, Uri url]) {
-    if (_socketConn != null) {
-      throw new HttpException("Cannot redirect with body data pending");
+  void _retry() {
+    var retry = new _RedirectInfo(_response.statusCode, _method, _request._uri);
+    if (_isAllDone) {

[Mads Ager (google), 2012/11/06 09:30:57]

Maybe add a comment to retry and redirect that we postpone the actual
retry/redirect until both request and response are done.

[Søren Gjesse, 2012/11/06 11:28:46]

Done.

+      _doRetry(retry);
+    } else {
+      // Prepare for retry.
+      _pendingRetry = retry;
+    }
+  }
+
+  void _doRedirect(_RedirectInfo redirect) {
+    assert(_socketConn == null);
+
+    if (_redirects == null) {
+      _redirects = new List<_RedirectInfo>();
     }
+    _redirects.add(redirect);
+    _doRetry(redirect);
+  }
+
+  void redirect([String method, Uri url]) {
     if (method == null) method = _method;
     if (url == null) {
       url = new Uri.fromString(_response.headers.value(HttpHeaders.LOCATION));
     }
-    if (_redirects == null) {
-      _redirects = new List<_RedirectInfo>();
+    var redirect = new _RedirectInfo(_response.statusCode, method, url);
+    if (_isAllDone) {
+      _doRedirect(redirect);
+    } else {
+      // Prepare for redirect.
+      _pendingRedirect = redirect;
     }
-    _redirects.add(new _RedirectInfo(_response.statusCode, method, url));
-    _request = null;
-    _response = null;
-    // Open redirect URL using the same connection instance.
-    _client._openUrl(method, url, this);
   }
 
+  int _state = NONE;
+
   List<RedirectInfo> get redirects => _redirects;
 
   Function _onRequest;
@@ -2100,6 +2150,8 @@ class _HttpClientConnection
   bool followRedirects = true;
   int maxRedirects = 5;
   List<_RedirectInfo> _redirects;
+  _RedirectInfo _pendingRedirect;
+  _RedirectInfo _pendingRetry;
 
   // Callbacks.
   var requestReceived;