Enable fire-and-forget Destroy of HW video decoder, and misc other improvements.

- Instead of requiring the client to wait for NotifyDestroyDone
  we do the asynchronous OMX teardown dance on the client's
  behalf.
- Prettify/simplify error handling in OVDA for easier matching of
  errors to OMX_Core.h and to remove redundant information.
- Enable previously-DISABLED_ early-teardown unittests!
- Remove passing VideoDecoder_Dev object in PPP_VideoDecoder_Dev
  calls, because it was unnecssary, and because the
  ~VideoDecoder_Dev dtor is now not a no-op so we don't want to
  call it spuriously.
- Remove accidentally re-added
  gpu_video_service_host.cc (originally removed in 92251,
  accidentally reintroduced by a bad rebase in 92383).

BUG=none
TEST=ovdatest passes (incl. early-teardown tests) and gles2 works (including reload and EOS handling, no crashes)

Review URL: http://codereview.chromium.org/7361010

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

9 files changed, 55 insertions, 97 deletions

diff --git a/ppapi/c/dev/ppb_video_decoder_dev.h b/ppapi/c/dev/ppb_video_decoder_dev.h
index 6564521..5b8f0d4 100644
--- a/ppapi/c/dev/ppb_video_decoder_dev.h
+++ b/ppapi/c/dev/ppb_video_decoder_dev.h
@@ -9,8 +9,8 @@
 #include "ppapi/c/pp_completion_callback.h"
 #include "ppapi/c/pp_var.h"
 
-#define PPB_VIDEODECODER_DEV_INTERFACE_0_11 "PPB_VideoDecoder(Dev);0.11"
-#define PPB_VIDEODECODER_DEV_INTERFACE PPB_VIDEODECODER_DEV_INTERFACE_0_11
+#define PPB_VIDEODECODER_DEV_INTERFACE_0_12 "PPB_VideoDecoder(Dev);0.12"
+#define PPB_VIDEODECODER_DEV_INTERFACE PPB_VIDEODECODER_DEV_INTERFACE_0_12
 
 // Video decoder interface.
 //
@@ -27,7 +27,7 @@
 //   callback.
 // - To reset the decoder (e.g. to implement Seek): call Reset() and wait for
 //   NotifyResetDone callback.
-// - To tear down the decoder call Destroy() and wait for NotifyDestroyDone.
+// - To tear down the decoder call Destroy().
 //
 // See PPP_VideoDecoder_Dev for the notifications the decoder may send the
 // plugin.
@@ -132,18 +132,14 @@ struct PPB_VideoDecoder_Dev {
                    struct PP_CompletionCallback callback);
 
   // Tear down the decoder as quickly as possible.  Pending inputs and outputs
-  // are dropped and the decoder frees all of its resources.  When |callback| is
-  // called the plugin is guaranteed not to receive any further callbacks and
-  // must make no further calls on |video_decoder|.
+  // are dropped and the decoder frees all of its resources.  Although resources
+  // may be freed asynchronously, after this method returns no more callbacks
+  // will be made on the client.  Any resources held by the client at that point
+  // may be freed.
   //
   // Parameters:
   //   |video_decoder| is the previously created handle to the decoder resource.
-  //   |callback| is one-time callback that will be called once the destroy
-  //   request has been completed.
-  //
-  // Returns an error code from pp_errors.h.
-  int32_t (*Destroy)(PP_Resource video_decoder,
-                     struct PP_CompletionCallback callback);
+  void (*Destroy)(PP_Resource video_decoder);
 };
 
 #endif  /* PPAPI_C_DEV_PPB_VIDEO_DECODER_DEV_H_ */
diff --git a/ppapi/c/dev/ppp_video_decoder_dev.h b/ppapi/c/dev/ppp_video_decoder_dev.h
index 1940665..cc9c721 100644
--- a/ppapi/c/dev/ppp_video_decoder_dev.h
+++ b/ppapi/c/dev/ppp_video_decoder_dev.h
@@ -9,7 +9,7 @@
 #include "ppapi/c/pp_resource.h"
 #include "ppapi/c/dev/pp_video_dev.h"
 
-#define PPP_VIDEODECODER_DEV_INTERFACE "PPP_VideoDecoder(Dev);0.4"
+#define PPP_VIDEODECODER_DEV_INTERFACE "PPP_VideoDecoder(Dev);0.5"
 
 // PPP_VideoDecoder_Dev structure contains the function pointers that the
 // plugin MUST implement to provide services needed by the video decoder
@@ -25,13 +25,11 @@ struct PPP_VideoDecoder_Dev {
   //
   // Parameters:
   //  |instance| the plugin instance to which the callback is responding.
-  //  |decoder| is pointer to the Pepper Video Decoder resource.
   //  |req_num_of_bufs| tells how many buffers are needed by the decoder.
   //  |dimensions| tells the dimensions of the buffer to allocate.
   //  |type| specifies whether the buffer lives in system memory or GL texture.
   void (*ProvidePictureBuffers)(
       PP_Instance instance,
-      PP_Resource decoder,
       uint32_t req_num_of_bufs,
       struct PP_Size dimensions,
       enum PP_PictureBufferType_Dev type);
@@ -41,11 +39,8 @@ struct PPP_VideoDecoder_Dev {
   //
   // Parameters:
   //  |instance| the plugin instance to which the callback is responding.
-  //  |decoder| is pointer to the Pepper Video Decoder resource.
   //  |picture_buffer| points to the picture buffer that is no longer needed.
-  void (*DismissPictureBuffer)(PP_Instance instance,
-                               PP_Resource decoder,
-                               int32_t picture_buffer_id);
+  void (*DismissPictureBuffer)(PP_Instance instance, int32_t picture_buffer_id);
 
   // Callback function for decoder to deliver decoded pictures ready to be
   // displayed. Decoder expects the plugin to return the buffer back to the
@@ -53,11 +48,8 @@ struct PPP_VideoDecoder_Dev {
   //
   // Parameters:
   //  |instance| the plugin instance to which the callback is responding.
-  //  |decoder| is pointer to the Pepper Video Decoder resource.
   //  |picture| is the picture that is ready.
-  void (*PictureReady)(PP_Instance instance,
-                       PP_Resource decoder,
-                       struct PP_Picture_Dev picture);
+  void (*PictureReady)(PP_Instance instance, struct PP_Picture_Dev picture);
 
   // Callback function to tell the plugin that decoder has decoded end of stream
   // marker and output all the pictures that should be displayed from the
@@ -65,8 +57,7 @@ struct PPP_VideoDecoder_Dev {
   //
   // Parameters:
   //  |instance| the plugin instance to which the callback is responding.
-  //  |decoder| is pointer to the Pepper Video Decoder resource.
-  void (*EndOfStream)(PP_Instance instance, PP_Resource decoder);
+  void (*EndOfStream)(PP_Instance instance);
 
   // Error handler callback for decoder to deliver information about detected
   // errors to the plugin.
@@ -75,11 +66,8 @@ struct PPP_VideoDecoder_Dev {
   //
   // Parameters:
   //  |instance| the plugin instance to which the callback is responding.
-  //  |decoder| is pointer to the Pepper Video Decoder resource.
   //  |error| error is the enumeration specifying the error.
-  void (*NotifyError)(PP_Instance instance,
-                      PP_Resource decoder,
-                      enum PP_VideoDecodeError_Dev error);
+  void (*NotifyError)(PP_Instance instance, enum PP_VideoDecodeError_Dev error);
 };
 
 #endif  /* PPAPI_C_DEV_PPP_VIDEO_DECODER_DEV_H_ */
diff --git a/ppapi/cpp/dev/video_decoder_client_dev.cc b/ppapi/cpp/dev/video_decoder_client_dev.cc
index aa00c6f..e0c174c 100644
--- a/ppapi/cpp/dev/video_decoder_client_dev.cc
+++ b/ppapi/cpp/dev/video_decoder_client_dev.cc
@@ -18,7 +18,6 @@ const char kPPPVideoDecoderInterface[] = PPP_VIDEODECODER_DEV_INTERFACE;
 
 // Callback to provide buffers for the decoded output pictures.
 void ProvidePictureBuffers(PP_Instance instance,
-                           PP_Resource decoder_id,
                            uint32_t req_num_of_bufs,
                            struct PP_Size dimensions,
                            enum PP_PictureBufferType_Dev type) {
@@ -27,49 +26,41 @@ void ProvidePictureBuffers(PP_Instance instance,
   if (!object)
     return;
   static_cast<VideoDecoderClient_Dev*>(object)->ProvidePictureBuffers(
-      VideoDecoder_Dev(decoder_id), req_num_of_bufs, dimensions, type);
+      req_num_of_bufs, dimensions, type);
 }
 
 void DismissPictureBuffer(PP_Instance instance,
-                          PP_Resource decoder_id,
                           int32_t picture_buffer_id) {
   void* object = pp::Instance::GetPerInstanceObject(
       instance, kPPPVideoDecoderInterface);
   if (!object)
     return;
   static_cast<VideoDecoderClient_Dev*>(object)->DismissPictureBuffer(
-      VideoDecoder_Dev(decoder_id), picture_buffer_id);
+      picture_buffer_id);
 }
 
-void PictureReady(PP_Instance instance,
-                  PP_Resource decoder_id,
-                  PP_Picture_Dev picture) {
+void PictureReady(PP_Instance instance, PP_Picture_Dev picture) {
   void* object = pp::Instance::GetPerInstanceObject(
       instance, kPPPVideoDecoderInterface);
   if (!object)
     return;
-  static_cast<VideoDecoderClient_Dev*>(object)->PictureReady(
-      VideoDecoder_Dev(decoder_id), picture);
+  static_cast<VideoDecoderClient_Dev*>(object)->PictureReady(picture);
 }
 
-void EndOfStream(PP_Instance instance, PP_Resource decoder_id) {
+void EndOfStream(PP_Instance instance) {
   void* object = pp::Instance::GetPerInstanceObject(
       instance, kPPPVideoDecoderInterface);
   if (!object)
     return;
-  static_cast<VideoDecoderClient_Dev*>(object)->EndOfStream(
-      VideoDecoder_Dev(decoder_id));
+  static_cast<VideoDecoderClient_Dev*>(object)->EndOfStream();
 }
 
-void NotifyError(PP_Instance instance,
-                 PP_Resource decoder_id,
-                 PP_VideoDecodeError_Dev error) {
+void NotifyError(PP_Instance instance, PP_VideoDecodeError_Dev error) {
   void* object = pp::Instance::GetPerInstanceObject(
       instance, kPPPVideoDecoderInterface);
   if (!object)
     return;
-  static_cast<VideoDecoderClient_Dev*>(object)->NotifyError(
-      VideoDecoder_Dev(decoder_id), error);
+  static_cast<VideoDecoderClient_Dev*>(object)->NotifyError(error);
 }
 
 static PPP_VideoDecoder_Dev videodecoder_interface = {
diff --git a/ppapi/cpp/dev/video_decoder_client_dev.h b/ppapi/cpp/dev/video_decoder_client_dev.h
index 776d54b..d1c5742 100644
--- a/ppapi/cpp/dev/video_decoder_client_dev.h
+++ b/ppapi/cpp/dev/video_decoder_client_dev.h
@@ -24,29 +24,23 @@ class VideoDecoderClient_Dev {
 
   // Callback to provide buffers for the decoded output pictures.
   virtual void ProvidePictureBuffers(
-      VideoDecoder_Dev decoder,
       uint32_t req_num_of_bufs,
       struct PP_Size dimensions,
       enum PP_PictureBufferType_Dev type) = 0;
 
-  // Callback for decoder to delivered unneeded picture buffers back to the
+  // Callback for decoder to deliver unneeded picture buffers back to the
   // plugin.
-  virtual void DismissPictureBuffer(
-      VideoDecoder_Dev decoder,
-      int32_t picture_buffer_id) = 0;
+  virtual void DismissPictureBuffer(int32_t picture_buffer_id) = 0;
 
   // Callback to deliver decoded pictures ready to be displayed.
-  virtual void PictureReady(
-      VideoDecoder_Dev decoder,
-      const PP_Picture_Dev& picture) = 0;
+  virtual void PictureReady(const PP_Picture_Dev& picture) = 0;
 
   // Callback to notify that decoder has decoded end of stream marker and has
   // outputted all displayable pictures.
-  virtual void EndOfStream(VideoDecoder_Dev decoder) = 0;
+  virtual void EndOfStream() = 0;
 
   // Callback to notify about decoding errors.
-  virtual void NotifyError(
-      VideoDecoder_Dev decoder, PP_VideoDecodeError_Dev error) = 0;
+  virtual void NotifyError(PP_VideoDecodeError_Dev error) = 0;
 
  private:
   Instance* associated_instance_;
diff --git a/ppapi/cpp/dev/video_decoder_dev.cc b/ppapi/cpp/dev/video_decoder_dev.cc
index 89fe6ae..2ef19fc 100644
--- a/ppapi/cpp/dev/video_decoder_dev.cc
+++ b/ppapi/cpp/dev/video_decoder_dev.cc
@@ -32,7 +32,10 @@ VideoDecoder_Dev::VideoDecoder_Dev(const Instance& instance) {
 VideoDecoder_Dev::VideoDecoder_Dev(PP_Resource resource) : Resource(resource) {
 }
 
-VideoDecoder_Dev::~VideoDecoder_Dev() {}
+VideoDecoder_Dev::~VideoDecoder_Dev() {
+  get_interface<PPB_VideoDecoder_Dev>()->Destroy(pp_resource());
+}
+
 
 int32_t VideoDecoder_Dev::Initialize(const PP_VideoConfigElement* config,
                                      const Context3D_Dev& context,
@@ -82,11 +85,4 @@ int32_t VideoDecoder_Dev::Reset(CompletionCallback callback) {
       pp_resource(), callback.pp_completion_callback());
 }
 
-int32_t VideoDecoder_Dev::Destroy(CompletionCallback callback) {
-  if (!has_interface<PPB_VideoDecoder_Dev>())
-    return callback.MayForce(PP_ERROR_NOINTERFACE);
-  return get_interface<PPB_VideoDecoder_Dev>()->Destroy(
-      pp_resource(), callback.pp_completion_callback());
-}
-
 }  // namespace pp
diff --git a/ppapi/cpp/dev/video_decoder_dev.h b/ppapi/cpp/dev/video_decoder_dev.h
index 03ddc59..3d6f2b9 100644
--- a/ppapi/cpp/dev/video_decoder_dev.h
+++ b/ppapi/cpp/dev/video_decoder_dev.h
@@ -42,7 +42,10 @@ class VideoDecoder_Dev : public Resource {
   void ReusePictureBuffer(int32_t picture_buffer_id);
   int32_t Flush(CompletionCallback callback);
   int32_t Reset(CompletionCallback callback);
-  int32_t Destroy(CompletionCallback callback);
+
+ private:
+  // Disallow copy-construction to ensure Destroy() is called exactly once.
+  VideoDecoder_Dev(const VideoDecoder_Dev&);
 };
 
 }  // namespace pp
diff --git a/ppapi/examples/gles2/gles2.cc b/ppapi/examples/gles2/gles2.cc
index 60c5bdf..a7e076e 100644
--- a/ppapi/examples/gles2/gles2.cc
+++ b/ppapi/examples/gles2/gles2.cc
@@ -58,15 +58,12 @@ class GLES2DemoInstance : public pp::Instance, public pp::Graphics3DClient_Dev,
 
   // pp::VideoDecoderClient_Dev implementation.
   virtual void ProvidePictureBuffers(
-      pp::VideoDecoder_Dev decoder, uint32_t req_num_of_bufs,
-      PP_Size dimensions, PP_PictureBufferType_Dev type);
-  virtual void DismissPictureBuffer(
-      pp::VideoDecoder_Dev decoder, int32_t picture_buffer_id);
-  virtual void PictureReady(
-      pp::VideoDecoder_Dev decoder, const PP_Picture_Dev& picture);
-  virtual void EndOfStream(pp::VideoDecoder_Dev decoder);
-  virtual void NotifyError(
-      pp::VideoDecoder_Dev decoder, PP_VideoDecodeError_Dev error);
+      uint32_t req_num_of_bufs, PP_Size dimensions,
+      PP_PictureBufferType_Dev type);
+  virtual void DismissPictureBuffer(int32_t picture_buffer_id);
+  virtual void PictureReady(const PP_Picture_Dev& picture);
+  virtual void EndOfStream();
+  virtual void NotifyError(PP_VideoDecodeError_Dev error);
 
  private:
   enum { kNumConcurrentDecodes = 7 };
@@ -78,7 +75,6 @@ class GLES2DemoInstance : public pp::Instance, public pp::Graphics3DClient_Dev,
   void DecoderInitDone(int32_t result);
   void DecoderBitstreamDone(int32_t result, int bitstream_buffer_id);
   void DecoderFlushDone(int32_t result);
-  void DecoderAbortDone(int32_t result);
 
   // Decode helpers.
   void DecodeNextNALUs();
@@ -146,7 +142,7 @@ GLES2DemoInstance::GLES2DemoInstance(PP_Instance instance, pp::Module* module)
 }
 
 GLES2DemoInstance::~GLES2DemoInstance() {
-  delete video_decoder_;
+  delete video_decoder_;  // May be NULL, which is fine.
   delete surface_;
   delete context_;
 }
@@ -194,9 +190,8 @@ void GLES2DemoInstance::DecoderFlushDone(int32_t result) {
   // Check that each bitstream buffer ID we handed to the decoder got handed
   // back to us.
   assert(bitstream_ids_at_decoder_.empty());
-}
-
-void GLES2DemoInstance::DecoderAbortDone(int32_t result) {
+  delete video_decoder_;
+  video_decoder_ = NULL;
 }
 
 static bool LookingAtNAL(const unsigned char* encoded, size_t pos) {
@@ -256,7 +251,7 @@ void GLES2DemoInstance::DecodeNextNALU() {
 }
 
 void GLES2DemoInstance::ProvidePictureBuffers(
-    pp::VideoDecoder_Dev decoder, uint32_t req_num_of_bufs, PP_Size dimensions,
+    uint32_t req_num_of_bufs, PP_Size dimensions,
     PP_PictureBufferType_Dev type) {
   std::vector<PP_GLESBuffer_Dev> buffers;
   for (uint32_t i = 0; i < req_num_of_bufs; i++) {
@@ -270,16 +265,14 @@ void GLES2DemoInstance::ProvidePictureBuffers(
   video_decoder_->AssignGLESBuffers(buffers);
 }
 
-void GLES2DemoInstance::DismissPictureBuffer(
-    pp::VideoDecoder_Dev decoder, int32_t picture_buffer_id) {
+void GLES2DemoInstance::DismissPictureBuffer(int32_t picture_buffer_id) {
   PictureBufferMap::iterator it = buffers_by_id_.find(picture_buffer_id);
   assert(it != buffers_by_id_.end());
   DeleteTexture(it->second.texture_id);
   buffers_by_id_.erase(it);
 }
 
-void GLES2DemoInstance::PictureReady(
-    pp::VideoDecoder_Dev decoder, const PP_Picture_Dev& picture) {
+void GLES2DemoInstance::PictureReady(const PP_Picture_Dev& picture) {
   if (first_frame_delivered_ticks_ == -1)
     assert((first_frame_delivered_ticks_ = core_if_->GetTimeTicks()) != -1);
   if (is_painting_) {
@@ -292,11 +285,10 @@ void GLES2DemoInstance::PictureReady(
   Render(it->second);
 }
 
-void GLES2DemoInstance::EndOfStream(pp::VideoDecoder_Dev decoder) {
+void GLES2DemoInstance::EndOfStream() {
 }
 
-void GLES2DemoInstance::NotifyError(
-    pp::VideoDecoder_Dev decoder, PP_VideoDecodeError_Dev error) {
+void GLES2DemoInstance::NotifyError(PP_VideoDecodeError_Dev error) {
 }
 
 // This object is the global object representing this plugin library as long
@@ -366,11 +358,12 @@ void GLES2DemoInstance::PaintFinished(int32_t result, int picture_buffer_id) {
               << fps << ", with average ms/swap of: " << ms_per_swap
               << std::endl;
   }
-  video_decoder_->ReusePictureBuffer(picture_buffer_id);
+  if (video_decoder_)
+    video_decoder_->ReusePictureBuffer(picture_buffer_id);
   while (!pictures_pending_paint_.empty() && !is_painting_) {
     PP_Picture_Dev picture = pictures_pending_paint_.front();
     pictures_pending_paint_.pop_front();
-    PictureReady(*video_decoder_, picture);
+    PictureReady(picture);
   }
 }
 
diff --git a/ppapi/thunk/ppb_video_decoder_api.h b/ppapi/thunk/ppb_video_decoder_api.h
index d019e8e..15c540a 100644
--- a/ppapi/thunk/ppb_video_decoder_api.h
+++ b/ppapi/thunk/ppb_video_decoder_api.h
@@ -24,7 +24,7 @@ class PPB_VideoDecoder_API {
   virtual void ReusePictureBuffer(int32_t picture_buffer_id) = 0;
   virtual int32_t Flush(PP_CompletionCallback callback) = 0;
   virtual int32_t Reset(PP_CompletionCallback callback) = 0;
-  virtual int32_t Destroy(PP_CompletionCallback callback) = 0;
+  virtual void Destroy() = 0;
 };
 
 }  // namespace thunk
diff --git a/ppapi/thunk/ppb_video_decoder_thunk.cc b/ppapi/thunk/ppb_video_decoder_thunk.cc
index 434d804..afc3bde 100644
--- a/ppapi/thunk/ppb_video_decoder_thunk.cc
+++ b/ppapi/thunk/ppb_video_decoder_thunk.cc
@@ -81,13 +81,10 @@ int32_t Reset(PP_Resource video_decoder,
   return MayForceCallback(callback, result);
 }
 
-int32_t Destroy(PP_Resource video_decoder,
-                PP_CompletionCallback callback) {
+void Destroy(PP_Resource video_decoder) {
   EnterVideoDecoder enter(video_decoder, true);
-  if (enter.failed())
-    return MayForceCallback(callback, PP_ERROR_BADRESOURCE);
-  int32_t result = enter.object()->Destroy(callback);
-  return MayForceCallback(callback, result);
+  if (enter.succeeded())
+    enter.object()->Destroy();
 }
 
 const PPB_VideoDecoder_Dev g_ppb_videodecoder_thunk = {