[Android,VideoFling] Never add null players to alternative ones

chrome/browser/media/android/remote/remote_media_player_manager.cc

Index: chrome/browser/media/android/remote/remote_media_player_manager.cc
diff --git a/chrome/browser/media/android/remote/remote_media_player_manager.cc b/chrome/browser/media/android/remote/remote_media_player_manager.cc
index 90b70ea60454f890deffdc2cdd9d74a486e4b345..5d21245cb1ee120a5aa4a3d1f5a57d8cc1d3a777 100644
--- a/chrome/browser/media/android/remote/remote_media_player_manager.cc
+++ b/chrome/browser/media/android/remote/remote_media_player_manager.cc
@@ -150,6 +150,9 @@ void RemoteMediaPlayerManager::SwapCurrentPlayer(int player_id) {
   MediaPlayerAndroid* new_player = *it;
   std::unique_ptr<MediaPlayerAndroid> old_player =
       SwapPlayer(player_id, new_player);
+  if (!old_player)

[dgn, 2016/04/18 13:18:49]

In which case can this be null? Is it safe to run SwapPlayer even though you are
not going to be able to swap back later? Wouldn't it be better to find the
player first?

[whywhat, 2016/04/18 13:21:51]

SwapPlayer doesn't do anything if it can't find the player to swap - it will
return null if it doesn't find one. As I understand, that would happen if we
have a player id that's out of sync with the C++/renderer side. Before the
refactoring mentioned in the bug I think we did nothing in the case player id
wasn't found in the list of players. After that we potentially store null
pointers sometimes, leading to the crashes causing the bug.

[aberent, 2016/04/18 13:44:57]

Is all this one line too high? It seems that this code would leave |new_player|
as both the current player and the alternative player. I think we want to remove
it as the alternative player, otherwise things will get confused the next time
this is called.

I think (but haven't fully checked) that all calls to GetAlternativePlayer check
for not getting one.

[whywhat, 2016/04/18 14:54:45]

Good point, remove and delete the alternative player now if the current player
is not found.

+    return;
+
   alternative_players_.weak_erase(it);
   alternative_players_.push_back(old_player.release());
 }