Refactor Clank cast connect logic

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

Index: chrome/browser/media/android/remote/remote_media_player_manager.h
diff --git a/chrome/browser/media/android/remote/remote_media_player_manager.h b/chrome/browser/media/android/remote/remote_media_player_manager.h
index 1cbe8fd9e7b9ffeb396c2ad1f0ed6788f97a2842..ae3e6dab59823697d5821b1f1f859cbf9d1fdfeb 100644
--- a/chrome/browser/media/android/remote/remote_media_player_manager.h
+++ b/chrome/browser/media/android/remote/remote_media_player_manager.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_MEDIA_ANDROID_REMOTE_REMOTE_MEDIA_PLAYER_MANAGER_H_
 #define CHROME_BROWSER_MEDIA_ANDROID_REMOTE_REMOTE_MEDIA_PLAYER_MANAGER_H_
 
+#include <set>
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
@@ -28,9 +29,6 @@ class RemoteMediaPlayerManager : public content::BrowserMediaPlayerManager {
   void OnPlaying(int player_id);
   void OnPaused(int player_id);
 
-  // Callback to trigger when a remote device has been selected.
-  void OnRemoteDeviceSelected(int player_id);
-
   // Callback to trigger when a remote device has been unselected.
   void OnRemoteDeviceUnselected(int player_id);
 
@@ -46,6 +44,11 @@ class RemoteMediaPlayerManager : public content::BrowserMediaPlayerManager {
                               int height,
                               bool success) override;
 
+  // Swap which player is currently in use (local or remote).
+  void SwitchToRemotePlayer(int player_id);
+  void SwitchToLocalPlayer(int player_id);
+  void OnPlayingRemotely(int player_id, const std::string& casting_message);
+
  protected:
   void OnSetPoster(int player_id, const GURL& url) override;
 
@@ -54,18 +57,9 @@ class RemoteMediaPlayerManager : public content::BrowserMediaPlayerManager {
   RemoteMediaPlayerBridge* CreateRemoteMediaPlayer(
       media::MediaPlayerAndroid* local_player);
 
-  // Replaces the given local player with the remote one. Does nothing if the
-  // player is remote already.
-  void ReplaceLocalPlayerWithRemote(media::MediaPlayerAndroid* player);
-
   // Replaces the remote player with the local player this class is holding.
   // Does nothing if there is no remote player.
-  void ReplaceRemotePlayerWithLocal();
-
-  // Checks if the URL managed by the player should be played remotely.
-  // Returns true if the manager should do nothing, false if it needs to
-  // proceed.
-  bool MaybeStartPlayingRemotely(int player_id);
+  void ReplaceRemotePlayerWithLocal(int player_id);
 
   // content::BrowserMediaPlayerManager overrides.
   void OnStart(int player_id) override;
@@ -73,6 +67,8 @@ class RemoteMediaPlayerManager : public content::BrowserMediaPlayerManager {
       const MediaPlayerHostMsg_Initialize_Params& media_player_params) override;
   void OnDestroyPlayer(int player_id) override;
   void OnReleaseResources(int player_id) override;
+  void OnSuspend(int player_id) override;
+  void OnResume(int player_id) override;
   void OnRequestRemotePlayback(int player_id) override;
   void OnRequestRemotePlaybackControl(int player_id) override;
 
@@ -91,18 +87,26 @@ class RemoteMediaPlayerManager : public content::BrowserMediaPlayerManager {
   // -1 in case something goes wrong.
   int GetTabId();
 
-  // Get the player id of current remote player, if any, or -1 if none.
-  int RemotePlayerId();
+  // Get the player that is not currently selected
+  ScopedVector<media::MediaPlayerAndroid>::iterator GetAlternativePlayer(

[dgn, 2015/10/14 16:05:49]

Return media::MediaPlayerAndroid* instead, and nullptr if there is none? It
doesn't look like usage makes a difference between the entry being erased and
having never existed in the first place.

[aberent, 2015/10/15 10:31:38]

No, I need the iterator in SwapCurrentPlayer, If I return MediaPlayerAndroid*
then I have to duplicate this code in SwapCurrentPlayer.

[dgn, 2015/10/15 11:07:31]

OK I missed that.
nit: How about having 2 methods then? returning a foo_list::iterator from getFoo
is weird and feels like leaking implementation details IMO. But it's a private
method so that's OK. 

[aberent, 2015/10/15 12:57:54]

Seems unnecessary. Not done.

+      int player_id);
 
   // Get the remote player for a given player id, whether or not it is currently
   // playing remotely.
   RemoteMediaPlayerBridge* GetRemotePlayer(int player_id);
 
-  // The local player that we have replaced with a remote player. This is NULL
-  // if we do not have a remote player currently running.
-  scoped_ptr<media::MediaPlayerAndroid> replaced_local_player_;
+  // Get the local player for a given player id, whether or not it is currently
+  // playing locally.
+  media::MediaPlayerAndroid* GetLocalPlayer(int player_id);
+
+  void SwapCurrentPlayer(int player_id);
+
+  // Contains the alternative players that are not currently in use, i.e. the
+  // remote players for videos that are playing locally, and the local players
+  // for videos that are playing remotely.
+  ScopedVector<media::MediaPlayerAndroid> alternative_players_;
 
-  ScopedVector<RemoteMediaPlayerBridge> remote_players_;
+  std::set<int> players_playing_remotely_;
 
   base::WeakPtrFactory<RemoteMediaPlayerManager> weak_ptr_factory_;