Remove reference counting from media::VideoDecoder and friends.

media/filters/gpu_video_decoder.cc

Index: media/filters/gpu_video_decoder.cc
diff --git a/media/filters/gpu_video_decoder.cc b/media/filters/gpu_video_decoder.cc
index cbad87d37051b4dba304e580c4ed5582fdccd187..ddb9233f65dde9d0f997e403a3286d799518f29e 100644
--- a/media/filters/gpu_video_decoder.cc
+++ b/media/filters/gpu_video_decoder.cc
@@ -9,6 +9,7 @@
 #include "base/cpu.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
+#include "base/synchronization/waitable_event.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/demuxer_stream.h"
@@ -54,6 +55,7 @@ GpuVideoDecoder::GpuVideoDecoder(
     const scoped_refptr<base::MessageLoopProxy>& message_loop,
     const scoped_refptr<Factories>& factories)
     : gvd_loop_proxy_(message_loop),
+      weak_factory_(this),
       vda_loop_proxy_(factories->GetMessageLoop()),
       factories_(factories),
       state_(kNormal),
@@ -71,7 +73,7 @@ void GpuVideoDecoder::Reset(const base::Closure& closure)  {
 
   if (state_ == kDrainingDecoder) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::Reset, this, closure));
+        &GpuVideoDecoder::Reset, weak_this_, closure));
     // NOTE: if we're deferring Reset() until a Flush() completes, return
     // queued pictures to the VDA so they can be used to finish that Flush().
     if (pending_read_cb_.is_null())
@@ -110,6 +112,8 @@ void GpuVideoDecoder::Initialize(const scoped_refptr<DemuxerStream>& stream,
                                  const PipelineStatusCB& orig_status_cb,
                                  const StatisticsCB& statistics_cb) {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  weak_this_ = weak_factory_.GetWeakPtr();
+
   PipelineStatusCB status_cb = CreateUMAReportingPipelineCB(
       "Media.GpuVideoDecoderInitializeStatus",
       BindToCurrentLoop(orig_status_cb));
@@ -163,7 +167,7 @@ void GpuVideoDecoder::Initialize(const scoped_refptr<DemuxerStream>& stream,
   DVLOG(1) << "GpuVideoDecoder::Initialize() succeeded.";
   vda_loop_proxy_->PostTaskAndReply(
       FROM_HERE,
-      base::Bind(&GpuVideoDecoder::SetVDA, this, vda),
+      base::Bind(&GpuVideoDecoder::SetVDA, weak_this_, vda),
       base::Bind(status_cb, PIPELINE_OK));
 }
 
@@ -183,17 +187,24 @@ void GpuVideoDecoder::DestroyTextures() {
   picture_buffers_in_decoder_.clear();
 }
 
+static void DestroyAndSignal(
+    base::WeakPtr<VideoDecodeAccelerator> weak_vda,
+    base::WaitableEvent* event) {
+  // XXX: how else would weak_vda be null?

[scherkus (not reviewing), 2013/03/22 00:04:20]

FYI

[xhwang, 2013/03/22 01:00:32]

Defer this part to fischman :)

[Ami GONE FROM CHROMIUM, 2013/03/22 23:48:01]

VDA unfortunately SupportsWeakPtr, so others can invalidate its weakptrs, and
you might not know about it.
E.g. if an error is signaled from the GPU process the VDA (GVDAH) in the
renderer may be released without telling you.
(I think?)

+  if (weak_vda)
+    weak_vda->Destroy();
+  event->Signal();
+}
+
 void GpuVideoDecoder::DestroyVDA() {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
   VideoDecodeAccelerator* vda ALLOW_UNUSED = vda_.release();
-  // Tricky: |this| needs to stay alive until after VDA::Destroy is actually
-  // called, not just posted, so we take an artificial ref to |this| and release
-  // it as |reply| after VDA::Destroy() returns.
-  AddRef();
-  vda_loop_proxy_->PostTaskAndReply(
-      FROM_HERE,
-      base::Bind(&VideoDecodeAccelerator::Destroy, weak_vda_),
-      base::Bind(&GpuVideoDecoder::Release, this));
+
+  // Wait until VideoDecodeAccelerator::Destroy() completes.

[scherkus (not reviewing), 2013/03/22 00:04:20]

fischman:
http://blog.operationreality.org/wp-content/uploads/2012/03/chemistry-dog-no-...

+  base::WaitableEvent event(false, false);
+  vda_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+      &DestroyAndSignal, weak_vda_, &event));
+  event.Wait();

[Ami GONE FROM CHROMIUM, 2013/03/22 23:48:01]

This looks like a deadlock: Stop() calls this function, so the render thread is
blocked waiting for this to return, so DestroyAndSignal won't get a chance to
run.

 
   DestroyTextures();
 }
@@ -245,7 +256,7 @@ void GpuVideoDecoder::RequestBufferDecode(
 
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::RequestBufferDecode, this, status, buffer));
+        &GpuVideoDecoder::RequestBufferDecode, weak_this_, status, buffer));
     return;
   }
   demuxer_read_in_progress_ = false;
@@ -351,7 +362,7 @@ void GpuVideoDecoder::ProvidePictureBuffers(uint32 count,
                                             uint32 texture_target) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::ProvidePictureBuffers, this, count, size,
+        &GpuVideoDecoder::ProvidePictureBuffers, weak_this_, count, size,
         texture_target));
     return;
   }
@@ -386,7 +397,7 @@ void GpuVideoDecoder::ProvidePictureBuffers(uint32 count,
 void GpuVideoDecoder::DismissPictureBuffer(int32 id) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::DismissPictureBuffer, this, id));
+        &GpuVideoDecoder::DismissPictureBuffer, weak_this_, id));
     return;
   }
   std::map<int32, PictureBuffer>::iterator it =
@@ -402,7 +413,7 @@ void GpuVideoDecoder::DismissPictureBuffer(int32 id) {
 void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::PictureReady, this, picture));
+        &GpuVideoDecoder::PictureReady, weak_this_, picture));
     return;
   }
   std::map<int32, PictureBuffer>::iterator it =
@@ -428,7 +439,7 @@ void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
           base::Bind(&Factories::ReadPixels, factories_, pb.texture_id(),
                      decoder_texture_target_,
                      gfx::Size(visible_rect.width(), visible_rect.height())),
-          base::Bind(&GpuVideoDecoder::ReusePictureBuffer, this,
+          base::Bind(&GpuVideoDecoder::ReusePictureBuffer, weak_this_,
                      picture.picture_buffer_id())));
   CHECK_GT(available_pictures_, 0);
   available_pictures_--;
@@ -460,7 +471,7 @@ void GpuVideoDecoder::EnqueueFrameAndTriggerFrameDelivery(
 void GpuVideoDecoder::ReusePictureBuffer(int64 picture_buffer_id) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::ReusePictureBuffer, this, picture_buffer_id));
+        &GpuVideoDecoder::ReusePictureBuffer, weak_this_, picture_buffer_id));
     return;
   }
   CHECK_GE(available_pictures_, 0);
@@ -495,7 +506,7 @@ void GpuVideoDecoder::PutSHM(SHMBuffer* shm_buffer) {
 void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyEndOfBitstreamBuffer, this, id));
+        &GpuVideoDecoder::NotifyEndOfBitstreamBuffer, weak_this_, id));
     return;
   }
 
@@ -547,13 +558,13 @@ void GpuVideoDecoder::EnsureDemuxOrDecode() {
   demuxer_read_in_progress_ = true;
   gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
       &DemuxerStream::Read, demuxer_stream_.get(),
-      base::Bind(&GpuVideoDecoder::RequestBufferDecode, this)));
+      base::Bind(&GpuVideoDecoder::RequestBufferDecode, weak_this_)));
 }
 
 void GpuVideoDecoder::NotifyFlushDone() {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyFlushDone, this));
+        &GpuVideoDecoder::NotifyFlushDone, weak_this_));
     return;
   }
   DCHECK_EQ(state_, kDrainingDecoder);
@@ -564,7 +575,7 @@ void GpuVideoDecoder::NotifyFlushDone() {
 void GpuVideoDecoder::NotifyResetDone() {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyResetDone, this));
+        &GpuVideoDecoder::NotifyResetDone, weak_this_));
     return;
   }
 
@@ -587,7 +598,7 @@ void GpuVideoDecoder::NotifyResetDone() {
 void GpuVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyError, this, error));
+        &GpuVideoDecoder::NotifyError, weak_this_, error));
     return;
   }
   if (!vda_.get())