Multiprocess test client: Android child process launcher rework.

base/test/android/java/src/org/chromium/base/MultiprocessTestClientService.java

+// Copyright 2016 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.base;
+
+import android.app.Service;
+import android.content.Intent;
+import android.os.Handler;
+import android.os.IBinder;
+import android.os.Process;
+
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.annotations.SuppressFBWarnings;
+import org.chromium.base.library_loader.LibraryLoader;
+import org.chromium.base.library_loader.LibraryProcessType;
+import org.chromium.base.library_loader.ProcessInitException;
+
+import javax.annotation.concurrent.GuardedBy;
+
+/**
+ * The service implementation used to host all multiprocess test client code.
+ */
+@JNINamespace("base::android")
+public class MultiprocessTestClientService extends Service {
+    private static final String TAG = "cr_TestClient";
+
+    private static boolean sAlreadyInitialized = false;
+
+    private final Handler mHandler = new Handler();
+
+    private final Object mResultLock = new Object();
+
+    @GuardedBy("mResultLock")
+    private MainReturnCodeResult mResult;

[Robert Sesek, 2016/12/20 20:44:34]

Could you use a Future<MainResultCodeResult> instead of this and the lock?

[Jay Civelli, 2016/12/20 23:07:26]

Do you mean having a FutureTask<MainResultCodeResult> that calls main and having
waitForMainToReturn() calling get on it?
I am not sure, as I don't know if we have guarantees on the ordering of AIDL
calls.
So we would have to protect the FutureTask with a lock to deal with the case
where waitForMainToReturn() ends up being called before launch().

[Robert Sesek, 2016/12/21 19:26:07]

Ah, I see your point. I was actually thinking of CompletableFuture<T> (but
that's only in API 24) or SettableFuture <T> (but that's only in Guava), but
since neither of those are options and given your comment, just ignore this.

+
+    private final ITestClient.Stub mBinder = new ITestClient.Stub() {
+        @Override
+        public int launch(final String[] commandLine, FileDescriptorInfo[] fdsToMap) {
+            final int[] fdIds = new int[fdsToMap.length];
+            final int[] fdFds = new int[fdsToMap.length];
+            for (int i = 0; i < fdsToMap.length; i++) {
+                fdIds[i] = fdsToMap[i].id;
+                // Take ownership of the file descriptor so they outlive the FileDescriptorInfo
+                // instances. Native code will own them.
+                fdFds[i] = fdsToMap[i].fd.detachFd();
+            }
+            // Don't run main directly, it would block and the response would not be returned.
+            // We post to the main thread as this thread does not have a Looper.
+            mHandler.post(new Runnable() {
+                @Override
+                public void run() {
+                    nativeRunMain(commandLine, fdIds, fdFds);
+                }
+            });
+            return Process.myPid();
+        }
+
+        @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE")
+        @Override
+        public MainReturnCodeResult waitForMainToReturn(int timeoutMs) {
+            synchronized (mResultLock) {
+                while (mResult == null) {
+                    try {
+                        mResultLock.wait(timeoutMs);
+                    } catch (InterruptedException ie) {
+                        continue;
+                    }
+                    // Check if we timed-out.
+                    if (mResult == null) {
+                        Log.e(TAG, "Failed to wait for main return value.");
+                        return new MainReturnCodeResult(0, true /* timed-out */);
+                    }
+                }
+                return mResult;
+            }
+        }
+
+        @SuppressFBWarnings("DM_EXIT")
+        @Override
+        public boolean forceStopSynchronous(int exitCode) {
+            System.exit(exitCode);
+            return true;
+        }
+
+        @SuppressFBWarnings("DM_EXIT")
+        @Override
+        public void forceStop(int exitCode) {
+            System.exit(exitCode);
+        }
+    };
+
+    @SuppressFBWarnings("DM_EXIT")
+    @Override
+    public void onCreate() {
+        super.onCreate();
+
+        if (sAlreadyInitialized) {
+            // The framework controls how services are reused and even though nothing is bound to a
+            // service it might be kept around. Since we really want to fork a new process when we
+            // bind, we'll kill the process early when a service is reused, forcing the framework to
+            // recreate the service in a new process.
+            // This is not ideal, but there are no clear alternatives at this point.
+            Log.e(TAG, "Service being reused, forcing stoppage.");
+            System.exit(0);
+            return;
+        }
+        markInitialized();
+
+        ContextUtils.initApplicationContext(getApplicationContext());
+
+        PathUtils.setPrivateDataDirectorySuffix("chrome_multiprocess_test_client_service");
+
+        loadLibraries();
+    }
+
+    @Override
+    public IBinder onBind(Intent intent) {
+        return mBinder;
+    }
+
+    private void loadLibraries() {
+        try {
+            LibraryLoader.get(LibraryProcessType.PROCESS_CHILD).loadNow();
+        } catch (ProcessInitException pie) {
+            Log.e(TAG, "Unable to load native libraries.", pie);
+        }
+        ContextUtils.initApplicationContextForNative();
+    }
+
+    @CalledByNative
+    private void setMainReturnValue(int result) {
+        synchronized (mResultLock) {
+            mResult = new MainReturnCodeResult(result, false /* timed-out */);
+            mResultLock.notifyAll();
+        }
+    }
+
+    private static void markInitialized() {

[Robert Sesek, 2016/12/20 20:44:34]

This is called exactly once, so inline it?

[Jay Civelli, 2016/12/20 23:07:26]

Is to avoid FindBugs complaining that a static field is being set from a
non-static method.
I added a comment.

+        sAlreadyInitialized = true;
+    }
+
+    private native void nativeRunMain(String[] commandLine, int[] fdsToMapIds, int[] fdsToMapFds);
+}