Unregister BroadcastReceiver and OnSubscriptionsChangedListener.
1. Unregister BroadcastReceiver of default subscription id.
2. Use a CompletableFuture to store and unregister the
OnSubscriptionsChangedListener. Note this has to be done since the
listener cannot be constructed in the handler thread.
Bug: 288059409
Test: atest FrameworksNetTests
Change-Id: Ia5fc3e53305a99c32ad2f6d5b1b6a367dc20c1d7
diff --git a/service/src/com/android/server/connectivity/KeepaliveStatsTracker.java b/service/src/com/android/server/connectivity/KeepaliveStatsTracker.java
index 698c457..0c2ed18 100644
--- a/service/src/com/android/server/connectivity/KeepaliveStatsTracker.java
+++ b/service/src/com/android/server/connectivity/KeepaliveStatsTracker.java
@@ -56,6 +56,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicBoolean;
/**
@@ -289,16 +290,32 @@
this(context, handler, new Dependencies());
}
+ private final Context mContext;
+ private final SubscriptionManager mSubscriptionManager;
+
+ private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {
+ @Override
+ public void onReceive(Context context, Intent intent) {
+ mCachedDefaultSubscriptionId =
+ intent.getIntExtra(
+ SubscriptionManager.EXTRA_SUBSCRIPTION_INDEX,
+ SubscriptionManager.INVALID_SUBSCRIPTION_ID);
+ }
+ };
+
+ private final CompletableFuture<OnSubscriptionsChangedListener> mListenerFuture =
+ new CompletableFuture<>();
+
@VisibleForTesting
public KeepaliveStatsTracker(
@NonNull Context context,
@NonNull Handler handler,
@NonNull Dependencies dependencies) {
- Objects.requireNonNull(context);
+ mContext = Objects.requireNonNull(context);
mDependencies = Objects.requireNonNull(dependencies);
mConnectivityServiceHandler = Objects.requireNonNull(handler);
- final SubscriptionManager subscriptionManager =
+ mSubscriptionManager =
Objects.requireNonNull(context.getSystemService(SubscriptionManager.class));
mLastUpdateDurationsTimestamp = mDependencies.getElapsedRealtime();
@@ -308,15 +325,7 @@
}
context.registerReceiver(
- new BroadcastReceiver() {
- @Override
- public void onReceive(Context context, Intent intent) {
- mCachedDefaultSubscriptionId =
- intent.getIntExtra(
- SubscriptionManager.EXTRA_SUBSCRIPTION_INDEX,
- SubscriptionManager.INVALID_SUBSCRIPTION_ID);
- }
- },
+ mBroadcastReceiver,
new IntentFilter(SubscriptionManager.ACTION_DEFAULT_SUBSCRIPTION_CHANGED),
/* broadcastPermission= */ null,
mConnectivityServiceHandler);
@@ -326,27 +335,30 @@
// this will throw. Therefore, post a runnable that creates it there.
// When the callback is called on the BackgroundThread, post a message on the CS handler
// thread to update the caches, which can only be touched there.
- BackgroundThread.getHandler().post(() ->
- subscriptionManager.addOnSubscriptionsChangedListener(
- r -> r.run(), new OnSubscriptionsChangedListener() {
- @Override
- public void onSubscriptionsChanged() {
- final List<SubscriptionInfo> activeSubInfoList =
- subscriptionManager.getActiveSubscriptionInfoList();
- // A null subInfo list here indicates the current state is unknown
- // but not necessarily empty, simply ignore it. Another call to the
- // listener will be invoked in the future.
- if (activeSubInfoList == null) return;
- mConnectivityServiceHandler.post(() -> {
- mCachedCarrierIdPerSubId.clear();
+ BackgroundThread.getHandler().post(() -> {
+ final OnSubscriptionsChangedListener listener =
+ new OnSubscriptionsChangedListener() {
+ @Override
+ public void onSubscriptionsChanged() {
+ final List<SubscriptionInfo> activeSubInfoList =
+ mSubscriptionManager.getActiveSubscriptionInfoList();
+ // A null subInfo list here indicates the current state is unknown
+ // but not necessarily empty, simply ignore it. Another call to the
+ // listener will be invoked in the future.
+ if (activeSubInfoList == null) return;
+ mConnectivityServiceHandler.post(() -> {
+ mCachedCarrierIdPerSubId.clear();
- for (final SubscriptionInfo subInfo : activeSubInfoList) {
- mCachedCarrierIdPerSubId.put(subInfo.getSubscriptionId(),
- subInfo.getCarrierId());
- }
- });
- }
- }));
+ for (final SubscriptionInfo subInfo : activeSubInfoList) {
+ mCachedCarrierIdPerSubId.put(subInfo.getSubscriptionId(),
+ subInfo.getCarrierId());
+ }
+ });
+ }
+ };
+ mListenerFuture.complete(listener);
+ mSubscriptionManager.addOnSubscriptionsChangedListener(r -> r.run(), listener);
+ });
}
/** Ensures the list of duration metrics is large enough for number of registered keepalives. */
@@ -685,7 +697,11 @@
return;
}
Log.wtf(TAG, msg + ". Disabling KeepaliveStatsTracker");
- // TODO: Unregister the OnSubscriptionsChangedListener and BroadcastReceiver.
+ mContext.unregisterReceiver(mBroadcastReceiver);
+ // The returned future is ignored since it is void and the is never completed exceptionally.
+ final CompletableFuture<Void> unused = mListenerFuture.thenAcceptAsync(
+ listener -> mSubscriptionManager.removeOnSubscriptionsChangedListener(listener),
+ BackgroundThread.getExecutor());
}
/** Whether this tracker is enabled. This method is thread safe. */
diff --git a/tests/unit/java/com/android/server/connectivity/KeepaliveStatsTrackerTest.java b/tests/unit/java/com/android/server/connectivity/KeepaliveStatsTrackerTest.java
index 90576a4..fa703eb 100644
--- a/tests/unit/java/com/android/server/connectivity/KeepaliveStatsTrackerTest.java
+++ b/tests/unit/java/com/android/server/connectivity/KeepaliveStatsTrackerTest.java
@@ -31,6 +31,7 @@
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.doReturn;
@@ -120,16 +121,25 @@
@Mock private KeepaliveStatsTracker.Dependencies mDependencies;
@Mock private SubscriptionManager mSubscriptionManager;
- private void triggerBroadcastDefaultSubId(int subId) {
+ private BroadcastReceiver getBroadcastReceiver() {
final ArgumentCaptor<BroadcastReceiver> receiverCaptor =
ArgumentCaptor.forClass(BroadcastReceiver.class);
- verify(mContext).registerReceiver(receiverCaptor.capture(), /* filter= */ any(),
- /* broadcastPermission= */ any(), eq(mTestHandler));
+ verify(mContext).registerReceiver(
+ receiverCaptor.capture(),
+ argThat(intentFilter -> intentFilter.matchAction(
+ SubscriptionManager.ACTION_DEFAULT_SUBSCRIPTION_CHANGED)),
+ /* broadcastPermission= */ any(),
+ eq(mTestHandler));
+
+ return receiverCaptor.getValue();
+ }
+
+ private void triggerBroadcastDefaultSubId(int subId) {
final Intent intent =
- new Intent(TelephonyManager.ACTION_SUBSCRIPTION_CARRIER_IDENTITY_CHANGED);
+ new Intent(SubscriptionManager.ACTION_DEFAULT_SUBSCRIPTION_CHANGED);
intent.putExtra(SubscriptionManager.EXTRA_SUBSCRIPTION_INDEX, subId);
- receiverCaptor.getValue().onReceive(mContext, intent);
+ getBroadcastReceiver().onReceive(mContext, intent);
}
private OnSubscriptionsChangedListener getOnSubscriptionsChangedListener() {
@@ -441,6 +451,18 @@
assertTrue(mKeepaliveStatsTracker.isEnabled());
}
+ private void assertKeepaliveStatsTrackerDisabled() {
+ assertFalse(mKeepaliveStatsTracker.isEnabled());
+
+ final OnSubscriptionsChangedListener listener = getOnSubscriptionsChangedListener();
+ // BackgroundThread will remove the OnSubscriptionsChangedListener.
+ HandlerUtils.waitForIdle(BackgroundThread.getHandler(), TIMEOUT_MS);
+ verify(mSubscriptionManager).removeOnSubscriptionsChangedListener(listener);
+
+ final BroadcastReceiver receiver = getBroadcastReceiver();
+ verify(mContext).unregisterReceiver(receiver);
+ }
+
@Test
public void testNoKeepalive() {
final int writeTime = 5000;
@@ -978,7 +1000,7 @@
onStartKeepalive(startTime2, TEST_SLOT);
// Starting a 2nd keepalive on the same slot is unexpected and an error so the stats tracker
// is disabled.
- assertFalse(mKeepaliveStatsTracker.isEnabled());
+ assertKeepaliveStatsTrackerDisabled();
final DailykeepaliveInfoReported dailyKeepaliveInfoReported =
buildKeepaliveMetrics(writeTime);