Revert 194993 "Remove reference counting from media::VideoDecode..."

trunk/src/media/filters/gpu_video_decoder.cc

Index: trunk/src/media/filters/gpu_video_decoder.cc
===================================================================
--- trunk/src/media/filters/gpu_video_decoder.cc	(revision 195011)
+++ trunk/src/media/filters/gpu_video_decoder.cc	(working copy)
@@ -9,7 +9,6 @@
 #include "base/cpu.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
-#include "base/task_runner_util.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/demuxer_stream.h"
@@ -19,112 +18,6 @@
 
 namespace media {
 
-// Proxies calls to a VideoDecodeAccelerator::Client from the calling thread to
-// the client's thread.
-//
-// TODO(scherkus): VDAClientProxy should hold onto GpuVideoDecoder::Factories
-// and take care of some of the work that GpuVideoDecoder does to minimize
-// thread hopping. See following for discussion:
-//
-// https://codereview.chromium.org/12989009/diff/27035/media/filters/gpu_video_decoder.cc#newcode23
-class VDAClientProxy
-    : public base::RefCountedThreadSafe<VDAClientProxy>,
-      public VideoDecodeAccelerator::Client {
- public:
-  explicit VDAClientProxy(VideoDecodeAccelerator::Client* client);
-
-  // Detaches the proxy. |weak_client_| will no longer be called and can be
-  // safely deleted. Any pending/future calls will be discarded.
-  //
-  // Must be called on |client_loop_|.
-  void Detach();
-
-  // VideoDecodeAccelerator::Client implementation.
-  virtual void NotifyInitializeDone() OVERRIDE;
-  virtual void ProvidePictureBuffers(uint32 count,
-                                     const gfx::Size& size,
-                                     uint32 texture_target) OVERRIDE;
-  virtual void DismissPictureBuffer(int32 id) OVERRIDE;
-  virtual void PictureReady(const media::Picture& picture) OVERRIDE;
-  virtual void NotifyEndOfBitstreamBuffer(int32 id) OVERRIDE;
-  virtual void NotifyFlushDone() OVERRIDE;
-  virtual void NotifyResetDone() OVERRIDE;
-  virtual void NotifyError(media::VideoDecodeAccelerator::Error error) OVERRIDE;
-
- private:
-  friend class base::RefCountedThreadSafe<VDAClientProxy>;
-  virtual ~VDAClientProxy();
-
-  scoped_refptr<base::MessageLoopProxy> client_loop_;
-
-  // Weak pointers are used to invalidate tasks posted to |client_loop_| after
-  // Detach() has been called.
-  base::WeakPtrFactory<VideoDecodeAccelerator::Client> weak_client_factory_;
-  base::WeakPtr<VideoDecodeAccelerator::Client> weak_client_;
-
-  DISALLOW_COPY_AND_ASSIGN(VDAClientProxy);
-};
-
-VDAClientProxy::VDAClientProxy(VideoDecodeAccelerator::Client* client)
-    : client_loop_(base::MessageLoopProxy::current()),
-      weak_client_factory_(client),
-      weak_client_(weak_client_factory_.GetWeakPtr()) {
-  DCHECK(weak_client_);
-}
-
-VDAClientProxy::~VDAClientProxy() {}
-
-void VDAClientProxy::Detach() {
-  DCHECK(client_loop_->BelongsToCurrentThread());
-  DCHECK(weak_client_) << "Detach() already called";
-  weak_client_factory_.InvalidateWeakPtrs();
-}
-
-void VDAClientProxy::NotifyInitializeDone() {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::NotifyInitializeDone, weak_client_));
-}
-
-void VDAClientProxy::ProvidePictureBuffers(uint32 count,
-                                           const gfx::Size& size,
-                                           uint32 texture_target) {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::ProvidePictureBuffers, weak_client_,
-      count, size, texture_target));
-}
-
-void VDAClientProxy::DismissPictureBuffer(int32 id) {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::DismissPictureBuffer, weak_client_, id));
-}
-
-void VDAClientProxy::PictureReady(const media::Picture& picture) {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::PictureReady, weak_client_, picture));
-}
-
-void VDAClientProxy::NotifyEndOfBitstreamBuffer(int32 id) {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::NotifyEndOfBitstreamBuffer, weak_client_,
-      id));
-}
-
-void VDAClientProxy::NotifyFlushDone() {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::NotifyFlushDone, weak_client_));
-}
-
-void VDAClientProxy::NotifyResetDone() {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::NotifyResetDone, weak_client_));
-}
-
-void VDAClientProxy::NotifyError(media::VideoDecodeAccelerator::Error error) {
-  client_loop_->PostTask(FROM_HERE, base::Bind(
-      &VideoDecodeAccelerator::Client::NotifyError, weak_client_, error));
-}
-
-
 // Maximum number of concurrent VDA::Decode() operations GVD will maintain.
 // Higher values allow better pipelining in the GPU, but also require more
 // resources.
@@ -161,7 +54,6 @@
     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),
@@ -179,7 +71,7 @@
 
   if (state_ == kDrainingDecoder && !factories_->IsAborted()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::Reset, weak_this_, closure));
+        &GpuVideoDecoder::Reset, 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())
@@ -221,8 +113,6 @@
                                  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));
@@ -260,9 +150,8 @@
     }
   }
 
-  client_proxy_ = new VDAClientProxy(this);
   VideoDecodeAccelerator* vda =
-      factories_->CreateVideoDecodeAccelerator(config.profile(), client_proxy_);
+      factories_->CreateVideoDecodeAccelerator(config.profile(), this);
   if (!vda) {
     status_cb.Run(DECODER_ERROR_NOT_SUPPORTED);
     return;
@@ -275,21 +164,17 @@
   statistics_cb_ = statistics_cb;
 
   DVLOG(1) << "GpuVideoDecoder::Initialize() succeeded.";
-  PostTaskAndReplyWithResult(
-      vda_loop_proxy_, FROM_HERE,
-      base::Bind(&VideoDecodeAccelerator::AsWeakPtr, base::Unretained(vda)),
-      base::Bind(&GpuVideoDecoder::SetVDA, weak_this_, status_cb, vda));
+  vda_loop_proxy_->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&GpuVideoDecoder::SetVDA, this, vda),
+      base::Bind(status_cb, PIPELINE_OK));
 }
 
-void GpuVideoDecoder::SetVDA(
-    const PipelineStatusCB& status_cb,
-    VideoDecodeAccelerator* vda,
-    base::WeakPtr<VideoDecodeAccelerator> weak_vda) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+void GpuVideoDecoder::SetVDA(VideoDecodeAccelerator* vda) {
+  DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
   DCHECK(!vda_.get());
   vda_.reset(vda);
-  weak_vda_ = weak_vda;
-  status_cb.Run(PIPELINE_OK);
+  weak_vda_ = vda->AsWeakPtr();
 }
 
 void GpuVideoDecoder::DestroyTextures() {
@@ -301,25 +186,17 @@
   picture_buffers_in_decoder_.clear();
 }
 
-static void DestroyVDAWithClientProxy(
-    const scoped_refptr<VDAClientProxy>& client_proxy,
-    base::WeakPtr<VideoDecodeAccelerator> weak_vda) {
-  if (weak_vda) {
-    weak_vda->Destroy();
-    DCHECK(!weak_vda);  // Check VDA::Destroy() contract.
-  }
-}
-
 void GpuVideoDecoder::DestroyVDA() {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
-
-  // |client_proxy| must stay alive until |weak_vda_| has been destroyed.
-  vda_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-      &DestroyVDAWithClientProxy, client_proxy_, weak_vda_));
-
   VideoDecodeAccelerator* vda ALLOW_UNUSED = vda_.release();
-  client_proxy_->Detach();
-  client_proxy_ = NULL;
+  // 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));
 
   DestroyTextures();
 }
@@ -367,9 +244,13 @@
 void GpuVideoDecoder::RequestBufferDecode(
     DemuxerStream::Status status,
     const scoped_refptr<DecoderBuffer>& buffer) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
   DCHECK_EQ(status != DemuxerStream::kOk, !buffer) << status;
 
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::RequestBufferDecode, this, status, buffer));
+    return;
+  }
   demuxer_read_in_progress_ = false;
 
   if (status != DemuxerStream::kOk) {
@@ -424,7 +305,7 @@
   if (CanMoreDecodeWorkBeDone()) {
     // Force post here to prevent reentrancy into DemuxerStream.
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::EnsureDemuxOrDecode, weak_this_));
+        &GpuVideoDecoder::EnsureDemuxOrDecode, this));
   }
 }
 
@@ -477,7 +358,12 @@
 void GpuVideoDecoder::ProvidePictureBuffers(uint32 count,
                                             const gfx::Size& size,
                                             uint32 texture_target) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::ProvidePictureBuffers, this, count, size,
+        texture_target));
+    return;
+  }
 
   std::vector<uint32> texture_ids;
   decoder_texture_target_ = texture_target;
@@ -507,8 +393,11 @@
 }
 
 void GpuVideoDecoder::DismissPictureBuffer(int32 id) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
-
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::DismissPictureBuffer, this, id));
+    return;
+  }
   std::map<int32, PictureBuffer>::iterator it =
       picture_buffers_in_decoder_.find(id);
   if (it == picture_buffers_in_decoder_.end()) {
@@ -520,8 +409,11 @@
 }
 
 void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
-
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::PictureReady, this, picture));
+    return;
+  }
   std::map<int32, PictureBuffer>::iterator it =
       picture_buffers_in_decoder_.find(picture.picture_buffer_id());
   if (it == picture_buffers_in_decoder_.end()) {
@@ -545,9 +437,8 @@
           base::Bind(&Factories::ReadPixels, factories_, pb.texture_id(),
                      decoder_texture_target_,
                      gfx::Size(visible_rect.width(), visible_rect.height())),
-          BindToCurrentLoop(base::Bind(
-              &GpuVideoDecoder::ReusePictureBuffer, weak_this_,
-              picture.picture_buffer_id()))));
+          base::Bind(&GpuVideoDecoder::ReusePictureBuffer, this,
+                     picture.picture_buffer_id())));
   CHECK_GT(available_pictures_, 0);
   available_pictures_--;
 
@@ -576,7 +467,11 @@
 }
 
 void GpuVideoDecoder::ReusePictureBuffer(int64 picture_buffer_id) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::ReusePictureBuffer, this, picture_buffer_id));
+    return;
+  }
   CHECK_GE(available_pictures_, 0);
   available_pictures_++;
 
@@ -609,7 +504,11 @@
 }
 
 void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::NotifyEndOfBitstreamBuffer, this, id));
+    return;
+  }
 
   std::map<int32, BufferPair>::iterator it =
       bitstream_buffers_in_decoder_.find(id);
@@ -664,18 +563,27 @@
 
   demuxer_read_in_progress_ = true;
   demuxer_stream_->Read(base::Bind(
-      &GpuVideoDecoder::RequestBufferDecode, weak_this_));
+      &GpuVideoDecoder::RequestBufferDecode, this));
 }
 
 void GpuVideoDecoder::NotifyFlushDone() {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::NotifyFlushDone, this));
+    return;
+  }
   DCHECK_EQ(state_, kDrainingDecoder);
   state_ = kDecoderDrained;
   EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEmptyFrame());
 }
 
 void GpuVideoDecoder::NotifyResetDone() {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::NotifyResetDone, this));
+    return;
+  }
+
   DCHECK(ready_video_frames_.empty());
 
   // This needs to happen after the Reset() on vda_ is done to ensure pictures
@@ -690,7 +598,11 @@
 }
 
 void GpuVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
-  DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
+    gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecoder::NotifyError, this, error));
+    return;
+  }
   if (!vda_.get())
     return;