Minimize window during which default sub may be invalid.
Verified from logs that the window is reduced from ~475ms
to ~28ms.
Test: manually tested single sim<->multi sim transitions
Test: manually verified window duration from logs
Test: atest PhoneConfigurationManagerTest
Bug: 163582235
Change-Id: I75967e2099424997dbb32e4f4c96bd2bf961aac1
diff --git a/src/java/com/android/internal/telephony/MultiSimSettingController.java b/src/java/com/android/internal/telephony/MultiSimSettingController.java
index a90daf9..c69a115 100644
--- a/src/java/com/android/internal/telephony/MultiSimSettingController.java
+++ b/src/java/com/android/internal/telephony/MultiSimSettingController.java
@@ -37,6 +37,7 @@
import android.content.IntentFilter;
import android.os.AsyncResult;
import android.os.Handler;
+import android.os.Looper;
import android.os.Message;
import android.os.ParcelUuid;
import android.provider.Settings;
@@ -226,6 +227,7 @@
* Notify subscription info change.
*/
public void notifySubscriptionInfoChanged() {
+ log("notifySubscriptionInfoChanged");
obtainMessage(EVENT_SUBSCRIPTION_INFO_CHANGED).sendToTarget();
}
@@ -332,6 +334,22 @@
}
/**
+ * This method is called when a phone object is removed (for example when going from multi-sim
+ * to single-sim).
+ * NOTE: This method does not post a message to self, instead it calls reEvaluateAll() directly.
+ * so it should only be called from the main thread. The reason is to update defaults asap
+ * after multi_sim_config property has been updated (see b/163582235).
+ */
+ public void onPhoneRemoved() {
+ if (DBG) log("onPhoneRemoved");
+ if (Looper.myLooper() != this.getLooper()) {
+ throw new RuntimeException("This method must be called from the same looper as "
+ + "MultiSimSettingController.");
+ }
+ reEvaluateAll();
+ }
+
+ /**
* Called when carrier config changes on any phone.
*/
@VisibleForTesting
diff --git a/src/java/com/android/internal/telephony/PhoneConfigurationManager.java b/src/java/com/android/internal/telephony/PhoneConfigurationManager.java
index fba016c..03c6cbe 100644
--- a/src/java/com/android/internal/telephony/PhoneConfigurationManager.java
+++ b/src/java/com/android/internal/telephony/PhoneConfigurationManager.java
@@ -362,22 +362,35 @@
log("onMultiSimConfigChanged: Rebooting is not required.");
mMi.notifyPhoneFactoryOnMultiSimConfigChanged(mContext, numOfActiveModems);
broadcastMultiSimConfigChange(numOfActiveModems);
-
+ boolean subInfoCleared = false;
// if numOfActiveModems is decreasing, deregister old RILs
// eg if we are going from 2 phones to 1 phone, we need to deregister RIL for the
// second phone. This loop does nothing if numOfActiveModems is increasing.
- for (int i = numOfActiveModems; i < oldNumOfActiveModems; i++) {
- mPhones[i].mCi.onSlotActiveStatusChange(SubscriptionManager.isValidPhoneId(i));
+ for (int phoneId = numOfActiveModems; phoneId < oldNumOfActiveModems; phoneId++) {
+ SubscriptionController.getInstance().clearSubInfoRecord(phoneId);
+ subInfoCleared = true;
+ mPhones[phoneId].mCi.onSlotActiveStatusChange(
+ SubscriptionManager.isValidPhoneId(phoneId));
+ }
+ if (subInfoCleared) {
+ // This triggers update of default subs. This should be done asap after
+ // setMultiSimProperties() to avoid (minimize) duration for which default sub can be
+ // invalid and can map to a non-existent phone.
+ // If forexample someone calls a TelephonyManager API on default sub after
+ // setMultiSimProperties() and before onSubscriptionsChanged() below -- they can be
+ // using an invalid sub, which can map to a non-existent phone and can cause an
+ // exception (see b/163582235).
+ MultiSimSettingController.getInstance().onPhoneRemoved();
}
// old phone objects are not needed now; mPhones can be updated
mPhones = PhoneFactory.getPhones();
// if numOfActiveModems is increasing, register new RILs
// eg if we are going from 1 phone to 2 phones, we need to register RIL for the second
// phone. This loop does nothing if numOfActiveModems is decreasing.
- for (int i = oldNumOfActiveModems; i < numOfActiveModems; i++) {
- Phone phone = mPhones[i];
+ for (int phoneId = oldNumOfActiveModems; phoneId < numOfActiveModems; phoneId++) {
+ Phone phone = mPhones[phoneId];
registerForRadioState(phone);
- phone.mCi.onSlotActiveStatusChange(SubscriptionManager.isValidPhoneId(i));
+ phone.mCi.onSlotActiveStatusChange(SubscriptionManager.isValidPhoneId(phoneId));
}
}
}
diff --git a/src/java/com/android/internal/telephony/SubscriptionInfoUpdater.java b/src/java/com/android/internal/telephony/SubscriptionInfoUpdater.java
index dc842d2..5d7ad9a 100644
--- a/src/java/com/android/internal/telephony/SubscriptionInfoUpdater.java
+++ b/src/java/com/android/internal/telephony/SubscriptionInfoUpdater.java
@@ -340,7 +340,6 @@
Context.TELEPHONY_SERVICE)).getActiveModemCount();
// For inactive modems, reset its states.
for (int phoneId = activeModemCount; phoneId < SUPPORTED_MODEM_COUNT; phoneId++) {
- SubscriptionController.getInstance().clearSubInfoRecord(phoneId);
sIccId[phoneId] = null;
sSimCardState[phoneId] = TelephonyManager.SIM_STATE_UNKNOWN;
sSimApplicationState[phoneId] = TelephonyManager.SIM_STATE_UNKNOWN;
diff --git a/tests/telephonytests/src/com/android/internal/telephony/MultiSimSettingControllerTest.java b/tests/telephonytests/src/com/android/internal/telephony/MultiSimSettingControllerTest.java
index af72c70..2033af5 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/MultiSimSettingControllerTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/MultiSimSettingControllerTest.java
@@ -37,6 +37,7 @@
import static org.mockito.Mockito.verify;
import android.content.Intent;
+import android.os.HandlerThread;
import android.os.ParcelUuid;
import android.os.PersistableBundle;
import android.provider.Settings;
@@ -51,6 +52,7 @@
import com.android.internal.telephony.dataconnection.DataEnabledSettings;
import org.junit.After;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -60,6 +62,9 @@
import java.util.Arrays;
import java.util.List;
import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
@RunWith(AndroidTestingRunner.class)
@TestableLooper.RunWithLooper
@@ -653,4 +658,41 @@
// This time user data should be disabled on phone1.
verify(mDataEnabledSettingsMock2).setUserDataEnabled(false);
}
+
+ @Test
+ @SmallTest
+ public void testOnPhoneRemoved() {
+ try {
+ mMultiSimSettingControllerUT.onPhoneRemoved();
+ } catch (RuntimeException re) {
+ Assert.fail("Exception not expected when calling from the same thread");
+ }
+ }
+
+ @Test
+ @SmallTest
+ public void testOnPhoneRemoved_DifferentThread() {
+ AtomicBoolean result = new AtomicBoolean(false);
+ CountDownLatch latch = new CountDownLatch(1);
+ HandlerThread handlerThread = new HandlerThread("MultiSimSettingControllerTest") {
+ public void onLooperPrepared() {
+ try {
+ mMultiSimSettingControllerUT.onPhoneRemoved();
+ } catch (RuntimeException re) {
+ result.set(true); // true to indicate that the test passed
+ }
+ latch.countDown();
+ }
+ };
+ handlerThread.start();
+ try {
+ if (!latch.await(5, TimeUnit.SECONDS)) {
+ Assert.fail("CountDownLatch did not reach 0");
+ } else if (!result.get()) {
+ Assert.fail("Exception expected when not calling from the same thread");
+ }
+ } catch (InterruptedException ie) {
+ Assert.fail("InterruptedException during latch.await");
+ }
+ }
}
diff --git a/tests/telephonytests/src/com/android/internal/telephony/PhoneConfigurationManagerTest.java b/tests/telephonytests/src/com/android/internal/telephony/PhoneConfigurationManagerTest.java
index d8094c0..a702aac 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/PhoneConfigurationManagerTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/PhoneConfigurationManagerTest.java
@@ -203,4 +203,58 @@
verify(mMockCi1, times(1)).registerForAvailable(any(), anyInt(), any());
verify(mMockCi1, times(1)).onSlotActiveStatusChange(anyBoolean());
}
+
+ @Test
+ @SmallTest
+ public void testSwitchMultiSimConfig_multiSimToSingleSim() throws Exception {
+ mPhones = new Phone[]{mPhone, mPhone1};
+ replaceInstance(PhoneFactory.class, "sPhones", null, mPhones);
+ init(2);
+ verify(mMockCi0, times(1)).registerForAvailable(any(), anyInt(), any());
+ verify(mMockCi1, times(1)).registerForAvailable(any(), anyInt(), any());
+
+ // Register for multi SIM config change.
+ mPcm.registerForMultiSimConfigChange(mHandler, EVENT_MULTI_SIM_CONFIG_CHANGED, null);
+ verify(mHandler, never()).sendMessageAtTime(any(), anyLong());
+
+ // Switch to single sim.
+ setRebootRequiredForConfigSwitch(false);
+ mPcm.switchMultiSimConfig(1);
+ ArgumentCaptor<Message> captor = ArgumentCaptor.forClass(Message.class);
+ verify(mMockRadioConfig).setModemsConfig(eq(1), captor.capture());
+
+ // Send message back to indicate switch success.
+ Message message = captor.getValue();
+ AsyncResult.forMessage(message, null, null);
+ message.sendToTarget();
+ processAllMessages();
+
+ // Verify set system property being called.
+ verify(mMi).setMultiSimProperties(1);
+ verify(mMi).notifyPhoneFactoryOnMultiSimConfigChanged(any(), eq(1));
+
+ // Capture and verify registration notification.
+ verify(mHandler).sendMessageAtTime(captor.capture(), anyLong());
+ message = captor.getValue();
+ assertEquals(EVENT_MULTI_SIM_CONFIG_CHANGED, message.what);
+ assertEquals(1, ((AsyncResult) message.obj).result);
+
+ // Capture and verify broadcast.
+ ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
+ verify(mContext).sendBroadcast(intentCaptor.capture());
+ Intent intent = intentCaptor.getValue();
+ assertEquals(ACTION_MULTI_SIM_CONFIG_CHANGED, intent.getAction());
+ assertEquals(1, intent.getIntExtra(
+ EXTRA_ACTIVE_SIM_SUPPORTED_COUNT, 0));
+
+ // Verify clearSubInfoRecord() and onSlotActiveStatusChange() are called for second phone,
+ // and not for the first one
+ verify(mSubscriptionController).clearSubInfoRecord(1);
+ verify(mMockCi1).onSlotActiveStatusChange(anyBoolean());
+ verify(mSubscriptionController, never()).clearSubInfoRecord(0);
+ verify(mMockCi0, never()).onSlotActiveStatusChange(anyBoolean());
+
+ // Verify onPhoneRemoved() gets called on MultiSimSettingController phone
+ verify(mMultiSimSettingController).onPhoneRemoved();
+ }
}
