diff options
| author | Tommy Steimel <steimel@chromium.org> | 2024-05-24 13:57:53 +0000 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2024-08-23 11:27:58 +0000 |
| commit | b82c835c2e79bdfa62625a13a3fb760d77042f34 (patch) | |
| tree | 9ee3df5fef76b9cceb2a77ceba6f01b2d69c298f | |
| parent | dad670a3e2a5a8fdeb110faf63a3676726cfdf54 (diff) | |
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5564794:
MediaSession: Use a MediaSessionImpl WeakPtr in MediaSessionServiceImpl
Currently, every time MediaSessionServiceImpl wants to talk to its
associated MediaSessionImpl, it recalculates it from its
RenderFrameHostId. This can lead to issues where a
MediaSessionServiceImpl of a disconnected RenderFrameHost can no longer
access the MediaSessionImpl to tell it that it is being deleted,
leaving MediaSessionImpl with a dangling raw_ptr.
Bug: 338929744
Change-Id: I8f404c1a39510a24643c1f973a32bf6c0bbde123
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5564794
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1305678}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/566837
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/584571
4 files changed, 44 insertions, 29 deletions
diff --git a/chromium/content/browser/media/session/media_session_impl.cc b/chromium/content/browser/media/session/media_session_impl.cc index 6aeb68f5ea0..cf9d8f6b4b6 100644 --- a/chromium/content/browser/media/session/media_session_impl.cc +++ b/chromium/content/browser/media/session/media_session_impl.cc @@ -1661,6 +1661,10 @@ const base::UnguessableToken& MediaSessionImpl::GetRequestId() const { return delegate_->request_id(); } +base::WeakPtr<MediaSessionImpl> MediaSessionImpl::GetWeakPtr() { + return weak_factory_.GetWeakPtr(); +} + void MediaSessionImpl::RebuildAndNotifyActionsChanged() { std::set<media_session::mojom::MediaSessionAction> actions = routed_service_ ? routed_service_->actions() diff --git a/chromium/content/browser/media/session/media_session_impl.h b/chromium/content/browser/media/session/media_session_impl.h index 4fd63732beb..7f366b2401c 100644 --- a/chromium/content/browser/media/session/media_session_impl.h +++ b/chromium/content/browser/media/session/media_session_impl.h @@ -15,6 +15,8 @@ #include "base/containers/flat_set.h" #include "base/containers/id_map.h" #include "base/memory/raw_ptr.h" +#include "base/memory/raw_ptr_exclusion.h" +#include "base/memory/weak_ptr.h" #include "base/timer/timer.h" #include "build/build_config.h" #include "content/browser/media/session/audio_focus_delegate.h" @@ -341,6 +343,9 @@ class MediaSessionImpl : public MediaSession, // Returns the Audio Focus request ID associated with this media session. const base::UnguessableToken& GetRequestId() const; + // Returns a WeakPtr to `this`. + base::WeakPtr<MediaSessionImpl> GetWeakPtr(); + CONTENT_EXPORT bool HasImageCacheForTest(const GURL& image_url) const; private: @@ -622,6 +627,8 @@ class MediaSessionImpl : public MediaSession, media_session::mojom::RemotePlaybackMetadataPtr remote_playback_metadata_; + base::WeakPtrFactory<MediaSessionImpl> weak_factory_{this}; + WEB_CONTENTS_USER_DATA_KEY_DECL(); }; diff --git a/chromium/content/browser/media/session/media_session_service_impl.cc b/chromium/content/browser/media/session/media_session_service_impl.cc index 92230f7429c..863dd9d9cb6 100644 --- a/chromium/content/browser/media/session/media_session_service_impl.cc +++ b/chromium/content/browser/media/session/media_session_service_impl.cc @@ -22,14 +22,16 @@ MediaSessionServiceImpl::MediaSessionServiceImpl( : render_frame_host_id_(render_frame_host->GetGlobalId()), playback_state_(blink::mojom::MediaSessionPlaybackState::NONE) { MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnServiceCreated(this); + if (session) { + media_session_ = session->GetWeakPtr(); + media_session_->OnServiceCreated(this); + } } MediaSessionServiceImpl::~MediaSessionServiceImpl() { - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnServiceDestroyed(this); + if (media_session_) { + media_session_->OnServiceDestroyed(this); + } } // static @@ -70,17 +72,17 @@ void MediaSessionServiceImpl::SetClient( void MediaSessionServiceImpl::SetPlaybackState( blink::mojom::MediaSessionPlaybackState state) { playback_state_ = state; - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnMediaSessionPlaybackStateChanged(this); + if (media_session_) { + media_session_->OnMediaSessionPlaybackStateChanged(this); + } } void MediaSessionServiceImpl::SetPositionState( const absl::optional<media_session::MediaPosition>& position) { position_ = position; - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->RebuildAndNotifyMediaPositionChanged(); + if (media_session_) { + media_session_->RebuildAndNotifyMediaPositionChanged(); + } } void MediaSessionServiceImpl::SetMetadata( @@ -102,48 +104,48 @@ void MediaSessionServiceImpl::SetMetadata( metadata_ = std::move(metadata); } - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnMediaSessionMetadataChanged(this); + if (media_session_) { + media_session_->OnMediaSessionMetadataChanged(this); + } } void MediaSessionServiceImpl::SetMicrophoneState( media_session::mojom::MicrophoneState microphone_state) { microphone_state_ = microphone_state; - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnMediaSessionInfoChanged(this); + if (media_session_) { + media_session_->OnMediaSessionInfoChanged(this); + } } void MediaSessionServiceImpl::SetCameraState( media_session::mojom::CameraState camera_state) { camera_state_ = camera_state; - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnMediaSessionInfoChanged(this); + if (media_session_) { + media_session_->OnMediaSessionInfoChanged(this); + } } void MediaSessionServiceImpl::EnableAction( media_session::mojom::MediaSessionAction action) { actions_.insert(action); - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnMediaSessionActionsChanged(this); + if (media_session_) { + media_session_->OnMediaSessionActionsChanged(this); + } } void MediaSessionServiceImpl::DisableAction( media_session::mojom::MediaSessionAction action) { actions_.erase(action); - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnMediaSessionActionsChanged(this); + if (media_session_) { + media_session_->OnMediaSessionActionsChanged(this); + } } void MediaSessionServiceImpl::ClearActions() { actions_.clear(); - MediaSessionImpl* session = GetMediaSession(); - if (session) - session->OnMediaSessionActionsChanged(this); + if (media_session_) { + media_session_->OnMediaSessionActionsChanged(this); + } } MediaSessionImpl* MediaSessionServiceImpl::GetMediaSession() { diff --git a/chromium/content/browser/media/session/media_session_service_impl.h b/chromium/content/browser/media/session/media_session_service_impl.h index aae5d94e59c..e2bfcdd200e 100644 --- a/chromium/content/browser/media/session/media_session_service_impl.h +++ b/chromium/content/browser/media/session/media_session_service_impl.h @@ -85,6 +85,8 @@ class CONTENT_EXPORT MediaSessionServiceImpl const GlobalRenderFrameHostId render_frame_host_id_; + base::WeakPtr<MediaSessionImpl> media_session_; + mojo::Remote<blink::mojom::MediaSessionClient> client_; blink::mojom::MediaSessionPlaybackState playback_state_; blink::mojom::SpecMediaMetadataPtr metadata_; |