Commit 39c7bd71 authored by boliu's avatar boliu Committed by Commit bot

android: Async setupConnection binder call

Make setupConnection oneway/async. Add a ICallbackInt interface to
return the PID of the child process. The reply is then posted back to
launcher thread. Connection setup is already mostly async so only
involves moving related code to the reply callback.

One caveat is that getCallingPid no longer works and just returns 0
for oneway calls. The pid check is not strictly necessary, so dropped
the from setupConnection instead.

BUG=689758, 690118

Review-Url: https://codereview.chromium.org/2810433002
Cr-Commit-Position: refs/heads/master@{#463332}
parent b2726a53
...@@ -2550,6 +2550,7 @@ if (is_android) { ...@@ -2550,6 +2550,7 @@ if (is_android) {
android_aidl("base_java_aidl") { android_aidl("base_java_aidl") {
import_include = [ "android/java/src" ] import_include = [ "android/java/src" ]
sources = [ sources = [
"android/java/src/org/chromium/base/process_launcher/ICallbackInt.aidl",
"android/java/src/org/chromium/base/process_launcher/IChildProcessService.aidl", "android/java/src/org/chromium/base/process_launcher/IChildProcessService.aidl",
] ]
} }
......
// Copyright 2017 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.process_launcher;
oneway interface ICallbackInt {
void call(int value);
}
...@@ -6,14 +6,17 @@ package org.chromium.base.process_launcher; ...@@ -6,14 +6,17 @@ package org.chromium.base.process_launcher;
import android.os.Bundle; import android.os.Bundle;
import org.chromium.base.process_launcher.ICallbackInt;
interface IChildProcessService { interface IChildProcessService {
// On the first call to this method, the service will record the calling PID // On the first call to this method, the service will record the calling PID
// and return true. Subsequent calls will only return true if the calling PID // and return true. Subsequent calls will only return true if the calling PID
// is the same as the recorded one. // is the same as the recorded one.
boolean bindToCaller(); boolean bindToCaller();
// Sets up the initial IPC channel and returns the pid of the child process. // Sets up the initial IPC channel.
int setupConnection(in Bundle args, IBinder callback); oneway void setupConnection(in Bundle args, ICallbackInt pidCallback,
IBinder gpuCallback);
// Asks the child service to crash so that we can test the termination logic. // Asks the child service to crash so that we can test the termination logic.
oneway void crashIntentionallyForTesting(); oneway void crashIntentionallyForTesting();
......
...@@ -32,6 +32,7 @@ import org.chromium.base.library_loader.Linker; ...@@ -32,6 +32,7 @@ import org.chromium.base.library_loader.Linker;
import org.chromium.base.library_loader.ProcessInitException; import org.chromium.base.library_loader.ProcessInitException;
import org.chromium.base.process_launcher.ChildProcessCreationParams; import org.chromium.base.process_launcher.ChildProcessCreationParams;
import org.chromium.base.process_launcher.FileDescriptorInfo; import org.chromium.base.process_launcher.FileDescriptorInfo;
import org.chromium.base.process_launcher.ICallbackInt;
import org.chromium.base.process_launcher.IChildProcessService; import org.chromium.base.process_launcher.IChildProcessService;
import org.chromium.content.browser.ChildProcessConstants; import org.chromium.content.browser.ChildProcessConstants;
import org.chromium.content.common.ContentSwitches; import org.chromium.content.common.ContentSwitches;
...@@ -126,23 +127,20 @@ public class ChildProcessServiceImpl { ...@@ -126,23 +127,20 @@ public class ChildProcessServiceImpl {
} }
@Override @Override
public int setupConnection(Bundle args, IBinder callback) { public void setupConnection(Bundle args, ICallbackInt pidCallback, IBinder gpuCallback)
int callingPid = Binder.getCallingPid(); throws RemoteException {
synchronized (mBinderLock) { synchronized (mBinderLock) {
if (mBindToCallerCheck && mBoundCallingPid != callingPid) { if (mBindToCallerCheck && mBoundCallingPid == 0) {
if (mBoundCallingPid == 0) { Log.e(TAG, "Service has not been bound with bindToCaller()");
Log.e(TAG, "Service has not been bound with bindToCaller()"); pidCallback.call(-1);
} else { return;
Log.e(TAG, "Client pid %d does not match the bound pid %d", callingPid,
mBoundCallingPid);
}
return -1;
} }
} }
mGpuCallback = callback != null ? IGpuProcessCallback.Stub.asInterface(callback) : null; pidCallback.call(Process.myPid());
mGpuCallback =
gpuCallback != null ? IGpuProcessCallback.Stub.asInterface(gpuCallback) : null;
getServiceInfo(args); getServiceInfo(args);
return Process.myPid();
} }
@Override @Override
......
...@@ -21,6 +21,7 @@ import org.chromium.base.TraceEvent; ...@@ -21,6 +21,7 @@ import org.chromium.base.TraceEvent;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.base.process_launcher.ChildProcessCreationParams; import org.chromium.base.process_launcher.ChildProcessCreationParams;
import org.chromium.base.process_launcher.FileDescriptorInfo; import org.chromium.base.process_launcher.FileDescriptorInfo;
import org.chromium.base.process_launcher.ICallbackInt;
import org.chromium.base.process_launcher.IChildProcessService; import org.chromium.base.process_launcher.IChildProcessService;
import java.io.IOException; import java.io.IOException;
...@@ -389,6 +390,18 @@ public class ChildProcessConnectionImpl implements ChildProcessConnection { ...@@ -389,6 +390,18 @@ public class ChildProcessConnectionImpl implements ChildProcessConnection {
} }
} }
private void onSetupConnectionResult(int pid) {
synchronized (mLock) {
mPid = pid;
assert mPid != 0 : "Child service claims to be run by a process of pid=0.";
if (mConnectionCallback != null) {
mConnectionCallback.onConnected(mPid);
}
mConnectionCallback = null;
}
}
/** /**
* Called after the connection parameters have been set (in setupConnection()) *and* a * Called after the connection parameters have been set (in setupConnection()) *and* a
* connection has been established (as signaled by onServiceConnected()). These two events can * connection has been established (as signaled by onServiceConnected()). These two events can
...@@ -402,10 +415,20 @@ public class ChildProcessConnectionImpl implements ChildProcessConnection { ...@@ -402,10 +415,20 @@ public class ChildProcessConnectionImpl implements ChildProcessConnection {
Bundle bundle = ChildProcessLauncher.createsServiceBundle( Bundle bundle = ChildProcessLauncher.createsServiceBundle(
mConnectionParams.mCommandLine, mConnectionParams.mFilesToBeMapped); 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 { try {
mPid = mService.setupConnection(bundle, mConnectionParams.mCallback); mService.setupConnection(bundle, pidCallback, mConnectionParams.mCallback);
assert mPid != 0 : "Child service claims to be run by a process of pid=0."; } catch (RemoteException re) {
} catch (android.os.RemoteException re) {
Log.e(TAG, "Failed to setup connection.", re); Log.e(TAG, "Failed to setup connection.", re);
} }
// We proactively close the FDs rather than wait for GC & finalizer. // We proactively close the FDs rather than wait for GC & finalizer.
...@@ -417,11 +440,6 @@ public class ChildProcessConnectionImpl implements ChildProcessConnection { ...@@ -417,11 +440,6 @@ public class ChildProcessConnectionImpl implements ChildProcessConnection {
Log.w(TAG, "Failed to close FD.", ioe); Log.w(TAG, "Failed to close FD.", ioe);
} }
mConnectionParams = null; mConnectionParams = null;
if (mConnectionCallback != null) {
mConnectionCallback.onConnected(mPid);
}
mConnectionCallback = null;
} finally { } finally {
TraceEvent.end("ChildProcessConnectionImpl.doConnectionSetupLocked"); TraceEvent.end("ChildProcessConnectionImpl.doConnectionSetupLocked");
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment