android: Allow registering multiple CreationParams

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

 // Copyright 2012 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.content.browser;
 
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
@@ skipping 552 matching lines @@
     /**
      * 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.
      * @param context the application context used for the connection.
      */
     public static void warmUp(Context context) {
         synchronized (sSpareConnectionLock) {
             assert !ThreadUtils.runningOnUiThread();
             if (sSpareSandboxedConnection == null) {
-                ChildProcessCreationParams params = ChildProcessCreationParams.get();
-                if (params != null) {
-                    params = params.copy();
-                }
-
+                ChildProcessCreationParams params = ChildProcessCreationParams.getDefault();
                 sSpareConnectionStarting = true;
 
                 ChildProcessConnection.StartCallback startCallback =
                         new ChildProcessConnection.StartCallback() {
                             @Override
                             public void onChildStarted() {
                                 synchronized (sSpareConnectionLock) {
                                     sSpareConnectionStarting = false;
                                     sSpareConnectionLock.notify();
                                 }
@@ skipping 37 matching lines @@
         return new FileDescriptorInfo(id, pFd, offset, size);
     }
 
     /**
      * Spawns and connects to a child process. May be called on any thread. It will not block, but
      * will instead callback to {@link #nativeOnChildProcessStarted} when the connection is
      * established. Note this callback will not necessarily be from the same thread (currently it
      * always comes from the main thread).
      *
      * @param context Context used to obtain the application context.
+     * @param paramId Key used to retrieve ChildProcessCreationParams.
      * @param commandLine The child process command line argv.
      * @param filesToBeMapped File IDs, FDs, offsets, and lengths to pass through.
      * @param clientContext Arbitrary parameter used by the client to distinguish this connection.
      */
+    // TODO(boliu): All tests should use this over startForTesting.
+    @VisibleForTesting
     @CalledByNative
-    private static void start(Context context, final String[] commandLine, int childProcessId,
+    static void start(Context context, int paramId, final String[] commandLine, int childProcessId,
             FileDescriptorInfo[] filesToBeMapped, long clientContext) {
-        assert clientContext != 0;
-
         int callbackType = CALLBACK_FOR_UNKNOWN_PROCESS;
         boolean inSandbox = true;
         String processType =
                 ContentSwitches.getSwitchValue(commandLine, ContentSwitches.SWITCH_PROCESS_TYPE);
-        ChildProcessCreationParams params = ChildProcessCreationParams.get();
-        if (params != null) {
-            params = params.copy();
+        ChildProcessCreationParams params = ChildProcessCreationParams.get(paramId);
+        if (paramId != ChildProcessCreationParams.DEFAULT_ID && params == null) {

[Ted C, 2017/02/27 22:25:17]

why do we want to allow DEFAULT_ID not to exist in the map?

Seems like we should force DEFAULT_ID to be passed in, and it should always
exist in the map.

i.e. why isn't this just if (params == null) FREAK OUT!

[boliu, 2017/02/27 23:43:02]

regular chrome (ie not monochrome) doesn't need it, and I don't think we should
force chrome to set one just for fun..

(but if we do.. I'd want to get rid of "default params" entirely, and force
everything to pass down a specific param id)

+            throw new RuntimeException("CreationParams id " + paramId + " not found");
         }
         if (ContentSwitches.SWITCH_RENDERER_PROCESS.equals(processType)) {
             callbackType = CALLBACK_FOR_RENDERER_PROCESS;
         } else {
             if (params != null && !params.getPackageName().equals(context.getPackageName())) {
                 // WebViews and WebAPKs have renderer processes running in their applications.
                 // When launching these renderer processes, {@link ChildProcessConnectionImpl}
                 // requires the package name of the application which holds the renderer process.
                 // Therefore, the package name in ChildProcessCreationParams could be the package
                 // name of WebViews, WebAPKs, or Chrome, depending on the host application.
                 // Except renderer process, all other child processes should use Chrome's package
                 // name. In WebAPK, ChildProcessCreationParams are initialized with WebAPK's
                 // package name. Make a copy of the WebAPK's params, but replace the package with
                 // Chrome's package to use when initializing a non-renderer processes.
+                // TODO(boliu): Should fold into |paramId|. Investigate why this is needed.
                 params = new ChildProcessCreationParams(context.getPackageName(),
                         params.getIsExternalService(), params.getLibraryProcessType());
             }
             if (ContentSwitches.SWITCH_GPU_PROCESS.equals(processType)) {
                 callbackType = CALLBACK_FOR_GPU_PROCESS;
                 inSandbox = false;
             } else if (ContentSwitches.SWITCH_UTILITY_PROCESS.equals(processType)) {
                 // We only support sandboxed right now.
                 callbackType = CALLBACK_FOR_UTILITY_PROCESS;
             } else {
@@ skipping 15 matching lines @@
             final boolean inSandbox,
             final ChildProcessCreationParams creationParams) {
         try {
             TraceEvent.begin("ChildProcessLauncher.startInternal");
 
             ChildProcessConnection allocatedConnection = null;
             String packageName = creationParams != null ? creationParams.getPackageName()
                     : context.getPackageName();
             synchronized (sSpareConnectionLock) {
                 if (inSandbox && sSpareSandboxedConnection != null
-                        && sSpareSandboxedConnection.getPackageName().equals(packageName)) {
+                        && sSpareSandboxedConnection.getPackageName().equals(packageName)
+                        // Object identity check for getDefault should be enough. The default is
+                        // not supposed to change once set.
+                        && creationParams == ChildProcessCreationParams.getDefault()) {
                     while (sSpareConnectionStarting) {
                         try {
                             sSpareConnectionLock.wait();
                         } catch (InterruptedException ex) {
                         }
                     }
                     allocatedConnection = sSpareSandboxedConnection;
                     sSpareSandboxedConnection = null;
                 }
             }
@@ skipping 253 matching lines @@
 
         return true;
     }
 
     private static native void nativeOnChildProcessStarted(long clientContext, int pid);
     private static native void nativeCompleteScopedSurfaceRequest(
             UnguessableToken requestToken, Surface surface);
     private static native boolean nativeIsSingleProcess();
     private static native Surface nativeGetViewSurface(int surfaceId);
 }