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

android notifs: Fix regression where notifs dropped when avatar not fetched #5138

Merged
merged 2 commits into from
Nov 23, 2021

Conversation

chrisbobbe
Copy link
Contributor

IconCompat.createWithBitmap [1] expects a non-null Bitmap object.
Since abd605d, we've been directly passing the value returned from
our own fetchBitmap function, which sometimes returns null. It
seems like the symptom would be a failure to deliver a notification
when the value is null.

So, go back to not calling IconCompat.createWithBitmap unless we
get a non-null value from fetchBitmap.

See discussion at
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/sentry.20IllegalArgumentException.3A.20Bitmap.20must.20not.20be.20null.2E/near/1283842.

[1] https://developer.android.com/reference/androidx/core/graphics/drawable/IconCompat#createWithBitmap(android.graphics.Bitmap)

chrisbobbe and others added 2 commits November 23, 2021 12:56
…tched

`IconCompat.createWithBitmap` [1] expects a non-null Bitmap object.
Since abd605d, we've been directly passing the value returned from
our own `fetchBitmap` function, which sometimes returns null. It
seems like the symptom would be a failure to deliver a notification
when the value is null.

So, go back to not calling `IconCompat.createWithBitmap` unless we
get a non-null value from `fetchBitmap`.

See discussion at
  https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/sentry.20IllegalArgumentException.3A.20Bitmap.20must.20not.20be.20null.2E/near/1283842.

[1] https://developer.android.com/reference/androidx/core/graphics/drawable/IconCompat#createWithBitmap(android.graphics.Bitmap)
This makes the boring parts of this pattern a bit more compact
(just a single line at the start and another at the end), and
the same as we do for the notification builders below.
@gnprice gnprice merged commit 503b06a into zulip:main Nov 23, 2021
@gnprice
Copy link
Member

gnprice commented Nov 23, 2021

Thanks for debugging and fixing this! Looks good -- merged, with a small formatting tweak on top.

@chrisbobbe
Copy link
Contributor Author

Thanks for the review and the formatting tweak!

@chrisbobbe chrisbobbe deleted the pr-fetch-bitmap-null branch November 24, 2021 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants