search

e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

⬆️ Upgrade the native code the latest versions #1079

Closed shankari closed 2 months ago

shankari commented 3 months ago

It is that time of the year again, and we need to upgrade the native code to the most recent version of the android API by the end of August. We do have some time to do this, but I would ideally like ~ 1 month of testing before it goes to production, so we also don't have that much time. I was hoping that @louisg1337 would be able to tackle it this time, but I am not sure if he has enough bandwidth given the changes needed for the carbon footprint improvements. So I might just have to step up and do it one more time.

shankari commented 3 months ago

Here's the list of potential android API changes: https://developer.android.com/about/versions/14/summary

The ones that are potentially relevant to us are:

  1. Schedule exact alarms are denied by default The SCHEDULE_EXACT_ALARM permission is no longer being pre-granted to most newly-installed apps targeting Android 13 and higher—the permission is denied by default.
    • we already get an alert about this in the play console. I assume that we use alarms in the local notifications, and we don't actually need those to be shown at a particular time
  2. Context-registered broadcasts are queued while apps are cached The system may place context-registered broadcasts in a queue when these broadcasts are queued for delivery to an app that's in the cached state.
    • I am fairly sure that we use context registered broadcasts to send the geofence exit, for example. Need to investigate what the cached state is and whether our app ever gets into it
  3. New reason an app can be placed in the restricted standby bucket Android 14 introduces a new reason an app can be placed into the restricted standby bucket.
    • The reason is if "The app's jobs trigger ANR errors multiple times due to onStartJob, onStopJob, or onBind method timeouts." I don't think this should happen to use, but we may want to test this functionality anyway, and check to see if how the app behaves if it is restricted.
  4. Foreground service types are required If your app targets Android 14 (API level 34) or higher, it must specify at least one foreground service type for each foreground service within your app.
    • This is partially done; we just need to add a new permission to the manifest
  5. OpenJDK 17 updates As part of the OpenJDK 17 updates, there are some changes that can affect app compatibility, such as changes to regular expressions and UUID handling.
    • Upgrade to JDK 17 and deal with compile time issues
  6. Regional preferences Apps can receive notifications when a user changes their regional preferences and mirror these preferences in app.
    • would be an enhancement, not a requirement
  7. Restrictions to implicit and pending intents For apps targeting Android 14 (API level 34) or higher, Android restricts apps from sending implicit intents to internal app components.
    • I don't think we use implicit intents, unless it is for sharing/exporting data
  8. Runtime-registered broadcasts receivers must specify export behavior Apps and services that target Android 14 (API level 34) or higher and use context-registered receivers are required to specify a flag to indicate whether or not the receiver should be exported to all other apps on the device.
    • I am pretty sure we use runtime registered receivers
  9. Additional restrictions on starting activities from the background Apps that target Android 14 (API level 34) or higher must opt in if they want to grant their background activity launch privileges to another app either when sending that app's PendingIntent, or binding that app's service.
    • We don't want to grant privileges to other apps
  10. Changes to how users experience non-dismissible notifications If your app shows non-dismissable foreground notifications to users, Android 14 has changed the behavior to allow users to dismiss such notifications.
    • not sure what, if anything, we want to do about this or can do about it
  11. Data safety information is more visible Your app's data safety information, such as data-sharing practices, now appears in some permission rationale system dialogs and in system notifications.
    • we are open-source so we don't care
shankari commented 2 months ago

The SCHEDULE_EXACT_ALARM permission: This is indeed from local notifications

$ grep -r SCHEDULE_EXACT_ALARM plugins/
plugins//cordova-plugin-local-notification-12/plugin.xml:            <uses-permission android:name="android.permission.SCHEDULE_EXACT_ALARM" />

Fortunately, we use our own plugin, so we can change it however we want. Here's where the alarms are scheduled, and they are in fact scheduled using setExact and setAlarmClock. Let's change setExact to set, and setAlarmClock (for the high priority case) to setAndAllowWhileIdle consistent with https://developer.android.com/develop/background-work/services/alarms/schedule#inexact-after-specific-time. set will not show the alarm in doze mode, setAndAllowWhenIdle will

Like set, but this alarm will be allowed to execute even when the system is in low-power idle (a.k.a. doze) modes")

but with some limits

To reduce abuse, there are restrictions on how frequently these alarms will go off for a particular application. Under normal system operation, it will not dispatch these alarms more than about every minute (at which point every such pending alarm is dispatched); when in low-power idle modes this duration may be significantly longer, such as 15 minutes

that we easily meet since we only generate the alarm once a day.

shankari commented 2 months ago

Context-registered broadcasts are queued while apps are cached

Do we have context-registered broadcasts? Context-registered broadcasts are those which are not in the manifest. > Apps can receive broadcasts in two ways: through manifest-declared receivers and context-registered receivers. These are the receivers that we register for. We can ignore the bluetooth classic ones, since we only use that in the foreground for testing. ``` $ grep -r registerReceiver plugins/ plugins//cordova-plugin-em-datacollection/src/android/sensors/BatteryUtils.java: Intent batteryIntent = applicationContext.registerReceiver(null, plugins//cordova-plugin-em-datacollection/src/android/location/actions/GeofenceActions.java: LocalBroadcastManager.getInstance(mCtxt).registerReceiver(new BroadcastReceiver() { plugins//cordova-plugin-em-datacollection/src/android/DataCollectionPlugin.java: ctxt.registerReceiver(tripDiaryReceiver, filter); plugins//cordova-plugin-bluetooth-classic-serial-port/src/android/nz/co/soltius/cordova/BluetoothClassicSerial.java: cordova.getActivity().unregisterReceiver(this); plugins//cordova-plugin-bluetooth-classic-serial-port/src/android/nz/co/soltius/cordova/BluetoothClassicSerial.java: activity.registerReceiver(discoverReceiver, new IntentFilter(BluetoothDevice.ACTION_FOUND)); plugins//cordova-plugin-bluetooth-classic-serial-port/src/android/nz/co/soltius/cordova/BluetoothClassicSerial.java: activity.registerReceiver(discoverReceiver, new IntentFilter(BluetoothAdapter.ACTION_DISCOVERY_FINISHED)); plugins//com.unarin.cordova.beacon/src/android/LocationManager.java: cordova.getActivity().unregisterReceiver(broadcastReceiver); plugins//com.unarin.cordova.beacon/src/android/LocationManager.java: cordova.getActivity().registerReceiver(broadcastReceiver, filter); ``` For the others, let's see which messages, they register for, and whether they are defined in the manifest 1. `Intent batteryIntent = applicationContext.registerReceiver(null`: in the manifest `` 4. `ctxt.registerReceiver(tripDiaryReceiver, filter);`: in the manifest `` 5. `LocalBroadcastManager.getInstance(mCtxt).registerReceiver(new BroadcastReceiver() {`: not in the manifest, inner class created using `new BroadcastReceiver()` 6. `cordova.getActivity().registerReceiver(broadcastReceiver, filter);`: not in the manifest, inner class created using `new BroadcastReceiver() {` Searching again for the inner class, I can confirm that there are only these three matches ``` $ grep -r "new BroadcastReceiver" plugins/ plugins//cordova-plugin-em-datacollection/src/android/location/actions/GeofenceActions.java: LocalBroadcastManager.getInstance(mCtxt).registerReceiver(new BroadcastReceiver() { plugins//cordova-plugin-bluetooth-classic-serial-port/src/android/nz/co/soltius/cordova/BluetoothClassicSerial.java: final BroadcastReceiver discoverReceiver = new BroadcastReceiver() { plugins//com.unarin.cordova.beacon/src/android/LocationManager.java: broadcastReceiver = new BroadcastReceiver() { ``` Note that there are also other receivers defined in the manifest that don't seem to call `registerReceiver`. Let's spot check a few of those... 1. `` and similar from the local notification plugin. a. `TriggerReceiver` listens to alarms, which is a different call b. `ClickReceiver` uses a `NotificationTrampolineActivity` not a `BroadcastReceiver` now, consistent with https://developer.android.com/about/versions/12/behavior-changes-12#notification-trampolines - We need to double check whether the local notifications work. I see https://github.com/katzer/cordova-plugin-local-notifications/issues/1990 but it is not clear whether `NotificationTrampolineActivity` fixes it or not (seems like it should, but the issue is still open! c. `ClearReceiver` is a broadcast receiver but is registered as a deleteIntent in the builder. So to make sure we capture all the broadcast locations, we also need to find the `PendingIntent.getBroadcast` calls. These are ``` $ grep -r PendingIntent.getBroadcast plugins/ plugins//@havesource/cordova-plugin-push/src/android/com/adobe/phonegap/push/FCMService.kt: val deleteIntent = PendingIntent.getBroadcast( plugins//@havesource/cordova-plugin-push/src/android/com/adobe/phonegap/push/FCMService.kt: PendingIntent.getBroadcast( plugins//@havesource/cordova-plugin-push/src/android/com/adobe/phonegap/push/FCMService.kt: pIntent = PendingIntent.getBroadcast( plugins//cordova-plugin-local-notification-12/src/android/notification/util/LaunchUtils.java: public static PendingIntent getBroadcastPendingIntent(Context context, Intent intent) { plugins//cordova-plugin-local-notification-12/src/android/notification/util/LaunchUtils.java: return PendingIntent.getBroadcast(context, getRandomCode(), intent, getIntentFlags()); plugins//cordova-plugin-x-socialsharing/src/android/nl/xservices/plugins/SocialSharing.java: final PendingIntent pendingIntent = PendingIntent.getBroadcast(cordova.getActivity().getApplicationContext(), 0, receiverIntent, PendingIntent.FLAG_UPDATE_CURRENT|PendingIntent.FLAG_IMMUTABLE); ``` and ``` $ grep -r getBroadcastPendingIntent plugins/ plugins//cordova-plugin-local-notification-12/src/android/notification/Builder.java: PendingIntent deleteIntent = LaunchUtils.getBroadcastPendingIntent(context, intent); plugins//cordova-plugin-local-notification-12/src/android/notification/util/LaunchUtils.java: public static PendingIntent getBroadcastPendingIntent(Context context, Intent intent) { plugins//cordova-plugin-local-notification-12/src/android/notification/Notification.java: PendingIntent pi = LaunchUtils.getBroadcastPendingIntent(context, intent); plugins//cordova-plugin-local-notification-12/src/android/notification/Notification.java: PendingIntent pi = LaunchUtils.getBroadcastPendingIntent(context, intent); ``` These cover: - com.adobe.phonegap.push.BackgroundActionButtonHandler" - com.adobe.phonegap.push.PushDismissedHandler - nl.xservices.plugins.ShareChooserPendingIntent - de.appplant.cordova.plugin.localnotification.TriggerReceiver - de.appplant.cordova.plugin.localnotification.ClearReceiver - de.appplant.cordova.plugin.localnotification.RestoreReceiver - edu.berkeley.eecs.emission.cordova.tracker.BootReceiver - edu.berkeley.eecs.emission.cordova.tracker.location.TripDiaryStateMachineReceiver

So the only context-registered broadcasts are the two in plugins//cordova-plugin-em-datacollection/src/android/location/actions/GeofenceActions.java and plugins//com.unarin.cordova.beacon/src/android/LocationManager.java.

what is the cached state? > A cached process is one that is not currently needed, so the system is free to kill it as needed when resources like memory are needed elsewhere. In a normally behaving system, these are the only processes involved in resource management. contrasted with > A visible process is doing work that the user is currently aware of, so killing it has a noticeable negative impact on the user experience. A process is considered visible in the following conditions: > It has a Service that is running as a foreground service, through Service.startForeground() (which asks the system to treat the service as something the user is aware of, or essentially as if it were visible).

So unless the foreground service is killed, we will not have a cached process, so this won't apply to us.

What happens if we don't receive these broadcasts when the foreground service is running? 1. Geofence code is while finding the location to create the geofence. If the foreground service was not running, we wouldn't get fine-grained notifications anyway. ``` LocalBroadcastManager.getInstance(mCtxt).registerReceiver(new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { Log.i(mCtxt, TAG, "recieved broadcast intent "+intent); synchronized(GeofenceActions.this) { GeofenceActions.this.newLastLocation = intent.getParcelableExtra(GeofenceLocationIntentService.INTENT_RESULT_KEY); GeofenceActions.this.notify(); } } }, new IntentFilter(GeofenceLocationIntentService.INTENT_NAME)); ``` And we do restart the foreground service anyway eventually, and then this will be fixed. 2. Bluetooth code listens to changes to https://developer.android.com/reference/android/bluetooth/BluetoothAdapter#ACTION_STATE_CHANGED > Broadcast Action: The state of the local Bluetooth adapter has been changed. > For example, Bluetooth has been turned on or off. If that happens when the foreground service is not running, we won't generate proper errors. When the foreground service is re-generated, we need to make sure that we re-check the bluetooth permissions and prompt the user to re-enable.

The geofence is fine; we should just verify that, if the foreground service is killed and the bluetooth status changes in the interim, we re-check the status when the foreground service is recreated, and prompt the user to fix the permissions.

shankari commented 2 months ago

Foreground service types are required

✔️ If your app targets Android 14, it must specify appropriate foreground service types. https://developer.android.com/about/versions/14/changes/fgs-types-required 

        <service android:enabled="true" android:exported="false" android:foregroundServiceType="location" android:name="edu.berkeley.eecs.emission.cordova.tracker.location.TripDiaryStateMachineForegroundService" />

❌ If apps that target Android 14 use a foreground service, they must declare a specific permission, based on the foreground service type, that Android 14 introduces.

❌ ACTIVITY_RECOGNITION permission seems to imply health as part of the foreground service as well

❌ The best practice for applications starting foreground services is to use the ServiceCompat version of startForeground() (available in androidx-core 1.12 and higher) where you pass in a bitwise integer of foreground service types. You can choose to pass one or more type values.

public static void startForeground(
    @NonNull Service service,
    int id,
    @NonNullnotification,
    int foregroundServiceType
)

But the example is 

ServiceCompat.startForeground(0, notification, FOREGROUND_SERVICE_TYPE_LOCATION)

Poking around a bit...

Ah! We first call startForegroundService and then we call startForeground while starting the service. https://developer.android.com/develop/background-work/services/foreground-services#java

So we only need to change startForeground in handleStart.

shankari commented 2 months ago

While poking around at the documentation related to the foreground service changes in Android 14, (API 34) I also found https://proandroiddev.com/foreground-services-in-android-14-whats-changing-dcd56ad72788 which indicated that the

Starting in Android 14 (API level 34), all long-running workers must be foreground services.

But do we still need that if we have a foreground service for location anyway? I don't think so, given that we have a long-running (always on) foreground service

https://developer.android.com/reference/android/content/pm/ServiceInfo#FOREGROUND_SERVICE_TYPE_SHORT_SERVICE

The type has a 3 minute timeout. A foreground service of this type must be stopped within the timeout by Service.stopSelf(), Context.stopService(android.content.Intent) or their overloads). Service.stopForeground(int) will also work, which will demote the service to a "background" service, which will soon be stopped by the system.

If the service isn't stopped within the timeout, Service.onTimeout(int) will be called. Note, even when the system calls this callback, it will not stop the service automatically. You still need to stop the service using one of the aforementioned ways even when you get this callback.

If the service is still not stopped after the callback, the app will be declared an ANR, after a short grace period of several seconds.

Need to test!

shankari commented 2 months ago

Runtime-registered broadcasts receivers must specify export behavior

Building on the prior investigation into runtime registered broadcasts https://github.com/e-mission/e-mission-docs/issues/1079#issuecomment-2309021838 we need to focus on

Some system broadcasts come from highly privileged apps, such as Bluetooth and telephony, that are part of the Android framework but do not run under the system's unique process ID (UID). To receive all system broadcasts, including broadcasts from highly privileged apps, flag your receiver with RECEIVER_EXPORTED.

We are listening to bluetooth, but for the enabled/disabled flags. So it is not clear if that comes from permissions or bluetooth app.

Should test.

shankari commented 2 months ago

Restrictions to implicit and pending intents

Searching for all intents and seeing if they are implicit or explicit: - `plugins//cordova-plugin-file/src/android/LocalFilesystem.java`: `broadcastNewFile` is implicit; but broadcasts the intent externally - `plugins//cordova-plugin-em-datacollection/src/android/sensors/BatteryUtils.java`: `getBatteryInfo`; registering for intent - `plugins/cordova-plugin-em-datacollection/src/android/location/TripDiaryStateMachineService.java`: `handleStart` is commented out - `plugins/cordova-plugin-em-datacollection/src/android/location/TripDiaryStateMachineService.java`: `handleTripStart` is explicit - `plugins/cordova-plugin-em-datacollection/src/android/location/ForegroundServiceComm.java`: explicit - `plugins/cordova-plugin-em-datacollection/src/android/location/OPGeofenceExitActivityActions.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/LocationChangeIntentService.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/TripDiaryStateMachineForegroundService.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/TripDiaryStateMachineForegroundService.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/TripDiaryStateMachineForegroundService.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/TripDiaryStateMachineForegroundService.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/actions/GeofenceLocationIntentService.java`: **implicit** - `plugins//cordova-plugin-em-datacollection/src/android/location/actions/GeofenceActions.java`: **unsure** - `plugins//cordova-plugin-em-datacollection/src/android/location/actions/GeofenceActions.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/actions/LocationTrackingActions.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/actions/ActivityRecognitionActions.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/location/TripDiaryStateMachineReceiver.java`: explicit - `plugins//cordova-plugin-em-datacollection/src/android/verification/SensorControlForegroundDelegate.java`: **implicit, not internal** - `plugins//cordova-plugin-inappbrowser/src/android/InAppBrowser.java`: **implicit, not internal** - `plugins//cordova-plugin-em-unifiedlogger/src/android/NotificationHelper.java`: explicit - `plugins//com.unarin.cordova.beacon/src/android/LocationManager.java`: registering for intent - `plugins//phonegap-plugin-barcodescanner/src/android/com/phonegap/plugins/barcodescanner/BarcodeScanner.java`: explicit - `plugins//cordova-plugin-local-notification-12/src/android/LocalNotification.java`: explicit - `plugins//cordova-plugin-local-notification-12/src/android/LocalNotification.java`: implicit, launching external - `plugins//cordova-plugin-local-notification-12/src/android/LocalNotification.java`: unsure, but in `requestIgnoreBatteryOptimizations`, which we don't invoke - `plugins//cordova-plugin-local-notification-12/src/android/LocalNotification.java`: `cancelScheduledAlarms` seems to be implicit - `plugins//cordova-plugin-x-socialsharing/src/android/nl/xservices/plugins/SocialSharing.java`: **implicit, not internal** - `plugins//cordova-plugin-em-serversync/src/android/ServerSyncAdapter.java`: **implicit**

So the locations we need to fix are:

The locations we need to test, and potentially fix, are:

**Follow up 1:** Address MUTABLE local notifications While testing [Schedule Exact Alarms](https://github.com/e-mission/e-mission-docs/issues/1079#issuecomment-2308917271) found that the LocalNotification code is an issue, notably in `LaunchUtils`, where we use mutable flags in `getIntentFlags()`. ``` java.lang.RuntimeException: Unable to start activity ComponentInfo{edu.berkeley.eecs.emission/de.appplant.cordova.plugin.localnotification.ClickReceiver}: java.lang.IllegalArgumentException: edu.berkeley.eecs.emission: Targeting U+ (version 34 and above) disallows creating or retrieving a PendingIntent with FLAG_MUTABLE, an implicit Intent within and without FLAG_NO_CREATE and FLAG_ALLOW_UNSAFE_IMPLICIT_INTENT for security reasons. To retrieve an already existing PendingIntent, use FLAG_NO_CREATE, however, to create a new PendingIntent with an implicit Intent use FLAG_IMMUTABLE. ``` This was added in https://github.com/e-mission/cordova-plugin-local-notification-12/commit/0b6aefe0aac71cf44feb62a975516e13adfd010e#diff-893ce41eb8e5292aff4ea85879e33ed745caa82f2c97300f9bd7a90e3c67a277. It was also mutable when the file was copied over to the bhandaribhumin repo https://github.com/e-mission/cordova-plugin-local-notification-12/blame/e79584161eb555e18a0178e4fdcda43ddbd902d6/src/android/notification/Builder.java#L202-L208C10 with the `init` commit (https://github.com/e-mission/cordova-plugin-local-notification-12/commit/86b997a575f28128af5c217038817c946b8ed264). In the original katzer repo (https://github.com/katzer/cordova-plugin-local-notifications/blob/master/src/android/notification/Builder.java), it was IMMUTABLE. ``` PendingIntent deleteIntent; if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.S) { deleteIntent = PendingIntent.getBroadcast( context, reqCode, intent, PendingIntent.FLAG_IMMUTABLE | FLAG_UPDATE_CURRENT); } else { deleteIntent = PendingIntent.getBroadcast( context, reqCode, intent, FLAG_UPDATE_CURRENT); } ``` with the immutable flag added in https://github.com/katzer/cordova-plugin-local-notifications/commit/b7896cbb1686fd5a4deb672cae8c9dbd6a58c350 to avoid crashes on android 12. Let's try to change it back to the katzer version and see if it fixes the problem. It does!
**Follow up 2:** Server sync implicit intent doesn't seem to fail, but let's remove it anyway This intent to indicate that we have new data is clearly implicit ``` UserCache.TimeQuery tq = new UserCache.TimeQuery("write_ts", 0, System.currentTimeMillis()/1000); biuc.clearObsoleteDocs(tq); JSONArray entriesReceived = edu.berkeley.eecs.emission.cordova.serversync.CommunicationHelper.server_to_phone( cachedContext, userToken); biuc.sync_server_to_phone(entriesReceived); biuc.checkAfterPull(); biuc.putMessage(R.string.key_usercache_client_time, new StatsEvent(cachedContext, R.string.pull_duration, t.elapsedSecs())); Intent localIntent = new Intent("edu.berkeley.eecs.emission.sync.NEW_DATA"); Bundle b = new Bundle(); b.putString( "userdata", "{}" ); localIntent.putExtras(b); Log.i(cachedContext, TAG, "Finished sync, sending local broadcast"); LocalBroadcastManager.getInstance(cachedContext).sendBroadcastSync(localIntent); biuc.putMessage(R.string.key_usercache_client_time, new StatsEvent(cachedContext, R.string.sync_duration, to.elapsedSecs())); ``` However, it appears to be sent properly without crashing ``` 08-27 17:26:02.209 11959 12158 I ServerSyncAdapter: Push complete, now pulling 08-27 17:26:02.219 11959 12158 D BuiltinUserCache: Args = type = 'document' 08-27 17:26:02.227 11959 12158 I BuiltinUserCache: Deleted 0 document entries ... 08-27 17:26:02.240 11959 12158 I System.out: Posting data to https://openpath-stage.nrel.gov/api//usercache/get 08-27 17:26:02.643 11959 12158 I CommunicationHelper: Got response org.apache.http.message.BasicHttpResponse@b0f3f3e with status HTTP/1.1 200 OK ... 08-27 17:26:02.795 11959 12158 I System.out: Result Summary JSON = {"server_to_phone": [{"metadata": {"write_ts": 1687907246.5748034, "type": "document", "key": "diary/trips-2016-07-27"}, "data": [{"type": "FeatureCollection", "properties": {"source": "DwellSegmentat length 295649 08-27 17:26:02.802 11959 12158 I CommunicationHelper: Result Summary JSON = {"server_to_phone": [{"metadata": {"write_ts": 1687907246.5748034, "type": "document", "key": "diary/trips-2016-07-27"}, "data": [{"type": "FeatureCollection", "properties": {"source": "DwellSegmentat length 295649 ... 08-27 17:26:03.341 11959 12158 D BuiltinUserCache: received 1 items 08-27 17:26:03.829 11959 12158 D BuiltinUserCache: for key diary/trips-2016-07-27, storing data of length 279493 08-27 17:26:03.848 11959 12158 D BuiltinUserCache: result of insert = 32 ... 08-27 17:26:03.864 11959 12158 D BuiltinUserCache: Added value for key stats/client_time at time 1.724804763855E9 08-27 17:26:03.868 11959 12158 I ServerSyncAdapter: Finished sync, sending local broadcast 08-27 17:26:03.877 11959 12158 D BuiltinUserCache: Added value for key stats/client_time at time 1.72480476387E9 08-27 17:26:03.881 11959 12154 I System.out: About to execute query SELECT * FROM userCache WHERE key = 'statemachine/transition' AND type = 'message' AND write_ts >= 0.0 AND write_ts <= 1.724804763879E9 ORDER BY write_ts DESC ... 08-27 17:26:03.889 11959 11959 I chromium: [INFO:CONSOLE(49)] "DEBUG:sensorDataList.length = 3, 08-27 17:26:03.889 11959 11959 I chromium: syncLaunchedCalls.length = 0, 08-27 17:26:03.889 11959 11959 I chromium: syncPending? = false", source: https://localhost/plugins/cordova-plugin-em-unifiedlogger/www/unifiedlogger.js (49) ``` However, we don't currently use the `usercache/get` call or the `diary/trips-2016-07-27` call anywhere. ``` $ grep -r edu.berkeley.eecs.emission.sync.NEW_DATA . ./platforms/ios/emission/Plugins/cordova-plugin-em-serversync/BEMServerSync/BEMServerSyncCommunicationHelper.m: [[NSNotificationCenter defaultCenter] postNotificationName:@"edu.berkeley.eecs.emission.sync.NEW_DATA" Binary file ./platforms/ios/build/Debug-iphonesimulator/emission.app/emission matches Binary file ./platforms/android/app/build/intermediates/project_dex_archive/debug/dexBuilderDebug/out/edu/berkeley/eecs/emission/cordova/serversync/ServerSyncAdapter.dex matches Binary file ./platforms/android/app/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes/edu/berkeley/eecs/emission/cordova/serversync/ServerSyncAdapter.class matches Binary file ./platforms/android/app/build/intermediates/dex/debug/mergeProjectDexDebug/6/classes.dex matches ./platforms/android/app/src/main/java/edu/berkeley/eecs/emission/cordova/serversync/ServerSyncAdapter.java: Intent localIntent = new Intent("edu.berkeley.eecs.emission.sync.NEW_DATA"); ./plugins/cordova-plugin-em-serversync/src/ios/BEMServerSyncCommunicationHelper.m: [[NSNotificationCenter defaultCenter] postNotificationName:@"edu.berkeley.eecs.emission.sync.NEW_DATA" ./plugins/cordova-plugin-em-serversync/src/android/ServerSyncAdapter.java: Intent localIntent = new Intent("edu.berkeley.eecs.emission.sync.NEW_DATA"); ./node_modules/cordova-plugin-em-serversync/src/ios/BEMServerSyncCommunicationHelper.m: [[NSNotificationCenter defaultCenter] postNotificationName:@"edu.berkeley.eecs.emission.sync.NEW_DATA" ./node_modules/cordova-plugin-em-serversync/src/android/ServerSyncAdapter.java: Intent localIntent = new Intent("edu.berkeley.eecs.emission.sync.NEW_DATA"); ``` We can just remove this intent. Note, however, that we do use the `usercache/get` calls to determine when the app is installed even if the user is not submitting any data. So that seems like a server-side change to not return anything on the get. Let's comment out the intent for now, and make the server side change to convert to a NOP. This gives us: ``` 08-27 18:39:10.607 15925 16081 I SYNC : PERFORMING SYNC 08-27 18:39:10.851 15925 16081 I ServerSyncAdapter: Push complete, now pulling 08-27 18:39:10.859 15925 16081 D BuiltinUserCache: Args = type = 'document' 08-27 18:39:10.881 15925 16081 I BuiltinUserCache: Deleted 0 document entries 08-27 18:39:10.891 15925 16081 D ConnectionSettings: in getConnectURL, returning http://10.0.2.2:8080 08-27 18:39:10.892 15925 16081 I System.out: Posting data to http://10.0.2.2:8080/usercache/get 08-27 18:39:10.912 15925 16081 I CommunicationHelper: Got response org.apache.http.message.BasicHttpResponse@1245861 with status HTTP/1.1 200 OK 08-27 18:39:10.912 15925 16081 I TrafficStats: untagSocket(183) 08-27 18:39:10.912 15925 16081 I System.out: Result Summary JSON = {"server_to_phone": []} 08-27 18:39:10.913 15925 16081 I System.out: length 24 08-27 18:39:10.918 15925 16081 I CommunicationHelper: Result Summary JSON = {"server_to_phone": []} 08-27 18:39:10.918 15925 16081 I CommunicationHelper: length 24 08-27 18:39:10.941 15925 16081 D BuiltinUserCache: Added value for key stats/client_time at time 1.724809150931E9 08-27 18:39:10.952 15925 16081 D BuiltinUserCache: Added value for key stats/client_time at time 1.724809150942E9 08-27 18:39:10.956 15925 16079 I System.out: About to execute query SELECT * FROM userCache WHERE key = 'statemachine/transition' AND type = 'message' AND write_ts >= 0.0 AND write_ts <= 1.724809150954E9 ORDER BY write_ts DESC 08-27 18:39:10.962 15925 15925 I chromium: [INFO:CONSOLE(49)] "DEBUG:sensorDataList.length = 1, 08-27 18:39:10.962 15925 15925 I chromium: syncLaunchedCalls.length = 0, 08-27 18:39:10.962 15925 15925 I chromium: syncPending? = false", source: https://localhost/plugins/cordova-plugin-em-unifiedlogger/www/unifiedlogger.js (49) ```

Other updates at the end of the issue

shankari commented 2 months ago

Of the remaining issues flagged in https://github.com/e-mission/e-mission-docs/issues/1079#issuecomment-2244192028 the others are either postponeable or not very relevant

The ones we are going to postpone are:

  1. Regional preferences Apps can receive notifications when a user changes their regional preferences and mirror these preferences in app. would be an enhancement, not a requirement
  2. New reason an app can be placed in the restricted standby bucket Android 14 introduces a new reason an app can be placed into the restricted standby bucket.
  3. OpenJDK 17 updates As part of the OpenJDK 17 updates, there are some changes that can affect app compatibility, such as changes to regular expressions and UUID handling.

The ones that we will not fix are:

  1. Additional restrictions on starting activities from the background Apps that target Android 14 (API level 34) or higher must opt in if they want to grant their background activity launch privileges to another app either when sending that app's PendingIntent, or binding that app's service. We don't want to grant privileges to other apps
  2. Changes to how users experience non-dismissible notifications If your app shows non-dismissable foreground notifications to users, Android 14 has changed the behavior to allow users to dismiss such notifications. not sure what, if anything, we want to do about this or can do about it
  3. Data safety information is more visible. Your app's data safety information, such as data-sharing practices, now appears in some permission rationale system dialogs and in system notifications. we are open-source so we don't care
shankari commented 2 months ago

Summary of the 11 points from earlier and their resolution

id Topic Resolution
1 Schedule exact alarms are denied by default Remove permission and change code to use inexact alarms
2 Context-registered broadcasts are queued while apps are cached Need to test to verify that the bluetooth permissions still work
3 New reason an app can be placed in the restricted standby bucket Test out the relevant case
4 Foreground service types are required done
5 OpenJDK 17 updates Already at JDK 17
6 Regional preferences Apps can receive notifications when a user changes their regional preferences and mirror these preferences in app good idea but not a priority right now.
7 Restrictions to implicit and pending intents Found two implicit intents, that we must fix before the end of the year.
8 Runtime-registered broadcasts receivers must specify export behavior found two locations that seem to be fine wrt exporting, but must test local notifications
9 Additional restrictions on starting activities from the background Not applicable
10 Changes to how users experience non-dismissible notifications Nothing we can do anyway, defer testing
11 Data safety information is more visible Fine by us, nothing to do
shankari commented 2 months ago

Next steps:

Will test and fix tomorrow!

shankari commented 2 months ago

While trying to rebuild, ran into an issue with loading the dependencies

Could not resolve androidx.core:core:1.6.+. ``` Required by: project :app > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml. > Could not GET 'https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml. > Could not GET 'https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml. > Could not GET 'https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml. > Could not GET 'https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://jcenter.bintray.com/androidx/core/core/maven-metadata.xml. > Could not GET 'https://jcenter.bintray.com/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://jcenter.bintray.com/androidx/core/core/maven-metadata.xml. > Could not GET 'https://jcenter.bintray.com/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml. > Could not GET 'https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target > Failed to list versions for androidx.core:core. > Unable to load Maven meta-data from https://jcenter.bintray.com/androidx/core/core/maven-metadata.xml. > Could not GET 'https://jcenter.bintray.com/androidx/core/core/maven-metadata.xml'. > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target ```
This is because the locations for the metadata have changed ``` $ curl https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml 404 Not Found

404 Not Found

nginx
$ curl https://jcenter.bintray.com/androidx/core/core/maven-metadata.xml 301 Moved Permanently

301 Moved Permanently

nginx
```

The dependency is in platforms/android/app/build.gradle, added as

    // SUB-PROJECT DEPENDENCIES START
    implementation(project(path: ":CordovaLib"))
    implementation "androidx.core:core:1.6.+"
    implementation "me.leolin:ShortcutBadger:1.1.22@aar"
    implementation "com.google.firebase:firebase-messaging:23.+"
    implementation "androidx.webkit:webkit:1.4.0"
    implementation "androidx.legacy:legacy-support-v4:1.0.0"
    implementation "androidx.appcompat:appcompat:1.3.1"
    implementation "androidx.legacy:legacy-support-core-utils:1.0.0"
    implementation "com.google.code.gson:gson:2.10.1"
    implementation "com.google.android.gms:play-services-location:21.0.1"
    implementation "androidx.core:core:1.8.0"
    implementation "androidx.work:work-runtime:2.7.1"
    // SUB-PROJECT DEPENDENCIES END

I looked up the dependency, and the library is here https://mvnrepository.com/artifact/androidx.core/core/1.6.0 and the instructions to include it in gradle are at

// https://mvnrepository.com/artifact/androidx.core/core
runtimeOnly 'androidx.core:core:1.6.0'

The push plugin has "ANDROIDX_CORE_VERSION": "1.6.+" and the data collection plugin has "ANDROIDX_CORE_VERSION": "1.8.0"

shankari commented 2 months ago

Hm... given that this appears to be a common issue with certs that people have encountered before https://stackoverflow.com/questions/69180079/how-to-resolve-gradle-tls-connection-after-upgrading-from-5-4-to-6-8-3 maybe this is a cert issue after all. The CI/CD is working fine.

CI/CD Our setup
openjdk version "17.0.12" 2024-07-16 openjdk 17.0.9 2023-10-17
Gradle 8.10, 2024-08-14 11:07:45 UTC Gradle 7.6, 2022-11-25 13:35:10 UTC

Since the cert is in the JRE

You get this message when the JRE is missing a certificate.

Let's try upgrading both of these and see if it fixes the problem.

shankari commented 2 months ago
Upgraded to java 17.0.12 and gradle 8.10 ``` $ java --version openjdk 17.0.12 2024-07-16 OpenJDK Runtime Environment Temurin-17.0.12+7 (build 17.0.12+7) OpenJDK 64-Bit Server VM Temurin-17.0.12+7 (build 17.0.12+7, mixed mode) ``` ``` $ gradle --version Welcome to Gradle 8.10! Here are the highlights of this release: - Support for Java 23 - Faster configuration cache - Better configuration cache reports For more details see https://docs.gradle.org/8.10/release-notes.html ------------------------------------------------------------ Gradle 8.10 ------------------------------------------------------------ Build time: 2024-08-14 11:07:45 UTC Revision: fef2edbed8af1022cefaf44d4c0514c5f89d7b78 Kotlin: 1.9.24 Groovy: 3.0.22 Ant: Apache Ant(TM) version 1.10.14 compiled on August 16 2023 Launcher JVM: 17.0.12 (Eclipse Adoptium 17.0.12+7) Daemon JVM: /Library/Java/JavaVirtualMachines/temurin-17.jdk/Contents/Home (no JDK specified, using current Java home) OS: Mac OS X 13.6.7 x86_64 ```

Did not fix it

   > Could not resolve androidx.core:core:1.6.+.
     Required by:
         project :app
      > Failed to list versions for androidx.core:core.
         > Unable to load Maven meta-data from https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml.
            > Could not GET 'https://repo.maven.apache.org/maven2/androidx/core/core/maven-metadata.xml'.
               > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties
                  > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target

Checking the certs installed in the JRE...

shankari commented 2 months ago

Aha! If I go to the missing URL in the browser, I still get the 404 error, but the certificate includes a root that is not part of the standard bundle

Screenshot 2024-08-26 at 12 25 02 PM

and is probably an "enterprise root" https://support.mozilla.org/en-US/kb/how-disable-enterprise-roots-preference?as=u&utm_source=inproduct

Aha! the cert chain is: repo1.maven.org <- netskope.nrel.gov <- NREL CA 01 <- NREL root CA

@louisg1337 you were right that it is due to netskope! I still don't see how this is supposed to work given that the path does not exist, but I just need to figure out how to import the enterprise root into the cacerts

shankari commented 2 months ago

So I tried to look up the roots in multiple ways:

We ended up with multiple copies of the same root key in the cacert (nrelrootca, nrelrootcabrowser, nrelrootcakeychain) but I still get the same error.

Note also that the error trace has

               > The server may not support the client's requested TLS protocol versions: (TLSv1.2, TLSv1.3). You may need to configure the client to allow other protocols to be used. See: https://docs.gradle.org/7.6/userguide/build_environment.html#gradle_system_properties
                  > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target

while the examples on stackoverflow have

                   > sun.security.validator.ValidatorException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
                               > PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
                                              > unable to find valid certification path to requested target

Note the lack of TLS 1.3. Maybe the issue is that the netskope routing puts in TLS 1.3 somewhere along the way, while CI/CD goes out to the internet directly.

Note also that even after I installed gradle 8.10, the build is still using gradle 7.1... 

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.6/userguide/command_line_interface.html#sec:command_line_warnings

The gradle version is associated with the cordova version because the gradle files are automatically created by cordova. We have to bump up to the most recent version of cordova-android and cordova-ios anyway; let's do that and see if it fixes the problem. Note that the most recent version of cordova-android uses gradle 8.7

https://cordova.apache.org/blog/

shankari commented 2 months ago

Upgrading to cordova-android@13.0.0 https://cordova.apache.org/announcements/2024/05/23/cordova-android-13.0.0.html

Bingo! That worked!

shankari commented 2 months ago

Per the instructions, we need to have SDK Platform 34 and SDK Build Tools 34.0.0. Which we do.

Older build tools version can be uninstalled if older versions of cordova-android is no longer used in your projects.

This is interesting because removing the older platforms may fix our issues with the SDK install CI/CD task, which is failing because of lack of space. Let's try doing that and see if the build still works...

+ '[' '!' -d /tmp/new-install/Android/sdk/emulator ']'
+ '[' '!' -d /tmp/new-install/Android/sdk/build-tools ']'
+ exit 1
New SDK root /tmp/new-install/Android/sdk
total 8
drwxr-xr-x   8 runner  wheel  256 Jul 14 04:36 .
drwxr-xr-x   3 runner  wheel   96 Jul 14 04:27 ..
-rw-r--r--   1 runner  wheel   16 Jul 14 04:36 .knownPackages
drwxr-xr-x   2 runner  wheel   64 Jul 14 04:36 .temp
drwxr-xr-x   3 runner  wheel   96 Jul 14 04:27 cmdline-tools
drwxr-xr-x  29 runner  wheel  928 Jul 14 04:27 emulator
drwxr-xr-x   5 runner  wheel  160 Jul 14 04:27 licenses
drwxr-xr-x  14 runner  wheel  448 Jul 14 04:35 system-images
Error: Process completed with exit code 1.

Note that the minSDK is now 24 (Android 7). This has not changed since the last release (https://cordova.apache.org/announcements/2023/05/22/cordova-android-12.0.0.html), but it does mean that we don't need system images for API 22 and 23.

shankari commented 2 months ago

Similarly upgrading the iOS version to 7.1.1 Changes needed:

Failed with

clang: error: SDK does not contain 'libarclite' at the path '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/arc/libarclite_iphonesimulator.a'; try increasing the minimum deployment target

This is because although we overwrite the IPHONEOS_DEPLOYMENT_TARGET in a hook (consistent with https://github.com/e-mission/e-mission-phone/pull/1149), and right after the hook, we only see that target, others get added later. Looking at the file, it looks like there are multiple copies of the target. Let's delete everything and see if it helps.

Did not help. I wonder if this is related to "Moved CODE_SIGN_ENTITLEMENTS to the target settings of the application target within the Xcode project."

shankari commented 2 months ago

Upgrading to the most recent version of the cordova CLI fixed it. Looking through the other upgrades that are relevant to us: ⬆️ cordova@12.0.0 ✔️ file@8.0.0 🚫 media-capture 👍 cordova-android@12.0.1 👍 cordova-ios@7.0.1 🚫 camera, media, file-transfer 🚫 geolocation ⬆️ in-app-browser@6.0.0 🚫 electron 👍 cordova-ios@7.1.0 ⬆️ cordova-android@13.0.0 ⬆️ cordova-ios@7.1.1

Let's bump these up and also bump up nvm (0.40.0), node (20.9.0) and npm (10.8.2) to their LTS versions. Worked!

shankari commented 2 months ago

Note also that the gradle version is automatically configured and downloaded by cordova https://github.com/e-mission/e-mission-docs/issues/1079#issuecomment-2310740124 so I don't think we need to have it installed via sdkman

Let's try removing that and see if it works. Worked! Let's pause here for a second and get these basic changes pushed.

shankari commented 2 months ago

Geofence Action intent, improve security

This is a bit tricky because of the way in which we have structured the code. We launch the GeofenceLocationIntentService service through an explicit intent. When we get the location, we want to get it back here, and then modify the location field and have the geofence action continue. So we register a BroadcastReceiver as an anonymous inner class. Right now, we send an implicit intent and filter by it in the broadcast receiver.

So the flow is: we receive location, we broadcast location, we receive location, we modify GeofenceActions fields. To make this an explicit intent, we need to:

        LocalBroadcastManager.getInstance(mCtxt).registerReceiver(new BroadcastReceiver() {
            @Override
            public void onReceive(Context context, Intent intent) {
                Log.i(mCtxt, TAG, "recieved broadcast intent "+intent);
                synchronized(GeofenceActions.this) {
                    GeofenceActions.this.newLastLocation = intent.getParcelableExtra(GeofenceLocationIntentService.INTENT_RESULT_KEY);
                    GeofenceActions.this.notify();
                }
            }
        }, new IntentFilter(GeofenceLocationIntentService.INTENT_NAME));
First, we verify that the change is not required and the code works anyway After making this change to force us to read the location again ``` diff --git a/src/android/location/actions/GeofenceActions.java b/src/android/location/actions/GeofenceActions.java index 3bc8dd9..d538deb 100644 --- a/src/android/location/actions/GeofenceActions.java +++ b/src/android/location/actions/GeofenceActions.java @@ -76,11 +76,6 @@ public class GeofenceActions { public Task create() { try { Location mLastLocation = Tasks.await(LocationServices.getFusedLocationProviderClient(mCtxt).getLastLocation(), 30, TimeUnit.SECONDS); - Log.d(mCtxt, TAG, "Last location would have been " + mLastLocation +" if we hadn't reset it"); - if (isValidLocation(mCtxt, mLastLocation)) { - Log.d(mCtxt, TAG, "Last location is " + mLastLocation + " using it"); - return createGeofenceAtLocation(mLastLocation); - } else { Log.w(mCtxt, TAG, "mLastLocationTime = null, launching callback to read it and then" + "create the geofence"); Location newLoc = readAndReturnCurrentLocation(); @@ -91,7 +86,6 @@ public class GeofenceActions { Log.d(mCtxt, TAG, "Was not able to read new location, skipping geofence creation"); return null; } - } } catch (SecurityException e) { ``` we get ``` 08-27 19:17:57.913 18339 18339 D GeofenceLocationIntentService: onCreate called 08-27 19:17:57.983 18339 18490 I CreateGeofenceAction: isValidLocation = true. Yay! 08-27 19:17:57.994 18339 18490 D GeofenceLocationIntentService: Found most recent valid location = null 08-27 19:17:58.033 18339 18490 D GeofenceLocationIntentService: location is valid, broadcast it Location[fused 37.400770,-122.035570 hAcc=5.0 et=+3h36m36s474ms alt=0.0 vAcc=0.5 vel=81.32331 sAcc=0.5 bear=243.0 bAcc=30.0] 08-27 19:17:58.045 18339 18490 I GeofenceLocationIntentService: broadcasting intent Intent { act=GEOFENCE_LOCATION_RESULT (has extras) } 08-27 19:17:58.059 18339 18339 I CreateGeofenceAction: recieved broadcast intent Intent { act=GEOFENCE_LOCATION_RESULT (has extras) } 08-27 19:17:58.103 18339 18448 D CreateGeofenceAction: After waiting for location reading result, location is Location[fused 37.400770,-122.035570 hAcc=5.0 et=+3h36m36s474ms alt=0.0 vAcc=0.5 vel=81.32331 sAcc=0.5 bear=243.0 bAcc=30.0] 08-27 19:17:58.138 18339 18448 D CreateGeofenceAction: creating geofence at location Location[fused 37.400770,-122.035570 hAcc=5.0 et=+3h36m36s474ms alt=0.0 vAcc=0.5 vel=81.3 2331 sAcc=0.5 bear=243.0 bAcc=30.0] ```
Now, let's try to send the intent to `GeofenceActions` (`new Intent(this, GeofenceActions.class);`) Did not work ``` 08-27 19:40:27.967 19420 19868 I CreateGeofenceAction: isValidLocation = true. Yay! 08-27 19:40:28.073 19420 19868 D GeofenceLocationIntentService: location is valid, broadcast it Location[fused 37.400767,-122.035600 hAcc=5.0 et=+3h59m5s833ms alt=0.0 vAcc=0.5 vel=9.918221 sAcc=0.5 bear=261.771 bAcc=30.0] 08-27 19:40:28.178 19420 19868 I GeofenceLocationIntentService: broadcasting intent Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceActions (has extras) } 08-27 19:40:28.218 538 602 W ProcessStats: Tracking association SourceState{acb1dda edu.berkeley.eecs.emission/10193 BTop #21732} whose proc state 2 is better than process ProcessState{8ac79ba com.google.android.gms/10128 pkg=com.google.android.gms} proc state 8 (15 skipped) 08-27 19:40:28.267 19420 19420 D GeofenceLocationIntentService: onDestroy called ```

Let's try to determine the anonymous class name by printing it out (Log.i(mCtxt, TAG, "recieved broadcast intent "+intent+" in class "+getClass().getName());). After restoring the old intent, we get

08-27 19:51:42.147 20451 20451 I CreateGeofenceAction: recieved broadcast intent Intent
{ act=GEOFENCE_LOCATION_RESULT (has extras) } in class edu.berkeley.eecs.emission.cordov
a.tracker.location.actions.GeofenceActions$1
Now, let's try to send to this class name (`new Intent(this, GeofenceActions$1.class);`), this generates a compile error ``` /Users/kshankar/Desktop/data/e-mission/native_code_upgrade/platforms/android/app/src/main/java/edu/berkeley/eecs/emission/cordova/tracker/location/actions/GeofenceLocationIntentService.java:114: error: cannot find symbol Intent answerIntent = new Intent(this, GeofenceActions$1.class); ^ symbol: class GeofenceActions$1 ```
Let's make this a named inner class (`new Intent(this, GeofenceActions$1.class);`) instead of an anonymous one. This is better anyway to be robust to other inner classes being added to the `GeofenceActions` class. If this doesn't work, I am going to punt on this for now. Did not work. ``` 08-27 20:16:16.803 21392 22827 I GeofenceLocationIntentService: broadcasting intent Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceActions$Ne wLocationCallback (has extras) } 08-27 20:16:16.893 21392 21392 D GeofenceLocationIntentService: onDestroy called ... 08-27 20:17:02.451 21392 21392 D GeofenceLocationIntentService: onCreate called 08-27 20:17:02.497 21392 21392 D GeofenceLocationIntentService: onStart called with Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } startId 1 ``` I suspect this may be due to the filter for the name. We listen to the filter ``` LocalBroadcastManager.getInstance(mCtxt).registerReceiver( new NewLocationCallback(), new IntentFilter(GeofenceLocationIntentService.INTENT_NAME)); ``` but we don't send it anywhere ``` Intent answerIntent = new Intent(this, GeofenceActions.NewLocationCallback.class); ```
The intentFilter cannot be null, but we can make it blank. > Action matches if any of the given values match the Intent action; if the filter specifies no actions, then it will only match Intents that do not contain an action. This also did not fix it ``` 08-27 20:51:56.242 5454 6961 I GeofenceLocationIntentService: broadcasting intent Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceActions$NewLocationCallback (has extras) } 08-27 20:51:56.259 5454 5454 D GeofenceLocationIntentService: onDestroy called ... 08-27 20:51:57.109 5454 5454 D GeofenceLocationIntentService: onCreate called 08-27 20:51:57.164 5454 5454 D GeofenceLocationIntentService: onStart called with Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } startId 1 08-27 20:51:57.315 5454 6972 D GeofenceLocationIntentService: FINALLY! Got location update, intent is Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } ```

There is also this argument that if you just need to notify one particular class, we should not use a receiver. https://stackoverflow.com/questions/4134203/how-to-use-registerreceiver-method I am using this pattern to ensure loose coupling - there is also not really a mechanism to pass in the GeofenceActions object to the service. We can, of course, also refactor to not require a broadcast; maybe using a singleton, or converting GeofenceActions to a service that we can communicate with the other service or passing in the GeofenceActions as an object to the pending intent (if such as thing is possible).

But I am not going to tackle that more complex restructuring now.

Instead, I will just make the changes to address the security best practices https://developer.android.com/develop/background-work/background-tasks/broadcasts#java

  1. specifying a package while broadcasting the intent
  2. set the exported to false wherever possible (this may need us to change from LocalBroadcastManager to ContextCompat.registerReceiver
(1) works but (2) doesn't - just switching `LocalBroadcastManager.getInstance().getReceiver` to `ContextCompat.registerReceiver` means that we don't get the broadcast messages No package works ``` 08-27 22:36:18.466 4901 5581 I GeofenceLocationIntentService: broadcasting intent Intent { act=GEOFENCE_LOCATION_RESULT (has extras) } 08-27 22:36:18.471 4901 4901 I CreateGeofenceAction: recieved broadcast intent Intent { act=GEOFENCE_LOCATION_RESULT (has extras) } ``` Package works ``` 08-27 22:42:29.312 6785 7442 I GeofenceLocationIntentService: broadcasting intent Inte nt { act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) } 08-27 22:42:29.319 6785 6785 I CreateGeofenceAction: recieved broadcast intent Intent { act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) } ``` Using `ContextCompat does not work ``` 08-27 22:59:46.540 15122 17868 D CreateGeofenceAction: Successfully started tracking location, about to start waiting for location update 08-27 22:59:46.547 15122 17868 D CreateGeofenceAction: About to start waiting for location 08-27 22:59:50.629 15122 17901 I GeofenceLocationIntentService: broadcasting intent Intent { act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) } 08-27 22:59:50.636 15122 15122 D GeofenceLocationIntentService: onDestroy called ... 08-27 22:59:51.564 15122 15122 D GeofenceLocationIntentService: onCreate called 08-27 22:59:51.571 15122 15122 D GeofenceLocationIntentService: onStart called with Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } startId 1 ```

But note that registerReceiver was added in 1.9.0 and our current version is 1.8.0.

Versions after bumping up I also changed the default version in the plugin. Everything has been updated ``` $ grep -r ANDROIDX.*CORE . ./package.cordovabuild.json: "ANDROIDX_CORE_VERSION": "1.6.+", ./package.cordovabuild.json: "ANDROIDX_CORE_VERSION": "1.9.0", ./platforms/android/app/build.gradle: implementation "androidx.core:core-splashscreen:${cordovaConfig.ANDROIDX_CORE_SPLASHSCREEN_VERSION}" ./platforms/android/android.json: "ANDROIDX_CORE_VERSION": "1.6.+", ./platforms/android/android.json: "ANDROIDX_CORE_VERSION": "1.9.0", ./platforms/android/cdv-gradle-config.json:{"MIN_SDK_VERSION":24,"SDK_VERSION":34,"COMPILE_SDK_VERSION":null,"GRADLE_VERSION":"8.7","MIN_BUILD_TOOLS_VERSION":"34.0.0","AGP_VERSION":"8.3.0","KOTLIN_VERSION":"1.7.10","ANDROIDX_APP_COMPAT_VERSION":"1.6.1","ANDROIDX_WEBKIT_VERSION":"1.6.0","ANDROIDX_CORE_SPLASHSCREEN_VERSION":"1.0.0","GRADLE_PLUGIN_GOOGLE_SERVICES_VERSION":"4.3.3","IS_GRADLE_PLUGIN_GOOGLE_SERVICES_ENABLED":true,"IS_GRADLE_PLUGIN_KOTLIN_ENABLED":true,"PACKAGE_NAMESPACE":"edu.berkeley.eecs.emission","JAVA_SOURCE_COMPATIBILITY":8,"JAVA_TARGET_COMPATIBILITY":8,"KOTLIN_JVM_TARGET":null} ./platforms/android/CordovaLib/build.gradle: implementation "androidx.core:core-splashscreen:${cordovaConfig.ANDROIDX_CORE_SPLASHSCREEN_VERSION}" ./plugins/@havesource/cordova-plugin-push/plugin.xml: ./plugins/@havesource/cordova-plugin-push/plugin.xml: ./plugins/android.json: "ANDROIDX_CORE_VERSION": "1.6.+", ./plugins/android.json: "ANDROIDX_CORE_VERSION": "1.9.0", ./plugins/cordova-plugin-em-datacollection/plugin.latest.androidx.patch:+ ./plugins/cordova-plugin-em-datacollection/plugin.latest.androidx.patch:+ ./plugins/cordova-plugin-em-datacollection/plugin.xml: ./plugins/cordova-plugin-em-datacollection/plugin.xml: ./plugins/fetch.json: "ANDROIDX_CORE_VERSION": "1.6.+", ./package.orig.json: "ANDROIDX_CORE_VERSION": "1.6.+", ./package.orig.json: "ANDROIDX_CORE_VERSION": "1.8.0", ./node_modules/@havesource/cordova-plugin-push/plugin.xml: ./node_modules/@havesource/cordova-plugin-push/plugin.xml: ./node_modules/cordova-android/framework/build.gradle: implementation "androidx.core:core-splashscreen:${cordovaConfig.ANDROIDX_CORE_SPLASHSCREEN_VERSION}" ./node_modules/cordova-android/framework/cdv-gradle-config-defaults.json: "ANDROIDX_CORE_SPLASHSCREEN_VERSION": "1.0.0", ./node_modules/cordova-android/templates/project/app/build.gradle: implementation "androidx.core:core-splashscreen:${cordovaConfig.ANDROIDX_CORE_SPLASHSCREEN_VERSION}" ./package.json: "ANDROIDX_CORE_VERSION": "1.6.+", ./package.json: "ANDROIDX_CORE_VERSION": "1.9.0", ```

And it still failed

08-27 23:16:16.852 24596 27052 D CreateGeofenceAction: About to start waiting for locati
08-27 23:16:20.588 24596 24596 D GeofenceLocationIntentService: onCreate called
08-27 23:16:20.709 24596 27083 I GeofenceLocationIntentService: broadcasting intent Intent {act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) }
08-27 23:16:20.722 24596 24596 D GeofenceLocationIntentService: onDestroy called
...
08-27 23:16:21.576 24596 24596 D GeofenceLocationIntentService: onCreate called
...
08-27 23:16:21.670 24596 27089 I GeofenceLocationIntentService: broadcasting intent Intent { act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) }
08-27 23:16:21.679 24596 24596 D GeofenceLocationIntentService: onDestroy called
...
08-27 23:16:22.582 24596 24596 D GeofenceLocationIntentService: onCreate called
....
08-27 23:16:22.672 24596 27103 I GeofenceLocationIntentService: broadcasting intent Inte
nt { act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) }
08-27 23:16:22.680 24596 24596 D GeofenceLocationIntentService: onDestroy called

Since I can't figure out why the ContextCompat doens't work:

shankari commented 2 months ago

Note also that if the ContextCompat does not work, we can't actually change any of the LocalBroadcastManager instances to ContextCompat. https://github.com/e-mission/e-mission-docs/issues/1079#issuecomment-2309319107

The one in the Bluetooth code is actually not a local broadcast receiver

        IntentFilter filter = new IntentFilter(BluetoothAdapter.ACTION_STATE_CHANGED);
        cordova.getActivity().registerReceiver(broadcastReceiver, filter);
Testing to see if it works now Works when the foreground service is present ``` 08-28 08:32:01.569 1630 1630 D com.unarin.beacon: Bluetooth Service state changed from STATE_OFF to STATE_TURNING_ON 08-28 08:32:02.176 1630 1630 D com.unarin.beacon: Bluetooth Service state changed from STATE_TURNING_ON to STATE_ON 08-28 08:32:05.902 1630 1630 D com.unarin.beacon: Bluetooth Service state changed from STATE_ON to STATE_TURNING_OFF ``` Killing the foreground service ``` $ adb shell am force-stop edu.berkeley.eecs.emission ``` Turning bluetooth on and off, **no logs** But that is not surprising since we have killed the entire app. Killing only the foreground service is trickier and requires rooting the emulator. Let's try that, and then declare this done.

Rooting the emulator using the instructions at: https://proandroiddev.com/root-an-android-emulator-avd-9f912328ca08 https://gitlab.com/newbit/rootAVD

Find and kill only the foreground service

$ ./adb shell dumpsys activity service edu.berkeley.eecs.emission

$ ./adb shell
emu64xa:/ $
emu64xa:/ # am stopservice edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineForegroundService
Stopping service: Intent { act=android.intent.action.MAIN cat=[android.intent.category.LAUNCHER] cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineForegroundService }
Service stopped
emu64xa:/ # dumpsys activity service edu.berkeley.eecs.emission
SERVICE edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineService c5b3d96 pid=(not running)

Turned Bluetooth on/off, no logs (kind of expected, because with the foreground service killed, the one service is not even running).

Launched the app

Try again - Bluetooth is off

$ ./adb shell dumpsys activity service edu.berkeley.eecs.emission
SERVICE edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineForegroundService 6b77a2c pid=10530 user=0
  Client:
    nothing to dump

SERVICE edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineService c5b3d96 pid=10530 user=0
  Client:
    nothing to dump

SERVICE edu.berkeley.eecs.emission/org.chromium.content.app.SandboxedProcessService0:0 2139204 pid=10600 user=0
  Client:
    Failure while dumping the service: java.io.IOException: Timeout

Kill the foreground service

$ ./adb shell
emu64xa:/ $ su
ordova.tracker.location.TripDiaryStateMachineForegroundService                        <
Stopping service: Intent { act=android.intent.action.MAIN cat=[android.intent.category.LAUNCHER] cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineForegroundService }
Service stopped
255|emu64xa:/ # dumpsys activity service edu.berkeley.eecs.emission
SERVICE edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineService c5b3d96 pid=14309 user=0
  Client:
    Failure while dumping the service: java.io.IOException: Timeout

SERVICE edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService 480fa7e pid=14309 user=0
  Client:

*** SERVICE 'activity' DUMP TIMEOUT (10000ms) EXPIRED ***

*** SERVICE 'activity' DUMP TIMEOUT (10000ms) EXPIRED ***

dumpsys activity service edu.berkeley.eecs.emission                                    <
SERVICE edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineService c5b3d96 pid=14309 user=0
  Client:
    Failure while dumping the service: java.io.IOException: Timeout
emu64xa:/ #
popup error that the app keeps closing, app is gone ``` umpsys activity service edu.berkeley.eecs.emission < No services match: edu.berkeley.eecs.emission Use -h for help. ``` ``` 08-28 09:21:08.199 549 581 W ActivityManager: Bringing down service while still waiting for start foreground: ServiceRecord{2a56e7 u0 edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineForegroundService} 08-28 09:21:08.239 14309 14309 D TripDiaryStateMachineForegroundService: onStartCommand called with intent = Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.TripDiaryStateMachineForegroundService } flags = 0 and startId = 1 08-28 09:21:08.432 14309 14309 D GeofenceLocationIntentService: onCreate called 08-28 09:21:08.441 14309 14309 D GeofenceLocationIntentService: onStart called with Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } startId 1 08-28 09:21:08.450 14309 14309 D GeofenceLocationIntentService: onStart called with Inte nt { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } startId 2 08-28 09:21:08.628 14309 15164 I GeofenceLocationIntentService: broadcasting intent Intent { act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) } 08-28 09:21:08.701 14309 14309 D TripDiaryStateMachineForegroundService: onDestroy calle d, removing notification 08-28 09:21:08.704 14309 14309 D AndroidRuntime: Shutting down VM 08-28 09:21:08.705 14309 14309 E AndroidRuntime: FATAL EXCEPTION: main 08-28 09:21:08.705 14309 14309 E AndroidRuntime: Process: edu.berkeley.eecs.emission, PI D: 14309 08-28 09:21:08.705 14309 14309 E AndroidRuntime: android.app.RemoteServiceException$Fore groundServiceDidNotStartInTimeException: Context.startForegroundService() did not then c all Service.startForeground(): ServiceRecord{2a56e7 u0 edu.berkeley.eecs.emission/.cordo va.tracker.location.TripDiaryStateMachineForegroundService} 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.app.ActivityThread.ge nerateForegroundServiceDidNotStartInTimeException(ActivityThread.java:2104) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.app.ActivityThread.th rowRemoteServiceException(ActivityThread.java:2075) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.app.ActivityThread.-$ $Nest$mthrowRemoteServiceException(Unknown Source:0) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.app.ActivityThread$H. handleMessage(ActivityThread.java:2369) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.os.Handler.dispatchMe ssage(Handler.java:106) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.os.Looper.loopOnce(Lo oper.java:205) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.os.Looper.loop(Looper .java:294) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at android.app.ActivityThread.ma in(ActivityThread.java:8177) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at java.lang.reflect.Method.invo ke(Native Method) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at com.android.internal.os.Runti meInit$MethodAndArgsCaller.run(RuntimeInit.java:552) 08-28 09:21:08.705 14309 14309 E AndroidRuntime: at com.android.internal.os.Zygot eInit.main(ZygoteInit.java:971) ```

Launch app, no bluetooth updates

$ grep Bluetooth /tmp/logcat.log
08-28 09:29:04.405 18792 18792 D PluginManager: startupPlugins: put - BluetoothClassicSerial
08-28 09:29:04.425 18792 18870 W cr_media: registerBluetoothIntentsIfNeeded: Requires BLUETOOTH permission

Change bluetooth toggle

08-28 09:29:51.699 19355 19387 I bt_bta_dm: packages/modules/Bluetooth/system/bta/dm/bta_dm_act.cc:3153 bta_dm_set_eir: Generating extended inquiry response packet EIR
08-28 09:29:51.699 19355 19387 E bt_btif : packages/modules/Bluetooth/system/main/bte_logmsg.cc:105 LogMsg: toggleProfile: Unknown service 255 being enabled
08-28 09:29:51.734   549  1504 I MediaSessionStack: addSession to bottom of stack | record: com.google.android.bluetooth/BluetoothMediaBrowserService (userId=0)
08-28 09:29:52.043 18792 18792 D com.unarin.beacon: Bluetooth Service state changed from STATE_TURNING_ON to STATE_ON
08-28 09:29:52.059  1353  2658 D BluetoothAdapter: isLeEnabled(): ON
08-28 09:29:52.063  1353  2658 D BluetoothAdapter: isLeEnabled(): ON
08-28 09:29:52.064  1353  4086 I NearbySharing: (REDACTED) Bluetooth is %s
08-28 09:29:52.070  1353  1798 D BluetoothLeScanner: onScannerRegistered() - status=0 scannerId=1 mScannerId=0
08-28 09:29:52.090 19355 19420 I bt_btif : packages/modules/Bluetooth/system/btif/src/bluetooth.cc:829 get_profile_interface: get_profile_interface: id = activity_attribution
08-28 09:29:52.105 19355 19392 I bluetooth: packages/modules/Bluetooth/system/gd/hci/le_scanning_manager.cc:588 stop_scan: Scanning already stopped, return!
08-28 09:29:52.109 19355 19392 I bluetooth: packages/modules/Bluetooth/system/gd/hci/le_address_manager.cc:175 register_client: Scheduled address rotation for first client registered
Comparing to android 12, behavior is the same - Foreground service running ``` $ grep "Bluetooth Servi" /tmp/logcat.log 08-28 10:21:12.316 7852 7852 D com.unarin.beacon: Bluetooth Service state changed from STATE_ON to STATE_TURNING_OFF 08-28 10:21:12.537 7852 7852 D com.unarin.beacon: Bluetooth Service state changed from STATE_TURNING_OFF to STATE_OFF ``` - Foreground service killed ``` $ grep "Bluetooth Servi" /tmp/logcat.log $ ``` - App restarted, foreground service started ``` $ grep "Bluetooth Servi" /tmp/logcat.log $ ``` - Bluetooth toggled again ``` $ grep "Bluetooth Servi" /tmp/logcat.log 08-28 10:33:44.844 3924 3924 D com.unarin.beacon: Bluetooth Service state changed from STATE_ON to STATE_TURNING_OFF 08-28 10:33:45.019 3924 3924 D com.unarin.beacon: Bluetooth Service state changed from STATE_TURNING_OFF to STATE_OFF ```

So we are not going to change this behavior now.

The manifest-registered receivers are not subject to this change

Manifest-declared broadcasts aren't queued, and apps are removed from the cached state for broadcast delivery.

So we don't need to change them. For the record, only these two receivers don't have the exported tag, and they are from a plugin that we haven't forked.

        <receiver android:name="com.adobe.phonegap.push.BackgroundActionButtonHandler" />
        <receiver android:name="com.adobe.phonegap.push.PushDismissedHandler" />

So I think we are done here.

shankari commented 2 months ago

Now that we have upgraded the base platforms and plugins, we can actually start the code changes outlined in https://github.com/e-mission/e-mission-docs/issues/1079#issuecomment-2309468607

id Topic
Resolution
Before fix
After fix
1 Schedule exact alarms are denied by default Remove permission and change code to use inexact alarms Generates notification (because we are on the exemption list "Apps that are on the power allowlist"). Still want to change this because we don't need the exact alarms, and we might as well lower power consumption, but now the test will just be that we don't crash with the new code Local notifications are generated and don't crash while either clicking or clearing
2 Context-registered broadcasts are queued while apps are cached we have a foreground service, so are typically not going to be cached. need to test behavior when foreground service is killed, unclear how we can fix other than making the broadcasts manifest-registered Bluetooth changes are captured when the foreground service is running. They are not captured when the service is not running. They are not queued or delivered when app restarts Unchanged
4 Foreground service types are required Added the appropriate permissions App crashes on launch with error Caused by: java.lang.SecurityException: Starting FGS with type location requires permissions: all of the permissions allOf=true [android.permission.FOREGROUND_SERVICE_LOCATION]... App does not crash, foreground service is visible
7 Restrictions to implicit and pending intents Found two implicit intents (1) Crashes with java.lang.RuntimeException: Unable to start activity ComponentInfo{ edu.berkeley.eecs.emission/ de.appplant.cordova.plugin.localnotification.ClickReceiver }: ... to create a new PendingIntent with an implicit Intent use FLAG_IMMUTABLE., (2) Two existing locations - server sync and geofence location check - continue to work. Trying to get two security improvements: (1) specifying the package in the intent, and creating a non-exported receiver using ContextCompat instead of LocalBroadcastManager (1) Removed server sync broadcast and changed the server impl to a NOP, (2) added package to geofence location intent for better security. Changing to ContextCompat caused the receiver to never receive broadcasts, so reverted it
8 Runtime-registered broadcasts receivers must specify export behavior found two locations that seem to be fine wrt exporting, but must test local notifications local notification works. Using ContextCompat for the new exporting functionality does not appear to work (as tested with the geofence action broadcast) code is unchanged
shankari commented 2 months ago

Next steps:

shankari commented 2 months ago

This is not actually closed.

shankari commented 2 months ago

While testing against the committed code, I didn't want to edit it to force reading the location again as in https://github.com/e-mission/e-mission-docs/issues/1079#issuecomment-2314436185

So I just turned the GPS off in the emulator

Tests passed ``` 08-29 11:59:50.351 19700 20990 D CreateGeofenceAction: Last location would have been Location[fused 37.400770,-122.035570 hAcc=754.457 et=+23h24m12s673ms alt=0.0 vAcc=0.5] if we hadn't reset it 08-29 11:59:50.359 19700 20990 I CreateGeofenceAction: testLoc.getAccuracy 754.457 > 200 isValidLocation = false 08-29 11:59:50.372 19700 20990 W CreateGeofenceAction: mLastLocationTime = null, launching callback to read it and thencreate the geofence 08-29 11:59:50.424 19700 20990 D CreateGeofenceAction: Successfully started tracking location, about to start waiting for location update 08-29 11:59:50.438 19700 20990 D CreateGeofenceAction: About to start waiting for location 08-29 12:00:06.453 19700 21025 D GeofenceLocationIntentService: Read location null from intent 08-29 12:00:11.524 19700 21029 D GeofenceLocationIntentService: Read location null from intent 08-29 12:00:21.551 19700 21037 D GeofenceLocationIntentService: Read location null from intent 08-29 12:00:43.187 19700 21058 D GeofenceLocationIntentService: Read location Location[fused 37.400770,-122.035571 hAcc=5.0 et=+23h25m34s752ms alt=0.0 vAcc=0.5 vel=36.17687 sAcc=0.5 bear=258.28284 bAcc=30.0] from intent 08-29 12:00:43.199 19700 21058 I CreateGeofenceAction: isValidLocation = true. Yay! 08-29 12:00:43.225 19700 21058 D GeofenceLocationIntentService: location is valid, broadcast it Location[fused 37.400770,-122.035571 hAcc=5.0 et=+23h25m34s752ms alt=0.0 vAcc=0.5 vel=36.17687 sAcc=0.5 bear=258.28284 bAcc=30.0] ```

Note that this looks like it also provides an explanation for why we see multiple exit activity actions and multiple geofences sometimes.

Once we start `GeofenceLocationIntentService` to read the new location, we get some updates. The first update creates the geofence and stops the `GeofenceLocationIntentService`. But now that it has received the broadcast, `CreateGeofenceAction` sees a valid "last location", creates another geofence, and multiple `OPGeofenceExitActivityAction`s. ``` 08-29 12:17:26.053 21469 21569 I CreateGeofenceAction: testLoc.getAccuracy 1506.805 > 200 isValidLocation = false 08-29 12:17:26.087 21469 21569 W CreateGeofenceAction: mLastLocationTime = null, launching callback to read it and thencreate the geofence 08-29 12:17:26.250 21469 21569 D CreateGeofenceAction: Successfully started tracking location, about to start waiting for location update 08-29 12:17:26.298 21469 21569 D CreateGeofenceAction: About to start waiting for location 08-29 12:17:28.487 21469 21469 D GeofenceLocationIntentService: onCreate called 08-29 12:17:28.504 21469 21469 D GeofenceLocationIntentService: onStart called with Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } startId 1 08-29 12:17:28.528 21469 21571 D GeofenceLocationIntentService: FINALLY! Got location update, intent is Intent { cmp=edu.berkeley.eecs.emission/.cordova.tracker.location.actions.GeofenceLocationIntentService (has extras) } 08-29 12:17:28.542 21469 21571 D GeofenceLocationIntentService: Extras bundle = Bundle[mParcelledData.dataSize=472] 08-29 12:17:28.556 21469 21571 D GeofenceLocationIntentService: Extras keys are [com.google.android.gms.location.EXTRA_LOCATION_RESULT_BYTES, com.google.android.location.LOCATION] 08-29 12:17:28.578 21469 21571 D GeofenceLocationIntentService: Extras values are [[B@836ae3a, Location[fused 37.400770,-122.035570 hAcc=5.0 et=+23h42m20s202ms alt=0.0 vAcc=0.5 vel=17.060932 sAcc=0.5 bear=260.91754 bAcc=30.0]] 08-29 12:17:28.593 21469 21571 I CreateGeofenceAction: isValidLocation = true. Yay! 08-29 12:17:28.612 21469 21571 D GeofenceLocationIntentService: Found most recent valid location = null 08-29 12:17:28.630 21469 21571 D GeofenceLocationIntentService: Read location Location[fused 37.400770,-122.035570 hAcc=5.0 et=+23h42m20s202ms alt=0.0 vAcc=0.5 vel=17.060932 sAcc=0.5 bear=260.91754 bAcc=30.0] from intent 08-29 12:17:28.644 21469 21571 I CreateGeofenceAction: isValidLocation = true. Yay! 08-29 12:17:28.658 21469 21571 D GeofenceLocationIntentService: location is valid, broadcast it Location[fused 37.400770,-122.035570 hAcc=5.0 et=+23h42m20s202ms alt=0.0 vAcc=0.5 vel=17.060932 sAcc=0.5 bear=260.91754 bAcc=30.0] 08-29 12:17:28.678 21469 21571 I GeofenceLocationIntentService: broadcasting intent Intent {act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) } 08-29 12:17:28.693 21469 21469 I CreateGeofenceAction: recieved broadcast intent Intent {act=GEOFENCE_LOCATION_RESULT pkg=edu.berkeley.eecs.emission (has extras) } 08-29 12:17:28.719 21469 21569 D CreateGeofenceAction: After waiting for location reading result, location is Location[fused 37.400770,-122.035570 hAcc=5.0 et=+23h42m20s202ms alt=0.0 vAcc=0.5 vel=17.060932 sAcc=0.5 bear=260.91754 bAcc=30.0] 08-29 12:17:28.746 21469 21469 D GeofenceLocationIntentService: onDestroy called 08-29 12:17:28.779 21469 21569 D CreateGeofenceAction: New last location is Location[fused 37.400770,-122.035570 hAcc=5.0 et=+23h42m20s202ms alt=0.0 vAcc=0.5 vel=17.060932 sAcc=0.5 bear=260.91754 bAcc=30.0] using it 08-29 12:17:28.803 21469 21569 D CreateGeofenceAction: creating geofence at location Location[fused 37.400770,-122.035570 hAcc=5.0 et=+23h42m20s202ms alt=0.0 vAcc=0.5 vel=17.060932 sAcc=0.5 bear=260.91754 bAcc=30.0] 08-29 12:17:28.957 21469 21569 D CreateGeofenceAction: creating geofence at location 37.40077, -122.03557 08-29 12:17:28.994 21469 21569 D OPGeofenceExitActivityActions: Starting listening to activity transitions for list = [ActivityTransition [mActivityType=7, mTransitionType=0], ActivityTransition [mActivityType=1, mTransitionType=0], ActivityTransition [mActivityType=8, mTransitionType=0], ActivityTransition [mActivityType=0, mTransitionType=0], ActivityTransition [mActivityType=3, mTransitionType=0]] 08-29 12:17:32.073 21469 21580 D CreateGeofenceAction: Last location would have been Location[fused 37.400766,-122.035601 hAcc=5.0 et=+23h42m21s207ms alt=0.0 vAcc=0.5 vel=12.72964 sAcc=0.5 bear=261.4832 bAcc=30.0] if we hadn't reset it 08-29 12:17:32.096 21469 21580 I CreateGeofenceAction: isValidLocation = true. Yay! 08-29 12:17:32.110 21469 21580 D CreateGeofenceAction: Last location is Location[fused 37.400766,-122.035601 hAcc=5.0 et=+23h42m21s207ms alt=0.0 vAcc=0.5 vel=12.72964 sAcc=0.5 bear=261.4832 bAcc=30.0] using it 08-29 12:17:32.128 21469 21580 D CreateGeofenceAction: creating geofence at location Location[fused 37.400766,-122.035601 hAcc=5.0 et=+23h42m21s207ms alt=0.0 vAcc=0.5 vel=12.72964 sAcc=0.5 bear=261.4832 bAcc=30.0] 08-29 12:17:32.213 21469 21580 D CreateGeofenceAction: creating geofence at location 37.4007664, -122.0356006 08-29 12:17:32.239 21469 21580 D OPGeofenceExitActivityActions: Starting listening to activity transitions for list = [ActivityTransition [mActivityType=7, mTransitionType=0], ActivityTransition [mActivityType=1, mTransitionType=0], ActivityTransition [mActivityType=8, mTransitionType=0], ActivityTransition [mActivityType=0, mTransitionType=0], ActivityTransition [mActivityType=3, mTransitionType=0]] ```

Logs: logcat.log.gz

shankari commented 1 month ago

This version (1.8.3) is now live on the android and iOS stores. Note that since we use a foreground service, I had to upload a video to the google play store describing what OpenPATH does, and wait for it to be manually reviewed. But it just got accepted right now.

@jf87 @idillon-sfl @asiripanich @ericafenyo @TTalex @AlirezaRa94 @MaliheTabasi for visibility