Remove threading from RendererGpuVideoAcceleratorFactories

media/filters/gpu_video_accelerator_factories.h

Index: media/filters/gpu_video_accelerator_factories.h
diff --git a/media/filters/gpu_video_accelerator_factories.h b/media/filters/gpu_video_accelerator_factories.h
index c152c2a4bda8ca51d0769ac047a85bb27e4ce2b3..ccca541fb59c7a9bbb9d70f8fa59752e503b0436 100644
--- a/media/filters/gpu_video_accelerator_factories.h
+++ b/media/filters/gpu_video_accelerator_factories.h
@@ -21,6 +21,14 @@ namespace media {
 
 // Helper interface for specifying factories needed to instantiate a hardware
 // video accelerator.
+// Threading model:
+// * The GpuVideoAcceleratorFactories has an associated message loop, which may
+//   be retrieved as |GetMessageLoop()|.
+// * Calls to the Factories must be made on its message loop.
+//   * With the exception of |ReadPixels()|, which for convenience maybe be
+//     called from any thread.

[sheu, 2013/10/16 00:57:45]

I'm still a little iffy about this.  My other thought is to make ReadPixels also
require call on the factory message loop, and to make the user of the
GpuVideoAcceleratorFactories handle the synchronous posting.  This would make
the interface here cleaner, but unfortunately require that we duplicate the
synchronous posting bit to two locations (see: RTCVideoDecoder,
GpuVideoDecoder).

[Ami GONE FROM CHROMIUM, 2013/10/17 22:22:17]

An alternative: make ReadPixels callable from only one thread, but make that
thread be the render/compositor thread (the one where ReadPixels as actually
called on!).  I think the only thing in ReadPixels' impl today that requires it
to run on the media thread is that it uses a context3d that's bound to that
thread.  I wonder if it makes sense to give GVAF an additional context3d, for
using on the compositing thread?
That would allow zero trampolining, either inside RGVAF or in its callers, for
ReadPixels.

+//   * This is because the main user of |ReadPixels()| is the |read_pixels_cb_|
+//     of media::VideoFrame, which needs to be synchronous and thread-safe.
 class MEDIA_EXPORT GpuVideoAcceleratorFactories
     : public base::RefCountedThreadSafe<GpuVideoAcceleratorFactories> {
  public:
@@ -43,7 +51,8 @@ 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 from a native texture and store into |pixels| as RGBA.  Safe to
+  // call from any thread.
   virtual void ReadPixels(uint32 texture_id,
                           const gfx::Size& size,
                           const SkBitmap& pixels) = 0;
@@ -52,16 +61,11 @@ class MEDIA_EXPORT GpuVideoAcceleratorFactories
   // Close()ing the returned pointer.
   virtual base::SharedMemory* CreateSharedMemory(size_t size) = 0;
 
-  // Returns the message loop the video accelerator runs on.
+  // Returns the message loop the GpuVideoAcceleratorFactories runs on.  Calls
+  // to interface functions must be made on this loop, with the exception of
+  // |ReadPixels()|.
   virtual scoped_refptr<base::MessageLoopProxy> GetMessageLoop() = 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();