ExoPlayerManager icon indicating copy to clipboard operation
ExoPlayerManager copied to clipboard

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

Open eneim opened this issue 7 years ago • 0 comments

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.

eneim avatar Oct 31 '18 06:10 eneim