0

Fix dangling raw_ptr in MojoVideoDecoder

Fixed: 1462083
Change-Id: Iaba377f479aedf6e826997b7eb32cf46185c2b3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4905242
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1205945}
This commit is contained in:
Rushan Suleymanov
2023-10-05 18:26:19 +00:00
committed by Chromium LUCI CQ
parent 8af12d1a2f
commit 4a2d5221ff
3 changed files with 15 additions and 4 deletions
chrome/browser/thumbnail/generator/android
media/mojo/clients
third_party/blink/renderer/modules/webcodecs

@ -111,9 +111,10 @@ class ThumbnailMediaParserImpl : public ThumbnailMediaParser,
// Objects used to decode the video into media::VideoFrame with
// MojoVideoDecoder.
media::VideoDecoderConfig config_;
// `gpu_factories_` must outlive `decoder_`.
std::unique_ptr<media::GpuVideoAcceleratorFactories> gpu_factories_;
std::unique_ptr<media::VideoThumbnailDecoder> decoder_;
mojo::Remote<media::mojom::InterfaceFactory> media_interface_factory_;
std::unique_ptr<media::GpuVideoAcceleratorFactories> gpu_factories_;
bool decode_done_;
base::WeakPtrFactory<ThumbnailMediaParserImpl> weak_factory_{this};

@ -120,8 +120,8 @@ class MojoVideoDecoder final : public VideoDecoder,
// Manages VideoFrame destruction callbacks.
scoped_refptr<MojoVideoFrameHandleReleaser> mojo_video_frame_handle_releaser_;
raw_ptr<GpuVideoAcceleratorFactories, LeakedDanglingUntriaged>
gpu_factories_ = nullptr;
// `gpu_factories_` is not immortal when provided by ThumbnailMediaParserImpl.
raw_ptr<GpuVideoAcceleratorFactories> gpu_factories_ = nullptr;
// Raw pointer is safe since both `this` and the `media_log` are owned by
// WebMediaPlayerImpl with the correct declaration order.

@ -196,6 +196,14 @@ class VideoDecoderBrokerTest : public testing::Test {
Platform::Current()->GetBrowserInterfaceBroker()->SetBinderForTesting(
media::mojom::InterfaceFactory::Name_,
base::RepeatingCallback<void(mojo::ScopedMessagePipeHandle)>());
// `decoder_broker` schedules deletion of internal data including decoders
// which keep pointers to `gpu_factories_`. The deletion is scheduled in
// `media_thread_`, wait for completion of all its tasks.
decoder_broker_.reset();
if (media_thread_) {
media_thread_->FlushForTesting();
}
}
void OnInitWithClosure(base::RepeatingClosure done_cb,
@ -326,9 +334,11 @@ class VideoDecoderBrokerTest : public testing::Test {
protected:
media::NullMediaLog null_media_log_;
std::unique_ptr<base::Thread> media_thread_;
// `gpu_factories_` must outlive `decoder_broker_` because it's stored as a
// raw_ptr.
std::unique_ptr<media::MockGpuVideoAcceleratorFactories> gpu_factories_;
std::unique_ptr<VideoDecoderBroker> decoder_broker_;
std::vector<scoped_refptr<media::VideoFrame>> output_frames_;
std::unique_ptr<media::MockGpuVideoAcceleratorFactories> gpu_factories_;
std::unique_ptr<FakeInterfaceFactory> interface_factory_;
base::test::ScopedFeatureList feature_list_;