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

Pepper: Change PPB_VideoDecoder::Reset behavior so plugin always manages textures.
Reset now will allow plugin to call RecyclePicture. The plugin must recycle any
texture it receives now. This makes plugin code simpler, since it can treat all
textures it receives in the same way (no special Reset handling).

Also fix a bug where Reset didn't return textures to the hardware decoder, which
already has this behavior.

BUG=281689
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283483

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

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
File ppapi/proxy/video_decoder_resource.cc (right):

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
ppapi/proxy/video_decoder_resource.cc:263: if (reset_callback_)
Should we now allow recycle during reset?

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
ppapi/proxy/video_decoder_resource.cc:477:
received_pictures_.front().texture_id));
nit: 4 space alignment

[igorc, 2014-07-14 23:16:43]

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
File ppapi/proxy/video_decoder_resource.cc (right):

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
ppapi/proxy/video_decoder_resource.cc:152:
DCHECK(!available_shm_buffers_.empty() ||
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]

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
File ppapi/proxy/video_decoder_resource.cc (right):

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
ppapi/proxy/video_decoder_resource.cc:152:
DCHECK(!available_shm_buffers_.empty() ||
On 2014/07/14 23:16:42, igorc wrote:
> 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;
>   }

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.

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
ppapi/proxy/video_decoder_resource.cc:263: if (reset_callback_)
On 2014/07/14 23:07:07, igorc wrote:
> Should we now allow recycle during reset?

Done.

https://codereview.chromium.org/390213002/diff/20001/ppapi/proxy/video_decode...
ppapi/proxy/video_decoder_resource.cc:477:
received_pictures_.front().texture_id));
On 2014/07/14 23:07:07, igorc wrote:
> nit: 4 space alignment

Done.

[bbudge, 2014-07-15 15:58:34]

+binji for nacl sdk.

[binji, 2014-07-15 17:56:50]

native_client_sdk lgtm

[dmichael (off chromium), 2014-07-15 20:46:51]

lgtm

[bbudge, 2014-07-16 01:12:33]

The CQ bit was checked by bbudge@chromium.org

[bbudge, 2014-07-16 01:12:34]

The CQ bit was unchecked by bbudge@chromium.org

[bbudge, 2014-07-16 01:12:34]

The CQ bit was checked by bbudge@chromium.org

[commit-bot: I haz the power, 2014-07-16 01:15:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bbudge@chromium.org/390213002/60001

[commit-bot: I haz the power, 2014-07-16 20:24:04]

Change committed as 283483