Fix shifting problem in location services page

Controller generates the injection location settings
in updateState() which happens in onResume.
That's the primary reason why we observed the shifting issue.

In a good practice, we generate preference in displayPreference(),
and then update the setting state in updateState().

In this cl, we create a base controller class to encapsulate
most implementation, and developer need to inject location
services in child class.

Test: Add work profile, and see correct services list.
Bug: 183169265
Change-Id: I5735ba974da87ad83b56791abd8a8637c2317571

src/com/android/settings/location/LocationInjectedServiceBasePreferenceController.java

@@ -0,0 +1,121 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settings.location;
+
+import android.content.BroadcastReceiver;
+import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
+import android.location.SettingInjectorService;
+import android.os.UserHandle;
+import android.util.Log;
+
+import androidx.annotation.VisibleForTesting;
+import androidx.lifecycle.Lifecycle;
+import androidx.lifecycle.OnLifecycleEvent;
+import androidx.preference.Preference;
+import androidx.preference.PreferenceScreen;
+
+import com.android.settings.Utils;
+import com.android.settings.dashboard.DashboardFragment;
+import com.android.settingslib.core.lifecycle.LifecycleObserver;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A abstract class which is responsible for creating the injected location services items.
+ * Developer needs to implement the {@link #injectLocationServices(PreferenceScreen)}.
+ */
+public abstract class LocationInjectedServiceBasePreferenceController
+        extends LocationBasePreferenceController implements LifecycleObserver{
+
+    private static final String TAG = "LocationPrefCtrl";
+    @VisibleForTesting
+    static final IntentFilter INTENT_FILTER_INJECTED_SETTING_CHANGED =
+            new IntentFilter(SettingInjectorService.ACTION_INJECTED_SETTING_CHANGED);
+
+    @VisibleForTesting
+    AppSettingsInjector mInjector;
+    /** Receives UPDATE_INTENT */
+    @VisibleForTesting
+    BroadcastReceiver mInjectedSettingsReceiver;
+
+    public LocationInjectedServiceBasePreferenceController(Context context, String key) {
+        super(context, key);
+    }
+
+    @Override
+    public void init(DashboardFragment fragment) {
+        super.init(fragment);
+        mInjector = new AppSettingsInjector(mContext, getMetricsCategory());
+    }
+
+    @Override
+    public void displayPreference(PreferenceScreen screen) {
+        super.displayPreference(screen);
+        injectLocationServices(screen);
+    }
+
+    /**
+     * Override this method to inject location services in preference screen.
+     * @param screen
+     */
+    protected abstract void injectLocationServices(PreferenceScreen screen);
+
+    @Override
+    public void onLocationModeChanged(int mode, boolean restricted) {
+        // As a safety measure, also reloads on location mode change to ensure the settings are
+        // up-to-date even if an affected app doesn't send the setting changed broadcast.
+        mInjector.reloadStatusMessages();
+    }
+
+    /** @OnLifecycleEvent(ON_RESUME) */
+    @OnLifecycleEvent(Lifecycle.Event.ON_RESUME)
+    public void onResume() {
+        if (mInjectedSettingsReceiver == null) {
+            mInjectedSettingsReceiver = new BroadcastReceiver() {
+                @Override
+                public void onReceive(Context context, Intent intent) {
+                    if (Log.isLoggable(TAG, Log.DEBUG)) {
+                        Log.d(TAG, "Received settings change intent: " + intent);
+                    }
+                    mInjector.reloadStatusMessages();
+                }
+            };
+        }
+        mContext.registerReceiver(
+                mInjectedSettingsReceiver, INTENT_FILTER_INJECTED_SETTING_CHANGED);
+    }
+
+    /** @OnLifecycleEvent(ON_PAUSE) */
+    @OnLifecycleEvent(Lifecycle.Event.ON_PAUSE)
+    public void onPause() {
+        mContext.unregisterReceiver(mInjectedSettingsReceiver);
+    }
+
+    protected Map<Integer, List<Preference>> getLocationServices() {
+        // If location access is locked down by device policy then we only show injected settings
+        // for the primary profile.
+        final int profileUserId = Utils.getManagedProfileId(mUserManager, UserHandle.myUserId());
+
+        return mInjector.getInjectedSettings(mFragment.getPreferenceManager().getContext(),
+                (profileUserId != UserHandle.USER_NULL
+                        && mLocationEnabler.getShareLocationEnforcedAdmin(profileUserId) != null)
+                        ? UserHandle.myUserId() : UserHandle.USER_CURRENT);
+    }
+}

src/com/android/settings/location/LocationInjectedServicesForWorkPreferenceController.java

@@ -20,6 +20,8 @@ import android.content.Context;
 import android.os.UserHandle;
 
 import androidx.preference.Preference;
+import androidx.preference.PreferenceCategory;
+import androidx.preference.PreferenceScreen;
 
 import com.android.settings.widget.RestrictedAppPreference;
 
@@ -30,7 +32,7 @@ import java.util.Map;
  * Retrieve the Location Services used in work profile user.
  */
 public class LocationInjectedServicesForWorkPreferenceController extends
-        LocationInjectedServicesPreferenceController {
+        LocationInjectedServiceBasePreferenceController {
     private static final String TAG = "LocationWorkPrefCtrl";
 
     public LocationInjectedServicesForWorkPreferenceController(Context context, String key) {
@@ -38,10 +40,10 @@ public class LocationInjectedServicesForWorkPreferenceController extends
     }
 
     @Override
-    public void updateState(Preference preference) {
+    protected void injectLocationServices(PreferenceScreen screen) {
-        mCategoryLocationServices.removeAll();
+        final PreferenceCategory categoryLocationServices =
+                screen.findPreference(getPreferenceKey());
         final Map<Integer, List<Preference>> prefs = getLocationServices();
-        boolean show = false;
         for (Map.Entry<Integer, List<Preference>> entry : prefs.entrySet()) {
             for (Preference pref : entry.getValue()) {
                 if (pref instanceof RestrictedAppPreference) {
@@ -49,11 +51,8 @@ public class LocationInjectedServicesForWorkPreferenceController extends
                 }
             }
             if (entry.getKey() != UserHandle.myUserId()) {
-                LocationSettings.addPreferencesSorted(entry.getValue(),
+                LocationSettings.addPreferencesSorted(entry.getValue(), categoryLocationServices);
-                        mCategoryLocationServices);
-                show = true;
             }
         }
-        mCategoryLocationServices.setVisible(show);
     }
 }

src/com/android/settings/location/LocationInjectedServicesPreferenceController.java

@@ -15,25 +15,14 @@
  */
 package com.android.settings.location;
 
-import android.content.BroadcastReceiver;
 import android.content.Context;
-import android.content.Intent;
-import android.content.IntentFilter;
-import android.location.SettingInjectorService;
 import android.os.UserHandle;
-import android.util.Log;
 
-import androidx.annotation.VisibleForTesting;
 import androidx.preference.Preference;
 import androidx.preference.PreferenceCategory;
 import androidx.preference.PreferenceScreen;
 
-import com.android.settings.Utils;
-import com.android.settings.dashboard.DashboardFragment;
 import com.android.settings.widget.RestrictedAppPreference;
-import com.android.settingslib.core.lifecycle.LifecycleObserver;
-import com.android.settingslib.core.lifecycle.events.OnPause;
-import com.android.settingslib.core.lifecycle.events.OnResume;
 
 import java.util.List;
 import java.util.Map;
@@ -41,42 +30,20 @@ import java.util.Map;
 /**
  * Preference controller for the injected Location Services.
  */
-public class LocationInjectedServicesPreferenceController extends LocationBasePreferenceController
+public class LocationInjectedServicesPreferenceController
-        implements LifecycleObserver, OnResume, OnPause {
+        extends LocationInjectedServiceBasePreferenceController {
 
     private static final String TAG = "LocationPrefCtrl";
-    @VisibleForTesting
-    static final IntentFilter INTENT_FILTER_INJECTED_SETTING_CHANGED =
-            new IntentFilter(SettingInjectorService.ACTION_INJECTED_SETTING_CHANGED);
-
-    protected PreferenceCategory mCategoryLocationServices;
-    @VisibleForTesting
-    AppSettingsInjector mInjector;
-    /** Receives UPDATE_INTENT */
-    @VisibleForTesting
-    BroadcastReceiver mInjectedSettingsReceiver;
 
     public LocationInjectedServicesPreferenceController(Context context, String key) {
         super(context, key);
     }
 
     @Override
-    public void init(DashboardFragment fragment) {
+    protected void injectLocationServices(PreferenceScreen screen) {
-        super.init(fragment);
+        final PreferenceCategory categoryLocationServices =
-        mInjector = new AppSettingsInjector(mContext, getMetricsCategory());
+                screen.findPreference(getPreferenceKey());
-    }
-
-    @Override
-    public void displayPreference(PreferenceScreen screen) {
-        super.displayPreference(screen);
-        mCategoryLocationServices = screen.findPreference(getPreferenceKey());
-    }
-
-    @Override
-    public void updateState(Preference preference) {
-        mCategoryLocationServices.removeAll();
         final Map<Integer, List<Preference>> prefs = getLocationServices();
-        boolean show = false;
         for (Map.Entry<Integer, List<Preference>> entry : prefs.entrySet()) {
             for (Preference pref : entry.getValue()) {
                 if (pref instanceof RestrictedAppPreference) {
@@ -84,53 +51,11 @@ public class LocationInjectedServicesPreferenceController extends LocationBasePr
                 }
             }
             if (entry.getKey() == UserHandle.myUserId()) {
-                if (mCategoryLocationServices != null) {
+                if (categoryLocationServices != null) {
                     LocationSettings.addPreferencesSorted(entry.getValue(),
-                            mCategoryLocationServices);
+                            categoryLocationServices);
-                }
+                }
-                show = true;
             }
         }
-        mCategoryLocationServices.setVisible(show);
-    }
-
-    @Override
-    public void onLocationModeChanged(int mode, boolean restricted) {
-        // As a safety measure, also reloads on location mode change to ensure the settings are
-        // up-to-date even if an affected app doesn't send the setting changed broadcast.
-        mInjector.reloadStatusMessages();
-    }
-
-    @Override
-    public void onResume() {
-        if (mInjectedSettingsReceiver == null) {
-            mInjectedSettingsReceiver = new BroadcastReceiver() {
-                @Override
-                public void onReceive(Context context, Intent intent) {
-                    if (Log.isLoggable(TAG, Log.DEBUG)) {
-                        Log.d(TAG, "Received settings change intent: " + intent);
-                    }
-                    mInjector.reloadStatusMessages();
-                }
-            };
-        }
-        mContext.registerReceiver(
-                mInjectedSettingsReceiver, INTENT_FILTER_INJECTED_SETTING_CHANGED);
-    }
-
-    @Override
-    public void onPause() {
-        mContext.unregisterReceiver(mInjectedSettingsReceiver);
-    }
-
-    protected Map<Integer, List<Preference>> getLocationServices() {
-        // If location access is locked down by device policy then we only show injected settings
-        // for the primary profile.
-        final int profileUserId = Utils.getManagedProfileId(mUserManager, UserHandle.myUserId());
-
-        return mInjector.getInjectedSettings(mFragment.getPreferenceManager().getContext(),
-                (profileUserId != UserHandle.USER_NULL
-                        && mLocationEnabler.getShareLocationEnforcedAdmin(profileUserId) != null)
-                        ? UserHandle.myUserId() : UserHandle.USER_CURRENT);
     }
 }

tests/robotests/src/com/android/settings/location/LocationInjectedServicesPreferenceControllerTest.java

@@ -67,8 +67,6 @@ public class LocationInjectedServicesPreferenceControllerTest {
     @Mock
     private PreferenceCategory mCategoryPrimary;
     @Mock
-    private PreferenceCategory mCategoryManaged;
-    @Mock
     private PreferenceScreen mScreen;
     @Mock
     private AppSettingsInjector mSettingsInjector;
@@ -114,31 +112,6 @@ public class LocationInjectedServicesPreferenceControllerTest {
         verify(mContext).unregisterReceiver(mController.mInjectedSettingsReceiver);
     }
 
-    @Test
-    public void updateState_shouldRemoveAllAndAddInjectedSettings() {
-        final List<Preference> preferences = new ArrayList<>();
-        final Map<Integer, List<Preference>> map = new ArrayMap<>();
-        final Preference pref1 = new Preference(mContext);
-        pref1.setTitle("Title1");
-        final Preference pref2 = new Preference(mContext);
-        pref2.setTitle("Title2");
-        preferences.add(pref1);
-        preferences.add(pref2);
-        map.put(UserHandle.myUserId(), preferences);
-        doReturn(map)
-                .when(mSettingsInjector).getInjectedSettings(any(Context.class), anyInt());
-        when(mFragment.getPreferenceManager().getContext()).thenReturn(mContext);
-        ShadowUserManager.getShadow().setProfileIdsWithDisabled(new int[]{UserHandle.myUserId()});
-
-        mController.displayPreference(mScreen);
-
-        mController.updateState(mCategoryPrimary);
-
-        verify(mCategoryPrimary).removeAll();
-        verify(mCategoryPrimary).addPreference(pref1);
-        verify(mCategoryPrimary).addPreference(pref2);
-    }
-
     @Test
     public void workProfileDisallowShareLocationOn_getParentUserLocationServicesOnly() {
         final int fakeWorkProfileId = 123;
@@ -158,7 +131,6 @@ public class LocationInjectedServicesPreferenceControllerTest {
         when(mDevicePolicyManager.getDeviceOwnerComponentOnAnyUser()).thenReturn(componentName);
 
         mController.displayPreference(mScreen);
-        mController.updateState(mCategoryPrimary);
         verify(mSettingsInjector).getInjectedSettings(
                 any(Context.class), eq(UserHandle.myUserId()));
     }
@@ -178,7 +150,6 @@ public class LocationInjectedServicesPreferenceControllerTest {
                 enforcingUsers);
 
         mController.displayPreference(mScreen);
-        mController.updateState(mCategoryPrimary);
         verify(mSettingsInjector).getInjectedSettings(
                 any(Context.class), eq(UserHandle.USER_CURRENT));
     }
@@ -216,7 +187,6 @@ public class LocationInjectedServicesPreferenceControllerTest {
         when(mDevicePolicyManager.getDeviceOwnerComponentOnAnyUser()).thenReturn(componentName);
 
         mController.displayPreference(mScreen);
-        mController.updateState(mCategoryPrimary);
 
         assertThat(pref.isEnabled()).isFalse();
         assertThat(pref.isDisabledByAdmin()).isTrue();