Update compileSdkVersion to 33

[irfano]

Fixes #17790

This PR updates compileSdkVersion to 33 and fixes errors and warnings caused by the update.

This contains a lot of changes, but they are small and similar. I could publish this via multiple PRs, but multiple PRs would create a burden of merging in order and creating multiple branches based on others. I believe it won't be hard to review this if you follow the commits.

I've added @ravishanker, but he'll be on support rotation next week. I'd appreciate it if you could review, @ParaskP7.

Commits

1. 61992b1: compileSdkVersion was updated to Android 33.
2. 9446c7f: onBackPressed was deprecated on Android 13. It's updated only on Kotlin files. Because only errors on Kotlin files prevent the build. onBackPressed on Java files can be updated while converting them to Kotlin.
3. a8d466a: CompatExtensions was added to use in the next commit.
4. d8deb99: Some functions of Bundle and Intent was deprecated. To update them for Android 13, there are functions in IntentCompat and BundleCompat but they're available on the 1.10.0-alpha version which is not stable yet. I used CompatExtensions that I created to update deprecated functions.
5. b767c46: Updated some AnimatorListener parameters which are not nullable anymore.
6. 44dfcad: Updated some MenuItem parameters which are not nullable anymore.
7. cf40f4e: Updated some SimpleOnGestureListener parameters which are not nullable anymore.
8. 73662bd: Updated androidx.core version to 1.9.0 because we'll need new ParcelCompat functions in the next commit.
9. 22a3914: Use ParcelCompat for deprecated functions.
10. 1287b22: Update deprecated PackageManager functions.
11. 37a3113: Added android.permission.POST_NOTIFICATIONS to fix the build error. Now NotificationManagerCompat.from(context).notify(id, notification) shows the error "Call requires permission which may be rejected by user: code should explicitly check to see if permission is available (with checkPermission) or explicitly handle a potential SecurityException" but notifications still works on Android 13 and lower versions. Runtime permission for notifications #17714 will be handled later.
12. 995e447: Fixed unit tests by updating deprecated functions and making superclass of T nullable in CompatExtensions. This also fixes possible null exceptions on runtime.

Note
I also took action for some warnings from Android Studio, like removing redundant SAM-constructor if the warning is just inside the function I work on.

To test:
Being able to build and basic smoke test should be enough.

Regression Notes

1. Potential unintended areas of impact
   Since this touches a lot of classes, every screen can be impacted, but no change is expected on runtime. But especially back gestures, passing data between screens and notifications.

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   I tested the back gesture manually to verify it's still working.

3. What automated tests I added (or what prevented me from doing so)
   None. This is just a library and deprecated functions update.

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[wpmobilebot]

📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17842-34085d4.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	WordPress
	Build Flavor	Jalapeno
	Build Type	Debug
	Commit	34085d4

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

[wpmobilebot]

📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17842-34085d4.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	Jetpack
	Build Flavor	Jalapeno
	Build Type	Debug
	Commit	34085d4

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

[ParaskP7]

👋 @irfano !

First of all, a big bravo for working on this compileSdkVersion = 33 upgrade and creating this PR, this work of yours will unblock many things for us, and it wasn't easy, I bet! 💯 🙇 ❤️

---

This contains a lot of changes, but they are small and similar. I could publish this via multiple PRs, but multiple PRs would create a burden of merging in order and creating multiple branches based on others. I believe it won't be hard to review this if you follow the commits.

Ah, actually, I would have still advised you to create multiple PRs. You will understand why I am saying that after you read through my code review change requests and comments below.

FYI: It took me almost a day to review this PR and more so to gather some feedback for you, that is, in a way that can make it easier for you to apply any of those changes I identified and can recommend to you. Also, I created some code batches for you, just to make it that bit easier for you to review those and apply them.

---

Another FYI, the fact that some commits, especially the big ones (9446c7f and d8deb99) weren't split into multiple commits, made it a bit more difficult for me to review this PR, so apologies for the delay.

• For example, for the SiteCreationActivity change, included in 9446c7f, you could have split the changes there into two commits, one for the SAM changes, and one for the main onBackPressed change.
• Another example, for the DomainSuggestionsFragment change, included in d8deb99, you could have split the changes there into 2 commits, one for the DI changes, and one for the main CompatExtensions change.

For the future, I would really recommend that kind of changes to be split into multiple commits, just so that to keep the main commit on focus on the actual change that warranties a bit more thoughtful code review in case things got slipped.

---

I reviewed this PR but not tested it as much as I would have liked. But, I did that on purpose, this is because:

1. Ideally, I would like us to bring a QE to the rescue here and have them test this PR thoroughly as well. For example, I did the same with the AndroidX Core dependency updates (see pdcxQM-1HP-p2) and I think this update deserves the same level of testing before merging. There are lots of changes here and even if they don't seem to big, they have the potential to break things.
2. I have some a few code review change requests for you. I would like them to be handled first before I can begin the testing part on my side.

---

Request Changes:

1. Warning (⚠️): For the d8deb99 change, I suggest redoing that change, almost reverting half of it. Instead, I recommend explicitly adding the requireNotNull(...) on all the changes that require a non-null value, that is, instead of guarding that and checking for nullability before proceeding forward. This is because by guarding that line and then checking whether that value is not null, instead of crashing the app, the flow will continue, but that will create more confusion and unexpected behavior later on. It is better to crash the app at that point, just like it would have crashed even before this update anyway. Wdyt? PS: I have created a patch for you on that just to make those change a bit more easier on you.

[Compile_SDK_33]_Compat_Extensions_Related_Code_Review_Changes.patch

2. Suggestion (💡) I also recommend you add the same CompatExtensions.kt file on the image-editor lib module and then use that on CropViewModel.kt and PreviewImageFragment.kt, the same way you did for the WordPress module. I understand that you didn't do it because you didn't want to create a duplication of extension files, but I believe it is better you do that now and then when the 1.10.0 becomes ready, to delete both files and use those IntentCompat and BundleCompat instead. Thus, I believe it is better to have those 2 temp extension files for now, as this will mean that when 1.10.0 becomes ready there would be no code changes, nowhere, only import changes. Wdyt? PS: The patch above includes this suggestion change as well.
3. Suggestion (💡): Consider explicitly setting the type on all the CompatExtensions.kt function calls, that is, instead of let it be referred sometimes. PS: The patch above includes this suggestion change as well.
4. Warning (⚠️): I did search for all usages of onBackPressed in order to verify that all of them have been updated away from their deprecated usage and into the new recommended custom back navigation, using the onBackPressedDispatcher. I found a few instances where this wasn't done and I recommend those Kotlin related activities and fragments to get that update too, no matter if the build doesn't complain about it (for an unknown to me reason). I also recommend we do the same change on all the Java related activities and fragments, no matter if our build succeeds without failing, just because it only complains on Kotlin classes, the idea is the same, later on, newer APIs and updates would no longer allow for these deprecated calls. Thus, if we can update them now, we should do that, especially given the fact that there are not that many such Java classes anyway, about 10 of them. Wdyt? PS: I have created a patch for you on that just to make those change a bit more easier on you, excluding the Java related changes.

[Compile_SDK_33]_On_Back_Pressed_Related_Code_Review_Changes.patch

5. Suggestion (💡): Also, I recommend refactor some classes that are using the old way of utilizing the onBackPressedDispatcher callback, like the LoginNoSitesFragment class. PS: The patch above includes this suggestion change as well.
6. Suggestion (💡): Consider reusing the PackageManagerWrapper.getPackageInfo(...) helper function instead of duplicating this logic in multiple places. I also suggest creating another such PackageManagerWrapper.getActivityInfo(...) helper function and reusing that were necessary. Wdyt? PS: I have created a patch for you on that just to make those change a bit more easier on you.

[Compile_SDK_33]_Package_Manager_Related_Code_Review_Changes.patch

---

I'll avoid writing any more comments for now, that is, until we discuss all the above first.

PS: Apologies for the wall of text and for adding lots on your plate before giving the 🟢 to merge this to trunk. But, I do believe this it is better for us to be careful with this change and invest some extra time to make sure this change is (a.) safe for merging and (b.) includes all of the necessary updates. I am here to support you on all that, all the way, so feel free to reach out for help with whatever you might need. 💯

[ParaskP7]

Minor (🔍): Consider using it.mutate instead.

[ParaskP7]

Minor (🔍): Consider making this lambda a one-liner.

[ravishanker]

I could publish this via multiple PRs, but multiple PRs would create a burden of merging in order and creating multiple branches based on others.

Enormous effort @irfano 👏

Is this supposed to go on a separate branch at first, not on the trunk direct!

Would it compile though, if it is split into multiple PRs (As @ParaskP7 suggested), unless the changes required are made first in separate PRs and then upgrade the compileSdkVersion at last.

[ParaskP7]

👋 @ravishanker @irfano !

Would it compile though, if it is split into multiple PRs (As @ParaskP7 suggested), unless the changes required are made first in separate PRs and then upgrade the compileSdkVersion at last.

Actually, you can do this the other way around, create a feature branch with the following "required" changes first:

1. compileSdkVersion = 33
2. kotlinOptions -> allWarningsAsErrors = false
3. AnimatorListener -> b767c46 (errors that need resolving, not warnings)
4. MenuItem -> 44dfcad (errors that need resolving, not warnings)
5. SimpleOnGestureListener -> cf40f4e (errors that need resolving, not warnings)

Maybe the above should be a PR of its own, so as for it to get reviewed first before it become the "feature branch" that all other PRs will target.

With the above, the project will compile. Then, you could create multiple PRs on top of those changes, PRs that are not depending to each other, but directly targeting this feature branch to resolve the other "major" warnings:

1. Resolve onBackPressed warnings -> 9446c7f (1st PR)
2. Resolve Intent and Bundle warnings -> d8deb99 (2nd PR)
3. Resolve every other warning left. (3rd PR)

Upon those 3 PRs merged into that feature branch, then revert the kotlinOptions -> allWarningsAsErrors = false change, flip the flag to back true and make this PR ready for review. This "feature branch" PR that will target trunk should not be reviewed again, as all the code changes should be already reviewed and smoke tested by the other PRs. However, I would recommend to request from a QE to do one final thorough testing before merging it to trunk, just like I did with this PR and that pdcxQM-1HP-p2 QE request.

Let me know if the above makes sense. 🙏

PS: With the above, I am just sharing one alternative strategy to progress with such work. That doesn't mean that I would like us to redo the work that was already done in this PR. We can progress with this PR now to keep things flowing. This is just an FYI for any future such work.

[irfano]

Thank you for your reviews, @ParaskP7 and @ravishanker! ❤️

---

Resolve onBackPressed warnings -> 9446c7f (1st PR)
Resolve Intent and Bundle warnings -> d8deb99 (2nd PR)

If I did this, I'd need to fix the conflict to merge the 2nd PR after the 1st PR is merged. I was about to start the rotation, and I wouldn't be able to work on conflicts. That's why I wanted to publish this work with a single PR. But after @ParaskP7's comments, I saw that multiple PRs would be better.

I'm converting this PR to the draft. As Petros suggested, I'll divide this PR into multiple PRs by making changes Petros requested next week after my rotation.

[ParaskP7]

👋 @irfano !

If I did this, I'd need to fix the conflict to merge the 2nd PR after the 1st PR is merged. I was about to start the rotation, and I wouldn't be able to work on conflicts. That's why I wanted to publish this work with a single PR. But after @ParaskP7's comments, I saw that multiple PRs would be better.

👍

I'm converting this PR to the draft. As Petros suggested, I'll divide this PR into multiple PRs by making changes Petros requested next week after my rotation.

Coolio, deal, but don't just do that because I suggested it. If you feel that it is better to included all these changes in this PR, please do that instead. No matter way you chose to progress with this work, I am here to support you all the way! 🚀 ❤️

THANK YOU! 🙇 🥇 🙏

[ParaskP7]

👋 @irfano !

Maybe you would like to close this draft PR now that #17790 is merged? 📕

[irfano]

I am closing this draft PR because the work has been reorganized, improved, and merged in #17947.