[HOLD for payment 2023-01-05] [HOLD #13392] [$1000] Android - Avatar is not displayed on push notification

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue found when executing PR #12540

Action Performed:

1. Sign into the Android app
2. Sign into another account on web and upload a custom profile picture for that account
3. Background the Android app
4. Send a message from the account signed in on web to the account signed in on Android
5. Verify that a push notification appears and that the notification appears a message style notification, including the avatar image of the web user.

Expected Result:

Avatar image of the web user is displayed after received a push notification in Android app

Actual Result:

Avatar image of the web user isn't displayed after received a push notification in Android app

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Android

Version Number: 1.2.30.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20221123-115704_New.Expensify.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0171d7d835c1969527
• Upwork Job ID: 1598107016845291520

[melvin-bot]

Triggered auto assignment to @arielgreen (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

@arielgreen Eep! 4 days overdue now. Issues have feelings too...

[arielgreen]

Bug0 Triage Checklist

Note: see this SO for more information.

• The "bug" is actually a bug
• The bug is not a duplicate report of an existing GH.
  • If it is, close the GH and add any novel details to the original GH instead
• The bug is reproducible, following the reproduction steps.
  • If the GH doesn’t have steps to reliably reproduce the bug and you figure it out, then please add them.
  • If you’re unable to reproduce the bug, add the Needs reproduction label.
  • Comment on the issue outlining the steps you took to try to reproduce the bug, your results and tag the issue reporter and the Applause QA member who created the issue. Ask them to clarify reproduction steps and/or to check the reproduction steps again. Close issue.
• The GH template is filled out as fully as possible
  • The GH body and title are clear (ie. could an external contributor understand it and work on it?)
• The GH was created by an Expensify employee or a QA tester
• If the bug is an OldDot issue, you should decide whether it is widespread enough to justify fixing or it is better to wait for reunification. If it’s better to wait, close the GH & provide this justification
• If there's a link to Slack, check the discussion to see if we decided not to fix it
• Decide if the GH should be resolved by an External contributor or Internal engineer, add the appropriate label

[melvin-bot]

Current assignee @arielgreen is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0171d7d835c1969527

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)

[melvin-bot]

Triggered auto assignment to @robertjchen (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[arielgreen]

Reproduced following the steps in OP:

[StefanNemeth]

I could reproduce the issue as well on the debug build and started to debug the issue.

The following error occurred when fetching the icon and appears to occur on Android O (onwards):

12-01 17:21:31.539 24579 24622 E NotificationProvider: Failed to fetch icon
12-01 17:21:31.539 24579 24622 E NotificationProvider: java.lang.IllegalArgumentException: Software rendering doesn't support hardware bitmaps
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at android.graphics.BaseCanvas.throwIfHwBitmapInSwMode(BaseCanvas.java:706)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at android.graphics.BaseCanvas.throwIfCannotDraw(BaseCanvas.java:81)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at android.graphics.BaseCanvas.drawBitmap(BaseCanvas.java:139)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at android.graphics.Canvas.drawBitmap(Canvas.java:1604)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.expensify.chat.customairshipextender.CustomNotificationProvider.getCroppedBitmap(CustomNotificationProvider.java:110)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.expensify.chat.customairshipextender.CustomNotificationProvider.fetchIcon(CustomNotificationProvider.java:265)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.expensify.chat.customairshipextender.CustomNotificationProvider.applyMessageStyle(CustomNotificationProvider.java:151)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.expensify.chat.customairshipextender.CustomNotificationProvider.onExtendBuilder(CustomNotificationProvider.java:76)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.urbanairship.push.notifications.AirshipNotificationProvider.onCreateNotification(AirshipNotificationProvider.java:210)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.urbanairship.push.IncomingPushRunnable.postProcessPush(IncomingPushRunnable.java:196)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.urbanairship.push.IncomingPushRunnable.processPush(IncomingPushRunnable.java:144)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.urbanairship.push.IncomingPushRunnable.run(IncomingPushRunnable.java:97)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:463)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at java.util.concurrent.FutureTask.run(FutureTask.java:264)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1137)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:637)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at com.urbanairship.util.AirshipThreadFactory$1.run(AirshipThreadFactory.java:50)
12-01 17:21:31.539 24579 24622 E NotificationProvider: 	at java.lang.Thread.run(Thread.java:1012)

To resolve the issue, we could convert the avatar to the desired bitmap (as used by the canvas we create in getCroppedBitmap)

// android/app/src/main/java/com/expensify/chat/customairshipextender/CustomNotificationProvider.java
public Bitmap getCroppedBitmap(Bitmap bitmap) {
        // Convert to ARGB_8888
        bitmap = bitmap.copy(Config.ARGB_8888, true);

        Bitmap output = Bitmap.createBitmap(bitmap.getWidth(),
                bitmap.getHeight(), Config.ARGB_8888);
        Canvas canvas = new Canvas(output);
        // ..
}

That would resolve the bug as seen here.

[robertjchen]

@StefanNemeth Thanks for the proposal, it looks like this is indeed the source of the issue! Just out of curiosity, is there a reason why we don't use PixelCopy? https://bumptech.github.io/glide/doc/hardwarebitmaps.html

[StefanNemeth]

@StefanNemeth Thanks for the proposal, it looks like this is indeed the source of the issue! Just out of curiosity, is there a reason why we don't use PixelCopy? https://bumptech.github.io/glide/doc/hardwarebitmaps.html

According to the documentation PixelCopy allows for copy operations from Surface to Bitmap (e.g. for screenshots). We are dealing with bitmaps here so bitmap.copy is the proper way to do it afaik.

[robertjchen]

Got it, that makes sense. @parasharrajat What are your thoughts on the above proposal? 🙏

[parasharrajat]

I am not an Android native guy but can we just use bitmap.setConfig(Config.ARGB_8888) before drawing? it seems that it will have the same effect.
https://github.com/Expensify/App/blame/main/android/app/src/main/java/com/expensify/chat/customairshipextender/CustomNotificationProvider.java#L107

I don't see where we create the Bitmap with Hardware config. It may be coming directly from AirShip.

[StefanNemeth]

Practically it will have the same effect, yes. However, I chose to use the immutable copy option as it is usually bad practice to alter input objects in a function like getCroppedBitmap as a hidden side-effect and the performance impact of copying is negligible. I'd be good with both options though.

Edit: Actually it won't work with bitmap.setConfig as only mutable bitmaps can be reconfigured, which the given one is not.

[parasharrajat]

Ok, Thanks for the explanation. I am good with both. I have asked internally to review it before we proceed.

[parasharrajat]

Ok @StefanNemeth 's proposal looks good to me.

Cc: @robertjchen

🎀 👀 🎀 C+ reviewed

[dylanexpensify]

@StefanNemeth offer sent! @parasharrajat please apply when you get a moment 😄

[dylanexpensify]

ah, @robertjchen looks like @arielgreen is the BZ member for this 😂

[robertjchen]

@dylanexpensify Ah, my bad! I was getting these issues mixed up 😵

[parasharrajat]

PR is on HOLD for #13392.

[melvin-bot]

@robertjchen, @StefanNemeth, @parasharrajat, @arielgreen Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[parasharrajat]

Having issues signing in the app. #13392 is done so we can remove the HOLD.

[melvin-bot]

@robertjchen, @StefanNemeth, @parasharrajat, @arielgreen Whoops! This issue is 2 days overdue. Let's get this updated quick!

[parasharrajat]

This is no more on hold and is ready for merge. Someone, please remove the HOLD from the title.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.45-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Convert hardware bitmap to software bitmap for Android notification avatar cropping #13391

If no regressions arise, payment will be issued on 2023-01-05. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter N/A
• Contributor that fixed the issue @StefanNemeth
• Contributor+ that helped on the issue and/or PR @parasharrajat

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@parasharrajat / @robertjchen] The PR that introduced the bug has been identified. Link to the PR: Icon rounding issue resolved #6770
• [@parasharrajat / @robertjchen] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: Icon rounding issue resolved #6770 (comment)
• [@parasharrajat / @robertjchen] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: https://expensify.slack.com/archives/C049HHMV9SM/p1673432839280069
• [@arielgreen] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: https://github.com/Expensify/Expensify/issues/256144

[StefanNemeth]

Am I eligible for the 50% bonus as the PR created within a few hours after assignment was held?

[arielgreen]

@StefanNemeth yes, I think that is fair. Paid. @parasharrajat sending offer now.

[parasharrajat]

[@parasharrajat / @robertjchen] The PR that introduced the bug has been identified. Link to the PR:

#6770

[@parasharrajat / @robertjchen] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

#6770 (comment)

[@parasharrajat / @robertjchen] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

https://expensify.slack.com/archives/C049HHMV9SM/p1673432839280069

@robertjchen Please update the checklist at #12967 (comment)

[Julesssss]

[@parasharrajat / @robertjchen] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

it's not clear to me what exactly caused the regression in the linked PR. I recall that notification profile images have been broken for weeks/months now. Perhaps they were recently fixed? But I can't recall.

[Julesssss]

Ahhh, I see. That PR was January 2022, not 2023 😄

[arielgreen]

All payments have been issued.

[arielgreen]

Just need to look into whether we can create a regression test and then this will be good to close out.

[Julesssss]

Hey @arielgreen I think this should be added as a new regression test, as it ensures we test for custom profile pics as well as the default icons:

Ensure rounded profile images are displayed for notifications

• Android and iOS only
• Sign in as a new account (userA) on the test platform (iOS/Android)
• Sign in as userB on a different device/browser/platformfor that account
• Background the app as userA
• Send a message from userB to userA
• Verify that a push notification appears for userA and that the notification includes a rounded default avatar
• As userB upload a custom profile picture for that account
• Send a message from userB to userA
• Verify that a push notification appears for userA and that the notification includes a rounded version of the custom profile image

[arielgreen]

Discussing regression test here.

[arielgreen]

https://github.com/Expensify/Expensify/issues/256144