Merge "Fix LOCATION_PROVIDERS_ALLOWED with provider status"

4 files changed, 96 insertions, 81 deletions

diff --git a/packages/SettingsProvider/res/values/defaults.xml b/packages/SettingsProvider/res/values/defaults.xml
index 26ea6ab2e8be..65e0c0fbc991 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 0ee16a9e9ed2..5e2b7c86ee93 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 @@ class DatabaseHelper extends SQLiteOpenHelper {
             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 ceafbfa4da15..d6c33a3b077c 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 @@ public class SettingsProvider extends ContentProvider {
                 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 @@ public class SettingsProvider extends ContentProvider {
                         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);
-                        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);
+                        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_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 4b8ba1056c85..d7cc19b6dbd4 100644
--- a/services/core/java/com/android/server/LocationManagerService.java
+++ b/services/core/java/com/android/server/LocationManagerService.java
@@ -337,7 +337,7 @@ public class LocationManagerService extends ILocationManager.Stub {
                     @Override
                     public void onChange(boolean selfChange) {
                         synchronized (mLock) {
-                            onProviderAllowedChangedLocked(true);
+                            onProviderAllowedChangedLocked();
                         }
                     }
                 }, UserHandle.USER_ALL);
@@ -436,6 +436,10 @@ public class LocationManagerService extends ILocationManager.Stub {
 
     @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 @@ public class LocationManagerService extends ILocationManager.Stub {
     }
 
     @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 @@ public class LocationManagerService extends ILocationManager.Stub {
             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 @@ public class LocationManagerService extends ILocationManager.Stub {
 
         // 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 class LocationManagerService extends ILocationManager.Stub {
         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 @@ public class LocationManagerService extends ILocationManager.Stub {
                         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 @@ public class LocationManagerService extends ILocationManager.Stub {
 
         @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 @@ public class LocationManagerService extends ILocationManager.Stub {
         }
 
         @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 @@ public class LocationManagerService extends ILocationManager.Stub {
             }
 
             updateProviderUseableLocked(this);
+
+            mContext.sendBroadcastAsUser(
+                    new Intent(LocationManager.PROVIDERS_CHANGED_ACTION),
+                    UserHandle.ALL);
         }
 
         @GuardedBy("mLock")
@@ -1720,7 +1740,7 @@ public class LocationManagerService extends ILocationManager.Stub {
         mProviders.add(provider);
 
         provider.onAllowedChangedLocked();  // allowed state may change while provider was inactive
-        provider.onUseableChangedLocked();
+        provider.onUseableChangedLocked(false);
     }
 
     @GuardedBy("mLock")
@@ -1728,7 +1748,7 @@ public class LocationManagerService extends ILocationManager.Stub {
         if (mProviders.remove(provider)) {
             long identity = Binder.clearCallingIdentity();
             try {
-                provider.onUseableChangedLocked();
+                provider.onUseableChangedLocked(false);
             } finally {
                 Binder.restoreCallingIdentity(identity);
             }
@@ -2113,7 +2133,6 @@ public class LocationManagerService extends ILocationManager.Stub {
             }
         }
 
-        if (D) Log.d(TAG, "provider request: " + provider + " " + providerRequest);
         provider.setRequestLocked(providerRequest, worksource);
     }
 
@@ -2507,7 +2526,6 @@ public class LocationManagerService extends ILocationManager.Stub {
 
     @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();