From 533a17f1f593f44cbc82fab6de5334ddbc7f2ecf Mon Sep 17 00:00:00 2001 From: Soonil Nagarkar Date: Wed, 10 Jul 2019 15:11:02 -0700 Subject: [PATCH] Remove obsolete broadcast behavior There is no need to broadcast when a footer is displayed/removed. The broadcasts were added specifically for gmscore to know when the footer was visible, so that GLS consent could be given. However, this was only very briefly actually used within GmsCore, and has been obsolete for several releases. Test: CTS Change-Id: I1df6c6214fe79c943cabd7b04357c8ab7d1c1c39 --- .../LocationFooterPreferenceController.java | 93 +++++-------------- .../settings/location/LocationSettings.java | 2 +- .../location/LocationEnablerTest.java | 4 - ...ocationFooterPreferenceControllerTest.java | 70 +------------- 4 files changed, 27 insertions(+), 142 deletions(-) diff --git a/src/com/android/settings/location/LocationFooterPreferenceController.java b/src/com/android/settings/location/LocationFooterPreferenceController.java index 55fea9f93c7..7c39fea47f6 100644 --- a/src/com/android/settings/location/LocationFooterPreferenceController.java +++ b/src/com/android/settings/location/LocationFooterPreferenceController.java @@ -28,9 +28,6 @@ import androidx.annotation.VisibleForTesting; import androidx.preference.Preference; import androidx.preference.PreferenceCategory; -import com.android.settingslib.core.lifecycle.Lifecycle; -import com.android.settingslib.core.lifecycle.LifecycleObserver; -import com.android.settingslib.core.lifecycle.events.OnPause; import com.android.settingslib.widget.FooterPreference; import java.util.ArrayList; @@ -41,24 +38,19 @@ import java.util.List; /** * Preference controller for location footer preference category */ -public class LocationFooterPreferenceController extends LocationBasePreferenceController - implements LifecycleObserver, OnPause { +public class LocationFooterPreferenceController extends LocationBasePreferenceController { + private static final String TAG = "LocationFooter"; private static final String KEY_LOCATION_FOOTER = "location_footer"; private static final Intent INJECT_INTENT = new Intent(LocationManager.SETTINGS_FOOTER_DISPLAYED_ACTION); - private final Context mContext; - private final PackageManager mPackageManager; - private Collection mFooterInjectors; - public LocationFooterPreferenceController(Context context, Lifecycle lifecycle) { - super(context, lifecycle); - mContext = context; - mPackageManager = mContext.getPackageManager(); - mFooterInjectors = new ArrayList<>(); - if (lifecycle != null) { - lifecycle.addObserver(this); - } + private final PackageManager mPackageManager; + + public LocationFooterPreferenceController(Context context) { + // we don't care location mode changes, so pass in a null lifecycle to disable listening + super(context, null); + mPackageManager = context.getPackageManager(); } @Override @@ -67,37 +59,30 @@ public class LocationFooterPreferenceController extends LocationBasePreferenceCo } /** - * Insert footer preferences. Send a {@link LocationManager#SETTINGS_FOOTER_DISPLAYED_ACTION} - * broadcast to receivers who have injected a footer + * Insert footer preferences. */ @Override public void updateState(Preference preference) { PreferenceCategory category = (PreferenceCategory) preference; category.removeAll(); - mFooterInjectors.clear(); Collection footerData = getFooterData(); for (FooterData data : footerData) { - // Generate a footer preference with the given text - FooterPreference footerPreference = new FooterPreference(preference.getContext()); - String footerString; try { - footerString = + String footerString = mPackageManager .getResourcesForApplication(data.applicationInfo) .getString(data.footerStringRes); + + // Generate a footer preference with the given text + FooterPreference footerPreference = new FooterPreference(preference.getContext()); + footerPreference.setTitle(footerString); + category.addPreference(footerPreference); } catch (NameNotFoundException exception) { Log.w( TAG, "Resources not found for application " + data.applicationInfo.packageName); - continue; } - footerPreference.setTitle(footerString); - // Inject the footer - category.addPreference(footerPreference); - // Send broadcast to the injector announcing a footer has been injected - sendBroadcastFooterDisplayed(data.componentName); - mFooterInjectors.add(data.componentName); } } @@ -116,37 +101,12 @@ public class LocationFooterPreferenceController extends LocationBasePreferenceCo return !getFooterData().isEmpty(); } - /** - * Send a {@link LocationManager#SETTINGS_FOOTER_REMOVED_ACTION} broadcast to footer injectors - * when LocationFragment is on pause - */ - @Override - public void onPause() { - // Send broadcast to the footer injectors. Notify them the footer is not visible. - for (ComponentName componentName : mFooterInjectors) { - final Intent intent = new Intent(LocationManager.SETTINGS_FOOTER_REMOVED_ACTION); - intent.setComponent(componentName); - mContext.sendBroadcast(intent); - } - } - - /** - * Send a {@link LocationManager#SETTINGS_FOOTER_DISPLAYED_ACTION} broadcast to a footer - * injector. - */ - @VisibleForTesting - void sendBroadcastFooterDisplayed(ComponentName componentName) { - Intent intent = new Intent(LocationManager.SETTINGS_FOOTER_DISPLAYED_ACTION); - intent.setComponent(componentName); - mContext.sendBroadcast(intent); - } - /** * Return a list of strings with text provided by ACTION_INJECT_FOOTER broadcast receivers. */ - private Collection getFooterData() { + private List getFooterData() { // Fetch footer text from system apps - final List resolveInfos = + List resolveInfos = mPackageManager.queryBroadcastReceivers( INJECT_INTENT, PackageManager.GET_META_DATA); if (resolveInfos == null) { @@ -158,10 +118,10 @@ public class LocationFooterPreferenceController extends LocationBasePreferenceCo Log.d(TAG, "Found broadcast receivers: " + resolveInfos); } - final Collection footerDataList = new ArrayList<>(resolveInfos.size()); + List footerDataList = new ArrayList<>(resolveInfos.size()); for (ResolveInfo resolveInfo : resolveInfos) { - final ActivityInfo activityInfo = resolveInfo.activityInfo; - final ApplicationInfo appInfo = activityInfo.applicationInfo; + ActivityInfo activityInfo = resolveInfo.activityInfo; + ApplicationInfo appInfo = activityInfo.applicationInfo; // If a non-system app tries to inject footer, ignore it if ((appInfo.flags & ApplicationInfo.FLAG_SYSTEM) == 0) { @@ -187,11 +147,7 @@ public class LocationFooterPreferenceController extends LocationBasePreferenceCo + LocationManager.METADATA_SETTINGS_FOOTER_STRING); continue; } - footerDataList.add( - new FooterData( - footerTextRes, - appInfo, - new ComponentName(activityInfo.packageName, activityInfo.name))); + footerDataList.add(new FooterData(footerTextRes, appInfo)); } return footerDataList; } @@ -207,14 +163,9 @@ public class LocationFooterPreferenceController extends LocationBasePreferenceCo // Application info of receiver injecting this footer final ApplicationInfo applicationInfo; - // The component that injected the footer. It must be a receiver of broadcast - // LocationManager.SETTINGS_FOOTER_DISPLAYED_ACTION - final ComponentName componentName; - - FooterData(int footerRes, ApplicationInfo appInfo, ComponentName componentName) { + FooterData(int footerRes, ApplicationInfo appInfo) { this.footerStringRes = footerRes; this.applicationInfo = appInfo; - this.componentName = componentName; } } } diff --git a/src/com/android/settings/location/LocationSettings.java b/src/com/android/settings/location/LocationSettings.java index 71424867d66..21b031b75bb 100644 --- a/src/com/android/settings/location/LocationSettings.java +++ b/src/com/android/settings/location/LocationSettings.java @@ -121,7 +121,7 @@ public class LocationSettings extends DashboardFragment { controllers.add(new RecentLocationRequestPreferenceController(context, fragment, lifecycle)); controllers.add(new LocationScanningPreferenceController(context)); controllers.add(new LocationServicePreferenceController(context, fragment, lifecycle)); - controllers.add(new LocationFooterPreferenceController(context, lifecycle)); + controllers.add(new LocationFooterPreferenceController(context)); return controllers; } diff --git a/tests/robotests/src/com/android/settings/location/LocationEnablerTest.java b/tests/robotests/src/com/android/settings/location/LocationEnablerTest.java index fbae2f45b16..4e1e600056b 100644 --- a/tests/robotests/src/com/android/settings/location/LocationEnablerTest.java +++ b/tests/robotests/src/com/android/settings/location/LocationEnablerTest.java @@ -158,10 +158,6 @@ public class LocationEnablerTest { Settings.Secure.LOCATION_MODE, Settings.Secure.LOCATION_MODE_OFF); mEnabler.setLocationEnabled(true); - verify(mContext).sendBroadcastAsUser( - argThat(actionMatches(LocationManager.MODE_CHANGING_ACTION)), - eq(UserHandle.of(ActivityManager.getCurrentUser())), - eq(WRITE_SECURE_SETTINGS)); assertThat(Settings.Secure.getInt(mContext.getContentResolver(), Settings.Secure.LOCATION_CHANGER, Settings.Secure.LOCATION_CHANGER_UNKNOWN)) .isEqualTo(Settings.Secure.LOCATION_CHANGER_SYSTEM_SETTINGS); diff --git a/tests/robotests/src/com/android/settings/location/LocationFooterPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/location/LocationFooterPreferenceControllerTest.java index 2abef4ed875..dc3d40a7400 100644 --- a/tests/robotests/src/com/android/settings/location/LocationFooterPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/location/LocationFooterPreferenceControllerTest.java @@ -22,11 +22,9 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.pm.ActivityInfo; @@ -38,12 +36,9 @@ import android.content.res.Resources; import android.location.LocationManager; import android.os.Bundle; -import androidx.lifecycle.LifecycleOwner; import androidx.preference.Preference; import androidx.preference.PreferenceCategory; -import com.android.settingslib.core.lifecycle.Lifecycle; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -65,22 +60,17 @@ public class LocationFooterPreferenceControllerTest { private PackageManager mPackageManager; @Mock private Resources mResources; - private Context mContext; private LocationFooterPreferenceController mController; - private LifecycleOwner mLifecycleOwner; - private Lifecycle mLifecycle; private static final int TEST_RES_ID = 1234; private static final String TEST_TEXT = "text"; @Before public void setUp() throws NameNotFoundException { MockitoAnnotations.initMocks(this); - mContext = spy(RuntimeEnvironment.application); - when(mContext.getPackageManager()).thenReturn(mPackageManager); - mLifecycleOwner = () -> mLifecycle; - mLifecycle = new Lifecycle(mLifecycleOwner); - when(mPreferenceCategory.getContext()).thenReturn(mContext); - mController = spy(new LocationFooterPreferenceController(mContext, mLifecycle)); + Context context = spy(RuntimeEnvironment.application); + when(context.getPackageManager()).thenReturn(mPackageManager); + when(mPreferenceCategory.getContext()).thenReturn(context); + mController = spy(new LocationFooterPreferenceController(context)); when(mPackageManager.getResourcesForApplication(any(ApplicationInfo.class))) .thenReturn(mResources); when(mResources.getString(TEST_RES_ID)).thenReturn(TEST_TEXT); @@ -118,32 +108,6 @@ public class LocationFooterPreferenceControllerTest { assertThat(mController.isAvailable()).isFalse(); } - @Test - public void sendBroadcastFooterInject() { - ArgumentCaptor intent = ArgumentCaptor.forClass(Intent.class); - final ActivityInfo activityInfo = - getTestResolveInfo(/*isSystemApp*/ true, /*hasRequiredMetadata*/ true).activityInfo; - mController.sendBroadcastFooterDisplayed( - new ComponentName(activityInfo.packageName, activityInfo.name)); - verify(mContext).sendBroadcast(intent.capture()); - assertThat(intent.getValue().getAction()) - .isEqualTo(LocationManager.SETTINGS_FOOTER_DISPLAYED_ACTION); - } - - @Test - public void updateState_sendBroadcast() { - final List testResolveInfos = new ArrayList<>(); - testResolveInfos.add( - getTestResolveInfo(/*isSystemApp*/ true, /*hasRequiredMetadata*/ true)); - when(mPackageManager.queryBroadcastReceivers(any(), anyInt())) - .thenReturn(testResolveInfos); - mController.updateState(mPreferenceCategory); - ArgumentCaptor intent = ArgumentCaptor.forClass(Intent.class); - verify(mContext).sendBroadcast(intent.capture()); - assertThat(intent.getValue().getAction()) - .isEqualTo(LocationManager.SETTINGS_FOOTER_DISPLAYED_ACTION); - } - @Test public void updateState_addPreferences() { final List testResolveInfos = new ArrayList<>(); @@ -166,32 +130,6 @@ public class LocationFooterPreferenceControllerTest { .thenReturn(testResolveInfos); mController.updateState(mPreferenceCategory); verify(mPreferenceCategory, never()).addPreference(any(Preference.class)); - verify(mContext, never()).sendBroadcast(any(Intent.class)); - } - - @Test - public void updateState_thenOnPause_sendBroadcasts() { - final List testResolveInfos = new ArrayList<>(); - testResolveInfos.add( - getTestResolveInfo(/*isSystemApp*/ true, /*hasRequiredMetadata*/ true)); - when(mPackageManager.queryBroadcastReceivers(any(Intent.class), anyInt())) - .thenReturn(testResolveInfos); - mController.updateState(mPreferenceCategory); - ArgumentCaptor intent = ArgumentCaptor.forClass(Intent.class); - verify(mContext).sendBroadcast(intent.capture()); - assertThat(intent.getValue().getAction()) - .isEqualTo(LocationManager.SETTINGS_FOOTER_DISPLAYED_ACTION); - - mController.onPause(); - verify(mContext, times(2)).sendBroadcast(intent.capture()); - assertThat(intent.getValue().getAction()) - .isEqualTo(LocationManager.SETTINGS_FOOTER_REMOVED_ACTION); - } - - @Test - public void onPause_doNotSendBroadcast() { - mController.onPause(); - verify(mContext, never()).sendBroadcast(any(Intent.class)); } /**