[Android] Allow JNI initialization on background thread

content/public/android/java/src/org/chromium/content/app/LibraryLoader.java

Index: content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
diff --git a/content/public/android/java/src/org/chromium/content/app/LibraryLoader.java b/content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
index 0facd0b31ebae5194207e36db3a03283334bb954..7f25b893f811b4fd2045b71d343e11f0e4b7b398 100644
--- a/content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
+++ b/content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
@@ -15,13 +15,11 @@ import org.chromium.content.common.TraceEvent;
 
 /**
  * This class provides functionality to load and register the native library.
- * In most cases, users will call ensureInitialized() from their main thread
- * (only) which ensures a post condition that the library is loaded,
- * initialized, and ready to use.
- * Optionally, an application may optimize startup be calling loadNow early on,
- * from a background thread, and then on completion of that method it must call
- * ensureInitialized() on the main thread before it tries to access any native
- * code.
+ * For compatibility with old code callers are allowed to separate loading the
+ * library from initializing it, although there is no longer any particular
+ * advantage in doing so.

[joth, 2013/03/18 16:32:55]

There was an extra need: for webview we need to do this in two parts: one that's
safe to do in the zygote process (specifically, than runs single threaded and
does not create any threads) and then the second part that is run per-app and
really starts chromium (launching threads etc).
The existing delineation suits this very well.
see AwBrowserProcess.loadLibrary vs. AwBrowserProcess.start


But now I look, I can't see anywhere obvious in LibraryLoadedOnMainThread() that
does create threads. It's always possible that someone adds code in e.g.
logging::Init() that creates a thread though.... ideas if there some way we can
stop this regressing in future? ScopedDisallowCreateThreads? :-)

[aberent, 2013/03/18 18:44:54]

I have changed the comment to mention the Webview case. 

I don't think my code change actually makes a difference to the scope for
incorrect thread creation in the first stage of initialization. I think all C++
threads in Chromium are created by PlatformThread class, so in theory one could
create a ScopedDisallowCreateThreads that was checked in PlatformThread.

On 2013/03/18 16:32:55, joth wrote:

+ * The library may be loaded and initialized from any thread, although we
+ * must prevent two threads trying to do so simultaneously.

[joth, 2013/03/18 16:32:55]

nit: prevent => handle. (prevent makes me look for an assert / exception thrown)

[aberent, 2013/03/18 18:44:54]

I have rewritten this paragraph to make this clearer.

  */
 @JNINamespace("content")
 public class LibraryLoader {
@@ -30,7 +28,7 @@ public class LibraryLoader {
     private static String sLibrary = null;
 
     // This object's lock guards sLoaded assignment and also the library load.
-    private static Object sLoadedLock = new Object();
+    private static Object sLoadLock = new Object();
     private static Boolean sLoaded = false;
 
     private static boolean sInitialized = false;
@@ -57,19 +55,20 @@ public class LibraryLoader {
     }
 
     /**
-     *  This method blocks until the library is fully loaded and initialized;
-     *  must be called on the thread that the native will call its "main" thread.
+     *  This method blocks until the library is fully loaded and initialized.
      */
     public static void ensureInitialized() throws ProcessInitException {
-        checkThreadUsage();
-        if (sInitialized) {
-            // Already initialized, nothing to do.
-            return;
+        synchronized(sLoadLock) {
+            if (sInitialized) {
+                // Already initialized, nothing to do.
+                return;
+            }
+            loadAlreadyLocked();
+            initializeAlreadyLocked(CommandLine.getJavaSwitchesOrNull());
         }
-        loadNow();
-        initializeOnMainThread();
     }
 
+
     /**
      * Loads the library and blocks until the load completes. The caller is responsible
      * for subsequently calling ensureInitialized().
@@ -80,11 +79,31 @@ public class LibraryLoader {
      * @throws ProcessInitException if the native library failed to load.
      */
     public static void loadNow() throws ProcessInitException {
+        synchronized(sLoadLock) {
+            loadAlreadyLocked();
+        }
+    }
+
+
+    /**
+     * initializes the library here and now: must be called on the thread that the
+     * native will call its "main" thread. The library must have previously been
+     * loaded with loadNow.
+     * @param initCommandLine The command line arguments that native command line will
+     * be initialized with.
+     */
+    static void initialize(String[] initCommandLine) throws ProcessInitException {
+        synchronized(sLoadLock) {

[bulach, 2013/03/18 16:21:11]

nit: space after synchronized

[aberent, 2013/03/18 18:44:54]

Done.

+            initializeAlreadyLocked(initCommandLine);
+        }
+    }
+
+
+    private static void loadAlreadyLocked() throws ProcessInitException {
         if (sLibrary == null) {
             assert false : "No library specified to load.  Call setLibraryToLoad before first.";
         }
         try {
-            synchronized (sLoadedLock) {
                 if (!sLoaded) {

[bulach, 2013/03/18 16:21:11]

nit: unindent this block 107-113

[aberent, 2013/03/18 18:44:54]

Done.

                     assert !sInitialized;
                     Log.i(TAG, "loading: " + sLibrary);
@@ -92,25 +111,18 @@ public class LibraryLoader {
                     Log.i(TAG, "loaded: " + sLibrary);
                     sLoaded = true;
                 }
-            }
         } catch (UnsatisfiedLinkError e) {
             throw new ProcessInitException(ResultCodes.RESULT_CODE_NATIVE_LIBRARY_LOAD_FAILED, e);
         }
     }
 
-    /**
-     * initializes the library here and now: must be called on the thread that the
-     * native will call its "main" thread. The library must have previously been
-     * loaded with loadNow.
-     * @param initCommandLine The command line arguments that native command line will
-     * be initialized with.
-     */
-    static void initializeOnMainThread(String[] initCommandLine) throws ProcessInitException {
-        checkThreadUsage();
+
+    private static void initializeAlreadyLocked(String[] initCommandLine)
+            throws ProcessInitException {
         if (sInitialized) {
             return;
         }
-        int resultCode = nativeLibraryLoadedOnMainThread(initCommandLine);
+        int resultCode = nativeLibraryLoaded(initCommandLine);
         if (resultCode != 0) {
             Log.e(TAG, "error calling nativeLibraryLoadedOnMainThread");
             throw new ProcessInitException(resultCode);
@@ -123,40 +135,9 @@ public class LibraryLoader {
         TraceEvent.setEnabledToMatchNative();
     }
 
-    static private void initializeOnMainThread() throws ProcessInitException {
-        checkThreadUsage();
-        if (!sInitialized) {
-            initializeOnMainThread(CommandLine.getJavaSwitchesOrNull());
-        }
-    }
-
-    private LibraryLoader() {
-    }
-
-    // This asserts that calls to ensureInitialized() will happen from the
-    // same thread.
-    private static Object sCheckThreadLock = new Object();
-    private static Thread sMyThread;
-    private static void checkThreadUsage() {
-        Thread currentThread = Thread.currentThread();
-        synchronized (sCheckThreadLock) {
-            if (sMyThread == null) {
-                sMyThread = currentThread;
-            } else {
-                if (sMyThread != currentThread) {
-                    Log.e(TAG, "Threading violation detected. My thread=" + sMyThread + " id=" +
-                            sMyThread.getId() + " but I'm being accessed from thread=" +
-                          currentThread + " id=" + currentThread.getId());
-                    assert false;
-                }
-            }
-        }
-    }
-
-    // This is the only method that is registered during System.loadLibrary, as it
-    // may happen on a different thread. We then call it on the main thread to register
-    // everything else.
+    // This is the only method that is registered during System.loadLibrary. We then call it
+    // to register everything else.
     // Return 0 on success, otherwise return the error code from
     // content/public/common/result_codes.h.
-    private static native int nativeLibraryLoadedOnMainThread(String[] initCommandLine);
+    private static native int nativeLibraryLoaded(String[] initCommandLine);
 }