Pepper: Change PPB_VideoDecoder::Reset behavior so plugin always manages textures.

ppapi/proxy/video_decoder_resource.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ppapi/proxy/video_decoder_resource.h"
 
 #include "base/bind.h"
 #include "gpu/command_buffer/client/gles2_cmd_helper.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "gpu/command_buffer/common/mailbox.h"
@@ skipping 131 matching lines @@
     return decoder_last_error_;
   if (flush_callback_ || reset_callback_)
     return PP_ERROR_FAILED;
   if (decode_callback_)
     return PP_ERROR_INPROGRESS;
   if (size > kMaximumBitstreamBufferSize)
     return PP_ERROR_NOMEMORY;
 
   // If we allow the plugin to call Decode again, we must have somewhere to
   // copy their buffer.
   DCHECK(!available_shm_buffers_.empty() ||

[igorc, 2014/07/14 23:16:42]

Perhaps we could change this to be a runtime check rather than DCHECK:

  if (available_shm_buffers_.empty() &&
      shm_buffers_.size() >= kMaximumPendingDecodes) {
    return PP_ERROR_INPROGRESS;
  }

[bbudge, 2014/07/15 15:58:08]

It's a DCHECK to catch programming errors in the proxy. It should never happen
in normal operation.

It is possible for a malicious plugin to bypass our checking such that this
condition was false in a release build, but the result would be only that it
would try to create one more shm buffer than our limit. The host will return an
error in this case since it does its own checking.

          shm_buffers_.size() < kMaximumPendingDecodes);
 
   // Count up, wrapping back to 0 before overflowing.
   int32_t uid = ++num_decodes_;
   if (uid == std::numeric_limits<int32_t>::max())
     num_decodes_ = 0;
 
   // Save decode_id in a ring buffer. The ring buffer is sized to store
   // decode_id for the maximum picture delay.
   decode_ids_[uid % kMaximumPictureDelay] = decode_id;
@@ skipping 90 matching lines @@
   }
 
   get_picture_callback_ = callback;
   get_picture_ = picture;
   return PP_OK_COMPLETIONPENDING;
 }
 
 void VideoDecoderResource::RecyclePicture(const PP_VideoPicture* picture) {
   if (decoder_last_error_)
     return;
   if (reset_callback_)

[igorc, 2014/07/14 23:07:07]

Should we now allow recycle during reset?

[bbudge, 2014/07/15 15:58:08]

Done.

     return;
 
   Post(RENDERER, PpapiHostMsg_VideoDecoder_RecyclePicture(picture->texture_id));
 }
 
 int32_t VideoDecoderResource::Flush(scoped_refptr<TrackedCallback> callback) {
   if (decoder_last_error_)
     return decoder_last_error_;
   if (reset_callback_)
     return PP_ERROR_FAILED;
@@ skipping 191 matching lines @@
   scoped_refptr<TrackedCallback> callback;
   callback.swap(flush_callback_);
   callback->Run(params.result());
 }
 
 void VideoDecoderResource::OnPluginMsgResetComplete(
     const ResourceMessageReplyParams& params) {
   // All shm buffers should have been made available by now.
   DCHECK_EQ(shm_buffers_.size(), available_shm_buffers_.size());
   // Received pictures are no longer valid.
-  while (!received_pictures_.empty())
+  while (!received_pictures_.empty()) {
+    Post(RENDERER, PpapiHostMsg_VideoDecoder_RecyclePicture(
+      received_pictures_.front().texture_id));

[igorc, 2014/07/14 23:07:07]

nit: 4 space alignment

[bbudge, 2014/07/15 15:58:08]

Done.

     received_pictures_.pop();
+  }
 
   scoped_refptr<TrackedCallback> callback;
   callback.swap(reset_callback_);
   callback->Run(params.result());
 }
 
 void VideoDecoderResource::RunCallbackWithError(
     scoped_refptr<TrackedCallback>* callback) {
   if (TrackedCallback::IsPending(*callback)) {
     scoped_refptr<TrackedCallback> temp;
@@ skipping 21 matching lines @@
     pp_picture->texture_target = it->second.texture_target;
     pp_picture->texture_size = it->second.size;
   } else {
     NOTREACHED();
   }
   received_pictures_.pop();
 }
 
 }  // namespace proxy
 }  // namespace ppapi