Create an app-wide `Disposable` to be used for app-wide actions

Stypox commented 2 years ago

Checklist

[X] This issue contains only one bug.
[X] I am able to reproduce the bug with the latest version.
[X] I have read and understood the contribution guidelines.
[X] I made sure that there are no existing issues - open or closed - which I could contribute my information to.
[X] I have taken the time to fill in all the required details. I understand that the bug report will be dismissed otherwise.

Affected version

After (but also before) #7570

The problem

In the NewPipe codebase there are many methods that use RxJava to perform resource/time/io-heavy actions in the background. When these actions finally result in a change in the activity/fragment/dialog that originally triggered them, then an RxJava Disposable should be passed to those functions, so that if the activity/fragment/dialog is dismissed before the action can complete, the action can be dispose()d. This is what is correctly being done in the text linkifier class: they take a text view and set its text once it has been linkified, but if in the meantime the activity/fragment/dialog associated to the text view was destroyed, the linkification of the text will stop (otherwise it would result in calling setText() on a non-existing text view).

Sometimes, though, an action is not really associated with the activity/fragment/dialog that triggered it. If the user want to start playing something in a popup, he may tap on the popup button in the long-press menu of the feed fragment, and then maybe he instantly (i.e. before the popup player had a chance to load) closes the feed fragment to switch to another app. If the disposable for playing in popup was added to the Disposables associated with the feed fragment, the loading of the popup player would stop. So currently what is being done instead is basically just not saving disposables anywhere so they are never disposed, for example checkout this. Though this behavior mostly works fine, I don't think it is a good practice and may lead to problems, so I think we should create an app-wide Disposable to which all disposables related to app-wide actions can be added.

AudricV commented 8 months ago

@Stypox As we want to rewrite the app and migrate it to Kotlin and use coroutines, is this issue still relevant?

Stypox commented 8 months ago

No, and it's also a bad idea. We should use dependency injection.

TeamNewPipe / NewPipe