GitLab

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}