Upstream: ChildProcessLauncher connects renderer processes of WebAPKs.

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 caf965ae8c36f030844bc39ee94d2f180067bc6a..a185fac9c7dec3eebb2eb4ec588cbc18d94513c6 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
@@ -26,6 +26,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.LibraryProcessType;
 import org.chromium.base.library_loader.Linker;
 import org.chromium.content.app.ChildProcessService;
 import org.chromium.content.app.ChromiumLinkerParams;
@@ -38,6 +39,7 @@ import org.chromium.content.common.SurfaceWrapper;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.Map;
 import java.util.Queue;
@@ -125,7 +127,7 @@ public class ChildProcessLauncher {
             }
         }
 
-        /** @return the count of connections managed by the allocator */
+        /** Returns the count of connections managed by the allocator */

[pkotwicz, 2016/05/28 01:04:14]

Why this change?

[Xi Han, 2016/05/30 19:59:30]

Due to the java style guide. I will revert them back since it is easy for
reviewing this CL.

[pkotwicz, 2016/05/31 00:38:58]

My bad. Yes, the Java style guide says you should make this change

         @VisibleForTesting
         int allocatedConnectionsCountForTesting() {
             return mChildProcessConnections.length - mFreeConnectionIndices.size();
@@ -206,7 +208,7 @@ public class ChildProcessLauncher {
             return sPendingSpawns.poll();
         }
 
-        /** @return the count of pending spawns in the queue */
+        /** Returns the count of pending spawns in the queue */

[pkotwicz, 2016/05/28 01:04:14]

Why this change?

[Xi Han, 2016/05/30 19:59:31]

Done.

         public int sizeLocked() {
             assert Thread.holdsLock(sPendingSpawnsLock);
             return sPendingSpawns.size();
@@ -215,9 +217,10 @@ public class ChildProcessLauncher {
 
     private static final PendingSpawnQueue sPendingSpawnQueue = new PendingSpawnQueue();
 
-    // Service class for child process. As the default value it uses SandboxedProcessService0 and
-    // PrivilegedProcessService0.
-    private static ChildConnectionAllocator sSandboxedChildConnectionAllocator;
+    // Service class for child process.
+    // Map from package name to ChildConnectionAllocator;
+    private static Map<String, ChildConnectionAllocator> sSandboxedChildConnectionAllocatorMap;
+    // As the default value it uses PrivilegedProcessService0.
     private static ChildConnectionAllocator sPrivilegedChildConnectionAllocator;
 
     private static final String NUM_SANDBOXED_SERVICES_KEY =
@@ -228,17 +231,16 @@ public class ChildProcessLauncher {
     @VisibleForTesting
     public static final String SWITCH_NUM_SANDBOXED_SERVICES_FOR_TESTING = "num-sandboxed-services";
 
-    private static int getNumberOfServices(Context context, boolean inSandbox) {
+    private static int getNumberOfServices(Context context, boolean inSandbox, String packageName) {
         try {
             PackageManager packageManager = context.getPackageManager();
-            ChildProcessCreationParams childProcessCreationParams =
-                    ChildProcessCreationParams.get();
-            final String packageName = childProcessCreationParams != null
-                    ? childProcessCreationParams.getPackageName() : context.getPackageName();
             ApplicationInfo appInfo = packageManager.getApplicationInfo(packageName,
                     PackageManager.GET_META_DATA);
-            int numServices = appInfo.metaData.getInt(inSandbox ? NUM_SANDBOXED_SERVICES_KEY
-                    : NUM_PRIVILEGED_SERVICES_KEY, -1);
+            int numServices = -1;
+            if (appInfo.metaData != null) {
+                numServices = appInfo.metaData.getInt(
+                        inSandbox ? NUM_SANDBOXED_SERVICES_KEY : NUM_PRIVILEGED_SERVICES_KEY, -1);
+            }
             if (inSandbox
                     && CommandLine.getInstance().hasSwitch(
                                SWITCH_NUM_SANDBOXED_SERVICES_FOR_TESTING)) {
@@ -262,26 +264,42 @@ public class ChildProcessLauncher {
         }
     }
 
-    private static void initConnectionAllocatorsIfNecessary(Context context) {
+    private static void initConnectionAllocatorsIfNecessary(
+            Context context, boolean inSandbox, String packageName) {
         synchronized (ChildProcessLauncher.class) {
-            if (sSandboxedChildConnectionAllocator == null) {
-                sSandboxedChildConnectionAllocator =
-                        new ChildConnectionAllocator(true, getNumberOfServices(context, true));
-            }
-            if (sPrivilegedChildConnectionAllocator == null) {
-                sPrivilegedChildConnectionAllocator =
-                        new ChildConnectionAllocator(false, getNumberOfServices(context, false));
+            if (inSandbox) {
+                if (sSandboxedChildConnectionAllocatorMap == null) {
+                    sSandboxedChildConnectionAllocatorMap =
+                            new HashMap<String, ChildConnectionAllocator>();
+                }
+                if (!sSandboxedChildConnectionAllocatorMap.containsKey(packageName)) {
+                    Log.w(TAG, "Create a new ChildConnectionAllocator with package name = %s,"
+                                    + " inSandbox = true",
+                            packageName);
+                    sSandboxedChildConnectionAllocatorMap.put(packageName,
+                            new ChildConnectionAllocator(true, getNumberOfServices(context,
+                                                                       inSandbox, packageName)));
+                }
+            } else if (sPrivilegedChildConnectionAllocator == null) {
+                sPrivilegedChildConnectionAllocator = new ChildConnectionAllocator(
+                        false, getNumberOfServices(context, false, packageName));
             }
+            // TODO(pkotwicz|hanxi): Figure out when old allocators should be removed from
+            // {@code sSandboxedChildConnectionAllocatorMap}.
         }
     }
 
-    private static ChildConnectionAllocator getConnectionAllocator(boolean inSandbox) {
-        return inSandbox
-                ? sSandboxedChildConnectionAllocator : sPrivilegedChildConnectionAllocator;
+    private static ChildConnectionAllocator getConnectionAllocator(
+            String packageName, boolean inSandbox) {
+        if (!inSandbox) {
+            return sPrivilegedChildConnectionAllocator;
+        }
+        return sSandboxedChildConnectionAllocatorMap.get(packageName);
     }
 
     private static ChildProcessConnection allocateConnection(Context context, boolean inSandbox,
-            ChromiumLinkerParams chromiumLinkerParams, boolean alwaysInForeground) {
+            ChromiumLinkerParams chromiumLinkerParams, boolean alwaysInForeground,
+            ChildProcessCreationParams creationParams) {
         ChildProcessConnection.DeathCallback deathCallback =
                 new ChildProcessConnection.DeathCallback() {
                     @Override
@@ -293,9 +311,11 @@ public class ChildProcessLauncher {
                         }
                     }
                 };
-        initConnectionAllocatorsIfNecessary(context);
-        return getConnectionAllocator(inSandbox).allocate(context, deathCallback,
-                chromiumLinkerParams, alwaysInForeground, ChildProcessCreationParams.get());
+        String packageName = creationParams.getPackageName();
+        initConnectionAllocatorsIfNecessary(context, inSandbox, packageName);
+        return getConnectionAllocator(packageName, inSandbox)
+                .allocate(context, deathCallback, chromiumLinkerParams, alwaysInForeground,
+                        creationParams);
     }
 
     private static boolean sLinkerInitialized = false;
@@ -329,14 +349,17 @@ public class ChildProcessLauncher {
     }
 
     private static ChildProcessConnection allocateBoundConnection(Context context,
-            String[] commandLine, boolean inSandbox, boolean alwaysInForeground) {
+            String[] commandLine, boolean inSandbox, boolean alwaysInForeground,
+            ChildProcessCreationParams creationParams) {
         ChromiumLinkerParams chromiumLinkerParams = getLinkerParamsForNewConnection();
-        ChildProcessConnection connection = allocateConnection(context, inSandbox,
-                chromiumLinkerParams, alwaysInForeground);
+        ChildProcessConnection connection = allocateConnection(
+                context, inSandbox, chromiumLinkerParams, alwaysInForeground, creationParams);
         if (connection != null) {
             connection.start(commandLine);
 
-            if (inSandbox && !sSandboxedChildConnectionAllocator.isFreeConnectionAvailable()) {
+            if (inSandbox
+                    && !sSandboxedChildConnectionAllocatorMap.get(creationParams.getPackageName())

[pkotwicz, 2016/05/28 01:04:14]

Nit: Use getConnectionAllocator() instead of
sSandboxedChildConnectionAllocatorMap

[Xi Han, 2016/05/30 19:59:31]

Done.

+                                .isFreeConnectionAvailable()) {
                 // Proactively releases all the moderate bindings once all the sandboxed services
                 // are allocated, which will be very likely to have some of them killed by OOM
                 // killer.
@@ -370,7 +393,7 @@ public class ChildProcessLauncher {
                             startInternal(pendingSpawn.context(), pendingSpawn.commandLine(),
                                     pendingSpawn.childProcessId(), pendingSpawn.filesToBeMapped(),
                                     pendingSpawn.clientContext(), pendingSpawn.callbackType(),
-                                    pendingSpawn.inSandbox());
+                                    pendingSpawn.inSandbox(), conn.getCreationParams());

[pkotwicz, 2016/05/28 01:04:15]

The params of the freed connection may be different from the params of the
pending connection

[Xi Han, 2016/05/30 19:59:31]

Good catch. Store ChildProcessCreationParams in PendingSpawnData.

                         }
                     }).start();
                 }
@@ -380,7 +403,8 @@ public class ChildProcessLauncher {
 
     private static PendingSpawnData freeConnectionAndDequeuePending(ChildProcessConnection conn) {
         synchronized (PendingSpawnQueue.sPendingSpawnsLock) {
-            getConnectionAllocator(conn.isInSandbox()).free(conn);
+            getConnectionAllocator(conn.getCreationParams().getPackageName(), conn.isInSandbox())
+                    .free(conn);
             return sPendingSpawnQueue.dequeueLocked();
         }
     }
@@ -414,7 +438,7 @@ public class ChildProcessLauncher {
         sBindingManager = manager;
     }
 
-    /** @return true iff the child process is protected from out-of-memory killing */
+    /** Returns true iff the child process is protected from out-of-memory killing */

[pkotwicz, 2016/05/28 01:04:15]

Why this change?

[Xi Han, 2016/05/30 19:59:30]

Done.

     @CalledByNative
     private static boolean isOomProtected(int pid) {
         return sBindingManager.isOomProtected(pid);
@@ -520,8 +544,9 @@ public class ChildProcessLauncher {
      */
     public static void startModerateBindingManagement(
             Context context, boolean moderateBindingTillBackgrounded) {

[pkotwicz, 2016/05/28 01:04:15]

{@link sBindingManager} is a singleton. This means that:
- WebAPKs and non WebAPKs shared the same moderate binding pool (I think this is
ok)
- The size of the shared moderate binding pool is set based on the number of
sandboxes processes used by Chrome. This is very confusing.

This should have an explanatory comment.

[Xi Han, 2016/05/30 19:59:31]

Added a comment here.

-        sBindingManager.startModerateBindingManagement(
-                context, getNumberOfServices(context, true), moderateBindingTillBackgrounded);
+        sBindingManager.startModerateBindingManagement(context,
+                getNumberOfServices(context, true, context.getPackageName()),
+                moderateBindingTillBackgrounded);
     }
 
     /**
@@ -544,14 +569,13 @@ public class ChildProcessLauncher {
      * in parallel to other startup work. Must not be called on the UI thread. Spare connection is
      * created in sandboxed child process.
      * @param context the application context used for the connection.
-     * @param params child process creation params.
      */
-    public static void warmUp(Context context, ChildProcessCreationParams params) {
-        ChildProcessCreationParams.set(params);
+    public static void warmUp(Context context) {
         synchronized (ChildProcessLauncher.class) {
             assert !ThreadUtils.runningOnUiThread();
             if (sSpareSandboxedConnection == null) {
-                sSpareSandboxedConnection = allocateBoundConnection(context, null, true, false);
+                sSpareSandboxedConnection = allocateBoundConnection(context, null, true, false,
+                      ChildProcessCreationParams.get().clone());
             }
         }
     }
@@ -594,11 +618,15 @@ public class ChildProcessLauncher {
         boolean inSandbox = true;
         String processType =
                 ContentSwitches.getSwitchValue(commandLine, ContentSwitches.SWITCH_PROCESS_TYPE);
+        ChildProcessCreationParams params = ChildProcessCreationParams.get().clone();
         if (ContentSwitches.SWITCH_RENDERER_PROCESS.equals(processType)) {
             callbackType = CALLBACK_FOR_RENDERER_PROCESS;
         } else if (ContentSwitches.SWITCH_GPU_PROCESS.equals(processType)) {
             callbackType = CALLBACK_FOR_GPU_PROCESS;
             inSandbox = false;
+            // For GPU process, always set the Chrome's package name.
+            params = new ChildProcessCreationParams(
+                    context.getPackageName(), 0, LibraryProcessType.PROCESS_CHILD);
         } else if (ContentSwitches.SWITCH_UTILITY_PROCESS.equals(processType)) {
             // We only support sandboxed right now.
             callbackType = CALLBACK_FOR_UTILITY_PROCESS;
@@ -607,7 +635,7 @@ public class ChildProcessLauncher {
         }
 
         startInternal(context, commandLine, childProcessId, filesToBeMapped, clientContext,
-                callbackType, inSandbox);
+                callbackType, inSandbox, params);
     }
 
     /**
@@ -650,7 +678,8 @@ public class ChildProcessLauncher {
             FileDescriptorInfo[] filesToBeMapped,
             long clientContext,
             int callbackType,
-            boolean inSandbox) {
+            boolean inSandbox,
+            ChildProcessCreationParams creationParams) {
         try {
             TraceEvent.begin("ChildProcessLauncher.startInternal");
 
@@ -666,7 +695,7 @@ public class ChildProcessLauncher {
                 if (callbackType == CALLBACK_FOR_GPU_PROCESS) alwaysInForeground = true;
                 synchronized (PendingSpawnQueue.sPendingSpawnsLock) {
                     allocatedConnection = allocateBoundConnection(
-                            context, commandLine, inSandbox, alwaysInForeground);
+                            context, commandLine, inSandbox, alwaysInForeground, creationParams);
                     if (allocatedConnection == null) {
                         Log.d(TAG, "Allocation of new service failed. Queuing up pending spawn.");

[pkotwicz, 2016/05/28 01:04:15]

This is suboptimal. There should be a {@link sPendingSpawnQueue} per allocator.

Ideally if:
- Chrome creates one sandboxed process
- If a WebAPK attempts to create 4 sandboxed processes (This should cause 1
ChildProcessConnection to be queued)
- Chrome sandboxed process is killed

ChildProcessLauncher will not try to start the 4th sandbox process (the number
of sandboxed processes is limited per package name)

[Xi Han, 2016/05/30 19:59:30]

Good catch! Make each allocator owns a PendingSpawnQueue.

                         sPendingSpawnQueue.enqueueLocked(new PendingSpawnData(context, commandLine,
@@ -848,8 +877,9 @@ public class ChildProcessLauncher {
     }
 
     @VisibleForTesting
-    static ChildProcessConnection allocateBoundConnectionForTesting(Context context) {
-        return allocateBoundConnection(context, null, true, false);
+    static ChildProcessConnection allocateBoundConnectionForTesting(Context context,
+            ChildProcessCreationParams creationParams) {
+        return allocateBoundConnection(context, null, true, false, creationParams);
     }
 
     /**
@@ -863,20 +893,24 @@ public class ChildProcessLauncher {
         }
     }
 
-    /** @return the count of sandboxed connections managed by the allocator */
+    /**
+     * Returns the number of sandboxed connections of given {@link packageName} managed by the
+     * allocator.
+     */
     @VisibleForTesting
-    static int allocatedConnectionsCountForTesting(Context context) {
-        initConnectionAllocatorsIfNecessary(context);
-        return sSandboxedChildConnectionAllocator.allocatedConnectionsCountForTesting();
+    static int allocatedSandboxedConnectionsCountForTesting(Context context, String packageName) {
+        initConnectionAllocatorsIfNecessary(context, true, packageName);
+        return sSandboxedChildConnectionAllocatorMap.get(packageName)
+                .allocatedConnectionsCountForTesting();
     }
 
-    /** @return the count of services set up and working */
+    /** Returns the count of services set up and working */
     @VisibleForTesting
     static int connectedServicesCountForTesting() {
         return sServiceMap.size();
     }
 
-    /** @return the count of pending spawns in the queue */
+    /** Returns the count of pending spawns in the queue */
     @VisibleForTesting
     static int pendingSpawnsCountForTesting() {
         synchronized (PendingSpawnQueue.sPendingSpawnsLock) {