nextcloud / news-android

📱🗞️ Android client for the Nextcloud news/feed reader app
https://play.google.com/store/apps/details?id=de.luhmer.owncloudnewsreader
GNU Affero General Public License v3.0
699 stars 257 forks source link

Use one Glide instance per NewsListRecyclerAdapter #1410

Closed Unpublished closed 8 months ago

Unpublished commented 8 months ago

Fixes the issue mentioned in #1405 without each view having its own glide instance

Also fixes leaking context via static reference to FavIconHolder


More infos:

Ref: https://github.com/nextcloud/news-android/pull/1405#issuecomment-2029563384 I think the main culprit is having a static FavIconHandler which leaks context.

Using a per fragment instance approach for Glide will still benefit from Glide's lifecycle awareness, pausing requests of not visible fragments. Also fragments context should be aware of configuration changes like theme AFAIK.

Unpublished commented 8 months ago

@DoHe Can you double check if this branch fixes the issue for you as well?

DoHe commented 8 months ago

@Unpublished yes, I can confirm this fixes the favion issues I was seeing 👍

David-Development commented 8 months ago

@Unpublished Thank you for implementing this so quickly! Looks like a great solution to me. I'm glad that you were able to remove the static reference.