Merge "Align GnssVisibilityControl location on/off with GnssLocationProvider" into qt-dev

services/core/java/com/android/server/location/GnssLocationProvider.java

@@ -935,6 +935,9 @@ public class GnssLocationProvider extends AbstractLocationProvider implements
             mGnssMeasurementsProvider.onGpsEnabledChanged();
             mGnssNavigationMessageProvider.onGpsEnabledChanged();
             mGnssBatchingProvider.enable();
+            if (mGnssVisibilityControl != null) {
+                mGnssVisibilityControl.onGpsEnabledChanged(mEnabled);
+            }
         } else {
             mEnabled = false;
             Log.w(TAG, "Failed to enable location provider");
@@ -951,6 +954,9 @@ public class GnssLocationProvider extends AbstractLocationProvider implements
         mAlarmManager.cancel(mWakeupIntent);
         mAlarmManager.cancel(mTimeoutIntent);
 
+        if (mGnssVisibilityControl != null) {
+            mGnssVisibilityControl.onGpsEnabledChanged(mEnabled);
+        }
         mGnssBatchingProvider.disable();
         // do this before releasing wakelock
         native_cleanup();

services/core/java/com/android/server/location/GnssVisibilityControl.java

@@ -23,13 +23,10 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.PackageManager;
-import android.database.ContentObserver;
-import android.location.LocationManager;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.PowerManager;
 import android.os.UserHandle;
-import android.provider.Settings;
 import android.text.TextUtils;
 import android.util.ArrayMap;
 import android.util.Log;
@@ -49,18 +46,14 @@ class GnssVisibilityControl {
     private static final String TAG = "GnssVisibilityControl";
     private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
 
-    // Constants related to non-framework (NFW) location access permission proxy apps.
-    private static final String NFW_PROXY_APP_PKG_ACTIVITY_NAME_SUFFIX =
-            ".NonFrameworkLocationAccessActivity";
-    private static final String NFW_INTENT_ACTION_NFW_LOCATION_ACCESS_SUFFIX =
-            ".intent.action.NON_FRAMEWORK_LOCATION_ACCESS";
-    private static final String NFW_INTENT_TYPE = "text/plain";
-
     private static final String LOCATION_PERMISSION_NAME =
             "android.permission.ACCESS_FINE_LOCATION";
 
     private static final String[] NO_LOCATION_ENABLED_PROXY_APPS = new String[0];
 
+    // Max wait time for synchronous method onGpsEnabledChanged() to run.
+    private static final long ON_GPS_ENABLED_CHANGED_TIMEOUT_MILLIS = 3 * 1000;
+
     // Wakelocks
     private static final String WAKELOCK_KEY = TAG;
     private static final long WAKELOCK_TIMEOUT_MILLIS = 60 * 1000;
@@ -72,7 +65,7 @@ class GnssVisibilityControl {
     private final Handler mHandler;
     private final Context mContext;
 
-    private boolean mIsDeviceLocationSettingsEnabled;
+    private boolean mIsGpsEnabled;
 
     // Number of non-framework location access proxy apps is expected to be small (< 5).
     private static final int ARRAY_MAP_INITIAL_CAPACITY_PROXY_APP_TO_LOCATION_PERMISSIONS = 7;
@@ -95,6 +88,30 @@ class GnssVisibilityControl {
         runOnHandler(this::handleInitialize);
     }
 
+    void onGpsEnabledChanged(boolean isEnabled) {
+        // The GnssLocationProvider's methods: handleEnable() calls this method after native_init()
+        // and handleDisable() calls this method before native_cleanup(). This method must be
+        // executed synchronously so that the NFW location access permissions are disabled in
+        // the HAL before native_cleanup() method is called.
+        //
+        // NOTE: Since improper use of runWithScissors() method can result in deadlocks, the method
+        // doc recommends limiting its use to cases where some initialization steps need to be
+        // executed in sequence before continuing which fits this scenario.
+        if (mHandler.runWithScissors(() -> handleGpsEnabledChanged(isEnabled),
+                ON_GPS_ENABLED_CHANGED_TIMEOUT_MILLIS)) {
+            return;
+        }
+
+        // After timeout, the method remains posted in the queue and hence future enable/disable
+        // calls to this method will all get executed in the correct sequence. But this timeout
+        // situation should not even arise because runWithScissors() will run in the caller's
+        // thread without blocking as it is the same thread as mHandler's thread.
+        if (!isEnabled) {
+            Log.w(TAG, "Native call to disable non-framework location access in GNSS HAL may"
+                    + " get executed after native_cleanup().");
+        }
+    }
+
     void updateProxyApps(List<String> nfwLocationAccessProxyApps) {
         runOnHandler(() -> handleUpdateProxyApps(nfwLocationAccessProxyApps));
     }
@@ -110,12 +127,6 @@ class GnssVisibilityControl {
     private void handleInitialize() {
         disableNfwLocationAccess(); // Disable until config properties are loaded.
         listenForProxyAppsPackageUpdates();
-        listenForDeviceLocationSettingsUpdate();
-        mIsDeviceLocationSettingsEnabled = getDeviceLocationSettings();
-    }
-
-    private boolean getDeviceLocationSettings() {
-        return mContext.getSystemService(LocationManager.class).isLocationEnabled();
     }
 
     private void listenForProxyAppsPackageUpdates() {
@@ -145,18 +156,6 @@ class GnssVisibilityControl {
         }, UserHandle.ALL, intentFilter, null, mHandler);
     }
 
-    private void listenForDeviceLocationSettingsUpdate() {
-        mContext.getContentResolver().registerContentObserver(
-                Settings.Secure.getUriFor(Settings.Secure.LOCATION_MODE),
-                true,
-                new ContentObserver(mHandler) {
-                    @Override
-                    public void onChange(boolean selfChange) {
-                        handleDeviceLocationSettingsUpdated();
-                    }
-                }, UserHandle.USER_ALL);
-    }
-
     private void handleProxyAppPackageUpdate(String pkgName, String action) {
         final Boolean locationPermission = mProxyAppToLocationPermissions.get(pkgName);
         if (locationPermission == null) {
@@ -213,22 +212,21 @@ class GnssVisibilityControl {
         return false;
     }
 
-    private void handleDeviceLocationSettingsUpdated() {
+    private void handleGpsEnabledChanged(boolean isEnabled) {
-        final boolean enabled = getDeviceLocationSettings();
+        if (DEBUG) Log.d(TAG, "handleGpsEnabledChanged, isEnabled: " + isEnabled);
-        Log.i(TAG, "Device location settings enabled: " + enabled);
 
-        if (mIsDeviceLocationSettingsEnabled == enabled) {
+        if (mIsGpsEnabled == isEnabled) {
             return;
         }
 
-        mIsDeviceLocationSettingsEnabled = enabled;
+        mIsGpsEnabled = isEnabled;
-        if (!mIsDeviceLocationSettingsEnabled) {
+        if (!mIsGpsEnabled) {
             disableNfwLocationAccess();
             return;
         }
 
-        // When device location settings was disabled, we already set the proxy app list
+        // When GNSS was disabled, we already set the proxy app list to empty in GNSS HAL.
-        // to empty in GNSS HAL. Update only if the proxy app list is not empty.
+        // Update only if the proxy app list is not empty.
         String[] locationPermissionEnabledProxyApps = getLocationPermissionEnabledProxyApps();
         if (locationPermissionEnabledProxyApps.length != 0) {
             setNfwLocationAccessProxyAppsInGnssHal(locationPermissionEnabledProxyApps);
@@ -335,7 +333,7 @@ class GnssVisibilityControl {
     }
 
     private void updateNfwLocationAccessProxyAppsInGnssHal() {
-        if (!mIsDeviceLocationSettingsEnabled) {
+        if (!mIsGpsEnabled) {
             return; // Keep non-framework location access disabled.
         }
         setNfwLocationAccessProxyAppsInGnssHal(getLocationPermissionEnabledProxyApps());