Bind Android ChildProcessServices to a specific client PID.

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 aed55c3f5328b61e80ea44f357b26f3418a24258..935d1c3a2b8c193e85507ebf6eccb2641acf83c2 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.ComponentName;
 import android.content.Context;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
+import android.os.AsyncTask;
 import android.os.Bundle;
 import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
@@ -413,12 +414,13 @@ public class ChildProcessLauncher {
 
     private static ChildProcessConnection allocateBoundConnection(Context context,
             String[] commandLine, boolean inSandbox, boolean alwaysInForeground,
-            ChildProcessCreationParams creationParams) {
+            ChildProcessCreationParams creationParams,
+            ChildProcessConnection.StartCallback startCallback) {
         ChromiumLinkerParams chromiumLinkerParams = getLinkerParamsForNewConnection();
         ChildProcessConnection connection = allocateConnection(
                 context, inSandbox, chromiumLinkerParams, alwaysInForeground, creationParams);
         if (connection != null) {
-            connection.start(commandLine);
+            connection.start(commandLine, startCallback);
 
             String packageName = creationParams != null ? creationParams.getPackageName()
                     : context.getPackageName();
@@ -436,7 +438,7 @@ public class ChildProcessLauncher {
     private static final long FREE_CONNECTION_DELAY_MILLIS = 1;
 
     private static void freeConnection(ChildProcessConnection connection) {
-        synchronized (ChildProcessLauncher.class) {
+        synchronized (sSpareConnectionLock) {
             if (connection.equals(sSpareSandboxedConnection)) sSpareSandboxedConnection = null;
         }
 
@@ -483,8 +485,16 @@ public class ChildProcessLauncher {
     private static Map<Integer, ChildProcessConnection> sServiceMap =
             new ConcurrentHashMap<Integer, ChildProcessConnection>();
 
+    // Lock and monitor for these members {{{
+    private static final Object sSpareConnectionLock = new Object();
     // A pre-allocated and pre-bound connection ready for connection setup, or null.
     private static ChildProcessConnection sSpareSandboxedConnection;
+    // If sSpareSandboxedConnection is not null, this indicates whether the service is
+    // ready for connection setup. Wait on the monitor lock to be notified when this
+    // state changes. sSpareSandboxedConnection may be null after waiting, if starting
+    // the service failed.
+    private static boolean sSpareConnectionStarting;
+    // }}}
 
     // Manages oom bindings used to bind chind services.
     private static BindingManager sBindingManager = BindingManagerImpl.createBindingManager();
@@ -567,15 +577,38 @@ public class ChildProcessLauncher {
      * @param context the application context used for the connection.
      */
     public static void warmUp(Context context) {
-        synchronized (ChildProcessLauncher.class) {
+        synchronized (sSpareConnectionLock) {
             assert !ThreadUtils.runningOnUiThread();
             if (sSpareSandboxedConnection == null) {
                 ChildProcessCreationParams params = ChildProcessCreationParams.get();
                 if (params != null) {
                     params = params.copy();
                 }
+

[Tobias Sargeant, 2017/01/14 23:24:58]

Presumably the fact that warmUp doesn't retry means that in second and
subsequent browser processes, warmUp will be non-functional?

[Robert Sesek, 2017/01/17 19:51:22]

Since warmUp is only used for the first connection, there's no point in
re-trying it if it fails.

+                sSpareConnectionStarting = true;
+
+                ChildProcessConnection.StartCallback startCallback =
+                        new ChildProcessConnection.StartCallback() {
+                            @Override
+                            public void onChildStarted() {
+                                synchronized (sSpareConnectionLock) {
+                                    sSpareConnectionStarting = false;
+                                    sSpareConnectionLock.notify();
+                                }
+                            }
+
+                            @Override
+                            public void onChildStartFailed() {
+                                Log.e(TAG, "Failed to warm up the spare sandbox service");
+                                synchronized (sSpareConnectionLock) {
+                                    sSpareSandboxedConnection = null;
+                                    sSpareConnectionStarting = false;
+                                    sSpareConnectionLock.notify();
+                                }
+                            }
+                        };
                 sSpareSandboxedConnection = allocateBoundConnection(context, null, true, false,
-                        params);
+                        params, startCallback);
             }
         }
     }
@@ -654,23 +687,29 @@ public class ChildProcessLauncher {
     }
 
     private static void startInternal(
-            Context context,
+            final Context context,
             final String[] commandLine,
-            int childProcessId,
-            FileDescriptorInfo[] filesToBeMapped,
-            long clientContext,
-            int callbackType,
-            boolean inSandbox,
-            ChildProcessCreationParams creationParams) {
+            final int childProcessId,
+            final FileDescriptorInfo[] filesToBeMapped,
+            final long clientContext,
+            final int callbackType,
+            final boolean inSandbox,
+            final ChildProcessCreationParams creationParams) {
         try {
             TraceEvent.begin("ChildProcessLauncher.startInternal");
 
             ChildProcessConnection allocatedConnection = null;
             String packageName = creationParams != null ? creationParams.getPackageName()
                     : context.getPackageName();
-            synchronized (ChildProcessLauncher.class) {
+            synchronized (sSpareConnectionLock) {
                 if (inSandbox && sSpareSandboxedConnection != null
                         && sSpareSandboxedConnection.getPackageName().equals(packageName)) {
+                    while (sSpareConnectionStarting) {
+                        try {
+                            sSpareConnectionLock.wait();
+                        } catch (InterruptedException ex) {
+                        }
+                    }
                     allocatedConnection = sSpareSandboxedConnection;
                     sSpareSandboxedConnection = null;
                 }
@@ -680,9 +719,29 @@ public class ChildProcessLauncher {
                 if (callbackType == CALLBACK_FOR_GPU_PROCESS) alwaysInForeground = true;
                 PendingSpawnQueue pendingSpawnQueue = getPendingSpawnQueue(
                         context, packageName, inSandbox);
+                ChildProcessConnection.StartCallback startCallback =
+                        new ChildProcessConnection.StartCallback() {
+                            @Override
+                            public void onChildStarted() {}
+
+                            @Override
+                            public void onChildStartFailed() {
+                                Log.e(TAG, "ChildProcessConnection.start failed, trying again");
+                                new AsyncTask<Void, Void, Void>() {

[Torne, 2017/01/16 12:44:43]

If the asynctask only has a doInBackground and is void, you can just pass a
Runnable to the execute() method of the executor, which looks a bit neater. The
AsyncTask machinery is only useful for the case where you pass a result back to
the UI thread.

[Robert Sesek, 2017/01/17 19:51:22]

Done.

+                                    @Override
+                                    protected Void doInBackground(Void... params) {
+                                        startInternal(context, commandLine, childProcessId,
+                                                filesToBeMapped, clientContext, callbackType,
+                                                inSandbox, creationParams);
+                                        return null;
+                                    }
+                                }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
+                            }
+                        };
                 synchronized (pendingSpawnQueue.mPendingSpawnsLock) {
                     allocatedConnection = allocateBoundConnection(
-                            context, commandLine, inSandbox, alwaysInForeground, creationParams);
+                            context, commandLine, inSandbox, alwaysInForeground, creationParams,
+                            startCallback);
                     if (allocatedConnection == null) {
                         Log.d(TAG, "Allocation of new service failed. Queuing up pending spawn.");
                         pendingSpawnQueue.enqueueLocked(new PendingSpawnData(context, commandLine,
@@ -836,7 +895,7 @@ public class ChildProcessLauncher {
     @VisibleForTesting
     static ChildProcessConnection allocateBoundConnectionForTesting(Context context,
             ChildProcessCreationParams creationParams) {
-        return allocateBoundConnection(context, null, true, false, creationParams);
+        return allocateBoundConnection(context, null, true, false, creationParams, null);
     }
 
     @VisibleForTesting