Merge "Refactor Network's use of mUrlConnectionFactory." am: f06634ae70

Change-Id: I3ad8fd8479e91bc34b21297b7953a38372431a8c

core/java/android/net/Network.java

@@ -27,6 +27,7 @@ import android.system.Os;
 import android.system.OsConstants;
 import android.util.proto.ProtoOutputStream;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.okhttp.internalandroidapi.Dns;
 import com.android.okhttp.internalandroidapi.HttpURLConnectionFactory;
 
@@ -70,9 +71,9 @@ public class Network implements Parcelable {
     // Objects used to perform per-network operations such as getSocketFactory
     // and openConnection, and a lock to protect access to them.
     private volatile NetworkBoundSocketFactory mNetworkBoundSocketFactory = null;
-    // mLock should be used to control write access to mUrlConnectionFactory.
+    // mUrlConnectionFactory is initialized lazily when it is first needed.
-    // maybeInitUrlConnectionFactory() must be called prior to reading this field.
+    @GuardedBy("mLock")
-    private volatile HttpURLConnectionFactory mUrlConnectionFactory;
+    private HttpURLConnectionFactory mUrlConnectionFactory;
     private final Object mLock = new Object();
 
     // Default connection pool values. These are evaluated at startup, just
@@ -284,36 +285,16 @@ public class Network implements Parcelable {
         return mNetworkBoundSocketFactory;
     }
 
-    // TODO: This creates a connection pool and host resolver for
+    private static HttpURLConnectionFactory createUrlConnectionFactory(Dns dnsLookup) {
-    // every Network object, instead of one for every NetId. This is
+        // Set configuration on the HttpURLConnectionFactory that will be good for all
-    // suboptimal, because an app could potentially have more than one
+        // connections created by this Network. Configuration that might vary is left
-    // Network object for the same NetId, causing increased memory footprint
+        // until openConnection() and passed as arguments.
-    // and performance penalties due to lack of connection reuse (connection
+        HttpURLConnectionFactory urlConnectionFactory = new HttpURLConnectionFactory();
-    // setup time, congestion window growth time, etc.).
+        urlConnectionFactory.setDns(dnsLookup); // Let traffic go via dnsLookup
-    //
+        // A private connection pool just for this Network.
-    // Instead, investigate only having one connection pool and host resolver
+        urlConnectionFactory.setNewConnectionPool(httpMaxConnections,
-    // for every NetId, perhaps by using a static HashMap of NetIds to
+                httpKeepAliveDurationMs, TimeUnit.MILLISECONDS);
-    // connection pools and host resolvers. The tricky part is deciding when
+        return urlConnectionFactory;
-    // to remove a map entry; a WeakHashMap shouldn't be used because whether
-    // a Network is referenced doesn't correlate with whether a new Network
-    // will be instantiated in the near future with the same NetID. A good
-    // solution would involve purging empty (or when all connections are timed
-    // out) ConnectionPools.
-    private void maybeInitUrlConnectionFactory() {
-        synchronized (mLock) {
-            if (mUrlConnectionFactory == null) {
-                // Set configuration on the HttpURLConnectionFactory that will be good for all
-                // connections created by this Network. Configuration that might vary is left
-                // until openConnection() and passed as arguments.
-                Dns dnsLookup = hostname -> Arrays.asList(Network.this.getAllByName(hostname));
-                HttpURLConnectionFactory urlConnectionFactory = new HttpURLConnectionFactory();
-                urlConnectionFactory.setDns(dnsLookup); // Let traffic go via dnsLookup
-                // A private connection pool just for this Network.
-                urlConnectionFactory.setNewConnectionPool(httpMaxConnections,
-                        httpKeepAliveDurationMs, TimeUnit.MILLISECONDS);
-                mUrlConnectionFactory = urlConnectionFactory;
-            }
-        }
     }
 
     /**
@@ -354,9 +335,31 @@ public class Network implements Parcelable {
      */
     public URLConnection openConnection(URL url, java.net.Proxy proxy) throws IOException {
         if (proxy == null) throw new IllegalArgumentException("proxy is null");
-        maybeInitUrlConnectionFactory();
+        // TODO: This creates a connection pool and host resolver for
+        // every Network object, instead of one for every NetId. This is
+        // suboptimal, because an app could potentially have more than one
+        // Network object for the same NetId, causing increased memory footprint
+        // and performance penalties due to lack of connection reuse (connection
+        // setup time, congestion window growth time, etc.).
+        //
+        // Instead, investigate only having one connection pool and host resolver
+        // for every NetId, perhaps by using a static HashMap of NetIds to
+        // connection pools and host resolvers. The tricky part is deciding when
+        // to remove a map entry; a WeakHashMap shouldn't be used because whether
+        // a Network is referenced doesn't correlate with whether a new Network
+        // will be instantiated in the near future with the same NetID. A good
+        // solution would involve purging empty (or when all connections are timed
+        // out) ConnectionPools.
+        final HttpURLConnectionFactory urlConnectionFactory;
+        synchronized (mLock) {
+            if (mUrlConnectionFactory == null) {
+                Dns dnsLookup = hostname -> Arrays.asList(getAllByName(hostname));
+                mUrlConnectionFactory = createUrlConnectionFactory(dnsLookup);
+            }
+            urlConnectionFactory = mUrlConnectionFactory;
+        }
         SocketFactory socketFactory = getSocketFactory();
-        return mUrlConnectionFactory.openConnection(url, socketFactory, proxy);
+        return urlConnectionFactory.openConnection(url, socketFactory, proxy);
     }
 
     /**