Moving some CPL logic to CPLH.

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

 // Copyright 2013 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.Intent;
 import android.content.ServiceConnection;
 import android.content.pm.PackageManager;
 import android.content.pm.ServiceInfo;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.IBinder;
 import android.os.RemoteException;
 
 import org.chromium.base.Log;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.process_launcher.ChildProcessCreationParams;
-import org.chromium.base.process_launcher.FileDescriptorInfo;
 import org.chromium.base.process_launcher.ICallbackInt;
 import org.chromium.base.process_launcher.IChildProcessService;
 
-import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
 import javax.annotation.Nullable;
 
 /**
  * Manages a connection between the browser activity and a child service.
  */
 public class ChildProcessConnection {
     private static final String TAG = "ChildProcessConn";
@@ skipping 122 matching lines @@
     private final ComponentName mServiceName;
 
     // Parameters passed to the child process through the service binding intent.
     // If the service gets recreated by the framework the intent will be reused, so these parameters
     // should be common to all processes of that type.
     private final Bundle mChildProcessCommonParameters;
 
     private final ChildProcessCreationParams mCreationParams;
 
     private static class ConnectionParams {
-        final String[] mCommandLine;
-        final FileDescriptorInfo[] mFilesToBeMapped;
+        final Bundle mBundle;

[boliu, 2017/05/15 21:38:22]

name this mConnectionBundle

[Jay Civelli, 2017/05/17 16:42:50]

Done.

         final IBinder mCallback;
 
-        ConnectionParams(
-                String[] commandLine, FileDescriptorInfo[] filesToBeMapped, IBinder callback) {
-            mCommandLine = commandLine;
-            mFilesToBeMapped = filesToBeMapped;
+        ConnectionParams(Bundle bundle, IBinder callback) {
+            mBundle = bundle;
             mCallback = callback;
         }
     }
 
     // This is set in start() and is used in onServiceConnected().
     private StartCallback mStartCallback;
 
-    // This is set in setupConnection() and is later used in doConnectionSetupLocked(), after which
-    // the variable is cleared. Therefore this is only valid while the connection is being set up.
+    // This is set in setupConnection() and is later used in doConnectionSetup(), after which the
+    // variable is cleared. Therefore this is only valid while the connection is being set up.
     private ConnectionParams mConnectionParams;
 
     // Callback provided in setupConnection() that will communicate the result to the caller. This
     // has to be called exactly once after setupConnection(), even if setup fails, so that the
     // caller can free up resources associated with the setup attempt. This is set to null after the
     // call.
     private ConnectionCallback mConnectionCallback;
 
     private IChildProcessService mService;
 
@@ skipping 92 matching lines @@
      * Starts a connection to an IChildProcessService. This must be followed by a call to
      * setupConnection() to setup the connection parameters. start() and setupConnection() are
      * separate to allow to pass whatever parameters are available in start(), and complete the
      * remainder addStrongBinding while reducing the connection setup latency.
      * @param useStrongBinding whether a strong binding should be bound by default. If false, an
      * initial moderate binding is used.
      * @param startCallback (optional) callback when the child process starts or fails to start.
      */
     public void start(boolean useStrongBinding, StartCallback startCallback) {
         assert LauncherThread.runningOnLauncherThread();
+
         try {
             TraceEvent.begin("ChildProcessConnection.start");
             assert LauncherThread.runningOnLauncherThread();
             assert mConnectionParams
                     == null : "setupConnection() called before start() in ChildProcessConnection.";
 
             mStartCallback = startCallback;
 
             if (!bind(useStrongBinding)) {
                 Log.e(TAG, "Failed to establish the service connection.");
                 // We have to notify the caller so that they can free-up associated resources.
                 // TODO(ppi): Can we hard-fail here?
                 mDeathCallback.onChildProcessDied(ChildProcessConnection.this);
             }
         } finally {
             TraceEvent.end("ChildProcessConnection.start");
         }
     }
 
     /**
-     * Setups the connection after it was started with start().
-     * @param commandLine (optional) will be ignored if the command line was already sent in start()
-     * @param filesToBeMapped a list of file descriptors that should be registered
+     * Sets-up the connection after it was started with start().
+     * @param connectionBundle a bundle passed to the service that can be used to pass various
+     *         parameters to the service
      * @param callback optional client specified callbacks that the child can use to communicate
      *                 with the parent process
      * @param connectionCallback will be called exactly once after the connection is set up or the
      *                           setup fails
      */
-    public void setupConnection(String[] commandLine, FileDescriptorInfo[] filesToBeMapped,
-            @Nullable IBinder callback, ConnectionCallback connectionCallback) {
+    public void setupConnection(Bundle connectionBundle, @Nullable IBinder callback,
+            ConnectionCallback connectionCallback) {
         assert LauncherThread.runningOnLauncherThread();
         assert mConnectionParams == null;
         if (mServiceDisconnected) {
             Log.w(TAG, "Tried to setup a connection that already disconnected.");
             connectionCallback.onConnected(null);
             return;
         }
         try {
             TraceEvent.begin("ChildProcessConnection.setupConnection");
             mConnectionCallback = connectionCallback;
-            mConnectionParams = new ConnectionParams(commandLine, filesToBeMapped, callback);
-            // Run the setup if the service is already connected. If not,
-            // doConnectionSetupLocked() will be called from onServiceConnected().
+            mConnectionParams = new ConnectionParams(connectionBundle, callback);
+            // Run the setup if the service is already connected. If not, doConnectionSetup() will
+            // be called from onServiceConnected().
             if (mServiceConnectComplete) {
-                doConnectionSetupLocked();
+                doConnectionSetup();
             }
         } finally {
             TraceEvent.end("ChildProcessConnection.setupConnection");
         }
     }
 
     /**
      * Terminates the connection to IChildProcessService, closing all bindings. It is safe to call
      * this multiple times.
      */
@@ skipping 39 matching lines @@
                 }
             }
 
             if (!boundToUs) {
                 return;
             }
 
             mServiceConnectComplete = true;
 
             // Run the setup if the connection parameters have already been provided. If
-            // not, doConnectionSetupLocked() will be called from setupConnection().
+            // not, doConnectionSetup() will be called from setupConnection().
             if (mConnectionParams != null) {
-                doConnectionSetupLocked();
+                doConnectionSetup();
             }
         } finally {
             TraceEvent.end("ChildProcessConnection.ChildServiceConnection.onServiceConnected");
         }
     }
 
     private void onServiceDisconnectedOnLauncherThread() {
         assert LauncherThread.runningOnLauncherThread();
         // Ensure that the disconnection logic runs only once (instead of once per each
         // ChildServiceConnection).
@@ skipping 18 matching lines @@
 
         if (mConnectionCallback != null) {
             mConnectionCallback.onConnected(this);
         }
         mConnectionCallback = null;
     }
 
     /**
      * Called after the connection parameters have been set (in setupConnection()) *and* a
      * connection has been established (as signaled by onServiceConnected()). These two events can
-     * happen in any order. Has to be called with mLock.
+     * happen in any order.
      */
-    private void doConnectionSetupLocked() {
+    private void doConnectionSetup() {
         try {
-            TraceEvent.begin("ChildProcessConnection.doConnectionSetupLocked");
+            TraceEvent.begin("ChildProcessConnection.doConnectionSetup");
             assert mServiceConnectComplete && mService != null;
             assert mConnectionParams != null;
 
-            Bundle bundle = ChildProcessLauncher.createsServiceBundle(
-                    mConnectionParams.mCommandLine, mConnectionParams.mFilesToBeMapped);
             ICallbackInt pidCallback = new ICallbackInt.Stub() {
                 @Override
                 public void call(final int pid) {
                     LauncherThread.post(new Runnable() {
                         @Override
                         public void run() {
                             onSetupConnectionResult(pid);
                         }
                     });
                 }
             };
             try {
-                mService.setupConnection(bundle, pidCallback, mConnectionParams.mCallback);
+                mService.setupConnection(
+                        mConnectionParams.mBundle, pidCallback, mConnectionParams.mCallback);
             } catch (RemoteException re) {
                 Log.e(TAG, "Failed to setup connection.", re);
             }
-            // We proactively close the FDs rather than wait for GC & finalizer.
-            try {
-                for (FileDescriptorInfo fileInfo : mConnectionParams.mFilesToBeMapped) {
-                    fileInfo.fd.close();
-                }
-            } catch (IOException ioe) {
-                Log.w(TAG, "Failed to close FD.", ioe);
-            }
             mConnectionParams = null;
         } finally {
-            TraceEvent.end("ChildProcessConnection.doConnectionSetupLocked");
+            TraceEvent.end("ChildProcessConnection.doConnectionSetup");
         }
     }
 
     private boolean bind(boolean useStrongBinding) {
         assert LauncherThread.runningOnLauncherThread();
         assert !mUnbound;
 
         boolean success = useStrongBinding ? mStrongBinding.bind() : mInitialBinding.bind();
         if (!success) return false;
 
@@ skipping 146 matching lines @@
     protected ChildServiceConnection createServiceConnection(int bindFlags) {
         assert LauncherThread.runningOnLauncherThread();
         return new ChildServiceConnectionImpl(bindFlags);
     }
 
     @VisibleForTesting
     public void crashServiceForTesting() throws RemoteException {
         mService.crashIntentionallyForTesting();
     }
 }