From 3e94966d4fd597bbf33273e99a421a91585d6e96 Mon Sep 17 00:00:00 2001 From: "K. Shankari" Date: Tue, 12 Mar 2019 11:30:27 -0700 Subject: [PATCH] Handle permission checks and prompts + modify location service prompts for greater consistency --- src/android/DataCollectionPlugin.java | 79 ++++++++++++++++--- .../TripDiaryStateMachineService.java | 67 ++++++++++++---- .../TripDiaryStateMachineServiceOngoing.java | 3 +- 3 files changed, 124 insertions(+), 25 deletions(-) diff --git a/src/android/DataCollectionPlugin.java b/src/android/DataCollectionPlugin.java index a10ce8a1..c16d056c 100644 --- a/src/android/DataCollectionPlugin.java +++ b/src/android/DataCollectionPlugin.java @@ -7,6 +7,7 @@ import org.json.JSONException; import org.json.JSONObject; +import android.Manifest; import android.app.Activity; import android.app.NotificationManager; import android.app.PendingIntent; @@ -14,6 +15,7 @@ import android.content.Intent; import android.content.IntentSender; import android.content.SharedPreferences; +import android.content.pm.PackageManager; import android.preference.PreferenceManager; import com.google.android.gms.common.ConnectionResult; @@ -37,7 +39,12 @@ public class DataCollectionPlugin extends CordovaPlugin { public static final String TAG = "DataCollectionPlugin"; - public static final int ENABLE_LOCATION_CODE = 362253; + public static String LOCATION_PERMISSION = Manifest.permission.ACCESS_FINE_LOCATION; + + public static final int ENABLE_LOCATION_SETTINGS = 362253738; + public static final int ENABLE_LOCATION_PERMISSION = 362253737; + + public static final String ENABLE_LOCATION_PERMISSION_ACTION = "ENABLE_LOCATION_PERMISSION"; @Override public void pluginInitialize() { @@ -64,7 +71,8 @@ public boolean execute(String action, JSONArray data, final CallbackContext call // Now, really initialize the state machine // Note that we don't call initOnUpgrade so that we can handle the case where the // user deleted the consent and re-consented, but didn't upgrade the app - ctxt.sendBroadcast(new ExplicitIntent(ctxt, R.string.transition_initialize)); + checkAndPromptPermissions(); + // ctxt.sendBroadcast(new ExplicitIntent(ctxt, R.string.transition_initialize)); // TripDiaryStateMachineReceiver.restartCollection(ctxt); callbackContext.success(); return true; @@ -132,28 +140,79 @@ private static Map getTransitionMap(Context ctxt) { return retVal; } - @Override - public void onNewIntent(Intent intent) { - Log.d(cordova.getActivity(), TAG, "onNewIntent(" + intent.getDataString() + ")"); - Log.d(cordova.getActivity(), TAG, "Found extras " + intent.getExtras()); - PendingIntent piFromIntent = intent.getParcelableExtra(NotificationHelper.RESOLUTION_PENDING_INTENT_KEY); - if (piFromIntent != null) { + private void checkAndPromptPermissions() { + if(cordova.hasPermission(LOCATION_PERMISSION)) { + TripDiaryStateMachineService.restartFSMIfStartState(cordova.getActivity()); + } else { + cordova.requestPermission(this, ENABLE_LOCATION_PERMISSION, LOCATION_PERMISSION); + } + } + + private void displayResolution(PendingIntent resolution) { + if (resolution != null) { try { // cordova.setActivityResultCallback(this); - cordova.getActivity().startIntentSenderForResult(piFromIntent.getIntentSender(), ENABLE_LOCATION_CODE, null, 0, 0, 0, null); + cordova.getActivity().startIntentSenderForResult(resolution.getIntentSender(), ENABLE_LOCATION_SETTINGS, null, 0, 0, 0, null); } catch (IntentSender.SendIntentException e) { NotificationHelper.createNotification(cordova.getActivity(), Constants.TRACKING_ERROR_ID, "Unable to resolve issue"); } } } + @Override + public void onNewIntent(Intent intent) { + Context mAct = cordova.getActivity(); + Log.d(mAct, TAG, "onNewIntent(" + intent.getAction() + ")"); + Log.d(mAct, TAG, "Found extras " + intent.getExtras()); + + if(ENABLE_LOCATION_PERMISSION_ACTION.equals(intent.getAction())) { + checkAndPromptPermissions(); + return; + } + if (NotificationHelper.DISPLAY_RESOLUTION_ACTION.equals(intent.getAction())) { + PendingIntent piFromIntent = intent.getParcelableExtra( + NotificationHelper.RESOLUTION_PENDING_INTENT_KEY); + displayResolution(piFromIntent); + return; + } + Log.i(mAct, TAG, "Action "+intent.getAction()+" unknown, ignoring "); + } + + @Override + public void onRequestPermissionResult(int requestCode, String[] permissions, + int[] grantResults) throws JSONException + { + /* + Let us figure out if we want to sent a javascript callback with the error. + This is currently only called from markConsented, and I don't think we listen to failures there + for(int r:grantResults) + { + if(r == PackageManager.PERMISSION_DENIED) + { + this.callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.ERROR, PERMISSION_DENIED_ERROR)); + return; + } + } + */ + switch(requestCode) + { + case ENABLE_LOCATION_PERMISSION: + if (grantResults[0] == PackageManager.PERMISSION_GRANTED) { + TripDiaryStateMachineService.restartFSMIfStartState(cordova.getActivity()); + } + break; + default: + Log.e(cordova.getActivity(), TAG, "Unknown permission code "+requestCode+" ignoring"); + } + } + /* @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { Log.d(cordova.getActivity(), TAG, "received onActivityResult("+requestCode+","+ resultCode+","+data.getDataString()+")"); switch (requestCode) { - case ENABLE_LOCATION_CODE: + case ENABLE_LOCATION_SETTINGS: Activity mAct = cordova.getActivity(); Log.d(mAct, TAG, requestCode + " is our code, handling callback"); cordova.setActivityResultCallback(null); diff --git a/src/android/location/TripDiaryStateMachineService.java b/src/android/location/TripDiaryStateMachineService.java index b2166f42..4149fc8e 100644 --- a/src/android/location/TripDiaryStateMachineService.java +++ b/src/android/location/TripDiaryStateMachineService.java @@ -1,13 +1,17 @@ package edu.berkeley.eecs.emission.cordova.tracker.location; +import android.Manifest; +import android.app.PendingIntent; import android.app.Service; import android.content.Context; import android.content.Intent; import android.content.SharedPreferences; +import android.content.pm.PackageManager; import android.location.LocationManager; import android.os.Bundle; import android.os.IBinder; import android.preference.PreferenceManager; +import android.support.v4.content.ContextCompat; import com.google.android.gms.common.ConnectionResult; import com.google.android.gms.common.api.Batch; @@ -29,10 +33,13 @@ import edu.berkeley.eecs.emission.cordova.tracker.ConfigManager; import edu.berkeley.eecs.emission.cordova.tracker.Constants; +import edu.berkeley.eecs.emission.cordova.tracker.DataCollectionPlugin; import edu.berkeley.eecs.emission.cordova.tracker.ExplicitIntent; import edu.berkeley.eecs.emission.cordova.tracker.sensors.BatteryUtils; import edu.berkeley.eecs.emission.cordova.unifiedlogger.NotificationHelper; import edu.berkeley.eecs.emission.R; +import edu.berkeley.eecs.emission.MainActivity; + import edu.berkeley.eecs.emission.cordova.tracker.location.actions.ActivityRecognitionActions; @@ -273,7 +280,7 @@ private void handleAction(Context ctxt, GoogleApiClient apiClient, String currSt handleStart(ctxt, apiClient, actionString); } else if (LocationManager.MODE_CHANGED_ACTION.equals(actionString)) { // should we do a handleXXX() wrapper for this too? - checkLocationSettings(ctxt, apiClient); + checkLocationSettingsAndPermissions(ctxt, apiClient); // stay in the current state, but do all the service cleanup stuff setNewState(currState); } else if (currState.equals(ctxt.getString(R.string.state_start))) { @@ -345,13 +352,13 @@ public void onResult(BatchResult batchResult) { } } else { if (batchResult.getStatus().hasResolution()) { - NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, + NotificationHelper.createResolveNotification(fCtxt, STATE_IN_NUMBERS, "Error " + batchResult.getStatus().getStatusCode()+" while creating geofence", batchResult.getStatus().getResolution()); } else { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, "Error " + batchResult.getStatus().getStatusCode()+" while creating geofence"); - checkLocationSettings(TripDiaryStateMachineService.this, mApiClient); + checkLocationSettingsAndPermissions(TripDiaryStateMachineService.this, mApiClient); } if (ConfigManager.getConfig(fCtxt).isSimulateUserInteraction()) { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, @@ -422,13 +429,13 @@ public void onResult(BatchResult batchResult) { } } else { if (batchResult.getStatus().hasResolution()) { - NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, + NotificationHelper.createResolveNotification(fCtxt, STATE_IN_NUMBERS, "Error " + batchResult.getStatus().getStatusCode()+" while creating geofence", batchResult.getStatus().getResolution()); } else { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, "Error " + batchResult.getStatus().getStatusCode()+" while creating geofence"); - checkLocationSettings(TripDiaryStateMachineService.this, mApiClient); + checkLocationSettingsAndPermissions(TripDiaryStateMachineService.this, mApiClient); } if (ConfigManager.getConfig(ctxt).isSimulateUserInteraction()) { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, @@ -515,12 +522,12 @@ public void onResult(Status status) { } else { setNewState(mCurrState); if (status.hasResolution()) { - NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, + NotificationHelper.createResolveNotification(fCtxt, STATE_IN_NUMBERS, "Error " + status.getStatusCode()+" while creating geofence", status.getResolution()); } else { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, "Error " + status.getStatusCode()+" while creating geofence"); - checkLocationSettings(TripDiaryStateMachineService.this, mApiClient); + checkLocationSettingsAndPermissions(TripDiaryStateMachineService.this, mApiClient); } if (ConfigManager.getConfig(fCtxt).isSimulateUserInteraction()) { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, @@ -533,7 +540,7 @@ public void onResult(Status status) { }); } else { // Geofence was not created properly. let's generate a tracking error so that the user is notified - checkLocationSettings(fCtxt, fApiClient); + checkLocationSettingsAndPermissions(fCtxt, fApiClient); setNewState(mCurrState); } } @@ -557,13 +564,13 @@ public void onResult(BatchResult batchResult) { } } else { if (batchResult.getStatus().hasResolution()) { - NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, + NotificationHelper.createResolveNotification(fCtxt, STATE_IN_NUMBERS, "Error " + batchResult.getStatus().getStatusCode() + " while creating geofence", batchResult.getStatus().getResolution()); } else { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, "Error " + batchResult.getStatus().getStatusCode() + " while creating geofence"); - checkLocationSettings(TripDiaryStateMachineService.this, mApiClient); + checkLocationSettingsAndPermissions(TripDiaryStateMachineService.this, mApiClient); } if (ConfigManager.getConfig(fCtxt).isSimulateUserInteraction()) { NotificationHelper.createNotification(fCtxt, STATE_IN_NUMBERS, @@ -611,10 +618,42 @@ public void onResult(BatchResult batchResult) { }); } - - public static void checkLocationSettings(final Context ctxt, GoogleApiClient apiClient) { + public static void checkLocationSettingsAndPermissions(final Context ctxt, final GoogleApiClient apiClient) { LocationRequest request = new LocationTrackingActions(ctxt, apiClient).getLocationRequest(); - Log.d(ctxt, TAG, "Checking location settings for request "+request); + Log.d(ctxt, TAG, "Checking location settings and permissions for request "+request); + // let's do the permission check first since it is synchronous + if (checkLocationPermissions(ctxt, apiClient, request)) { + Log.d(ctxt, TAG, "checkPermissions returned true, checking settings"); + checkLocationSettings(ctxt, apiClient, request); + } else { + Log.d(ctxt, TAG, "checkPermissions returned false, no point checking settings"); + } + } + + public static boolean checkLocationPermissions(final Context ctxt, + final GoogleApiClient apiClient, + final LocationRequest request) { + // Ideally, we would use the request accuracy to figure out the permissions requested + // but I can't find an authoritative mapping, and I'm running out of time for + // fancy stuff + int result = ContextCompat.checkSelfPermission(ctxt, DataCollectionPlugin.LOCATION_PERMISSION); + Log.d(ctxt, TAG, "checkSelfPermission returned "+result); + if (PackageManager.PERMISSION_GRANTED == result) { + return true; + } else { + Intent activityIntent = new Intent(ctxt, MainActivity.class); + activityIntent.setAction(DataCollectionPlugin.ENABLE_LOCATION_PERMISSION_ACTION); + PendingIntent pi = PendingIntent.getActivity(ctxt, DataCollectionPlugin.ENABLE_LOCATION_PERMISSION, + activityIntent, PendingIntent.FLAG_UPDATE_CURRENT); + NotificationHelper.createNotification(ctxt, DataCollectionPlugin.ENABLE_LOCATION_PERMISSION, + "Location permission off, click to enable", pi); + return false; + } + } + + public static void checkLocationSettings(final Context ctxt, + final GoogleApiClient apiClient, + final LocationRequest request) { LocationSettingsRequest.Builder builder = new LocationSettingsRequest.Builder() .addLocationRequest(request); @@ -639,7 +678,7 @@ public void onResult(LocationSettingsResult result) { // a dialog. ctxt.sendBroadcast(new ExplicitIntent(ctxt, R.string.transition_tracking_error)); if (status.hasResolution()) { - NotificationHelper.createNotification(ctxt, Constants.TRACKING_ERROR_ID, + NotificationHelper.createResolveNotification(ctxt, Constants.TRACKING_ERROR_ID, "Error " + status.getStatusCode() + " in location settings", status.getResolution()); } else { diff --git a/src/android/location/TripDiaryStateMachineServiceOngoing.java b/src/android/location/TripDiaryStateMachineServiceOngoing.java index 97f6c2f7..289965b2 100644 --- a/src/android/location/TripDiaryStateMachineServiceOngoing.java +++ b/src/android/location/TripDiaryStateMachineServiceOngoing.java @@ -27,6 +27,7 @@ import edu.berkeley.eecs.emission.cordova.unifiedlogger.NotificationHelper; import edu.berkeley.eecs.emission.R; + import edu.berkeley.eecs.emission.cordova.tracker.location.actions.ActivityRecognitionActions; import edu.berkeley.eecs.emission.cordova.tracker.location.actions.LocationTrackingActions; import edu.berkeley.eecs.emission.cordova.unifiedlogger.Log; @@ -238,7 +239,7 @@ private void handleAction(Context ctxt, GoogleApiClient apiClient, String currSt handleStart(ctxt, apiClient, actionString); } else if (LocationManager.MODE_CHANGED_ACTION.equals(actionString)) { // should we do a handleXXX() wrapper for this too? - TripDiaryStateMachineService.checkLocationSettings(ctxt, apiClient); + TripDiaryStateMachineService.checkLocationSettingsAndPermissions(ctxt, apiClient); } else if (currState.equals(ctxt.getString(R.string.state_start))) { handleStart(ctxt, apiClient, actionString); } else if (currState.equals(ctxt.getString(R.string.state_waiting_for_trip_start))) {