Merge "Change eq method for connection reuse w\ dun apn" into rvc-d1-dev

2 files changed, 109 insertions, 34 deletions

diff --git a/src/java/com/android/internal/telephony/dataconnection/DcTracker.java b/src/java/com/android/internal/telephony/dataconnection/DcTracker.java
index 3342b41ce1..9abc06b418 100644
--- a/src/java/com/android/internal/telephony/dataconnection/DcTracker.java
+++ b/src/java/com/android/internal/telephony/dataconnection/DcTracker.java
@@ -2398,7 +2398,9 @@ public class DcTracker extends Handler {
                 log("apnSetting: " + apnSetting);
                 if (dunSettings != null && dunSettings.size() > 0) {
                     for (ApnSetting dunSetting : dunSettings) {
-                        if (dunSetting.equals(apnSetting)) {
+                        //This ignore network type as a check which is ok because that's checked
+                        //when calculating dun candidates.
+                        if (areCompatible(dunSetting, apnSetting)) {
                             if (curDc.isActive()) {
                                 if (DBG) {
                                     log("checkForCompatibleDataConnection:"
@@ -4385,22 +4387,6 @@ public class DcTracker extends Handler {
         }
     }
 
-    private boolean containsAllApns(List<ApnSetting> oldApnList, List<ApnSetting> newApnList) {
-        for (ApnSetting newApnSetting : newApnList) {
-            boolean canHandle = false;
-            for (ApnSetting oldApnSetting : oldApnList) {
-                // Make sure at least one of the APN from old list can cover the new APN
-                if (oldApnSetting.equals(newApnSetting,
-                        mPhone.getServiceState().getDataRoamingFromRegistration())) {
-                    canHandle = true;
-                    break;
-                }
-            }
-            if (!canHandle) return false;
-        }
-        return true;
-    }
-
     private void cleanUpConnectionsOnUpdatedApns(boolean detach, String reason) {
         if (DBG) log("cleanUpConnectionsOnUpdatedApns: detach=" + detach);
         if (mAllApnSettings.isEmpty()) {
@@ -4419,8 +4405,7 @@ public class DcTracker extends Handler {
                             apnContext.getApnType(), getDataRat());
                     apnContext.setWaitingApns(waitingApns);
                     for (ApnSetting apnSetting : waitingApns) {
-                        if (apnSetting.equals(apnContext.getApnSetting(),
-                                mPhone.getServiceState().getDataRoamingFromRegistration())) {
+                        if (areCompatible(apnSetting, apnContext.getApnSetting())) {
                             cleanupRequired = false;
                             break;
                         }
@@ -5175,4 +5160,14 @@ public class DcTracker extends Handler {
     public void unregisterForPhysicalLinkStateChanged(Handler h) {
         mDcc.unregisterForPhysicalLinkStateChanged(h);
     }
+
+    // We use a specialized equals function in Apn setting when checking if an active
+    // data connection is still legitimate to use against a different apn setting.
+    // This method is extracted to a function to ensure that any future changes to this check will
+    // be applied to both cleanUpConnectionsOnUpdatedApns and checkForCompatibleDataConnection.
+    // Fix for b/158908392.
+    private boolean areCompatible(ApnSetting apnSetting1, ApnSetting apnSetting2) {
+        return apnSetting1.equals(apnSetting2,
+                mPhone.getServiceState().getDataRoamingFromRegistration());
+    }
 }
diff --git a/tests/telephonytests/src/com/android/internal/telephony/dataconnection/DcTrackerTest.java b/tests/telephonytests/src/com/android/internal/telephony/dataconnection/DcTrackerTest.java
index d194493ca7..3cb536d761 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/dataconnection/DcTrackerTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/dataconnection/DcTrackerTest.java
@@ -22,6 +22,7 @@ import static com.android.internal.telephony.dataconnection.ApnSettingTest.creat
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
@@ -109,9 +110,11 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
 
 public class DcTrackerTest extends TelephonyTest {
     public static final String FAKE_APN1 = "FAKE APN 1";
@@ -197,6 +200,18 @@ public class DcTrackerTest extends TelephonyTest {
     private class ApnSettingContentProvider extends MockContentProvider {
         private int mPreferredApnSet = 0;
 
+        private String mFakeApn1Types = "default,supl";
+
+        private int mRowIdOffset = 0;
+
+        public void setFakeApn1Types(String apnTypes) {
+            mFakeApn1Types = apnTypes;
+        }
+
+        public void setRowIdOffset(int rowIdOffset) {
+            mRowIdOffset = rowIdOffset;
+        }
+
         @Override
         public Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs,
                             String sortOrder) {
@@ -241,7 +256,7 @@ public class DcTrackerTest extends TelephonyTest {
                                     Telephony.Carriers.SKIP_464XLAT});
 
                     mc.addRow(new Object[]{
-                            2163,                   // id
+                            2163 + mRowIdOffset,    // id
                             FAKE_PLMN,              // numeric
                             "sp-mode",              // name
                             FAKE_APN1,              // apn
@@ -253,7 +268,7 @@ public class DcTrackerTest extends TelephonyTest {
                             "",                     // user
                             "",                     // password
                             -1,                     // authtype
-                            "default,supl",         // types
+                            mFakeApn1Types,         // types
                             "IP",                   // protocol
                             "IP",                   // roaming_protocol
                             1,                      // carrier_enabled
@@ -274,7 +289,7 @@ public class DcTrackerTest extends TelephonyTest {
                     });
 
                     mc.addRow(new Object[]{
-                            2164,                   // id
+                            2164 + mRowIdOffset,    // id
                             FAKE_PLMN,              // numeric
                             "mopera U",             // name
                             FAKE_APN2,              // apn
@@ -307,7 +322,7 @@ public class DcTrackerTest extends TelephonyTest {
                     });
 
                     mc.addRow(new Object[]{
-                            2165,                   // id
+                            2165 + mRowIdOffset,    // id
                             FAKE_PLMN,              // numeric
                             "b-mobile for Nexus",   // name
                             FAKE_APN3,              // apn
@@ -340,7 +355,7 @@ public class DcTrackerTest extends TelephonyTest {
                     });
 
                     mc.addRow(new Object[]{
-                            2166,                   // id
+                            2166 + mRowIdOffset,    // id
                             FAKE_PLMN,              // numeric
                             "sp-mode ehrpd",        // name
                             FAKE_APN4,              // apn
@@ -373,7 +388,7 @@ public class DcTrackerTest extends TelephonyTest {
                     });
 
                     mc.addRow(new Object[]{
-                            2167,                   // id
+                            2167 + mRowIdOffset,    // id
                             FAKE_PLMN,              // numeric
                             "b-mobile for Nexus",   // name
                             FAKE_APN5,              // apn
@@ -406,7 +421,7 @@ public class DcTrackerTest extends TelephonyTest {
                     });
 
                     mc.addRow(new Object[]{
-                            2168,                   // id
+                            2168 + mRowIdOffset,    // id
                             FAKE_PLMN,              // numeric
                             "sp-mode",              // name
                             FAKE_APN6,              // apn
@@ -621,20 +636,32 @@ public class DcTrackerTest extends TelephonyTest {
     }
 
     private void sendInitializationEvents() {
-        logd("Sending EVENT_CARRIER_CONFIG_CHANGED");
+        sendCarrierConfigChanged("");
+
+        sendSimStateUpdated("");
+
+        sendEventDataConnectionAttached("");
+
+        waitForMs(200);
+    }
+
+    private void sendCarrierConfigChanged(String messagePrefix) {
+        logd(messagePrefix + "Sending EVENT_CARRIER_CONFIG_CHANGED");
         mDct.sendEmptyMessage(DctConstants.EVENT_CARRIER_CONFIG_CHANGED);
         waitForLastHandlerAction(mDcTrackerTestHandler.getThreadHandler());
+    }
 
-        logd("Sending EVENT_SIM_STATE_UPDATED");
+    private void sendSimStateUpdated(String messagePrefix) {
+        logd(messagePrefix + "Sending EVENT_SIM_STATE_UPDATED");
         mDct.sendMessage(mDct.obtainMessage(DctConstants.EVENT_SIM_STATE_UPDATED,
                 TelephonyManager.SIM_STATE_LOADED, 0));
         waitForLastHandlerAction(mDcTrackerTestHandler.getThreadHandler());
+    }
 
-        logd("Sending EVENT_DATA_CONNECTION_ATTACHED");
+    private void sendEventDataConnectionAttached(String messagePrefix) {
+        logd(messagePrefix + "Sending EVENT_DATA_CONNECTION_ATTACHED");
         mDct.sendMessage(mDct.obtainMessage(DctConstants.EVENT_DATA_CONNECTION_ATTACHED, null));
         waitForLastHandlerAction(mDcTrackerTestHandler.getThreadHandler());
-
-        waitForMs(200);
     }
 
     // Test the unmetered APN setup when data is disabled.
@@ -1339,9 +1366,7 @@ public class DcTrackerTest extends TelephonyTest {
     @Test
     @SmallTest
     public void testFetchDunApnWithPreferredApnSet() {
-        logd("Sending EVENT_CARRIER_CONFIG_CHANGED");
-        mDct.sendEmptyMessage(DctConstants.EVENT_CARRIER_CONFIG_CHANGED);
-        waitForLastHandlerAction(mDcTrackerTestHandler.getThreadHandler());
+        sendCarrierConfigChanged("testFetchDunApnWithPreferredApnSet: ");
 
         // apnSetId=1
         String dunApnString1 = "[ApnSettingV5]HOT mobile PC,pc.hotm,,,,,,,,,440,10,,DUN,,,true,"
@@ -1387,6 +1412,61 @@ public class DcTrackerTest extends TelephonyTest {
                 Settings.Global.TETHER_DUN_APN, null);
     }
 
+    // This tests simulates the race case where the sim status change event is triggered, the
+    // default data connection is attached, and then the carrier config gets changed which bumps
+    // the database id which we want to ignore when cleaning up connections and matching against
+    // the dun APN.  Tests b/158908392.
+    @Test
+    @SmallTest
+    public void testCheckForCompatibleDataConnectionWithDunWhenIdsChange() throws Exception {
+        //Set dun as a support apn type of FAKE_APN1
+        mApnSettingContentProvider.setFakeApn1Types("default,supl,dun");
+
+        // Enable the default apn
+        mSimulatedCommands.setDataCallResult(true, createSetupDataCallResult());
+        mDct.enableApn(ApnSetting.TYPE_DEFAULT, DcTracker.REQUEST_TYPE_NORMAL, null);
+        waitForLastHandlerAction(mDcTrackerTestHandler.getThreadHandler());
+
+        //Load the sim and attach the data connection without firing the carrier changed event
+        final String logMsgPrefix = "testCheckForCompatibleDataConnectionWithDunWhenIdsChange: ";
+        sendSimStateUpdated(logMsgPrefix);
+        sendEventDataConnectionAttached(logMsgPrefix);
+        waitForMs(200);
+
+        // Confirm that FAKE_APN1 comes up as a dun candidate
+        ApnSetting dunApn = mDct.fetchDunApns().get(0);
+        assertEquals(dunApn.getApnName(), FAKE_APN1);
+        Map<Integer, ApnContext> apnContexts = mDct.getApnContexts()
+                .stream().collect(Collectors.toMap(ApnContext::getApnTypeBitmask, x -> x));
+
+        //Double check that the default apn content is connected while the dun apn context is not
+        assertEquals(apnContexts.get(ApnSetting.TYPE_DEFAULT).getState(),
+                DctConstants.State.CONNECTED);
+        assertNotEquals(apnContexts.get(ApnSetting.TYPE_DUN).getState(),
+                DctConstants.State.CONNECTED);
+
+
+        //Change the row ids the same way as what happens when we have old apn values in the
+        //carrier table
+        mApnSettingContentProvider.setRowIdOffset(100);
+        sendCarrierConfigChanged(logMsgPrefix);
+        waitForMs(200);
+
+        mDct.enableApn(ApnSetting.TYPE_DUN, DcTracker.REQUEST_TYPE_NORMAL, null);
+        waitForLastHandlerAction(mDcTrackerTestHandler.getThreadHandler());
+
+        Map<Integer, ApnContext> apnContextsAfterRowIdsChanged = mDct.getApnContexts()
+                .stream().collect(Collectors.toMap(ApnContext::getApnTypeBitmask, x -> x));
+
+        //Make sure that the data connection used earlier wasn't cleaned up and still in use.
+        assertEquals(apnContexts.get(ApnSetting.TYPE_DEFAULT).getDataConnection(),
+                apnContextsAfterRowIdsChanged.get(ApnSetting.TYPE_DEFAULT).getDataConnection());
+
+        //Check that the DUN is using the same active data connection
+        assertEquals(apnContexts.get(ApnSetting.TYPE_DEFAULT).getDataConnection(),
+                apnContextsAfterRowIdsChanged.get(ApnSetting.TYPE_DUN).getDataConnection());
+    }
+
     // Test oos
     @Test
     @SmallTest