(reland)

Remove threading from GpuVideoAcceleratorFactories

This change removes all the threading considerations from
GpuVideoAcceleratorFactories (and its implementation,
RendererGpuVideoAcceleratorFactories).  Most notably, it removes Abort() and
associated functions and state.  And with the removal of Abort() and friends,
we can also remove its Clone() interface.

All of the previously abortable operations on the RGVAF (with the exception of
ReadPixels()) can be made non-abortable, with no functional difference, due to
the way the users of RGVAF function.  These three users are
WebMediaPlayerImpl/GpuVideoDecoder, RTCVideoDecoder, and RTCVideoEncoder, and
they can be made non-abortable because:

WebMediaPlayerImpl/GpuVideoDecoder:
* Abort() is called from WebMediaPlayerImpl::Destroy().  It has no effect, as:
  * All the RGVAF entry points are called from the the RGVAF message loop
    from GpuVideoDecoder (except for ReadPixels()), so the Abort() has no
    effect on them.

RTCVideoDecoder:
* Abort() is called from RTCVideoDecoder::WillDestroyCurrentMessageLoop() for
  the RGVAF message loop.  It has no effect, as:
  * Amost all the RGVAF entry points are called from the RGVAF message loop
    (except for ReadPixels()), so Abort() has no effect on them.
  * The other exception is CreateVideoDecodeAccelerator(), which is called from
    RTC's main thread.  But as the Abort() is called from
    WillDestroyCurrentMessageLoop() for the RGVAF message loop itself, it is
    guaranteed to occur after any tasks posted to the RGVAF message loop by
    CreateVideoDecodeAccelerator() has completed, and so the Abort() has no
    effect.

RTCVideoEncoder:
* Abort() is called from RTCVideoDecoder::Release().  It has no effect, as:
  * All the RGVAF entry points are called from the RGVAF message loop.

The only functional difference remaining is that making ReadPixels()
non-abortable.  This is preferable, as as long as a completed video accelerator
texture is available, it should be readable.  We also specify that all calls to
ReadPixels must be made on the RGVAF message loop, like all other entry points,
and leave it up to the users of ReadPixels() to handle thread trampolining if
necessary.

This is a revert of a revert of a revert of a revert of r247480.  Yes.

BUG=306333
TEST=local build, run on CrOS snow; build, run unittests on desktop Linux
TBR=jamesr@chromium.org, wuchengli@chromium.org, fischman@chromium.org

Review URL: https://codereview.chromium.org/141663005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@249638 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 43 insertions, 25 deletions

diff --git a/media/cast/test/fake_gpu_video_accelerator_factories.cc b/media/cast/test/fake_gpu_video_accelerator_factories.cc
index b9f16a5..144f7c5 100644
--- a/media/cast/test/fake_gpu_video_accelerator_factories.cc
+++ b/media/cast/test/fake_gpu_video_accelerator_factories.cc
@@ -55,8 +55,6 @@ FakeGpuVideoAcceleratorFactories::CreateVideoDecodeAccelerator(
       static_cast<media::VideoDecodeAccelerator*>(NULL));
 }
 
-bool FakeGpuVideoAcceleratorFactories::IsAborted() { return false; }
-
 }  // namespace test
 }  // namespace cast
 }  // namespace media
diff --git a/media/cast/test/fake_gpu_video_accelerator_factories.h b/media/cast/test/fake_gpu_video_accelerator_factories.h
index 4cc2182..7b815fd 100644
--- a/media/cast/test/fake_gpu_video_accelerator_factories.h
+++ b/media/cast/test/fake_gpu_video_accelerator_factories.h
@@ -40,17 +40,13 @@ class FakeGpuVideoAcceleratorFactories : public GpuVideoAcceleratorFactories {
   virtual void WaitSyncPoint(uint32 sync_point) OVERRIDE {}
 
   virtual void ReadPixels(uint32 texture_id,
-                          const gfx::Size& size,
-                          const SkBitmap& pixels) OVERRIDE{};
+                          const gfx::Rect& visible_rect,
+                          const SkBitmap& pixels) OVERRIDE {};
 
   virtual scoped_ptr<VideoDecodeAccelerator> CreateVideoDecodeAccelerator(
       VideoCodecProfile profile,
       VideoDecodeAccelerator::Client* client) OVERRIDE;
 
-  virtual void Abort() OVERRIDE {}
-
-  virtual bool IsAborted() OVERRIDE;
-
  private:
   friend class base::RefCountedThreadSafe<FakeGpuVideoAcceleratorFactories>;
   virtual ~FakeGpuVideoAcceleratorFactories();
diff --git a/media/filters/gpu_video_accelerator_factories.h b/media/filters/gpu_video_accelerator_factories.h
index b6de58f..88d74b5 100644
--- a/media/filters/gpu_video_accelerator_factories.h
+++ b/media/filters/gpu_video_accelerator_factories.h
@@ -21,6 +21,12 @@ namespace media {
 
 // Helper interface for specifying factories needed to instantiate a hardware
 // video accelerator.
+// Threading model:
+// * The GpuVideoAcceleratorFactories may be constructed on any thread.
+// * The GpuVideoAcceleratorFactories has an associated message loop, which may
+//   be retrieved as |GetMessageLoop()|.
+// * All calls to the Factories after construction must be made on its message
+//   loop.
 class MEDIA_EXPORT GpuVideoAcceleratorFactories
     : public base::RefCountedThreadSafe<GpuVideoAcceleratorFactories> {
  public:
@@ -43,9 +49,10 @@ class MEDIA_EXPORT GpuVideoAcceleratorFactories
 
   virtual void WaitSyncPoint(uint32 sync_point) = 0;
 
-  // Read pixels from a native texture and store into |pixels| as RGBA.
+  // Read pixels within |visible_rect| boundaries from a native texture and
+  // store into |pixels| as RGBA.
   virtual void ReadPixels(uint32 texture_id,
-                          const gfx::Size& size,
+                          const gfx::Rect& visible_rect,
                           const SkBitmap& pixels) = 0;
 
   // Allocate & return a shared memory segment.  Caller is responsible for
@@ -55,13 +62,6 @@ class MEDIA_EXPORT GpuVideoAcceleratorFactories
   // Returns the task runner the video accelerator runs on.
   virtual scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() = 0;
 
-  // Abort any outstanding factory operations and error any future
-  // attempts at factory operations
-  virtual void Abort() = 0;
-
-  // Returns true if Abort() has been called.
-  virtual bool IsAborted() = 0;
-
  protected:
   friend class base::RefCountedThreadSafe<GpuVideoAcceleratorFactories>;
   virtual ~GpuVideoAcceleratorFactories();
diff --git a/media/filters/gpu_video_decoder.cc b/media/filters/gpu_video_decoder.cc
index 2699a44..b87041a 100644
--- a/media/filters/gpu_video_decoder.cc
+++ b/media/filters/gpu_video_decoder.cc
@@ -12,6 +12,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/task_runner_util.h"
 #include "gpu/command_buffer/common/mailbox_holder.h"
 #include "media/base/bind_to_current_loop.h"
@@ -21,6 +22,7 @@
 #include "media/base/pipeline_status.h"
 #include "media/base/video_decoder_config.h"
 #include "media/filters/gpu_video_accelerator_factories.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace media {
 
@@ -73,7 +75,7 @@ void GpuVideoDecoder::Reset(const base::Closure& closure)  {
   DVLOG(3) << "Reset()";
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
 
-  if (state_ == kDrainingDecoder && !factories_->IsAborted()) {
+  if (state_ == kDrainingDecoder) {
     base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
         &GpuVideoDecoder::Reset, weak_this_, closure));
     // NOTE: if we're deferring Reset() until a Flush() completes, return
@@ -421,6 +423,33 @@ void GpuVideoDecoder::DismissPictureBuffer(int32 id) {
   }
 }
 
+static void ReadPixelsSyncInner(
+    const scoped_refptr<media::GpuVideoAcceleratorFactories>& factories,
+    uint32 texture_id,
+    const gfx::Rect& visible_rect,
+    const SkBitmap& pixels,
+    base::WaitableEvent* event) {
+  factories->ReadPixels(texture_id, visible_rect, pixels);
+  event->Signal();
+}
+
+static void ReadPixelsSync(
+    const scoped_refptr<media::GpuVideoAcceleratorFactories>& factories,
+    uint32 texture_id,
+    const gfx::Rect& visible_rect,
+    const SkBitmap& pixels) {
+  base::WaitableEvent event(true, false);
+  if (!factories->GetTaskRunner()->PostTask(FROM_HERE,
+                                            base::Bind(&ReadPixelsSyncInner,
+                                                       factories,
+                                                       texture_id,
+                                                       visible_rect,
+                                                       pixels,
+                                                       &event)))
+    return;
+  event.Wait();
+}
+
 void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
   DVLOG(3) << "PictureReady()";
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
@@ -452,10 +481,7 @@ void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
       visible_rect,
       natural_size,
       timestamp,
-      base::Bind(&GpuVideoAcceleratorFactories::ReadPixels,
-                 factories_,
-                 pb.texture_id(),
-                 gfx::Size(visible_rect.width(), visible_rect.height()))));
+      base::Bind(&ReadPixelsSync, factories_, pb.texture_id(), visible_rect)));
   CHECK_GT(available_pictures_, 0);
   --available_pictures_;
   bool inserted =
diff --git a/media/filters/mock_gpu_video_accelerator_factories.h b/media/filters/mock_gpu_video_accelerator_factories.h
index aee918a..91a45e8 100644
--- a/media/filters/mock_gpu_video_accelerator_factories.h
+++ b/media/filters/mock_gpu_video_accelerator_factories.h
@@ -44,12 +44,10 @@ class MockGpuVideoAcceleratorFactories : public GpuVideoAcceleratorFactories {
   MOCK_METHOD1(WaitSyncPoint, void(uint32 sync_point));
   MOCK_METHOD3(ReadPixels,
                void(uint32 texture_id,
-                    const gfx::Size& size,
+                    const gfx::Rect& visible_rect,
                     const SkBitmap& pixels));
   MOCK_METHOD1(CreateSharedMemory, base::SharedMemory*(size_t size));
   MOCK_METHOD0(GetTaskRunner, scoped_refptr<base::SingleThreadTaskRunner>());
-  MOCK_METHOD0(Abort, void());
-  MOCK_METHOD0(IsAborted, bool());
 
   virtual scoped_ptr<VideoDecodeAccelerator> CreateVideoDecodeAccelerator(
       VideoCodecProfile profile,