GitLab

This reverts commit 4a8451e4.

Reason for revert: The issue wasn't fixed by [1], so lets reinstate the defensive fix.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2626920

Original change's description:
> 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/+/2632919
> Reviewed-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}

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: I6ff223f67ce4c45bf1a49722f98e0a3dc8eef8bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2636377Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844604}