Bug fix + clean up on component alias
- The "overrides" device config was supposed to use "," as a separator,
but accidentally using "+" instead.
- Improved BroadcastMessenger.
- ComponentAliasResolver.Resolution now is a generic class.
I'm using it with String for provider authority aliases.
Bug: 197264681
Test: atest ComponentAliasServiceTest
Change-Id: Iaa8f4e67bfd8cae93755f68d821da0ba9f48dd41
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index 49722d0..3ba1cb2 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -3211,7 +3211,7 @@
// See if the intent refers to an alias. If so, update the intent with the target component
// name. `resolution` will contain the alias component name, which we need to return
// to the client.
- final ComponentAliasResolver.Resolution resolution =
+ final ComponentAliasResolver.Resolution<ComponentName> resolution =
mAm.mComponentAliasResolver.resolveService(service, resolvedType,
/* match flags */ 0, userId, callingUid);
@@ -3448,7 +3448,7 @@
return null;
}
}
- return new ServiceLookupResult(r, resolution.getAliasComponent());
+ return new ServiceLookupResult(r, resolution.getAlias());
}
return null;
}
@@ -4448,6 +4448,8 @@
// being brought down. Mark it as dead.
cr.serviceDead = true;
cr.stopAssociation();
+ final ComponentName clientSideComponentName =
+ cr.aliasComponent != null ? cr.aliasComponent : r.name;
try {
cr.conn.connected(r.name, null, true);
} catch (Exception e) {
diff --git a/services/core/java/com/android/server/am/ComponentAliasResolver.java b/services/core/java/com/android/server/am/ComponentAliasResolver.java
index 6ed9447..3577f72 100644
--- a/services/core/java/com/android/server/am/ComponentAliasResolver.java
+++ b/services/core/java/com/android/server/am/ComponentAliasResolver.java
@@ -189,7 +189,7 @@
@GuardedBy("mLock")
private void loadOverridesLocked() {
if (DEBUG) Slog.d(TAG, "Loading aliases overrides ...");
- for (String line : mOverrideString.split("\\+")) {
+ for (String line : mOverrideString.split("\\,+")) {
final String[] fields = line.split("\\:+", 2);
final ComponentName from = ComponentName.unflattenFromString(fields[0]);
if (!validateComponentName(from)) {
@@ -247,50 +247,45 @@
/**
* Contains alias resolution information.
*/
- public static class Resolution {
- @NonNull
- public final Intent sourceIntent;
-
+ public static class Resolution<T> {
/** "From" component. Null if component alias is disabled. */
@Nullable
- public final ComponentName sourceComponent;
+ public final T source;
/** "To" component. Null if component alias is disabled, or the source isn't an alias. */
@Nullable
- public final ComponentName resolvedComponent;
+ public final T resolved;
- public Resolution(Intent sourceIntent,
- ComponentName sourceComponent, ComponentName resolvedComponent) {
- this.sourceIntent = sourceIntent;
- this.sourceComponent = sourceComponent;
- this.resolvedComponent = resolvedComponent;
+ public Resolution(T source, T resolved) {
+ this.source = source;
+ this.resolved = resolved;
}
@Nullable
public boolean isAlias() {
- return this.resolvedComponent != null;
+ return this.resolved != null;
}
@Nullable
- public ComponentName getAliasComponent() {
- return isAlias() ? sourceComponent : null;
+ public T getAlias() {
+ return isAlias() ? source : null;
}
@Nullable
- public ComponentName getTargetComponent() {
- return isAlias() ? resolvedComponent : null;
+ public T getTarget() {
+ return isAlias() ? resolved : null;
}
}
@Nullable
- public Resolution resolveService(
+ public Resolution<ComponentName> resolveService(
@NonNull Intent service, @Nullable String resolvedType,
int packageFlags, int userId, int callingUid) {
final long identity = Binder.clearCallingIdentity();
try {
synchronized (mLock) {
if (!mEnabled) {
- return new Resolution(service, null, null);
+ return new Resolution<>(null, null);
}
PackageManagerInternal pmi = LocalServices.getService(PackageManagerInternal.class);
@@ -317,7 +312,7 @@
+ " -> " + target.flattenToShortString());
}
}
- return new Resolution(service, alias, target);
+ return new Resolution<>(alias, target);
}
} finally {
Binder.restoreCallingIdentity(identity);
diff --git a/tests/componentalias/common/com/android/compatibility/common/util/BroadcastMessenger.java b/tests/componentalias/common/com/android/compatibility/common/util/BroadcastMessenger.java
index b6dc09f..175082e 100644
--- a/tests/componentalias/common/com/android/compatibility/common/util/BroadcastMessenger.java
+++ b/tests/componentalias/common/com/android/compatibility/common/util/BroadcastMessenger.java
@@ -20,10 +20,13 @@
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
+import android.os.HandlerThread;
import android.os.Parcelable;
+import android.os.SystemClock;
import android.util.Log;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.util.Preconditions;
import java.util.ArrayList;
@@ -37,39 +40,83 @@
private static final String ACTION_MESSAGE =
"com.android.compatibility.common.util.BroadcastMessenger.ACTION_MESSAGE";
+ private static final String ACTION_PING =
+ "com.android.compatibility.common.util.BroadcastMessenger.ACTION_PING";
private static final String EXTRA_MESSAGE =
"com.android.compatibility.common.util.BroadcastMessenger.EXTRA_MESSAGE";
- /** Send a message to the {@link Receiver} in a given package. */
- public static <T extends Parcelable> void send(@NonNull Context context,
- @NonNull String receiverPackage,
- @NonNull T message) {
- final Intent i = new Intent(ACTION_MESSAGE);
- i.putExtra(EXTRA_MESSAGE, message);
+ /**
+ * We need to drop messages that were sent before the receiver was created. We keep
+ * track of the message send time in this extra.
+ */
+ private static final String EXTRA_SENT_TIME =
+ "com.android.compatibility.common.util.BroadcastMessenger.EXTRA_SENT_TIME";
+
+ private static long getCurrentTime() {
+ return SystemClock.uptimeMillis();
+ }
+
+ private static void sendBroadcast(@NonNull Intent i,
+ @NonNull Context context, @NonNull String receiverPackage) {
i.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
i.setPackage(receiverPackage);
+ i.putExtra(EXTRA_SENT_TIME, getCurrentTime());
- Log.d(TAG, "Sending: " + message);
context.sendBroadcast(i);
}
+ /** Send a message to the {@link Receiver} in a given package. */
+ public static <T extends Parcelable> void send(@NonNull Context context,
+ @NonNull String receiverPackage, @NonNull T message) {
+ final Intent i = new Intent(ACTION_MESSAGE);
+ i.putExtra(EXTRA_MESSAGE, Preconditions.checkNotNull(message));
+
+ Log.i(TAG, "Sending: " + message);
+ sendBroadcast(i, context, receiverPackage);
+ }
+
+ private static void sendPing(@NonNull Context context, @NonNull String receiverPackage) {
+ final Intent i = new Intent(ACTION_PING);
+
+ Log.i(TAG, "Sending a ping");
+ sendBroadcast(i, context, receiverPackage);
+ }
+
/**
- * Receive messages from the test app.
+ * Receive messages sent with {@link #send}. Note it'll ignore all the messages that were
+ * sent before instantiated.
*/
public static final class Receiver<T extends Parcelable> implements AutoCloseable {
private final Context mContext;
+ private final HandlerThread mReceiverThread = new HandlerThread(TAG);
+
@GuardedBy("mMessages")
private final ArrayList<T> mMessages = new ArrayList<>();
+ private final long mCreatedTime = getCurrentTime();
private boolean mRegistered;
private final BroadcastReceiver mReceiver = new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
- if (!ACTION_MESSAGE.equals(intent.getAction())) {
- throw new RuntimeException("Unknown message received: " + intent);
+ // Log.d(TAG, "Received intent: " + intent);
+ switch (intent.getAction()) {
+ case ACTION_MESSAGE:
+ case ACTION_PING:
+ break;
+ default:
+ throw new RuntimeException("Unknown broadcast received: " + intent);
}
+ if (intent.getLongExtra(EXTRA_SENT_TIME, 0) < mCreatedTime) {
+ Log.i(TAG, "Dropping stale broadcast: " + intent);
+ return;
+ }
+
+ // Note for a PING, the message will be null.
final T message = intent.getParcelableExtra(EXTRA_MESSAGE);
- Log.d(TAG, "Received: " + message);
+ if (message != null) {
+ Log.i(TAG, "Received: " + message);
+ }
+
synchronized (mMessages) {
mMessages.add(message);
mMessages.notifyAll();
@@ -83,8 +130,13 @@
public Receiver(@NonNull Context context) {
mContext = context;
+ mReceiverThread.start();
+
final IntentFilter fi = new IntentFilter(ACTION_MESSAGE);
- context.registerReceiver(mReceiver, fi);
+ fi.addAction(ACTION_PING);
+
+ context.registerReceiver(mReceiver, fi, /** permission=*/ null,
+ mReceiverThread.getThreadHandler());
mRegistered = true;
}
@@ -92,6 +144,7 @@
public void close() {
if (mRegistered) {
mContext.unregisterReceiver(mReceiver);
+ mReceiverThread.quit();
mRegistered = false;
}
}
@@ -121,5 +174,21 @@
return mMessages.remove(0);
}
}
+
+ /**
+ * Ensure that no further messages have been received.
+ *
+ * Call it before {@link #close()}.
+ */
+ public void ensureNoMoreMessages() throws Exception {
+ // Send a ping to myself.
+ sendPing(mContext, mContext.getPackageName());
+
+ final T m = waitForNextMessage();
+ if (m == null) {
+ return; // Okay. Ping will deliver a null message.
+ }
+ throw new RuntimeException("No more messages expected, but received: " + m);
+ }
}
}
diff --git a/tests/componentalias/src/android/content/componentalias/tests/ComponentAliasServiceTest.java b/tests/componentalias/src/android/content/componentalias/tests/ComponentAliasServiceTest.java
index 1de6277..81fc9bf 100644
--- a/tests/componentalias/src/android/content/componentalias/tests/ComponentAliasServiceTest.java
+++ b/tests/componentalias/src/android/content/componentalias/tests/ComponentAliasServiceTest.java
@@ -21,7 +21,6 @@
import static android.content.componentalias.tests.common.ComponentAliasTestCommon.SUB1_PACKAGE;
import static android.content.componentalias.tests.common.ComponentAliasTestCommon.SUB2_PACKAGE;
import static android.content.componentalias.tests.common.ComponentAliasTestCommon.TAG;
-import static android.content.componentalias.tests.common.ComponentAliasTestCommon.TEST_PACKAGE;
import static com.google.common.truth.Truth.assertThat;
@@ -34,18 +33,16 @@
import android.provider.DeviceConfig;
import android.util.Log;
+import androidx.test.InstrumentationRegistry;
+
import com.android.compatibility.common.util.BroadcastMessenger;
import com.android.compatibility.common.util.BroadcastMessenger.Receiver;
import com.android.compatibility.common.util.DeviceConfigStateHelper;
import com.android.compatibility.common.util.ShellUtils;
import com.android.compatibility.common.util.TestUtils;
-import androidx.test.InstrumentationRegistry;
-
import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
-import org.junit.Ignore;
import org.junit.Test;
/**
@@ -95,7 +92,7 @@
.setMethodName("onServiceConnected")
.setComponent(name);
- BroadcastMessenger.send(sContext, TEST_PACKAGE, m);
+ BroadcastMessenger.send(sContext, sContext.getPackageName(), m);
}
@Override
@@ -107,7 +104,7 @@
.setMethodName("onServiceDisconnected")
.setComponent(name);
- BroadcastMessenger.send(sContext, TEST_PACKAGE, m);
+ BroadcastMessenger.send(sContext, sContext.getPackageName(), m);
}
@Override
@@ -118,7 +115,7 @@
.setSenderIdentity("sServiceConnection")
.setMethodName("onBindingDied");
- BroadcastMessenger.send(sContext, TEST_PACKAGE, m);
+ BroadcastMessenger.send(sContext, sContext.getPackageName(), m);
}
@Override
@@ -129,7 +126,7 @@
.setSenderIdentity("sServiceConnection")
.setMethodName("onNullBinding");
- BroadcastMessenger.send(sContext, TEST_PACKAGE, m);
+ BroadcastMessenger.send(sContext, sContext.getPackageName(), m);
}
};
@@ -165,6 +162,8 @@
m = receiver.waitForNextMessage();
assertThat(m.getMethodName()).isEqualTo("onDestroy");
+
+ receiver.ensureNoMoreMessages();
}
}
@@ -193,24 +192,29 @@
@Test
public void testStartAndStopService_override() throws Exception {
Intent i = new Intent().setPackage(APP_PACKAGE);
- i.setAction(APP_PACKAGE + ".IS_ALIAS_02");
+ i.setAction(APP_PACKAGE + ".IS_ALIAS_01");
- ComponentName alias = new ComponentName(APP_PACKAGE, APP_PACKAGE + ".s.Alias02");
+ // Change some of the aliases from what's defined in <meta-data>.
- // Note, alias02 originally points at sub*2* package, but we override it in this test.
- ComponentName target = new ComponentName(SUB1_PACKAGE, APP_PACKAGE + ".s.Target02");
+ ComponentName aliasA = new ComponentName(APP_PACKAGE, APP_PACKAGE + ".s.Alias01");
+ ComponentName targetA = new ComponentName(APP_PACKAGE, APP_PACKAGE + ".s.Target02");
+
+ ComponentName aliasB = new ComponentName(APP_PACKAGE, APP_PACKAGE + ".s.Alias02");
+ ComponentName targetB = new ComponentName(APP_PACKAGE, APP_PACKAGE + ".s.Target01");
sDeviceConfig.set("component_alias_overrides",
- alias.flattenToShortString() + ":" + target.flattenToShortString());
+ aliasA.flattenToShortString() + ":" + targetA.flattenToShortString()
+ + ","
+ + aliasB.flattenToShortString() + ":" + targetB.flattenToShortString());
TestUtils.waitUntil("Wait until component alias is actually enabled", () -> {
return ShellUtils.runShellCommand("dumpsys activity component-alias")
- .indexOf(alias.flattenToShortString() + " -> " + target.flattenToShortString())
- > 0;
+ .indexOf(aliasA.flattenToShortString()
+ + " -> " + targetA.flattenToShortString()) > 0;
});
- testStartAndStopService_common(i, alias, target);
+ testStartAndStopService_common(i, aliasA, targetA);
}
private void testBindAndUnbindService_common(
@@ -253,6 +257,7 @@
assertThat(m.getMethodName()).isEqualTo("onDestroy");
// Note onServiceDisconnected() won't be called in this case.
+ receiver.ensureNoMoreMessages();
}
}
@@ -275,4 +280,43 @@
new ComponentName(APP_PACKAGE, APP_PACKAGE + ".s.Alias02"),
new ComponentName(SUB2_PACKAGE, APP_PACKAGE + ".s.Target02"));
}
+
+ @Test
+ public void testBindService_serviceKilled() throws Exception {
+ Intent originalIntent = new Intent().setPackage(APP_PACKAGE);
+ originalIntent.setAction(APP_PACKAGE + ".IS_ALIAS_02");
+
+ final ComponentName componentNameForClient =
+ new ComponentName(APP_PACKAGE, APP_PACKAGE + ".s.Alias02");
+ final ComponentName componentNameForTarget =
+ new ComponentName(SUB2_PACKAGE, APP_PACKAGE + ".s.Target02");
+
+ ComponentAliasMessage m;
+
+ try (Receiver<ComponentAliasMessage> receiver = new Receiver<>(sContext)) {
+ // Bind to the service.
+ assertThat(sContext.bindService(
+ originalIntent, sServiceConnection, BIND_AUTO_CREATE)).isTrue();
+
+ // Check the target side behavior.
+ m = receiver.waitForNextMessage();
+
+ assertThat(m.getMethodName()).isEqualTo("onBind");
+
+ m = receiver.waitForNextMessage();
+ assertThat(m.getMethodName()).isEqualTo("onServiceConnected");
+ assertThat(m.getComponent()).isEqualTo(componentNameForClient);
+
+ // Now kill the service process.
+ ShellUtils.runShellCommand("su 0 killall %s", SUB2_PACKAGE);
+
+ // Check the target side behavior.
+ m = receiver.waitForNextMessage();
+
+ assertThat(m.getMethodName()).isEqualTo("onServiceDisconnected");
+ assertThat(m.getComponent()).isEqualTo(componentNameForClient);
+
+ receiver.ensureNoMoreMessages();
+ }
+ }
}