PPAPI: Fix interface functions that take PP_CompletionCallbacks, but don't

PPAPI: Fix interface functions that take PP_CompletionCallbacks, but don't
return codes from pp_errors.h

BUG=none
TEST=compile + manually run ppapi_tests::VideoDecoder
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=87905

[Ville-Mikko Rautio, 2011-06-01 11:44:15]

Added Ami and Victoria as reviewers, as this will affect their future work
substantially.

Basically LGTM.

However, video decoder related files in this CL affect heavily other CLs
currently in review with quite a bit of meat inside them (e.g.
http://codereview.chromium.org/7085030/,
http://codereview.chromium.org/6961018/) and that are just about to land. I
would probably rebase this CL after those issues land as it would mean only one
CL to be rebased instead of rebasing multiple CLs that are based on the
ppb_video_decoder_dev.h.

http://codereview.chromium.org/6975053/diff/2005/ppapi/c/dev/ppb_video_decode...
File ppapi/c/dev/ppb_video_decoder_dev.h (right):

http://codereview.chromium.org/6975053/diff/2005/ppapi/c/dev/ppb_video_decode...
ppapi/c/dev/ppb_video_decoder_dev.h:141: //   |vide_decoder| is the previously
created handle to the decoder resource.
NIT: vide_decoder => video_decoder

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
File webkit/plugins/ppapi/ppb_video_decoder_impl.h (right):

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
webkit/plugins/ppapi/ppb_video_decoder_impl.h:52: PP_CompletionCallback
callback);
Fix aligment.

[Ami GONE FROM CHROMIUM, 2011-06-01 15:45:37]

vmr: thanks for forwarding.
polina: what vmr@ said.  Outstanding CLs are expected to land by EOW.  Are you
OK with holding off this CL until the landscape clears up a bit?

Thanks for doing this!

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
File webkit/plugins/ppapi/ppb_video_decoder_impl.cc (right):

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
webkit/plugins/ppapi/ppb_video_decoder_impl.cc:236:
platform_video_decoder_->Initialize(copied);
We shouldn't be ignoring the return value here.  (pre-existing bug).

[Ami GONE FROM CHROMIUM, 2011-06-01 15:47:00]

> polina: what vmr@ said.

...by which I also meant that this looks good to me modulo the nits.

[vrk (LEFT CHROMIUM), 2011-06-01 20:50:34]

LGTM

[polina, 2011-06-01 21:45:43]

http://codereview.chromium.org/6975053/diff/2005/ppapi/c/dev/ppb_video_decode...
File ppapi/c/dev/ppb_video_decoder_dev.h (right):

http://codereview.chromium.org/6975053/diff/2005/ppapi/c/dev/ppb_video_decode...
ppapi/c/dev/ppb_video_decoder_dev.h:141: //   |vide_decoder| is the previously
created handle to the decoder resource.
On 2011/06/01 11:44:15, vmr1 wrote:
> NIT: vide_decoder => video_decoder

Done.

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
File webkit/plugins/ppapi/ppb_video_decoder_impl.cc (right):

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
webkit/plugins/ppapi/ppb_video_decoder_impl.cc:236:
platform_video_decoder_->Initialize(copied);
On 2011/06/01 15:45:37, Ami Fischman wrote:
> We shouldn't be ignoring the return value here.  (pre-existing bug).

Done.

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
File webkit/plugins/ppapi/ppb_video_decoder_impl.h (right):

http://codereview.chromium.org/6975053/diff/2005/webkit/plugins/ppapi/ppb_vid...
webkit/plugins/ppapi/ppb_video_decoder_impl.h:52: PP_CompletionCallback
callback);
On 2011/06/01 11:44:15, vmr1 wrote:
> Fix aligment.

Done.

[darin (slow to review), 2011-06-03 17:54:58]

http://codereview.chromium.org/6975053/diff/9001/ppapi/c/dev/ppb_video_decode...
File ppapi/c/dev/ppb_video_decoder_dev.h (right):

http://codereview.chromium.org/6975053/diff/9001/ppapi/c/dev/ppb_video_decode...
ppapi/c/dev/ppb_video_decoder_dev.h:146: int32_t (*Init)(PP_Resource
video_decoder,
nit: elsewhere we spell out Initialize.  I think we should be consistent and do
so here too.  Use words not abbreviations :)

http://codereview.chromium.org/6975053/diff/9001/ppapi/cpp/dev/video_decoder_...
File ppapi/cpp/dev/video_decoder_dev.cc (right):

http://codereview.chromium.org/6975053/diff/9001/ppapi/cpp/dev/video_decoder_...
ppapi/cpp/dev/video_decoder_dev.cc:37: CompletionCallback /* callback */) {
nit: indentation

int32_t VideoDecoder::Init(a,
                           b) {

http://codereview.chromium.org/6975053/diff/9001/ppapi/tests/test_video_decod...
File ppapi/tests/test_video_decoder.cc (right):

http://codereview.chromium.org/6975053/diff/9001/ppapi/tests/test_video_decod...
ppapi/tests/test_video_decoder.cc:31: std::string
TestVideoDecoder::TestCreateInit() {
nit: TestCreateAndInitialize

[darin (slow to review), 2011-06-03 17:55:10]

LGTM w/ those nits picked.

[polina, 2011-06-03 19:35:04]

http://codereview.chromium.org/6975053/diff/9001/ppapi/c/dev/ppb_video_decode...
File ppapi/c/dev/ppb_video_decoder_dev.h (right):

http://codereview.chromium.org/6975053/diff/9001/ppapi/c/dev/ppb_video_decode...
ppapi/c/dev/ppb_video_decoder_dev.h:146: int32_t (*Init)(PP_Resource
video_decoder,
On 2011/06/03 17:54:58, darin wrote:
> nit: elsewhere we spell out Initialize.  I think we should be consistent and
do
> so here too.  Use words not abbreviations :)

Done.

http://codereview.chromium.org/6975053/diff/9001/ppapi/cpp/dev/video_decoder_...
File ppapi/cpp/dev/video_decoder_dev.cc (right):

http://codereview.chromium.org/6975053/diff/9001/ppapi/cpp/dev/video_decoder_...
ppapi/cpp/dev/video_decoder_dev.cc:37: CompletionCallback /* callback */) {
On 2011/06/03 17:54:58, darin wrote:
> nit: indentation
> 
> int32_t VideoDecoder::Init(a,
>                            b) {

Done.

http://codereview.chromium.org/6975053/diff/9001/ppapi/tests/test_video_decod...
File ppapi/tests/test_video_decoder.cc (right):

http://codereview.chromium.org/6975053/diff/9001/ppapi/tests/test_video_decod...
ppapi/tests/test_video_decoder.cc:31: std::string
TestVideoDecoder::TestCreateInit() {
On 2011/06/03 17:54:58, darin wrote:
> nit: TestCreateAndInitialize

Done.

[polina, 2011-06-03 21:58:02]

Sync'ed passed one of vmr's CLs modifying these files.

[Ville-Mikko Rautio, 2011-06-06 12:55:54]

LGTM

Feel free to land this. The CL that was more affected by this landed on Friday.

I can do the rebase tomorrow on trunk with the CL that is still in the works.

[polina, 2011-06-06 21:26:19]

On 2011/06/06 12:55:54, vmr1 wrote:
> LGTM
> 
> Feel free to land this. The CL that was more affected by this landed on
Friday.
> 
> I can do the rebase tomorrow on trunk with the CL that is still in the works.

I have committed the CL, but it got blamed for a breakage of an unrelated
PPB_Scrollbar test and reverted. I am working to get it recommitted.

[jam, 2011-06-06 22:07:19]

I don't see how this could have caused any issue with the scrollbar tests.

[polina, 2011-06-06 22:33:49]

On 2011/06/06 22:07:19, John Abd-El-Malek wrote:
> I don't see how this could have caused any issue with the scrollbar tests.

Reverted the revert of this CL. Let's see if it lands now.
http://codereview.chromium.org/7112040/