Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(244)

Side by Side Diff: media/mojo/clients/mojo_renderer_impl.cc

Issue 2075193002: Fixes use-after-free in MojoDemuxerStreamImpl. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: addressed comments Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "media/mojo/clients/mojo_renderer_impl.h" 5 #include "media/mojo/clients/mojo_renderer_impl.h"
6 6
7 #include <utility> 7 #include <utility>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/callback_helpers.h" 10 #include "base/callback_helpers.h"
(...skipping 45 matching lines...) Expand 10 before | Expand all | Expand 10 after
56 init_cb_ = init_cb; 56 init_cb_ = init_cb;
57 57
58 // Create audio and video mojom::DemuxerStream and bind its lifetime to 58 // Create audio and video mojom::DemuxerStream and bind its lifetime to
59 // the pipe. 59 // the pipe.
60 DemuxerStream* const audio = 60 DemuxerStream* const audio =
61 demuxer_stream_provider_->GetStream(DemuxerStream::AUDIO); 61 demuxer_stream_provider_->GetStream(DemuxerStream::AUDIO);
62 DemuxerStream* const video = 62 DemuxerStream* const video =
63 demuxer_stream_provider_->GetStream(DemuxerStream::VIDEO); 63 demuxer_stream_provider_->GetStream(DemuxerStream::VIDEO);
64 64
65 mojom::DemuxerStreamPtr audio_stream; 65 mojom::DemuxerStreamPtr audio_stream;
66 if (audio) 66 if (audio) {
67 new MojoDemuxerStreamImpl(audio, GetProxy(&audio_stream)); 67 audio_stream_.reset(
68 new MojoDemuxerStreamImpl(audio, GetProxy(&audio_stream)));
69 // Using base::Unretained(this) is safe because |this| owns
70 // |audio_stream_|, and the error handler can't be invoked once
71 // |audio_stream_| is destroyed.
72 audio_stream_->set_connection_error_handler(
73 base::Bind(&MojoRendererImpl::OnDemuxerStreamConnectionError,
74 base::Unretained(this), DemuxerStream::AUDIO));
75 }
68 76
69 mojom::DemuxerStreamPtr video_stream; 77 mojom::DemuxerStreamPtr video_stream;
70 if (video) 78 if (video) {
71 new MojoDemuxerStreamImpl(video, GetProxy(&video_stream)); 79 video_stream_.reset(
80 new MojoDemuxerStreamImpl(video, GetProxy(&video_stream)));
81 // Using base::Unretained(this) is safe because |this| owns
82 // |video_stream_|, and the error handler can't be invoked once
83 // |video_stream_| is destroyed.
84 video_stream_->set_connection_error_handler(
85 base::Bind(&MojoRendererImpl::OnDemuxerStreamConnectionError,
86 base::Unretained(this), DemuxerStream::VIDEO));
87 }
72 88
73 BindRemoteRendererIfNeeded(); 89 BindRemoteRendererIfNeeded();
74 90
75 // Using base::Unretained(this) is safe because |this| owns 91 // Using base::Unretained(this) is safe because |this| owns
76 // |remote_renderer_|, and the callback won't be dispatched if 92 // |remote_renderer_|, and the callback won't be dispatched if
77 // |remote_renderer_| is destroyed. 93 // |remote_renderer_| is destroyed.
78 remote_renderer_->Initialize(binding_.CreateInterfacePtrAndBind(), 94 remote_renderer_->Initialize(binding_.CreateInterfacePtrAndBind(),
79 std::move(audio_stream), std::move(video_stream), 95 std::move(audio_stream), std::move(video_stream),
80 base::Bind(&MojoRendererImpl::OnInitialized, 96 base::Bind(&MojoRendererImpl::OnInitialized,
81 base::Unretained(this), client)); 97 base::Unretained(this), client));
(...skipping 146 matching lines...) Expand 10 before | Expand all | Expand 10 after
228 DVLOG(1) << __FUNCTION__; 244 DVLOG(1) << __FUNCTION__;
229 DCHECK(task_runner_->BelongsToCurrentThread()); 245 DCHECK(task_runner_->BelongsToCurrentThread());
230 246
231 encountered_error_ = true; 247 encountered_error_ = true;
232 CancelPendingCallbacks(); 248 CancelPendingCallbacks();
233 249
234 if (client_) 250 if (client_)
235 client_->OnError(PIPELINE_ERROR_DECODE); 251 client_->OnError(PIPELINE_ERROR_DECODE);
236 } 252 }
237 253
254 void MojoRendererImpl::OnDemuxerStreamConnectionError(
255 DemuxerStream::Type type) {
256 DVLOG(1) << __FUNCTION__ << ": " << type;
257 DCHECK(task_runner_->BelongsToCurrentThread());
258
259 if (type == DemuxerStream::AUDIO) {
260 audio_stream_.reset();
261 } else if (type == DemuxerStream::VIDEO) {
262 video_stream_.reset();
263 } else {
264 NOTREACHED() << "Unexpected demuxer stream type: " << type;
265 }
266 }
267
238 void MojoRendererImpl::BindRemoteRendererIfNeeded() { 268 void MojoRendererImpl::BindRemoteRendererIfNeeded() {
239 DVLOG(2) << __FUNCTION__; 269 DVLOG(2) << __FUNCTION__;
240 DCHECK(task_runner_->BelongsToCurrentThread()); 270 DCHECK(task_runner_->BelongsToCurrentThread());
241 271
242 // If |remote_renderer_| has already been bound, do nothing. 272 // If |remote_renderer_| has already been bound, do nothing.
243 // Note that after Bind() is called, |remote_renderer_| is always bound even 273 // Note that after Bind() is called, |remote_renderer_| is always bound even
244 // after connection error. 274 // after connection error.
245 if (remote_renderer_.is_bound()) 275 if (remote_renderer_.is_bound())
246 return; 276 return;
247 277
(...skipping 47 matching lines...) Expand 10 before | Expand all | Expand 10 after
295 base::ResetAndReturn(&init_cb_).Run(PIPELINE_ERROR_INITIALIZATION_FAILED); 325 base::ResetAndReturn(&init_cb_).Run(PIPELINE_ERROR_INITIALIZATION_FAILED);
296 326
297 if (!flush_cb_.is_null()) 327 if (!flush_cb_.is_null())
298 base::ResetAndReturn(&flush_cb_).Run(); 328 base::ResetAndReturn(&flush_cb_).Run();
299 329
300 if (!cdm_attached_cb_.is_null()) 330 if (!cdm_attached_cb_.is_null())
301 base::ResetAndReturn(&cdm_attached_cb_).Run(false); 331 base::ResetAndReturn(&cdm_attached_cb_).Run(false);
302 } 332 }
303 333
304 } // namespace media 334 } // namespace media
OLDNEW
« media/mojo/clients/mojo_demuxer_stream_impl.h ('K') | « media/mojo/clients/mojo_renderer_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698