NewPipe icon indicating copy to clipboard operation
NewPipe copied to clipboard

Keep strong references to Picasso notification icon loading targets

Open Stypox opened this issue 3 years ago • 1 comments

What is it?

  • [x] Bugfix (user facing)
  • [ ] Feature (user facing)
  • [ ] Codebase improvement (dev facing)
  • [ ] Meta improvement to the project (dev facing)

Description of the changes in your PR

Before the Target would sometimes be garbage collected before being called with the loaded channel icon, since Picasso holds weak references to targets. This meant that sometimes a new streams notification would not be shown, because the lambda that should have shown it had already been garbage collected.

I don't know if there was a way to properly reproduce something wrong with the previous behavior, but Picasso clearly states in RequestCreator#into(Target):

This method keeps a weak reference to the Target instance and will be garbage collected if you do not keep a strong reference to it.

Anyway, I tested the new code by subscribing to the usual Roel Van De Paar and it works.

Note that this is also one of the causes of a missing thumbnail in the notification, that will be fixed with #8678.

Fixes the following issue(s)

None that I could find

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

Stypox avatar Jul 22 '22 13:07 Stypox

Yes, just subscribe to Roel Van De Paar and enable its notifications, wait for a few minutes (until he uploads a new video, or find a channel which uploads more frequently), then manually trigger the streams notification update from debug settings.

Stypox avatar Jul 26 '22 16:07 Stypox