Skip to content

Commit ad5c6d5

Browse files
authored
binder: Replace queryIntentServices() hack with the new SystemApis.createContextAsUser() (#12280)
createContextAsUser() wrapper makes the call to PackageManager's resolveService() look the same in both the same-user and cross-user cases. This is how the Android team recommends accessing XXXAsUser() APIs in general. We can also remove all the apologies for using reflection since SystemApis already explains all that.
1 parent cdd3202 commit ad5c6d5

File tree

2 files changed

+28
-50
lines changed

2 files changed

+28
-50
lines changed

binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java

Lines changed: 19 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package io.grpc.binder.internal;
1818

1919
import static com.google.common.base.Preconditions.checkState;
20+
import static io.grpc.binder.internal.SystemApis.createContextAsUser;
2021

2122
import android.app.admin.DevicePolicyManager;
2223
import android.content.ComponentName;
@@ -32,13 +33,10 @@
3233
import androidx.annotation.AnyThread;
3334
import androidx.annotation.MainThread;
3435
import com.google.common.annotations.VisibleForTesting;
35-
import com.google.common.base.VerifyException;
3636
import com.google.errorprone.annotations.concurrent.GuardedBy;
3737
import io.grpc.Status;
3838
import io.grpc.StatusException;
3939
import io.grpc.binder.BinderChannelCredentials;
40-
import java.lang.reflect.Method;
41-
import java.util.List;
4240
import java.util.concurrent.Executor;
4341
import java.util.logging.Level;
4442
import java.util.logging.Logger;
@@ -93,8 +91,6 @@ public String methodName() {
9391
private final Observer observer;
9492
private final Executor mainThreadExecutor;
9593

96-
private static volatile Method queryIntentServicesAsUserMethod;
97-
9894
@GuardedBy("this")
9995
private State state;
10096

@@ -194,8 +190,10 @@ private static Status bindInternal(
194190
break;
195191
case BIND_SERVICE_AS_USER:
196192
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
193+
// We don't need SystemApis because bindServiceAsUser() is simply public in R+.
197194
bindResult = context.bindServiceAsUser(bindIntent, conn, flags, targetUserHandle);
198195
} else {
196+
// TODO(#12279): Use SystemApis to make this work pre-R.
199197
return Status.INTERNAL.withDescription("Cross user Channel requires Android R+");
200198
}
201199
break;
@@ -266,51 +264,9 @@ void unbindInternal(Status reason) {
266264
}
267265
}
268266

269-
// Sadly the PackageManager#resolveServiceAsUser() API we need isn't part of the SDK or even a
270-
// @SystemApi as of this writing. Modern Android prevents even system apps from calling it, by any
271-
// means (https://developer.android.com/guide/app-compatibility/restrictions-non-sdk-interfaces).
272-
// So instead we call queryIntentServicesAsUser(), which does more than we need but *is* a
273-
// @SystemApi in all the SDK versions where we support cross-user Channels.
274-
@Nullable
275-
private static ResolveInfo resolveServiceAsUser(
276-
PackageManager packageManager, Intent intent, int flags, UserHandle targetUserHandle) {
277-
List<ResolveInfo> results =
278-
queryIntentServicesAsUser(packageManager, intent, flags, targetUserHandle);
279-
// The first query result is "what would be returned by resolveService", per the javadoc.
280-
return (results != null && !results.isEmpty()) ? results.get(0) : null;
281-
}
282-
283-
// The cross-user Channel feature requires the client to be a system app so we assume @SystemApi
284-
// queryIntentServicesAsUser() is visible to us at runtime. It would be visible at build time too,
285-
// if our host system app were written to call it directly. We only have to use reflection here
286-
// because grpc-java is a library built outside the Android source tree where the compiler can't
287-
// see the "non-SDK" @SystemApis that we need.
288-
@Nullable
289-
@SuppressWarnings("unchecked") // Safe by PackageManager#queryIntentServicesAsUser spec in AOSP.
290-
private static List<ResolveInfo> queryIntentServicesAsUser(
291-
PackageManager packageManager, Intent intent, int flags, UserHandle targetUserHandle) {
292-
try {
293-
if (queryIntentServicesAsUserMethod == null) {
294-
synchronized (ServiceBinding.class) {
295-
if (queryIntentServicesAsUserMethod == null) {
296-
queryIntentServicesAsUserMethod =
297-
PackageManager.class.getMethod(
298-
"queryIntentServicesAsUser", Intent.class, int.class, UserHandle.class);
299-
}
300-
}
301-
}
302-
return (List<ResolveInfo>)
303-
queryIntentServicesAsUserMethod.invoke(packageManager, intent, flags, targetUserHandle);
304-
} catch (ReflectiveOperationException e) {
305-
throw new VerifyException(e);
306-
}
307-
}
308-
309267
@AnyThread
310268
@Override
311269
public ServiceInfo resolve() throws StatusException {
312-
checkState(sourceContext != null);
313-
PackageManager packageManager = sourceContext.getPackageManager();
314270
int flags = 0;
315271
if (Build.VERSION.SDK_INT >= 29) {
316272
// Filter out non-'directBootAware' <service>s when 'targetUserHandle' is locked. Here's why:
@@ -320,9 +276,9 @@ public ServiceInfo resolve() throws StatusException {
320276
flags |= PackageManager.MATCH_DIRECT_BOOT_AUTO;
321277
}
322278
ResolveInfo resolveInfo =
323-
targetUserHandle != null
324-
? resolveServiceAsUser(packageManager, bindIntent, flags, targetUserHandle)
325-
: packageManager.resolveService(bindIntent, flags);
279+
getContextForTargetUser("Cross-user pre-auth")
280+
.getPackageManager()
281+
.resolveService(bindIntent, flags);
326282
if (resolveInfo == null) {
327283
throw Status.UNIMPLEMENTED // Same status code as when bindService() returns false.
328284
.withDescription("resolveService(" + bindIntent + " / " + targetUserHandle + ") was null")
@@ -331,6 +287,19 @@ public ServiceInfo resolve() throws StatusException {
331287
return resolveInfo.serviceInfo;
332288
}
333289

290+
private Context getContextForTargetUser(String purpose) throws StatusException {
291+
checkState(sourceContext != null, "Already unbound!");
292+
try {
293+
return targetUserHandle == null
294+
? sourceContext
295+
: createContextAsUser(sourceContext, targetUserHandle, /* flags= */ 0);
296+
} catch (ReflectiveOperationException e) {
297+
throw Status.INTERNAL
298+
.withDescription(purpose + " requires SDK_INT >= R and @SystemApi visibility")
299+
.asException();
300+
}
301+
}
302+
334303
@MainThread
335304
private void clearReferences() {
336305
sourceContext = null;

binder/src/test/java/io/grpc/binder/internal/ServiceBindingTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,15 @@ public void testResolveWithTargetUserHandle() throws Exception {
304304
assertThat(resolvedServiceInfo.processName).isEqualTo(serviceInfo.processName);
305305
}
306306

307+
@Test
308+
@Config(sdk = 29)
309+
public void testResolveWithUnsupportedTargetUserHandle() throws Exception {
310+
binding = newBuilder().setTargetUserHandle(generateUserHandle(/* userId= */ 0)).build();
311+
StatusException statusException = assertThrows(StatusException.class, binding::resolve);
312+
assertThat(statusException.getStatus().getCode()).isEqualTo(Code.INTERNAL);
313+
assertThat(statusException.getStatus().getDescription()).contains("SDK_INT >= R");
314+
}
315+
307316
@Test
308317
public void testResolveNonExistentServiceThrows() throws Exception {
309318
ComponentName doesNotExistService = new ComponentName("does.not.exist", "NoService");

0 commit comments

Comments
 (0)