search

wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

Upgrade Wordpress-Android Target SDK to 34 #19917

Open notandyvee opened 4 months ago

notandyvee commented 4 months ago

Conducted a preliminary investigation based on the API changes being made to Android 14 devices. Listing them out here. There may be gaps with 3rd party libraries. For example, sentry, which needs an update.

Smoke test this and check it off.

We only use system registered broadcasts. Smoke test and check it off.

This is more of an FYI. We have the potential to log this to ensure the bucket never changes for us. Decide that and we can check it off.

We do not use that.

A quick search doesn't show any places where onStop is doing any background work. Checking off now.

We need to add foreground service types to all the services below.

Test a few screens that use the following Matcher methods:

Marking this as done since we are currently already declaring the ACCESS_NETWORK_STATE permission. It's recommended we migrate to WorkManager. But it doesn't appear required based on the changes we need to make.

Marked as done. We don't support SDKs under 24. Android 14 removes support for SDKs less than 23.

Not seeing usages anywhere.

I can't find an instance where we use implicit intents.

Test runtime broadcasts being registered using registerReceiver in the following classes:

Don't think we use this. Correct me if I am wrong.

Cannot find instances where we even start an activity from the background.

Do not see any of these usages in the app.

Cannot find any mention of MediaProjection.createVirtualDisplay in the app. So assuming we are good.

I haven't tested this. Will update when I do.

After testing this, everything mostly works. There was a bug I noticed with my Samsung phone. Nothing major. PR up for this here: https://github.com/wordpress-mobile/WordPress-Android/pull/19979

We do not use fullscreen notifications.

Any non-dismissable notification in general will now be dismissable, with some caveats. Shouldn't cause any issues upgrading.

irfano commented 4 months ago

Thanks for listing all these changes as a task list, @notandyvee! I'll review items you investigated as a second pair of eyes.

👍🏻

System enforces cached-app resource usage A quick search doesn't show any places where onStop is doing any background work. Checking off now.

👍🏻 I think relying only on checking onStops is not enough to ensure we don't use any background work after onStop. This is because there may be active threads bound to the activity. However, this is not a new problem. If my understanding is correct, Android 14 only amplifies the limitation. If there are any instances of using any background threads after onStop, it must already be an open issue or reported on Sentry. So, I believe we don't need to be concerned about this within the scope of this project.

❓ We're using PendingIntent and bindService() in the project. Should we test them or are they already compatible?

notandyvee commented 4 months ago

Thanks for the questions @irfano .

System enforces cached-app resource usage A quick search doesn't show any places where onStop is doing any background work. Checking off now.

👍🏻 I think relying only on checking onStops is not enough to ensure we don't use any background work after onStop. This is because there may be active threads bound to the activity. However, this is not a new problem. If my understanding is correct, Android 14 only amplifies the limitation. If there are any instances of using any background threads after onStop, it must already be an open issue or reported on Sentry. So, I believe we don't need to be concerned about this within the scope of this project.

As per the android docs on these changes:

Apps that use typical framework-supported lifecycle APIs – such as services, JobScheduler, and Jetpack WorkManager – shouldn't be impacted by these changes.

Your understanding is correct.

Additional restrictions on starting activities from the background

❓ We're using PendingIntent and bindService() in the project. Should we test them or are they already compatible?

For PendingIntent, the docs seem to refer to the PendingIntent#send method. I could not find any usages of that. So we should be good unless I missed something. Not efficient, but I simply searched for "send" and checked if it was called from an intent. None are. So we are good.

For bindService, the docs mention when binding a service from a different app. We only bind services from internal services or system services. Doesn't affect us. But let me know if you are aware of a service we use that's from another app on the device.