[Cronet] Hook up library loader, system proxy and network change notifier to async api.

components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net;
 
 import android.content.Context;
 import android.os.Build;
+import android.os.ConditionVariable;
+import android.os.Handler;
+import android.os.Looper;
 import android.os.Process;
 import android.util.Log;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * UrlRequest context using Chromium HTTP stack implementation.
  */
 @JNINamespace("cronet")
 public class CronetUrlRequestContext extends UrlRequestContext  {
     private static final int LOG_NONE = 3;  // LOG(FATAL), no VLOG.
     private static final int LOG_DEBUG = -1;  // LOG(FATAL...INFO), VLOG(1)
     private static final int LOG_VERBOSE = -2;  // LOG(FATAL...INFO), VLOG(2)
     static final String LOG_TAG = "ChromiumNetwork";
 
+    /**
+     * Synchronize access to mUrlRequestContextAdapter and shutdown routine.
+     */
+    private final Object mLock = new Object();
     private long mUrlRequestContextAdapter = 0;
     private Thread mNetworkThread;
+    private final ConditionVariable mInitCompleted = new ConditionVariable(false);
+
     private AtomicInteger mActiveRequestCount = new AtomicInteger(0);
 
     public CronetUrlRequestContext(Context context,
                                    UrlRequestContextConfig config) {
+        CronetLibraryLoader.ensureInitialized(context, config);
         nativeSetMinLogLevel(getLoggingLevel());
         mUrlRequestContextAdapter = nativeCreateRequestContextAdapter(
                 context, config.toString());
         if (mUrlRequestContextAdapter == 0) {
             throw new NullPointerException("Context Adapter creation failed");
         }
+
+        // Post a task to UI thread to init native Chromium URLRequestContext.
+        // TODO(xunjieli): This constructor is not supposed to be invoked on
+        // the main thread. Consider making the following code into a blocking
+        // API to handle the case where we are already on main thread.

[mmenke, 2015/02/02 19:30:26]

Random comment:  Because this is a non-blocking API, I think this is something
we should do sooner rather than later.

[mef, 2015/02/03 01:28:57]

Done.

+        Runnable task = new Runnable() {
+            public void run() {
+                synchronized (mLock) {
+                    // mUrlRequestContextAdapter is guaranteed to exist until
+                    // initialization on main and network threads completes and
+                    // initNetworkThread is called back on network thread.
+                    nativeInitRequestContextOnMainThread(mUrlRequestContextAdapter);
+                    mInitCompleted.open();
+                }
+            }
+        };
+        new Handler(Looper.getMainLooper()).post(task);
     }
 
     @Override
     public UrlRequest createRequest(String url, UrlRequestListener listener,
                                     Executor executor) {
-        if (mUrlRequestContextAdapter == 0) {
-            throw new IllegalStateException(
-                    "Cannot create requests on shutdown context.");
+        synchronized (mLock) {
+            if (mUrlRequestContextAdapter == 0) {
+                throw new IllegalStateException("Context is shutdown.");
+            }
+            return new CronetUrlRequest(this, mUrlRequestContextAdapter, url,
+                    UrlRequest.REQUEST_PRIORITY_MEDIUM, listener, executor);
         }
-        return new CronetUrlRequest(this, mUrlRequestContextAdapter, url,
-                UrlRequest.REQUEST_PRIORITY_MEDIUM, listener, executor);
     }
 
     @Override
     public boolean isEnabled() {
         return Build.VERSION.SDK_INT >= 14;
     }
 
     @Override
     public String getVersionString() {
         return "Cronet/" + Version.getVersion();
     }
 
     @Override
     public void shutdown() {
-        if (mActiveRequestCount.get() != 0) {
-            throw new IllegalStateException(
-                    "Cannot shutdown with active requests.");
+        synchronized (mLock) {
+            if (mUrlRequestContextAdapter == 0) {
+                throw new IllegalStateException(
+                        "Context is already shutdown.");
+            }
+            if (mActiveRequestCount.get() != 0) {
+                throw new IllegalStateException(
+                        "Cannot shutdown with active requests.");
+            }
+            // Destroying adapter stops the network thread, so it cannot be
+            // called on network thread.
+            if (Thread.currentThread() == mNetworkThread) {
+                throw new IllegalThreadStateException(
+                        "Cannot shutdown from network thread.");
+            }
         }
-        // Destroying adapter stops the network thread, so it cannot be called
-        // on network thread.
-        if (Thread.currentThread() == mNetworkThread) {
-            throw new IllegalThreadStateException(
-                    "Cannot shutdown from network thread.");
+        // Wait for init to complete on main thread (without lock, so main thread
+        // could access it).
+        mInitCompleted.block();

[mmenke, 2015/02/02 19:30:26]

This doesn't work if shutdown is called on two threads at once.  Both could end
up blocking.  Then when both are released...Either both end up trying to delete
the adapter, one ends up trying to delete a null adapter.  Simplest solution is
just to add a check in the synchronized block below to early return if the
adapter is null.

[mef, 2015/02/03 01:28:57]

Done.

+
+        long urlRequestContextAdapter;
+        synchronized (mLock) {
+            urlRequestContextAdapter = mUrlRequestContextAdapter;
+            mUrlRequestContextAdapter = 0;
         }
-        nativeDestroyRequestContextAdapter(mUrlRequestContextAdapter);
-        mUrlRequestContextAdapter = 0;
+        nativeDestroyRequestContextAdapter(urlRequestContextAdapter);

[mmenke, 2015/02/02 19:30:26]

Can we just move this into the synchronized block, and get rid of the temporary?

[mef, 2015/02/03 01:28:57]

Unfortunately nativeDestroyRequestContextAdapter under lock deadlocks in
testShutdownDuringInit but I cannot find a plausible explanation why. I'll try
to debug it.

[mef, 2015/02/03 14:29:03]

Done. The deadlock was due to synchronized (mLock) in initNetworkThread().
I've moved mInitCompleted.open() after initialization completes on network
thread,
so there is no deadlock.

     }
 
     @Override
     public void startNetLogToFile(String fileName) {
-        nativeStartNetLogToFile(mUrlRequestContextAdapter, fileName);
+        synchronized (mLock) {
+            if (mUrlRequestContextAdapter == 0) {
+                throw new IllegalStateException(
+                        "Context is shutdown.");
+            }
+            nativeStartNetLogToFile(mUrlRequestContextAdapter, fileName);
+        }
     }
 
     @Override
     public void stopNetLog() {
-        nativeStopNetLog(mUrlRequestContextAdapter);
+        synchronized (mLock) {
+            if (mUrlRequestContextAdapter == 0) {
+                throw new IllegalStateException(
+                        "Context is shutdown.");
+            }
+            nativeStopNetLog(mUrlRequestContextAdapter);
+        }
     }
 
     /**
      * Mark request as started to prevent shutdown when there are active
      * requests.
      */
     void onRequestStarted(UrlRequest urlRequest) {
         mActiveRequestCount.incrementAndGet();
     }
 
     /**
      * Mark request as completed to allow shutdown when there are no active
      * requests.
      */
     void onRequestDestroyed(UrlRequest urlRequest) {
         mActiveRequestCount.decrementAndGet();
     }
 
     long getUrlRequestContextAdapter() {
-        if (mUrlRequestContextAdapter == 0) {
-            throw new IllegalStateException("Context Adapter is destroyed.");
+        synchronized (mLock) {
+            if (mUrlRequestContextAdapter == 0) {
+                throw new IllegalStateException("Context is shutdown.");
+            }
+            return mUrlRequestContextAdapter;
         }
-        return mUrlRequestContextAdapter;
     }
 
     /**
      * @return loggingLevel see {@link #LOG_NONE}, {@link #LOG_DEBUG} and
      *         {@link #LOG_VERBOSE}.
      */
     private int getLoggingLevel() {
         int loggingLevel;
         if (Log.isLoggable(LOG_TAG, Log.VERBOSE)) {
             loggingLevel = LOG_VERBOSE;
         } else if (Log.isLoggable(LOG_TAG, Log.DEBUG)) {
             loggingLevel = LOG_DEBUG;
         } else {
             loggingLevel = LOG_NONE;
         }
         return loggingLevel;
     }
 
     @SuppressWarnings("unused")
     @CalledByNative
     private void initNetworkThread() {
-        mNetworkThread = Thread.currentThread();
+        synchronized (mLock) {
+            mNetworkThread = Thread.currentThread();
+        }
         Thread.currentThread().setName("ChromiumNet");
         Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
     }
 
     // Native methods are implemented in cronet_url_request_context.cc.
     private native long nativeCreateRequestContextAdapter(Context context,
             String config);
 
     private native void nativeDestroyRequestContextAdapter(
             long urlRequestContextAdapter);
 
     private native void nativeStartNetLogToFile(
             long urlRequestContextAdapter, String fileName);
 
     private native void nativeStopNetLog(long urlRequestContextAdapter);
 
     private native int nativeSetMinLogLevel(int loggingLevel);
+
+    private native void nativeInitRequestContextOnMainThread(
+            long urlRequestContextAdapter);
 }