Merge _idleConnections and _activeConnections into one structure, _ConnectionTarget.

sdk/lib/io/http_impl.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of dart.io;
 
 const int _OUTGOING_BUFFER_SIZE = 8 * 1024;
 
 class _HttpIncoming extends Stream<List<int>> {
   final int _transferLength;
@@ skipping 1484 matching lines @@
 }
 
 class _ConnnectionInfo {
   final _HttpClientConnection connection;
   final _Proxy proxy;
 
   _ConnnectionInfo(this.connection, this.proxy);
 }
 
 
+class _ConnectionTarget {
+  // Unique key for this connection target.
+  final String key;
+  final Set<_HttpClientConnection> idle = new HashSet();

[Søren Gjesse, 2014/05/22 08:03:23]

Rename idle and active to _idle and _active to indicate they are private. And
add methods to avoid direct access to these sets.

[Anders Johnsen, 2014/05/23 07:47:55]

Done.

+  final Set<_HttpClientConnection> active = new HashSet();
+
+  _ConnectionTarget(this.key);
+
+  bool get isEmpty => idle.isEmpty && active.isEmpty;
+
+  void returnConnection(_HttpClientConnection connection) {
+    assert(active.contains(connection));
+    active.remove(connection);
+    idle.add(connection);
+  }
+
+  void connectionClosed(_HttpClientConnection connection) {

[Søren Gjesse, 2014/05/22 08:03:23]

Add the assert from above here as well.

[Anders Johnsen, 2014/05/23 07:47:55]

Done.

+    active.remove(connection);
+    idle.remove(connection);
+  }
+
+  void close(bool force) {
+    for (var c in idle.toList()) {
+      c.close();
+    }
+    if (force) {
+      for (var c in active.toList()) {
+        c.destroy();
+      }
+    }
+  }
+}
+
+
 class _HttpClient implements HttpClient {
   bool _closing = false;
-  final Map<String, Set<_HttpClientConnection>> _idleConnections
-      = new HashMap<String, Set<_HttpClientConnection>>();
-  final Map<String, Set<_HttpClientConnection>> _activeConnections
-      = new HashMap<String, Set<_HttpClientConnection>>();
+  final Map<String, _ConnectionTarget> _connectionTargets
+      = new HashMap<String, _ConnectionTarget>();
   final List<_Credentials> _credentials = [];
   final List<_ProxyCredentials> _proxyCredentials = [];
   Function _authenticate;
   Function _authenticateProxy;
   Function _findProxy = HttpClient.findProxyFromEnvironment;
   Duration _idleTimeout = const Duration(seconds: 15);
   Function _badCertificateCallback;
 
   Timer _noActiveTimer;
 
   Duration get idleTimeout => _idleTimeout;
 
   String userAgent = _getHttpVersion();
 
   void set idleTimeout(Duration timeout) {
     _idleTimeout = timeout;
-    _idleConnections.values.forEach(
-        (l) => l.forEach((c) {
-          // Reset timer. This is fine, as it's not happening often.
-          c.stopTimer();
-          c.startTimer();
-        }));
+    for (var c in _connectionTargets.values) {
+      for (var idle in c.idle) {
+        // Reset timer. This is fine, as it's not happening often.
+        idle.stopTimer();
+        idle.startTimer();
+      }
+    }
   }
 
   set badCertificateCallback(bool callback(X509Certificate cert,
                                            String host,
                                            int port)) {
     _badCertificateCallback = callback;
   }
 
 
   Future<HttpClientRequest> open(String method,
@@ skipping 35 matching lines @@
 
   Future<HttpClientRequest> headUrl(Uri url) => _openUrl("head", url);
 
   Future<HttpClientRequest> patch(String host, int port, String path)
       => open("patch", host, port, path);
 
   Future<HttpClientRequest> patchUrl(Uri url) => _openUrl("patch", url);
 
   void close({bool force: false}) {
     _closing = true;
-    // Create flattened copy of _idleConnections, as 'destory' will manipulate
-    // it.
-    var idle = _idleConnections.values.fold(
-        [],
-        (l, e) {
-          l.addAll(e);
-          return l;
-        });
-    idle.forEach((e) {
-      e.close();
-    });
-    assert(_idleConnections.isEmpty);
-    if (force) {
-      for (var connection in
-           _activeConnections.values.expand((s) => s).toList()) {
-        connection.destroy();
-      }
-      assert(_activeConnections.isEmpty);
-      _activeConnections.clear();
-    }
+    _connectionTargets.values.toList().forEach((c) => c.close(force));
+    assert(!_connectionTargets.values.any((s) => s.idle.isNotEmpty));
+    assert(!force || _connectionTargets.isEmpty);
   }
 
   set authenticate(Future<bool> f(Uri url, String scheme, String realm)) {
     _authenticate = f;
   }
 
   void addCredentials(Uri url, String realm, HttpClientCredentials cr) =>
       _credentials.add(new _SiteCredentials(url, realm, cr));
 
   set authenticateProxy(
@@ skipping 78 matching lines @@
             }
           }
           return request
               ..headers.chunkedTransferEncoding = false
               ..contentLength = 0;
         });
   }
 
   // Return a live connection to the idle pool.
   void _returnConnection(_HttpClientConnection connection) {
-    var key = connection.key;
-    _activeConnections[key].remove(connection);
-    if (_activeConnections[key].isEmpty) {
-      _activeConnections.remove(key);
-    }
-    if (_closing) {
-      connection.close();
-      return;
-    }
-    _idleConnections
-        .putIfAbsent(key, () => new HashSet())
-        .add(connection);
+    _connectionTargets[connection.key].returnConnection(connection);
     connection.startTimer();
     _updateTimers();
   }
 
   // Remove a closed connnection from the active set.
   void _connectionClosed(_HttpClientConnection connection) {
     connection.stopTimer();
-    var key = connection.key;
-    if (_activeConnections.containsKey(key)) {
-      _activeConnections[key].remove(connection);
-      if (_activeConnections[key].isEmpty) {
-        _activeConnections.remove(key);
+    var connectionTarget = _connectionTargets[connection.key];
+    if (connectionTarget != null) {
+      connectionTarget.connectionClosed(connection);
+      if (connectionTarget.isEmpty) {
+        _connectionTargets.remove(connection.key);
       }
+      _updateTimers();
     }
-    if (_idleConnections.containsKey(key)) {
-      _idleConnections[key].remove(connection);
-      if (_idleConnections[key].isEmpty) {
-        _idleConnections.remove(key);
-      }
-    }
-    _updateTimers();
   }
 
   void _updateTimers() {
-    if (_activeConnections.isEmpty) {
-      if (!_idleConnections.isEmpty && _noActiveTimer == null) {
+    bool hasActive = _connectionTargets.values.any((t) => t.active.isNotEmpty);

[Søren Gjesse, 2014/05/22 08:03:23]

Add hasActive (or hasActiveConnections) to ConnectionTarget to change

(t) => t.active.isNotEmpty

to

(t) => t.hasActiveConnections

[Anders Johnsen, 2014/05/23 07:47:55]

Done.

+    if (!hasActive) {
+      bool hasIdle = _connectionTargets.values.any((t) => t.idle.isNotEmpty);

[Søren Gjesse, 2014/05/22 08:03:23]

Add hasIdle (or hasIdleConnections) to ConnectionTarget.

[Anders Johnsen, 2014/05/23 07:47:55]

Done.

+      if (hasIdle && _noActiveTimer == null) {
         _noActiveTimer = new Timer(const Duration(milliseconds: 100), () {
           _noActiveTimer = null;
-          if (_activeConnections.isEmpty) {
+          bool hasActive =
+              _connectionTargets.values.any((t) => t.active.isNotEmpty);

[Søren Gjesse, 2014/05/22 08:03:23]

Ditto.

[Anders Johnsen, 2014/05/23 07:47:55]

Done.

+          if (!hasActive) {
             close();
             _closing = false;
           }
         });
       }
     } else if (_noActiveTimer != null) {
       _noActiveTimer.cancel();
       _noActiveTimer = null;
     }
   }
 
   // Get a new _HttpClientConnection, either from the idle pool or created from
   // a new Socket.
   Future<_ConnnectionInfo> _getConnection(String uriHost,
                                           int uriPort,
                                           _ProxyConfiguration proxyConf,
                                           bool isSecure) {
     Iterator<_Proxy> proxies = proxyConf.proxies.iterator;
 
     Future<_ConnnectionInfo> connect(error) {
       if (!proxies.moveNext()) return new Future.error(error);
       _Proxy proxy = proxies.current;
       String host = proxy.isDirect ? uriHost: proxy.host;
       int port = proxy.isDirect ? uriPort: proxy.port;
       String key = _HttpClientConnection.makeKey(isSecure, host, port);
-      if (_idleConnections.containsKey(key)) {
-        var connection = _idleConnections[key].first;
-        _idleConnections[key].remove(connection);
-        if (_idleConnections[key].isEmpty) {
-          _idleConnections.remove(key);
-        }
+      var connectionTarget = _connectionTargets[key];
+      if (connectionTarget != null && connectionTarget.idle.isNotEmpty) {

[Søren Gjesse, 2014/05/22 08:03:23]

Add method getConnection to ConnectionTarget to avoid direct access to idle and
active members,

[Anders Johnsen, 2014/05/23 07:47:55]

Done.

+        var connection = connectionTarget.idle.first;
+        connectionTarget.idle.remove(connection);
         connection.stopTimer();
-        _activeConnections
-            .putIfAbsent(key, () => new HashSet())
-            .add(connection);
+        connectionTarget.active.add(connection);
         _updateTimers();
         return new Future.value(new _ConnnectionInfo(connection, proxy));
       }
       var currentBadCertificateCallback = _badCertificateCallback;
       bool callback(X509Certificate certificate) =>
           currentBadCertificateCallback == null ? false :
           currentBadCertificateCallback(certificate, uriHost, uriPort);
       Future socketFuture = (isSecure && proxy.isDirect
           ? SecureSocket.connect(host,
                                  port,
                                  sendClientCertificate: true,
                                  onBadCertificate: callback)
           : Socket.connect(host, port));
       return socketFuture.then((socket) {
           socket.setOption(SocketOption.TCP_NODELAY, true);
           var connection = new _HttpClientConnection(key, socket, this);
           if (isSecure && !proxy.isDirect) {
             connection._dispose = true;
             return connection.createProxyTunnel(
                 uriHost, uriPort, proxy, callback)
                 .then((tunnel) {
-                  _activeConnections
-                      .putIfAbsent(tunnel.key, () => new HashSet())
-                      .add(tunnel);
+                  var connectionTarget = _connectionTargets
+                      .putIfAbsent(tunnel.key,
+                                   () => new _ConnectionTarget(tunnel.key));
+                  connectionTarget.active.add(tunnel);

[Søren Gjesse, 2014/05/22 08:03:23]

Add method addConnection to ConnectionTarget

[Anders Johnsen, 2014/05/23 07:47:55]

Done.

                   return new _ConnnectionInfo(tunnel, proxy);
                 });
           } else {
-            _activeConnections
-                .putIfAbsent(key, () => new HashSet())
-                .add(connection);
+            var connectionTarget = _connectionTargets
+                .putIfAbsent(key, () => new _ConnectionTarget(key));
+            connectionTarget.active.add(connection);
             return new _ConnnectionInfo(connection, proxy);
           }
         }, onError: (error) {
           // Continue with next proxy.
           return connect(error);
         });
     }
     return connect(new HttpException("No proxies given"));
   }
 
@@ skipping 814 matching lines @@
   const _RedirectInfo(this.statusCode, this.method, this.location);
 }
 
 String _getHttpVersion() {
   var version = Platform.version;
   // Only include major and minor version numbers.
   int index = version.indexOf('.', version.indexOf('.') + 1);
   version = version.substring(0, index);
   return 'Dart/$version (dart:io)';
 }