Execute operations of ChildProcessLauncher on launcher thread

content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java

Index: content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
index ee4d63a08a5dbfba1a514c1a688211cd241e7c7c..8ba09c10d111de8c788b129fa26f35affbf29283 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
@@ -8,6 +8,7 @@ import android.content.Context;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.graphics.SurfaceTexture;
+import android.os.AsyncTask;
 import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.text.TextUtils;
@@ -21,6 +22,7 @@ import org.chromium.base.TraceEvent;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.Linker;
 import org.chromium.content.app.ChildProcessService;
 import org.chromium.content.app.ChromiumLinkerParams;
@@ -218,6 +220,8 @@ public class ChildProcessLauncher {
     private static ChildConnectionAllocator sSandboxedChildConnectionAllocator;
     private static ChildConnectionAllocator sPrivilegedChildConnectionAllocator;
 
+    private static boolean sOnTesting;
+
     private static final String NUM_SANDBOXED_SERVICES_KEY =
             "org.chromium.content.browser.NUM_SANDBOXED_SERVICES";
     private static final String NUM_PRIVILEGED_SERVICES_KEY =
@@ -279,7 +283,16 @@ public class ChildProcessLauncher {
         ChildProcessConnection.DeathCallback deathCallback =
                 new ChildProcessConnection.DeathCallback() {
                     @Override
-                    public void onChildProcessDied(ChildProcessConnection connection) {
+                    public void onChildProcessDied(final ChildProcessConnection connection) {
+                        runOnLauncherThread(new Runnable() {
+                            @Override
+                            public void run() {
+                                childProcessDied(connection);
+                            }
+                        }, true);
+                    }
+
+                    void childProcessDied(ChildProcessConnection connection) {
                         if (connection.getPid() != 0) {
                             stop(connection.getPid());
                         } else {
@@ -360,7 +373,7 @@ public class ChildProcessLauncher {
 
                 final PendingSpawnData pendingSpawn = sPendingSpawnQueue.dequeue();
                 if (pendingSpawn != null) {
-                    new Thread(new Runnable() {
+                    runOnLauncherThread(new Runnable() {
                         @Override
                         public void run() {
                             startInternal(pendingSpawn.context(), pendingSpawn.commandLine(),
@@ -368,7 +381,7 @@ public class ChildProcessLauncher {
                                     pendingSpawn.clientContext(), pendingSpawn.callbackType(),
                                     pendingSpawn.inSandbox());
                         }
-                    }).start();
+                    }, true);
                 }
             }
         }, FREE_CONNECTION_DELAY_MILLIS);
@@ -385,7 +398,18 @@ public class ChildProcessLauncher {
     private static ChildProcessConnection sSpareSandboxedConnection = null;
 
     // Manages oom bindings used to bind chind services.
-    private static BindingManager sBindingManager = BindingManagerImpl.createBindingManager();
+    private static BindingManager sBindingManager = BindingManagerImpl.createBindingManager(
+            new BindingManagerImpl.LauncherThreadExecutor() {
+                @Override
+                public void execute(Runnable runnable) {
+                    runOnLauncherThread(runnable, 0, true);
+                }
+
+                @Override
+                public void executeDelayed(Runnable runnable, long delayInMS) {
+                    runOnLauncherThread(runnable, delayInMS, true);
+                }
+            });
 
     // Map from surface id to Surface.
     private static Map<Integer, Surface> sViewSurfaceMap =
@@ -403,7 +427,10 @@ public class ChildProcessLauncher {
         sBindingManager = manager;
     }
 
-    /** @return true iff the child process is protected from out-of-memory killing */
+    /**
+     * This may be called from any thread.
+     * @return true iff the child process is protected from out-of-memory killing
+     */
     @CalledByNative
     private static boolean isOomProtected(int pid) {
         return sBindingManager.isOomProtected(pid);
@@ -472,33 +499,55 @@ public class ChildProcessLauncher {
      * Called when the renderer commits a navigation. This signals a time at which it is safe to
      * rely on renderer visibility signalled through setInForeground. See http://crbug.com/421041.
      */
-    public static void determinedVisibility(int pid) {
-        sBindingManager.determinedVisibility(pid);
+    public static void determinedVisibility(final int pid) {
+        runOnLauncherThread(new Runnable() {
+            @Override
+            public void run() {
+                sBindingManager.determinedVisibility(pid);
+            }
+        }, true);
     }
 
     /**
      * Called when the embedding application is sent to background.
      */
     public static void onSentToBackground() {
-        sApplicationInForeground = false;
-        sBindingManager.onSentToBackground();
+        Runnable task = new Runnable() {
+            @Override
+            public void run() {
+                sApplicationInForeground = false;
+                sBindingManager.onSentToBackground();

[Yaron, 2015/09/14 22:30:59]

Why is this not happening in the case below ok? Because the thread isn't set up
yet so we couldn't have created a binding manager?

[Jaekyun Seok (inactive), 2015/09/14 23:44:11]

There will be no binding in this case.

+            }
+        };
+        if (!runOnLauncherThread(task, false)) sApplicationInForeground = false;
     }
 
     /**
      * Starts moderate binding management.
      */
     public static void startModerateBindingManagement(
-            Context context, float lowReduceRatio, float highReduceRatio) {
-        sBindingManager.startModerateBindingManagement(
-                context, getNumberOfServices(context, true), lowReduceRatio, highReduceRatio);
+            final Context context, final float lowReduceRatio, final float highReduceRatio) {
+        runOnLauncherThread(new Runnable() {
+            @Override
+            public void run() {
+                sBindingManager.startModerateBindingManagement(context,
+                        getNumberOfServices(context, true), lowReduceRatio, highReduceRatio);
+            }
+        }, true);
     }
 
     /**
      * Called when the embedding application is brought to foreground.
      */
     public static void onBroughtToForeground() {
-        sApplicationInForeground = true;
-        sBindingManager.onBroughtToForeground();
+        Runnable task = new Runnable() {
+            @Override
+            public void run() {
+                sApplicationInForeground = true;
+                sBindingManager.onBroughtToForeground();
+            }
+        };
+        if (!runOnLauncherThread(task, false)) sApplicationInForeground = true;
     }
 
     /**
@@ -510,17 +559,33 @@ public class ChildProcessLauncher {
 
     /**
      * Should be called early in startup so the work needed to spawn the child process can be done
-     * in parallel to other startup work. Must not be called on the UI thread. Spare connection is
-     * created in sandboxed child process.
+     * in parallel to other startup work. Spare connection is created in sandboxed child process.
      * @param context the application context used for the connection.
      */
-    public static void warmUp(Context context) {
-        synchronized (ChildProcessLauncher.class) {
-            assert !ThreadUtils.runningOnUiThread();
-            if (sSpareSandboxedConnection == null) {
-                sSpareSandboxedConnection = allocateBoundConnection(context, null, true, false);
+    public static void warmUp(final Context context) {
+        new AsyncTask<Void, Void, Void>() {

[Yaron, 2015/09/14 22:30:59]

Two of the callers to this are trivial to switch to not using AsyncTask. The
other already has a TODO to clean it up.

[Jaekyun Seok (inactive), 2015/09/14 23:44:11]

Done.

+            @Override
+            public Void doInBackground(Void... params) {
+                warmUpInternal(context);
+                return null;
             }
-        }
+        }.execute();

[Yaron, 2015/09/14 22:30:59]

Use AsyncTask.THREAD_POOL_EXECUTOR

[Jaekyun Seok (inactive), 2015/09/14 23:44:11]

Done.

+    }
+
+    private static void warmUpInternal(final Context context) {
+        Runnable allocator = new Runnable() {
+            @Override
+            public void run() {
+                synchronized (ChildProcessLauncher.class) {

[Yaron, 2015/09/14 22:30:59]

Darn - if we didn't have the failure case below, we could get rid of this
synchronization too.

[Jaekyun Seok (inactive), 2015/09/14 23:44:11]

I left TODO for the later investigation to avoid the failure case.

+                    if (sSpareSandboxedConnection == null) {
+                        sSpareSandboxedConnection =
+                                allocateBoundConnection(context, null, true, false);
+                    }
+                }
+            }
+        };
+        // LauncherThread might not be ready. In that case, we use the current thread.
+        if (!runOnLauncherThread(allocator, false)) allocator.run();
     }
 
     private static String getSwitchValue(final String[] commandLine, String switchKey) {
@@ -642,7 +707,16 @@ public class ChildProcessLauncher {
         ChildProcessConnection.ConnectionCallback connectionCallback =
                 new ChildProcessConnection.ConnectionCallback() {
                     @Override
-                    public void onConnected(int pid) {
+                    public void onConnected(final int pid) {
+                        runOnLauncherThread(new Runnable() {
+                            @Override
+                            public void run() {
+                                connect(pid);
+                            }
+                        }, true);
+                    }
+
+                    void connect(int pid) {
                         Log.d(TAG, "on connect callback, pid=%d context=%d callbackType=%d",
                                 pid, clientContext, callbackType);
                         if (pid != NULL_PROCESS_HANDLE) {
@@ -672,16 +746,21 @@ public class ChildProcessLauncher {
      * @param pid The pid (process handle) of the service connection obtained from {@link #start}.
      */
     @CalledByNative
-    static void stop(int pid) {
+    static void stop(final int pid) {
         Log.d(TAG, "stopping child connection: pid=%d", pid);
-        ChildProcessConnection connection = sServiceMap.remove(pid);
-        if (connection == null) {
-            logPidWarning(pid, "Tried to stop non-existent connection");
-            return;
-        }
-        sBindingManager.clearConnection(pid);
-        connection.stop();
-        freeConnection(connection);
+        runOnLauncherThread(new Runnable() {
+            @Override
+            public void run() {
+                ChildProcessConnection connection = sServiceMap.remove(pid);
+                if (connection == null) {
+                    logPidWarning(pid, "Tried to stop non-existent connection");
+                    return;
+                }
+                sBindingManager.clearConnection(pid);
+                connection.stop();
+                freeConnection(connection);
+            }
+        }, true);
     }
 
     /**
@@ -819,8 +898,40 @@ public class ChildProcessLauncher {
         return true;
     }
 
+    @CalledByNative
+    static void executeOnLauncherThread(Runnable runnable) {
+        runnable.run();
+    }
+
+    @VisibleForTesting
+    public static void setOnTesting(boolean onTesting) {
+        sOnTesting = onTesting;
+    }
+
+    private static boolean runOnLauncherThread(Runnable runnable, boolean asserted) {
+        return runOnLauncherThread(runnable, 0, asserted);
+    }
+
+    private static boolean runOnLauncherThread(
+            Runnable runnable, long delayInMS, boolean asserted) {

[Yaron, 2015/09/14 22:30:59]

s/asserted/assertOnPostingFailure/

(also in above function)

[Jaekyun Seok (inactive), 2015/09/14 23:44:11]

Done.

+        if (sOnTesting) {
+            if (delayInMS > 0) {
+                ThreadUtils.postOnUiThreadDelayed(runnable, delayInMS);
+            } else {
+                ThreadUtils.runOnUiThread(runnable);
+            }
+            return true;
+        }
+        if (!LibraryLoader.isInitialized()) return false;
+
+        boolean ret = nativeRunOnLauncherThread(runnable, delayInMS);

[Yaron, 2015/09/14 22:30:59]

s/ret/wasPosted/

[Jaekyun Seok (inactive), 2015/09/14 23:44:11]

Done.

+        if (!ret && asserted) throw new RuntimeException("Failed to run on launcher thread.");
+        return ret;
+    }
+
     private static native void nativeOnChildProcessStarted(long clientContext, int pid);
     private static native void nativeEstablishSurfacePeer(
             int pid, Surface surface, int primaryID, int secondaryID);
     private static native boolean nativeIsSingleProcess();
+    private static native boolean nativeRunOnLauncherThread(Runnable runnable, long delayInMS);
 }