Fix some types at the IPC layer. Sizes should be unsigned.

Fix some types at the IPC layer. Sizes should be unsigned.

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

[Chris Evans, 2012-12-07 23:18:04]

@brettw: Viet-Trung suggested I bounce this review over to you.

[brettw, 2012-12-07 23:21:54]

https://codereview.chromium.org/11474040/diff/1/ppapi/proxy/ppb_video_decoder...
File ppapi/proxy/ppb_video_decoder_proxy.cc (right):

https://codereview.chromium.org/11474040/diff/1/ppapi/proxy/ppb_video_decoder...
ppapi/proxy/ppb_video_decoder_proxy.cc:235: PP_VideoBitstreamBuffer_Dev
bitstream = { id, buffer.host_resource(), size };
This unsigned size is promptly assigned to a signed value on the other side. I'm
not quire sure what the point of this change is in this context.

[Chris Evans, 2012-12-07 23:28:40]

On 2012/12/07 23:21:54, brettw wrote:
>
https://codereview.chromium.org/11474040/diff/1/ppapi/proxy/ppb_video_decoder...
> File ppapi/proxy/ppb_video_decoder_proxy.cc (right):
> 
>
https://codereview.chromium.org/11474040/diff/1/ppapi/proxy/ppb_video_decoder...
> ppapi/proxy/ppb_video_decoder_proxy.cc:235: PP_VideoBitstreamBuffer_Dev
> bitstream = { id, buffer.host_resource(), size };
> This unsigned size is promptly assigned to a signed value on the other side.
I'm
> not quire sure what the point of this change is in this context.

Thanks. Yes, PP_VideoBitstreamBuffer_Dev::size should be unsigned too.

That's part of the idl / API, though, so are we at liberty to change it?
Hopefully yes because it's a "Dev"-level API?

[Chris Evans, 2012-12-07 23:45:57]

Thanks Brett.

As discussed over chat, made the size in the ppapi/api/dev/pp_video_dev.idl file
unsigned too.

And adding @wez to the review.

@wez: I think this is your API? Any objections to the change of the "size"
integer from signed to unsigned? Will that cause you any issues?

[Wez, 2012-12-07 23:52:19]

-wez, +fischman: I believe PP_Video_Dev is used by Ami's PPB_VideoDecoder_Dev
interface.

[Ami GONE FROM CHROMIUM, 2012-12-08 00:29:08]

LGTM % removing the static_cast<size_t>() in PPB_VideoDecoder_Impl::Decode().

[brettw, 2012-12-08 00:32:48]

lgtm

[Chris Evans, 2012-12-08 00:38:25]

Thanks. Added @ihf as FYI.

[commit-bot: I haz the power, 2012-12-10 21:27:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cevans@chromium.org/11474040/4004

[commit-bot: I haz the power, 2012-12-10 21:27:50]

Presubmit check for 11474040-4004 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    ppapi/proxy/ppapi_messages.h

Presubmit checks took 5.4s to calculate.

[Cris Neckar, 2012-12-12 00:44:09]

lgtm