Revert "Defensive fix for crash in MediaWebContentsobserver"

This reverts commit 79de0d4b.

Reason for revert: We expect another CL may be the correct fix. https://chromium-review.googlesource.com/c/chromium/src/+/2626920

Original change's description:
> Defensive fix for crash in MediaWebContentsobserver
>
> This defensive fix until MediaSession owners return from vacation.
> Reverting is challenging given the long string of CLs.
>
> GetMediaPlayerRemote(player_id) is crashing for failing to find the
> remote with that ID. The stack shows the call coming from
> MediaSessionController, which should have the same lifetime as its
> associated mojo::Remote<media::mojom::MediaPlayer>. So far, I have not
> found how those lifetimes are getting out of sync.
>
> Ownership looks like this (read -> as "owns", MWCO as MediaWebContentsObserver):
> MWCO -> MediaSessionControllersManager -> map<MediaPlayerId, MediaSessionController(s)>
> MWCO -> map<MediaPlayerId, mojo::Remote<media::mojom::MediaPlayer>>
>
> I've combed through all the places where MWCO removes from its map of
> MediaPlayer remotes, and looked for a failure to call
> MediaSessionControllersManger::OnEnd(Id) to remove the associated
> MediaSessionController. There are 3 cases where we remove from the
> map...
> 1. MediaWebContentsObserver::RenderFrameDeleted()
> 2. MediaWebContentsObserver::WebContentsDestroyed()
> 3. media_player_remotes_[player_id].set_disconnect_handler(base::BindOnce(...
>
> The only case where I don't see a call to
> MediaSessionControllersManager is #2. But that shouldn't matter since
> MWCO's destructor is called shortly (synchronously) after
> WebContentsDestroyed(). Its destructor will tear down the
> MediaSessionControllersManager.
>
> The defensive fix does not address the root lifetime issue. It is
> probably the case that the MediaSessionControllers are being leaked.
>
> Bug: 1161551, 1039252
> Change-Id: I85337eae106d8b56b7d48fc9759702dc579855ca
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2607579
> Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
> Commit-Queue: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Auto-Submit: Chrome Cunningham <chcunningham@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#839818}

TBR=dcheng@chromium.org,mlamouri@chromium.org,chcunningham@chromium.org,liberato@chromium.org,beccahughes@chromium.org,mario@igalia.com,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1161551
Bug: 1039252
Change-Id: Id12ba170758a4479342c0eb3b2c64bb1243f34b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2632919Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844208}

content/browser/media/media_web_contents_observer.cc

@@ -528,14 +528,11 @@ void MediaWebContentsObserver::OnReceivedTranslatedDeviceId(
       MediaPlayerId(render_frame_host, delegate_id), raw_device_id);
 }
 
-media::mojom::MediaPlayer* MediaWebContentsObserver::GetMediaPlayerRemote(
-    const MediaPlayerId& player_id) {
-  if (media_player_remotes_.contains(player_id)) {
-    DCHECK(media_player_remotes_[player_id].is_bound());
-    return media_player_remotes_.at(player_id).get();
-  }
-
-  return nullptr;
+mojo::Remote<media::mojom::MediaPlayer>&
+MediaWebContentsObserver::GetMediaPlayerRemote(const MediaPlayerId& player_id) {
+  DCHECK(media_player_remotes_.contains(player_id));
+  DCHECK(media_player_remotes_[player_id].is_bound());
+  return media_player_remotes_.at(player_id);
 }
 
 void MediaWebContentsObserver::OnMediaPlayerHostDisconnected(

content/browser/media/media_web_contents_observer.h

@@ -110,11 +110,9 @@ class CONTENT_EXPORT MediaWebContentsObserver : public WebContentsObserver {
                                     int delegate_id,
                                     const std::string& raw_device_id);
 
-  // Return an already bound mojo Remote for the MediaPlayer mojo interface.
-  // Return null if no player with |player_id| exists.
-  // TODO(https://crbug.com/1161551): Revert to returning a reference and make
-  // it an error to call this method if no MediaPlayer with |player_id| exists.
-  media::mojom::MediaPlayer* GetMediaPlayerRemote(
+  // Return an already bound mojo Remote for the MediaPlayer mojo interface. It
+  // is an error to call this method if no MediaPlayer with |player_id| exists.
+  mojo::Remote<media::mojom::MediaPlayer>& GetMediaPlayerRemote(
       const MediaPlayerId& player_id);
 
   // Creates a new MediaPlayerObserverHostImpl associated to |player_id| if

content/browser/media/session/media_session_controller.cc

@@ -50,62 +50,34 @@ bool MediaSessionController::OnPlaybackStarted() {
 
 void MediaSessionController::OnSuspend(int player_id) {
   DCHECK_EQ(player_id_, player_id);
-
-  media::mojom::MediaPlayer* remote =
-      web_contents_->media_web_contents_observer()->GetMediaPlayerRemote(id_);
-  if (!remote) {
-    // TODO(https://crbug.com/1161551): Remove this when lifetime bug is fixed.
-    NOTREACHED() << "Controller should not outlive remote MediaPlayer";
-    return;
-  }
-
   // TODO(crbug.com/953645): Set triggered_by_user to true ONLY if that action
   // was actually triggered by user as this will activate the frame.
-  remote->RequestPause(/*triggered_by_user=*/true);
+  web_contents_->media_web_contents_observer()
+      ->GetMediaPlayerRemote(id_)
+      ->RequestPause(/*triggered_by_user=*/true);
 }
 
 void MediaSessionController::OnResume(int player_id) {
   DCHECK_EQ(player_id_, player_id);
-
-  media::mojom::MediaPlayer* remote =
-      web_contents_->media_web_contents_observer()->GetMediaPlayerRemote(id_);
-  if (!remote) {
-    // TODO(https://crbug.com/1161551): Remove this when lifetime bug is fixed.
-    NOTREACHED() << "Controller should not outlive remote MediaPlayer";
-    return;
-  }
-
-  remote->RequestPlay();
+  web_contents_->media_web_contents_observer()
+      ->GetMediaPlayerRemote(id_)
+      ->RequestPlay();
 }
 
 void MediaSessionController::OnSeekForward(int player_id,
                                            base::TimeDelta seek_time) {
   DCHECK_EQ(player_id_, player_id);
-
-  media::mojom::MediaPlayer* remote =
-      web_contents_->media_web_contents_observer()->GetMediaPlayerRemote(id_);
-  if (!remote) {
-    // TODO(https://crbug.com/1161551): Remove this when lifetime bug is fixed.
-    NOTREACHED() << "Controller should not outlive remote MediaPlayer";
-    return;
-  }
-
-  remote->RequestSeekForward(seek_time);
+  web_contents_->media_web_contents_observer()
+      ->GetMediaPlayerRemote(id_)
+      ->RequestSeekForward(seek_time);
 }
 
 void MediaSessionController::OnSeekBackward(int player_id,
                                             base::TimeDelta seek_time) {
   DCHECK_EQ(player_id_, player_id);
-
-  media::mojom::MediaPlayer* remote =
-      web_contents_->media_web_contents_observer()->GetMediaPlayerRemote(id_);
-  if (!remote) {
-    // TODO(https://crbug.com/1161551): Remove this when lifetime bug is fixed.
-    NOTREACHED() << "Controller should not outlive remote MediaPlayer";
-    return;
-  }
-
-  remote->RequestSeekBackward(seek_time);
+  web_contents_->media_web_contents_observer()
+      ->GetMediaPlayerRemote(id_)
+      ->RequestSeekBackward(seek_time);
 }
 
 void MediaSessionController::OnSetVolumeMultiplier(int player_id,
@@ -119,29 +91,17 @@ void MediaSessionController::OnSetVolumeMultiplier(int player_id,
 void MediaSessionController::OnEnterPictureInPicture(int player_id) {
   DCHECK_EQ(player_id_, player_id);
 
-  media::mojom::MediaPlayer* remote =
-      web_contents_->media_web_contents_observer()->GetMediaPlayerRemote(id_);
-  if (!remote) {
-    // TODO(https://crbug.com/1161551): Remove this when lifetime bug is fixed.
-    NOTREACHED() << "Controller should not outlive remote MediaPlayer";
-    return;
-  }
-
-  remote->RequestEnterPictureInPicture();
+  web_contents_->media_web_contents_observer()
+      ->GetMediaPlayerRemote(id_)
+      ->RequestEnterPictureInPicture();
 }
 
 void MediaSessionController::OnExitPictureInPicture(int player_id) {
   DCHECK_EQ(player_id_, player_id);
 
-  media::mojom::MediaPlayer* remote =
-      web_contents_->media_web_contents_observer()->GetMediaPlayerRemote(id_);
-  if (!remote) {
-    // TODO(https://crbug.com/1161551): Remove this when lifetime bug is fixed.
-    NOTREACHED() << "Controller should not outlive remote MediaPlayer";
-    return;
-  }
-
-  remote->RequestExitPictureInPicture();
+  web_contents_->media_web_contents_observer()
+      ->GetMediaPlayerRemote(id_)
+      ->RequestExitPictureInPicture();
 }
 
 void MediaSessionController::OnSetAudioSinkId(