WordPress-Android icon indicating copy to clipboard operation
WordPress-Android copied to clipboard

Upgrade Wordpress-Android Target SDK to 34

Open notandyvee opened this issue 2 years ago • 2 comments

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.

  • [x] .ui.uploads.UploadService
  • [x] .ui.media.services.MediaDeleteService
  • [x] .ui.reader.services.update.ReaderUpdateService
  • [x] .ui.reader.services.update.ReaderUpdateJobService
  • [x] .ui.reader.services.discover.ReaderDiscoverService
  • [x] .ui.reader.services.discover.ReaderDiscoverJobService
  • [x] .ui.reader.services.post.ReaderPostService
  • [x] .ui.reader.services.post.ReaderPostJobService
  • [x] .ui.reader.services.search.ReaderSearchService
  • [x] .ui.reader.services.search.ReaderSearchJobService
  • [x] .ui.reader.services.comment.ReaderCommentService
  • [x] .ui.suggestion.service.SuggestionService
  • [x] .push.NotificationsProcessingService
  • [x] .ui.notifications.services.NotificationsUpdateService
  • [x] .ui.notifications.services.NotificationsUpdateJobService
  • [x] .util.analytics.service.InstallationReferrerService
  • [x] .util.analytics.service.InstallationReferrerJobService
  • [x] .login.LoginWpcomService
  • [x] .ui.sitecreation.services.SiteCreationService
  • [x] .ui.stats.refresh.lists.widget.WidgetService
  • [x] .push.GCMMessageService

  • Smoke test regex Matcher.
  • Smoke test production build to ensure there wasn't a proguard issue.

Test a few screens that use the following Matcher methods:

  • [ ] appendReplacement
  • [ ] end
  • [ ] group
  • [ ] start
  • [ ] usePattern

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:

  • [x] MediaBrowserActivity - (My Site -> More -> Media -> WordPress Media) If it doesn't crash, we are good.
  • [x] MediaSettingsActivity (Done via https://github.com/wordpress-mobile/WordPress-Android/pull/19979)
  • [x] AppInitializer - Load the app. Put the app in the background. Go back into the app. Not crashing is good.
  • [x] ReaderPostDetailFragment - This can be tested by going to the reader and clicking on any post (https://github.com/wordpress-mobile/WordPress-Android/pull/20116)
  • [x] ConnectionStatusLiveData - (My Site -> Posts) No crash and seeing your published posts means we are good.

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.


notandyvee avatar Jan 10 '24 22:01 notandyvee

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?

irfano avatar Jan 14 '24 18:01 irfano

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.

notandyvee avatar Jan 16 '24 21:01 notandyvee

@AjeshRPai I went ahead and created a drive link for you and created the video screengrabs. Let me know if you can't access it. There is a doc in that folder that contains the services and an explanation. Each service is accompanied by an mp4 which contains the example video it sounds the declaration needs. The exception is ReaderCommentService-and-SuggestionService-recording. That one doubles for both ReaderCommentService and SuggestionService.

At this point I will convert the PR so it can be reviewed. I will request two approvals before we can merge. One from you Ajesh.

notandyvee avatar May 24 '24 20:05 notandyvee

Hey @notandyvee Thanks for creating the videos. 🙌🏼

AjeshRPai avatar May 29 '24 09:05 AjeshRPai