[Cronet] Move initialization to a new thread rather than the UI thread.

components/cronet/android/java/src/org/chromium/net/impl/CronetLibraryLoader.java

 // Copyright 2015 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.impl;
 
 import android.content.Context;
 import android.os.Handler;
+import android.os.HandlerThread;
 import android.os.Looper;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.net.NetworkChangeNotifier;
 
 /**
- * CronetLibraryLoader loads and initializes native library on main thread.
+ * CronetLibraryLoader loads and initializes native library on init thread.
  */
 @JNINamespace("cronet")
 @VisibleForTesting
 public class CronetLibraryLoader {
     // Synchronize initialization.
     private static final Object sLoadLock = new Object();
     private static final String LIBRARY_NAME = "cronet." + ImplVersion.getCronetVersion();
     private static final String TAG = CronetLibraryLoader.class.getSimpleName();
+    // Thread used for initialization. This thread lives forever as things like the
+    // global singleton NetworkChangeNotifier live on it and are never killed.
+    private static final HandlerThread sInitThread = new HandlerThread("CronetInit");

[xunjieli, 2017/04/13 18:44:54]

I am fine with the word "init thread." However, since the thread lives forever
and handles messages from the UI thread, calling it initialization thread is a
slightly misleading. 

[pauljensen, 2017/05/01 15:35:53]

True, I've updated the comment appropriately.  I couldn't think of a variable
name that wasn't ridiculously long so I've left it called sInitThread.  Let me
know if you think I should rename the variable too.

     // Has library loading commenced?  Setting guarded by sLoadLock.
     private static volatile boolean sLibraryLoaded = false;
-    // Has ensureMainThreadInitialized() completed?  Only accessed on main thread.
-    private static volatile boolean sMainThreadInitDone = false;
+    // Has ensureInitThreadInitialized() completed?  Only accessed on init thread.
+    private static volatile boolean sInitThreadInitDone = false;
 
     /**
      * Ensure that native library is loaded and initialized. Can be called from
-     * any thread, the load and initialization is performed on main thread.
+     * any thread, the load and initialization is performed on init thread.
      */
     public static void ensureInitialized(
             final Context applicationContext, final CronetEngineBuilderImpl builder) {
         synchronized (sLoadLock) {
             if (!sLibraryLoaded) {
                 ContextUtils.initApplicationContext(applicationContext);
                 if (builder.libraryLoader() != null) {
                     builder.libraryLoader().loadLibrary(LIBRARY_NAME);
                 } else {
                     System.loadLibrary(LIBRARY_NAME);
                 }
                 ContextUtils.initApplicationContextForNative();
                 String implVersion = ImplVersion.getCronetVersion();
                 if (!implVersion.equals(nativeGetCronetVersion())) {
                     throw new RuntimeException(String.format("Expected Cronet version number %s, "
                                     + "actual version number %s.",
                             implVersion, nativeGetCronetVersion()));
                 }
                 Log.i(TAG, "Cronet version: %s, arch: %s", implVersion,
                         System.getProperty("os.arch"));
                 sLibraryLoaded = true;
             }
 
-            if (!sMainThreadInitDone) {
-                // Init native Chromium CronetEngine on Main UI thread.
-                Runnable task = new Runnable() {
+            if (!sInitThreadInitDone) {
+                if (!sInitThread.isAlive()) {

[xunjieli, 2017/04/13 18:44:54]

We could also lazily create sInitThread here to avoid the isAlive() check/state.

if (!sInitThread) {
   sInitThread = new HandlerThread();
   sInitThread.start();
}

[pauljensen, 2017/05/01 15:35:53]

We could but this would require making sInitThread non-final which somewhat
complicates reasoning about sInitThread.  For example one might then wonder if
synchronization to access sInitThread is necessary in postToInitThread() and
ensureInitializedOnInitThread() which I'd like to avoid as it goes against the
Chromium threading approach: "We discourage locking and threadsafe objects."
https://chromium.googlesource.com/chromium/src/+/master/docs/design/threading.md

I think sInitThread.isAlive() will generally only be called once, so I don't
think the performance impact will be significant.

+                    sInitThread.start();
+                }
+                postToInitThread(new Runnable() {
                     @Override
                     public void run() {
-                        ensureInitializedOnMainThread(applicationContext);
+                        ensureInitializedOnInitThread(applicationContext);
                     }
-                };
-                // Run task immediately or post it to the UI thread.
-                if (Looper.getMainLooper() == Looper.myLooper()) {
-                    task.run();
-                } else {
-                    // The initOnMainThread will complete on the main thread prior
-                    // to other tasks posted to the main thread.
-                    new Handler(Looper.getMainLooper()).post(task);
-                }
+                });
             }
         }
     }
 
     /**
-     * Ensure that the main thread initialization has completed. Can only be called from
-     * the main thread. Ensures that the NetworkChangeNotifier is initialzied and the
-     * main thread native MessageLoop is initialized.
+     * Ensure that the init thread initialization has completed. Can only be called from
+     * the init thread. Ensures that the NetworkChangeNotifier is initialzied and the
+     * init thread native MessageLoop is initialized.
      */
-    static void ensureInitializedOnMainThread(Context context) {
+    static void ensureInitializedOnInitThread(Context context) {
         assert sLibraryLoaded;
-        assert Looper.getMainLooper() == Looper.myLooper();
-        if (sMainThreadInitDone) {
+        assert sInitThread.getLooper() == Looper.myLooper();
+        if (sInitThreadInitDone) {
             return;
         }
         NetworkChangeNotifier.init(context);
         // Registers to always receive network notifications. Note
         // that this call is fine for Cronet because Cronet
         // embedders do not have API access to create network change
         // observers. Existing observers in the net stack do not
         // perform expensive work.
         NetworkChangeNotifier.registerToReceiveNotificationsAlways();
         // registerToReceiveNotificationsAlways() is called before the native
         // NetworkChangeNotifierAndroid is created, so as to avoid receiving
         // the undesired initial network change observer notification, which
         // will cause active requests to fail with ERR_NETWORK_CHANGED.
-        nativeCronetInitOnMainThread();
-        sMainThreadInitDone = true;
+        nativeCronetInitOnInitThread();
+        sInitThreadInitDone = true;
+    }
+
+    /**
+     * Run {@code r} on the initialization thread.
+     */
+    static void postToInitThread(Runnable r) {
+        new Handler(sInitThread.getLooper()).post(r);

[xunjieli, 2017/04/13 18:44:54]

Do we need to test to see if we aren't already on Init thread?
I know that embedder won't have access to Init thread, but if in the future we
might end up posting from the same thread. Maybe add a check so we can detect
that.

[pauljensen, 2017/05/01 15:35:53]

Done.

     }
 
     // Native methods are implemented in cronet_library_loader.cc.
-    private static native void nativeCronetInitOnMainThread();
+    private static native void nativeCronetInitOnInitThread();
     private static native String nativeGetCronetVersion();
 }