Fixes use-after-free in MojoDemuxerStreamImpl.

media/mojo/clients/mojo_renderer_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/mojo/clients/mojo_renderer_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 38 matching lines @@
   init_cb_ = init_cb;
 
   // Create audio and video mojom::DemuxerStream and bind its lifetime to
   // the pipe.
   DemuxerStream* const audio =
       demuxer_stream_provider_->GetStream(DemuxerStream::AUDIO);
   DemuxerStream* const video =
       demuxer_stream_provider_->GetStream(DemuxerStream::VIDEO);
 
   mojom::DemuxerStreamPtr audio_stream;
-  if (audio)
-    new MojoDemuxerStreamImpl(audio, GetProxy(&audio_stream));
+  if (audio) {
+    audio_stream_.reset(
+        new MojoDemuxerStreamImpl(audio, GetProxy(&audio_stream)));
+    audio_stream_->set_connection_error_handler(
+        base::Bind(&MojoRendererImpl::OnDemuxerStreamConnectionError,
+                   base::Unretained(this), DemuxerStream::AUDIO));

[xhwang, 2016/06/18 16:46:16]

please add a comment about why Unretained() is safe.

[alokp, 2016/06/19 04:57:45]

Done.

+  }
 
   mojom::DemuxerStreamPtr video_stream;
-  if (video)
-    new MojoDemuxerStreamImpl(video, GetProxy(&video_stream));
+  if (video) {
+    video_stream_.reset(
+        new MojoDemuxerStreamImpl(video, GetProxy(&video_stream)));
+    video_stream_->set_connection_error_handler(
+        base::Bind(&MojoRendererImpl::OnDemuxerStreamConnectionError,
+                   base::Unretained(this), DemuxerStream::VIDEO));
+  }
 
   BindRemoteRendererIfNeeded();
 
   // Using base::Unretained(this) is safe because |this| owns
   // |remote_renderer_|, and the callback won't be dispatched if
   // |remote_renderer_| is destroyed.
   remote_renderer_->Initialize(
       binding_.CreateInterfacePtrAndBind(), std::move(audio_stream),
       std::move(video_stream),
       base::Bind(&MojoRendererImpl::OnInitialized, base::Unretained(this)));
@@ skipping 120 matching lines @@
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   if (!init_cb_.is_null()) {
     base::ResetAndReturn(&init_cb_).Run(PIPELINE_ERROR_INITIALIZATION_FAILED);
     return;
   }
 
   client_->OnError(PIPELINE_ERROR_DECODE);
 }
 
+void MojoRendererImpl::OnDemuxerStreamConnectionError(
+    DemuxerStream::Type type) {
+  DVLOG(1) << __FUNCTION__ << ": " << type;
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  if (type == DemuxerStream::AUDIO) {
+    audio_stream_.reset();
+  } else {
+    DCHECK(type == DemuxerStream::VIDEO);
+    video_stream_.reset();
+  }

[xhwang, 2016/06/18 16:46:16]

Just to be safe, 

} else if (type == VIDEO) {
  ...
} else {
  NOTREACHED() << "wrong type..";
}

[alokp, 2016/06/19 04:57:45]

Done.

+}
+
 void MojoRendererImpl::BindRemoteRendererIfNeeded() {
   DVLOG(2) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // If |remote_renderer_| has already been bound, do nothing.
   if (remote_renderer_.is_bound())
     return;
 
   // Bind |remote_renderer_| to the |task_runner_|.
   remote_renderer_.Bind(std::move(remote_renderer_info_));
 
   // Otherwise, set an error handler to catch the connection error.
   // Using base::Unretained(this) is safe because |this| owns
   // |remote_renderer_|, and the error handler can't be invoked once
   // |remote_renderer_| is destroyed.
   remote_renderer_.set_connection_error_handler(
       base::Bind(&MojoRendererImpl::OnConnectionError, base::Unretained(this)));
 }
 
 void MojoRendererImpl::OnInitialized(bool success) {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!init_cb_.is_null());
 
   base::ResetAndReturn(&init_cb_).Run(
       success ? PIPELINE_OK : PIPELINE_ERROR_INITIALIZATION_FAILED);
 }
 
 }  // namespace media