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

Using static method PlayerView#switchTargetView instead of PlayerView#setPlayer() to update Player/PlayerView #30

Open
eneim opened this issue Oct 31, 2018 · 0 comments

Comments

@eneim
Copy link

eneim commented Oct 31, 2018

Here in this line
You directly call the setter of PlayerView to set its Player instance. There is a flaw in this, as below.

Consider the following scenario:

ExoPlayerManager has a non-null playerView, and client call playerManager.inject(otherPlayerView).

Current behaviour: immediately call otherPlayerView.player = player, and not consider to set the Player of old PlayerView to null.
Side effect: the old PlayerView instance is still holding the same Player instance, along with the ComponentListener added to it. ComponentListener instance is an inner non-static class of PlayerView, so without proper removing/releasing, the Player instance will hold a strong reference of this ComponentListener, and also the old PlayerView for longer than expected. Finally this can result in a memory leak.

Suggested change: using PlayerView.switchTargetView(player, oldPlayerView, newPlayerView) helper static method is a simple way to make it right. It simply set the Player instance of old PlayerView to null before updating new PlayerView instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant