Merge "Do not remove profile network preference for different uids" am: 42791188b2

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2076720

Change-Id: I75fceac40bba4ce17e3dce721554b87361187d89
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

framework/src/android/net/ProfileNetworkPreference.java

@@ -120,8 +120,8 @@ public final class ProfileNetworkPreference implements Parcelable {
     public String toString() {
         return "ProfileNetworkPreference{"
                 + "mPreference=" + getPreference()
-                + "mIncludedUids=" + mIncludedUids.toString()
-                + "mExcludedUids=" + mExcludedUids.toString()
+                + "mIncludedUids=" + Arrays.toString(mIncludedUids)
+                + "mExcludedUids=" + Arrays.toString(mExcludedUids)
                 + "mPreferenceEnterpriseId=" + mPreferenceEnterpriseId
                 + '}';
     }

service/src/com/android/server/ConnectivityService.java

@@ -10833,10 +10833,20 @@ public class ConnectivityService extends IConnectivityManager.Stub
     private void handleSetProfileNetworkPreference(
             @NonNull final List<ProfileNetworkPreferenceList.Preference> preferenceList,
             @Nullable final IOnCompleteListener listener) {
+        /*
+         * handleSetProfileNetworkPreference is always called for single user.
+         * preferenceList only contains preferences for different uids within the same user
+         * (enforced by getUidListToBeAppliedForNetworkPreference).
+         * Clear all the existing preferences for the user before applying new preferences.
+         *
+         */
+        mProfileNetworkPreferences = mProfileNetworkPreferences.clearUser(
+                preferenceList.get(0).user);
         for (final ProfileNetworkPreferenceList.Preference preference : preferenceList) {
             validateNetworkCapabilitiesOfProfileNetworkPreference(preference.capabilities);
             mProfileNetworkPreferences = mProfileNetworkPreferences.plus(preference);
         }
+
         removeDefaultNetworkRequestsForPreference(PREFERENCE_ORDER_PROFILE);
         addPerAppDefaultNetworkRequests(
                 createNrisFromProfileNetworkPreferences(mProfileNetworkPreferences));

service/src/com/android/server/connectivity/ProfileNetworkPreferenceList.java

@@ -70,23 +70,33 @@ public class ProfileNetworkPreferenceList {
     /**
      * Returns a new object consisting of this object plus the passed preference.
      *
-     * If a preference already exists for the same user, it will be replaced by the passed
-     * preference. Passing a Preference object containing a null capabilities object is equivalent
-     * to (and indeed, implemented as) removing the preference for this user.
+     * It is not expected that unwanted preference already exists for the same user.
+     * All preferences for the user that were previously configured should be cleared before
+     * adding a new preference.
+     * Passing a Preference object containing a null capabilities object is equivalent
+     * to removing the preference for this user.
      */
     public ProfileNetworkPreferenceList plus(@NonNull final Preference pref) {
-        final ArrayList<Preference> newPrefs = new ArrayList<>();
-        for (final Preference existingPref : preferences) {
-            if (!existingPref.user.equals(pref.user)) {
-                newPrefs.add(existingPref);
-            }
-        }
+        final ArrayList<Preference> newPrefs = new ArrayList<>(preferences);
         if (null != pref.capabilities) {
             newPrefs.add(pref);
         }
         return new ProfileNetworkPreferenceList(newPrefs);
     }
 
+    /**
+     * Remove all preferences corresponding to a user.
+     */
+    public ProfileNetworkPreferenceList clearUser(UserHandle user) {
+        final ArrayList<Preference> newPrefs = new ArrayList<>();
+        for (final Preference existingPref : preferences) {
+            if (!existingPref.user.equals(user)) {
+                newPrefs.add(existingPref);
+            }
+        }
+        return new ProfileNetworkPreferenceList(newPrefs);
+    }
+
     public boolean isEmpty() {
         return preferences.isEmpty();
     }

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -14630,7 +14630,7 @@ public class ConnectivityServiceTest {
         profileNetworkPreferenceBuilder.setPreference(
                 PROFILE_NETWORK_PREFERENCE_ENTERPRISE_NO_FALLBACK);
         profileNetworkPreferenceBuilder.setPreferenceEnterpriseId(
-                NetworkCapabilities.NET_ENTERPRISE_ID_2);
+                NET_ENTERPRISE_ID_2);
         registerDefaultNetworkCallbacks();
         testPreferenceForUserNetworkUpDownForGivenPreference(
                 profileNetworkPreferenceBuilder.build(), true,
@@ -14730,14 +14730,13 @@ public class ConnectivityServiceTest {
                 workAgent2.getNetwork().netId,
                 uidRangeFor(testHandle, profileNetworkPreferenceBuilder2.build()),
                 PREFERENCE_ORDER_PROFILE));
-        // BUG: the second preference silently replaces the first because they are
-        // both for testHandle.
-        verify(mMockNetd, never()).networkAddUidRangesParcel(new NativeUidRangeConfig(
+        verify(mMockNetd).networkAddUidRangesParcel(new NativeUidRangeConfig(
                 workAgent1.getNetwork().netId,
                 uidRangeFor(testHandle, profileNetworkPreferenceBuilder1.build()),
                 PREFERENCE_ORDER_PROFILE));
 
-        assertNoCallbacks(mSystemDefaultNetworkCallback, mDefaultNetworkCallback, appCb1);
+        assertNoCallbacks(mSystemDefaultNetworkCallback, mDefaultNetworkCallback);
+        appCb1.expectAvailableCallbacksValidated(workAgent1);
         appCb2.expectAvailableCallbacksValidated(workAgent2);
 
         // Set preferences for testHandle to map testWorkProfileAppUid3 to
@@ -14760,16 +14759,15 @@ public class ConnectivityServiceTest {
                 workAgent2.getNetwork().netId,
                 uidRangeFor(testHandle, profileNetworkPreferenceBuilder2.build()),
                 PREFERENCE_ORDER_PROFILE));
-        // BUG: the second preference should also get removed here unless it was silently
-        // discarded
-        verify(mMockNetd, never()).networkRemoveUidRangesParcel(new NativeUidRangeConfig(
+        verify(mMockNetd).networkRemoveUidRangesParcel(new NativeUidRangeConfig(
                 workAgent1.getNetwork().netId,
                 uidRangeFor(testHandle, profileNetworkPreferenceBuilder1.build()),
                 PREFERENCE_ORDER_PROFILE));
 
-        assertNoCallbacks(mSystemDefaultNetworkCallback, mDefaultNetworkCallback, appCb1);
+        assertNoCallbacks(mSystemDefaultNetworkCallback, mDefaultNetworkCallback);
         appCb3.expectAvailableCallbacksValidated(workAgent1);
         appCb2.expectAvailableCallbacksValidated(mCellNetworkAgent);
+        appCb1.expectAvailableCallbacksValidated(mCellNetworkAgent);
 
         // Set the preferences for testHandle to default.
         ProfileNetworkPreference.Builder profileNetworkPreferenceBuilder =