Merge _idleConnections and _activeConnections into one structure, _ConnectionTarget.

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 70b81b1cb91e569d4dafe82d0b743354c84442f7..67869066d80d27393c53329f9b4821a4cae0b7cf 100644
--- a/sdk/lib/io/http_impl.dart
+++ b/sdk/lib/io/http_impl.dart
@@ -1502,12 +1502,44 @@ class _ConnnectionInfo {
 }
 
 
+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;
@@ -1524,12 +1556,13 @@ class _HttpClient implements HttpClient {
 
   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,
@@ -1585,26 +1618,9 @@ class _HttpClient implements HttpClient {
 
   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)) {
@@ -1703,18 +1719,7 @@ class _HttpClient implements HttpClient {
 
   // 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();
   }
@@ -1722,28 +1727,26 @@ class _HttpClient implements HttpClient {
   // 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;
           }
@@ -1769,16 +1772,12 @@ class _HttpClient implements HttpClient {
       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));
       }
@@ -1800,15 +1799,16 @@ class _HttpClient implements HttpClient {
             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) {