Fix LOCATION_PROVIDERS_ALLOWED with provider status
Ensures that LOCATION_PROVIDERS_ALLOWED is properly updated through all
location provider state changes, including location on/off. Remove all
initialization of LOCATION_PROVIDERS_ALLOWED, as the setting is now
completely controlled by LMS.
Bug: 124300200
Test: Manually
Change-Id: Ic715347515bcc417d873c48113ce4303685c0aa7
diff --git a/packages/SettingsProvider/res/values/defaults.xml b/packages/SettingsProvider/res/values/defaults.xml
index 26ea6ab..65e0c0f 100644
--- a/packages/SettingsProvider/res/values/defaults.xml
+++ b/packages/SettingsProvider/res/values/defaults.xml
@@ -40,8 +40,8 @@
<bool name="def_wifi_display_on">false</bool>
<bool name="def_install_non_market_apps">false</bool>
<bool name="def_package_verifier_enable">true</bool>
- <!-- Comma-separated list of location providers -->
- <string name="def_location_providers_allowed" translatable="false">gps,network</string>
+ <!-- 0 == off, 3 == on -->
+ <integer name="def_location_mode">3</integer>
<bool name="assisted_gps_enabled">true</bool>
<bool name="def_netstats_enabled">true</bool>
<bool name="def_usb_mass_storage_enabled">true</bool>
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/DatabaseHelper.java b/packages/SettingsProvider/src/com/android/providers/settings/DatabaseHelper.java
index 0ee16a9..5e2b7c8 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/DatabaseHelper.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/DatabaseHelper.java
@@ -2340,9 +2340,6 @@
stmt = db.compileStatement("INSERT OR IGNORE INTO secure(name,value)"
+ " VALUES(?,?);");
- loadStringSetting(stmt, Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
- R.string.def_location_providers_allowed);
-
// Don't do this. The SystemServer will initialize ADB_ENABLED from a
// persistent system property instead.
//loadSetting(stmt, Settings.Secure.ADB_ENABLED, 0);
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index ceafbfa..d6c33a3 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -969,6 +969,11 @@
try {
synchronized (mLock) {
Setting setting = getSecureSetting(
+ Settings.Secure.LOCATION_MODE, userId);
+ updateSecureSetting(Settings.Secure.LOCATION_MODE,
+ setting != null ? setting.getValue() : null, null,
+ true, userId, true);
+ setting = getSecureSetting(
Settings.Secure.LOCATION_PROVIDERS_ALLOWED, userId);
updateSecureSetting(Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
setting != null ? setting.getValue() : null, null,
@@ -4229,23 +4234,18 @@
final Setting locationProvidersAllowed = secureSettings.getSettingLocked(
Secure.LOCATION_PROVIDERS_ALLOWED);
- String defLocationMode = Integer.toString(
- !TextUtils.isEmpty(locationProvidersAllowed.getValue())
- ? Secure.LOCATION_MODE_ON
- : Secure.LOCATION_MODE_OFF);
+ final int defLocationMode;
+ if (locationProvidersAllowed.isNull()) {
+ defLocationMode = getContext().getResources().getInteger(
+ R.integer.def_location_mode);
+ } else {
+ defLocationMode =
+ !TextUtils.isEmpty(locationProvidersAllowed.getValue())
+ ? Secure.LOCATION_MODE_ON
+ : Secure.LOCATION_MODE_OFF;
+ }
secureSettings.insertSettingLocked(
- Secure.LOCATION_MODE, defLocationMode,
- null, true, SettingsState.SYSTEM_PACKAGE_NAME);
-
- // also reset LOCATION_PROVIDERS_ALLOWED back to the default value - this
- // setting is now only for debug/test purposes, and will likely be removed
- // in a later release. LocationManagerService is responsible for adjusting
- // these settings to the proper state.
-
- String defLocationProvidersAllowed = getContext().getResources().getString(
- R.string.def_location_providers_allowed);
- secureSettings.insertSettingLocked(
- Secure.LOCATION_PROVIDERS_ALLOWED, defLocationProvidersAllowed,
+ Secure.LOCATION_MODE, Integer.toString(defLocationMode),
null, true, SettingsState.SYSTEM_PACKAGE_NAME);
}
diff --git a/services/core/java/com/android/server/LocationManagerService.java b/services/core/java/com/android/server/LocationManagerService.java
index d8b96e4..2d62850 100644
--- a/services/core/java/com/android/server/LocationManagerService.java
+++ b/services/core/java/com/android/server/LocationManagerService.java
@@ -337,7 +337,7 @@
@Override
public void onChange(boolean selfChange) {
synchronized (mLock) {
- onProviderAllowedChangedLocked(true);
+ onProviderAllowedChangedLocked();
}
}
}, UserHandle.USER_ALL);
@@ -436,6 +436,10 @@
@GuardedBy("mLock")
private void onLocationModeChangedLocked(boolean broadcast) {
+ if (D) {
+ Log.d(TAG, "location enabled is now " + isLocationEnabled());
+ }
+
for (LocationProvider p : mProviders) {
p.onLocationModeChangedLocked();
}
@@ -448,16 +452,10 @@
}
@GuardedBy("mLock")
- private void onProviderAllowedChangedLocked(boolean broadcast) {
+ private void onProviderAllowedChangedLocked() {
for (LocationProvider p : mProviders) {
p.onAllowedChangedLocked();
}
-
- if (broadcast) {
- mContext.sendBroadcastAsUser(
- new Intent(LocationManager.PROVIDERS_CHANGED_ACTION),
- UserHandle.ALL);
- }
}
@GuardedBy("mLock")
@@ -827,6 +825,10 @@
return;
}
+ if (D) {
+ Log.d(TAG, "foreground user is changing to " + userId);
+ }
+
// let providers know the current user is on the way out before changing the user
for (LocationProvider p : mProviders) {
p.onUserChangingLocked();
@@ -839,7 +841,12 @@
// if the user changes, per-user settings may also have changed
onLocationModeChangedLocked(false);
- onProviderAllowedChangedLocked(false);
+ onProviderAllowedChangedLocked();
+
+ // always force useability to be rechecked, even if no per-user settings have changed
+ for (LocationProvider p : mProviders) {
+ p.onUseableChangedLocked(false);
+ }
}
private class LocationProvider implements AbstractLocationProvider.LocationProviderManager {
@@ -891,9 +898,13 @@
public void attachLocked(AbstractLocationProvider provider) {
checkNotNull(provider);
checkState(mProvider == null);
- mProvider = provider;
- onUseableChangedLocked();
+ if (D) {
+ Log.d(TAG, mName + " provider attached");
+ }
+
+ mProvider = provider;
+ onUseableChangedLocked(false);
}
public String getName() {
@@ -1054,26 +1065,12 @@
return;
}
- mEnabled = enabled;
-
- // update provider allowed settings to reflect enabled status
- if (mIsManagedBySettings) {
- if (mEnabled && !mAllowed) {
- Settings.Secure.putStringForUser(
- mContext.getContentResolver(),
- Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
- "+" + mName,
- mCurrentUserId);
- } else if (!mEnabled && mAllowed) {
- Settings.Secure.putStringForUser(
- mContext.getContentResolver(),
- Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
- "-" + mName,
- mCurrentUserId);
- }
+ if (D) {
+ Log.d(TAG, mName + " provider enabled is now " + mEnabled);
}
- onUseableChangedLocked();
+ mEnabled = enabled;
+ onUseableChangedLocked(false);
}
});
}
@@ -1091,41 +1088,28 @@
@GuardedBy("mLock")
public void onLocationModeChangedLocked() {
- onUseableChangedLocked();
- }
-
- private boolean isAllowed() {
- return isAllowedForUser(mCurrentUserId);
- }
-
- private boolean isAllowedForUser(int userId) {
- String allowedProviders = Settings.Secure.getStringForUser(
- mContext.getContentResolver(),
- Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
- userId);
- return TextUtils.delimitedStringContains(allowedProviders, ',', mName);
+ onUseableChangedLocked(false);
}
@GuardedBy("mLock")
public void onAllowedChangedLocked() {
if (mIsManagedBySettings) {
- boolean allowed = isAllowed();
+ String allowedProviders = Settings.Secure.getStringForUser(
+ mContext.getContentResolver(),
+ Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
+ mCurrentUserId);
+ boolean allowed = TextUtils.delimitedStringContains(allowedProviders, ',', mName);
+
if (allowed == mAllowed) {
return;
}
- mAllowed = allowed;
- // make a best effort to keep the setting matching the real enabled state of the
- // provider so that legacy applications aren't broken.
- if (mAllowed && !mEnabled) {
- Settings.Secure.putStringForUser(
- mContext.getContentResolver(),
- Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
- "-" + mName,
- mCurrentUserId);
+ if (D) {
+ Log.d(TAG, mName + " provider allowed is now " + mAllowed);
}
- onUseableChangedLocked();
+ mAllowed = allowed;
+ onUseableChangedLocked(true);
}
}
@@ -1140,17 +1124,49 @@
}
@GuardedBy("mLock")
- public void onUseableChangedLocked() {
+ private boolean isUseableIgnoringAllowedLocked() {
+ return mProvider != null && mProviders.contains(this) && isLocationEnabled()
+ && mEnabled;
+ }
+
+ @GuardedBy("mLock")
+ public void onUseableChangedLocked(boolean isAllowedChanged) {
// if any property that contributes to "useability" here changes state, it MUST result
// in a direct or indrect call to onUseableChangedLocked. this allows the provider to
// guarantee that it will always eventually reach the correct state.
- boolean useable = mProvider != null
- && mProviders.contains(this) && isLocationEnabled() && mAllowed && mEnabled;
+ boolean useableIgnoringAllowed = isUseableIgnoringAllowedLocked();
+ boolean useable = useableIgnoringAllowed && mAllowed;
+
+ // update deprecated provider allowed settings for backwards compatibility, and do this
+ // even if there is no change in overall useability state. this may result in trying to
+ // overwrite the same value, but Settings handles deduping this.
+ if (mIsManagedBySettings) {
+ // a "-" change derived from the allowed setting should not be overwritten, but a
+ // "+" change should be corrected if necessary
+ if (useableIgnoringAllowed && !isAllowedChanged) {
+ Settings.Secure.putStringForUser(
+ mContext.getContentResolver(),
+ Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
+ "+" + mName,
+ mCurrentUserId);
+ } else if (!useableIgnoringAllowed) {
+ Settings.Secure.putStringForUser(
+ mContext.getContentResolver(),
+ Settings.Secure.LOCATION_PROVIDERS_ALLOWED,
+ "-" + mName,
+ mCurrentUserId);
+ }
+ }
+
if (useable == mUseable) {
return;
}
mUseable = useable;
+ if (D) {
+ Log.d(TAG, mName + " provider useable is now " + mUseable);
+ }
+
if (!mUseable) {
// If any provider has been disabled, clear all last locations for all
// providers. This is to be on the safe side in case a provider has location
@@ -1160,6 +1176,10 @@
}
updateProviderUseableLocked(this);
+
+ mContext.sendBroadcastAsUser(
+ new Intent(LocationManager.PROVIDERS_CHANGED_ACTION),
+ UserHandle.ALL);
}
@GuardedBy("mLock")
@@ -1720,7 +1740,7 @@
mProviders.add(provider);
provider.onAllowedChangedLocked(); // allowed state may change while provider was inactive
- provider.onUseableChangedLocked();
+ provider.onUseableChangedLocked(false);
}
@GuardedBy("mLock")
@@ -1728,7 +1748,7 @@
if (mProviders.remove(provider)) {
long identity = Binder.clearCallingIdentity();
try {
- provider.onUseableChangedLocked();
+ provider.onUseableChangedLocked(false);
} finally {
Binder.restoreCallingIdentity(identity);
}
@@ -2113,7 +2133,6 @@
}
}
- if (D) Log.d(TAG, "provider request: " + provider + " " + providerRequest);
provider.setRequestLocked(providerRequest, worksource);
}
@@ -2507,7 +2526,6 @@
@Override
public Location getLastLocation(LocationRequest r, String packageName) {
- if (D) Log.d(TAG, "getLastLocation: " + r);
synchronized (mLock) {
LocationRequest request = r != null ? r : DEFAULT_LOCATION_REQUEST;
int allowedResolutionLevel = getCallerAllowedResolutionLevel();