Snap for 6664195 from 397c220a737fd218a128651e7daab5d843fb4c72 to rvc-d1-release

Change-Id: Idb2970dbf2bf246977d9a17172b4c0c3b45a854e

2 files changed, 142 insertions, 80 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..0f5ba36e73 100644
--- a/src/java/com/android/internal/telephony/dataconnection/DcTracker.java
+++ b/src/java/com/android/internal/telephony/dataconnection/DcTracker.java
@@ -123,7 +123,6 @@ import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -1854,11 +1853,14 @@ public class DcTracker extends Handler {
         }
 
         for (ApnSetting dunSetting : dunCandidates) {
-            if (!dunSetting.canSupportNetworkType(
+            if (dunSetting.canSupportNetworkType(
                     ServiceState.rilRadioTechnologyToNetworkType(bearer))) {
-                continue;
+                int preferredApnSetId = getPreferredApnSetId();
+                if (preferredApnSetId == Telephony.Carriers.NO_APN_SET_ID
+                        || preferredApnSetId == dunSetting.getApnSetId()) {
+                    retDunSettings.add(dunSetting);
+                }
             }
-            retDunSettings.add(dunSetting);
         }
 
         if (VDBG) log("fetchDunApns: dunSettings=" + retDunSettings);
@@ -2385,7 +2387,7 @@ public class DcTracker extends Handler {
         ArrayList<ApnSetting> dunSettings = null;
 
         if (ApnSetting.TYPE_DUN == apnType) {
-            dunSettings = sortApnListByPreferred(fetchDunApns());
+            dunSettings = fetchDunApns();
         }
         if (DBG) {
             log("checkForCompatibleDataConnection: apnContext=" + apnContext);
@@ -2398,7 +2400,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:"
@@ -3358,7 +3362,7 @@ public class DcTracker extends Handler {
                     apnList.add(dun);
                     if (DBG) log("buildWaitingApns: X added APN_TYPE_DUN apnList=" + apnList);
                 }
-                return sortApnListByPreferred(apnList);
+                return apnList;
             }
         }
 
@@ -3397,7 +3401,6 @@ public class DcTracker extends Handler {
                 if (mPreferredApn.canSupportNetworkType(
                         ServiceState.rilRadioTechnologyToNetworkType(radioTech))) {
                     apnList.add(mPreferredApn);
-                    apnList = sortApnListByPreferred(apnList);
                     if (DBG) log("buildWaitingApns: X added preferred apnList=" + apnList);
                     return apnList;
                 }
@@ -3412,8 +3415,15 @@ public class DcTracker extends Handler {
             if (apn.canHandleType(requestedApnTypeBitmask)) {
                 if (apn.canSupportNetworkType(
                         ServiceState.rilRadioTechnologyToNetworkType(radioTech))) {
-                    if (VDBG) log("buildWaitingApns: adding apn=" + apn);
-                    apnList.add(apn);
+                    int preferredApnSetId = getPreferredApnSetId();
+                    if (preferredApnSetId == Telephony.Carriers.NO_APN_SET_ID
+                            || preferredApnSetId == apn.getApnSetId()) {
+                        if (VDBG) log("buildWaitingApns: adding apn=" + apn);
+                        apnList.add(apn);
+                    } else {
+                        log("buildWaitingApns: APN set id " + apn.getApnSetId()
+                                + " does not match the preferred set id " + preferredApnSetId);
+                    }
                 } else {
                     if (DBG) {
                         log("buildWaitingApns: networkTypeBitmask:"
@@ -3428,44 +3438,10 @@ public class DcTracker extends Handler {
             }
         }
 
-        apnList = sortApnListByPreferred(apnList);
         if (DBG) log("buildWaitingApns: " + apnList.size() + " APNs in the list: " + apnList);
         return apnList;
     }
 
-    /**
-     * Sort a list of ApnSetting objects, with the preferred APNs at the front of the list
-     *
-     * e.g. if the preferred APN set = 2 and we have
-     *   1. APN with apn_set_id = 0 = Carriers.NO_SET_SET (no set is set)
-     *   2. APN with apn_set_id = 1 (not preferred set)
-     *   3. APN with apn_set_id = 2 (preferred set)
-     * Then the return order should be (3, 1, 2) or (3, 2, 1)
-     *
-     * e.g. if the preferred APN set = Carriers.NO_SET_SET (no preferred set) then the
-     * return order can be anything
-     */
-    @VisibleForTesting
-    public ArrayList<ApnSetting> sortApnListByPreferred(ArrayList<ApnSetting> list) {
-        if (list == null || list.size() <= 1) return list;
-        int preferredApnSetId = getPreferredApnSetId();
-        if (preferredApnSetId != Telephony.Carriers.NO_APN_SET_ID) {
-            list.sort(new Comparator<ApnSetting>() {
-                @Override
-                public int compare(ApnSetting apn1, ApnSetting apn2) {
-                    if (apn1.getApnSetId() == preferredApnSetId) {
-                        return -1;
-                    }
-                    if (apn2.getApnSetId() == preferredApnSetId) {
-                        return 1;
-                    }
-                    return 0;
-                }
-            });
-        }
-        return list;
-    }
-
     private String apnListToString (ArrayList<ApnSetting> apns) {
         StringBuilder result = new StringBuilder();
         for (int i = 0, size = apns.size(); i < size; i++) {
@@ -4385,22 +4361,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 +4379,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 +5134,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..58cac7a545 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,16 +1366,14 @@ 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,"
                 + "0,,,,,,,,,,1";
         // apnSetId=0
         String dunApnString2 = "[ApnSettingV5]HOT mobile PC,pc.coldm,,,,,,,,,440,10,,DUN,,,true,"
-                + "0,,,,,,,,,,0";
+                + "0,,,,,,,,,,2";
 
         ApnSetting dunApnExpected = ApnSetting.fromString(dunApnString1);
 
@@ -1362,8 +1387,21 @@ public class DcTrackerTest extends TelephonyTest {
         cr.update(PREFERAPN_URI, values, null, null);
 
         // return APN from Setting with apnSetId=1
-        ArrayList<ApnSetting> dunApns = mDct.sortApnListByPreferred(mDct.fetchDunApns());
-        assertEquals(2, dunApns.size());
+        ArrayList<ApnSetting> dunApns = mDct.fetchDunApns();
+        assertEquals(1, dunApns.size());
+        assertEquals(1, dunApns.get(0).getApnSetId());
+        assertTrue(dunApnExpected.equals(dunApns.get(0)));
+
+        // set that we prefer apn set 2
+        values = new ContentValues();
+        values.put(Telephony.Carriers.APN_SET_ID, 2);
+        cr.update(PREFERAPN_URI, values, null, null);
+
+        // return APN from Setting with apnSetId=2
+        dunApns = mDct.fetchDunApns();
+        assertEquals(1, dunApns.size());
+        assertEquals(2, dunApns.get(0).getApnSetId());
+        dunApnExpected = ApnSetting.fromString(dunApnString2);
         assertTrue(dunApnExpected.equals(dunApns.get(0)));
     }
 
@@ -1387,6 +1425,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