Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(messaging, android): Replace deprecated AsyncTask API and other deprecated API #12580

Merged
merged 12 commits into from Apr 15, 2024

Conversation

russellwheatley
Copy link
Member

@russellwheatley russellwheatley commented Apr 3, 2024

Description

Done

  • Replaced deprecated AsyncTask.
  • Replaced deprecated getParcelableExtra() and replaced with passing Parcel through Intent for background/quit app states.
  • Removed some unused code.

I tested messages received in open/background/quit and also when opening app with message. Working as intended.

Related Issues

Part of: #8073

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@russellwheatley russellwheatley marked this pull request as ready for review April 4, 2024 12:49
byte[] parcelBytes =
intent.getByteArrayExtra(FlutterFirebaseMessagingUtils.EXTRA_REMOTE_MESSAGE);
if (parcelBytes != null) {
Parcel parcel = Parcel.obtain();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot fail? Or it should be part of the try/catch?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should never fail.


public void execute() {
executor.execute(
new Runnable() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we store this runnable to cancel it manually inside cancel() if needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is necessary, it is a like for like replacement for deprecated AsyncTask. The execute(Runnable) is running in the background thread (like doInBackground for AyncTask). When that work is complete, it calls the processorFinished() finished on the main thread (like onPostExecute for AsyncTask). See documentation: https://developer.android.com/reference/android/os/AsyncTask#the-4-steps

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add to that, the only time cancel is called using previous AsyncTask, we also call cancel() here using latest implementation: https://github.com/firebase/flutterfire/pull/12580/files#diff-3d9ef1ece89413638d5bc6907c220242d3b2f489db5900782615c504073437f1R561

@russellwheatley russellwheatley merged commit ac089e5 into master Apr 15, 2024
21 checks passed
@russellwheatley russellwheatley deleted the messaging-deprecations-8073 branch April 15, 2024 07:26
@firebase firebase locked and limited conversation to collaborators May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants