Added idl file for Pepper 3D.

Added idl file for Pepper 3D.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=121419

[dmichael (off chromium), 2012-02-06 21:58:10]

lgtm

https://chromiumcodereview.appspot.com/9340003/diff/1/ppapi/c/ppb_graphics_3d.h
File ppapi/c/ppb_graphics_3d.h (right):

https://chromiumcodereview.appspot.com/9340003/diff/1/ppapi/c/ppb_graphics_3d...
ppapi/c/ppb_graphics_3d.h:95: * would share resources. If
<code>share_context</code> is not 0, then all
nit: would->will?

https://chromiumcodereview.appspot.com/9340003/diff/1/ppapi/c/ppb_graphics_3d...
ppapi/c/ppb_graphics_3d.h:274: * @param[in] callback The callback that will
executed when
nit: will "be" executed?

[noelallen_use_chromium, 2012-02-06 22:07:27]

Plz hold.  Got pulled into a meeting mid review, but there are some errors.

[alokp, 2012-02-06 22:08:44]

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h
File ppapi/c/ppb_graphics_3d.h (right):

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h#newco...
ppapi/c/ppb_graphics_3d.h:187: int32_t (*GetAttribs)(PP_Resource context,
int32_t* attrib_list[]);
I realized that this is wrong. attrib_list should be an array of integers, not
pointers.
Noel: How can it be fixed in idl?

[noelallen1, 2012-02-06 22:26:56]

http://codereview.chromium.org/9340003/diff/1/ppapi/api/ppb_graphics_3d.idl
File ppapi/api/ppb_graphics_3d.idl (right):

http://codereview.chromium.org/9340003/diff/1/ppapi/api/ppb_graphics_3d.idl#n...
ppapi/api/ppb_graphics_3d.idl:69: [out] int32_t[] value);
This is a value, not an array.  [out] int32_t value : An [out] will
automatically be converted to a non-const pointer for C.

http://codereview.chromium.org/9340003/diff/1/ppapi/api/ppb_graphics_3d.idl#n...
ppapi/api/ppb_graphics_3d.idl:178: [out] int32_t[] attrib_list);
inout not just out.  Out implies that we are allocating it.

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h
File ppapi/c/ppb_graphics_3d.h (right):

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h#newco...
ppapi/c/ppb_graphics_3d.h:85: int32_t* value[]);
* value []?

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h#newco...
ppapi/c/ppb_graphics_3d.h:187: int32_t (*GetAttribs)(PP_Resource context,
int32_t* attrib_list[]);
shouldn't be a pointer to an array.

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h#newco...
ppapi/c/ppb_graphics_3d.h:204: int32_t (*SetAttribs)(PP_Resource context, const
int32_t attrib_list[]);
You may need to fix sources.  This is a case of finding something that should
have been const to begin with.

[alokp, 2012-02-06 22:32:46]

http://codereview.chromium.org/9340003/diff/1/ppapi/api/ppb_graphics_3d.idl
File ppapi/api/ppb_graphics_3d.idl (right):

http://codereview.chromium.org/9340003/diff/1/ppapi/api/ppb_graphics_3d.idl#n...
ppapi/api/ppb_graphics_3d.idl:69: [out] int32_t[] value);
On 2012/02/06 22:26:57, noelallen1 wrote:
> This is a value, not an array.  [out] int32_t value : An [out] will
> automatically be converted to a non-const pointer for C.

done.

http://codereview.chromium.org/9340003/diff/1/ppapi/api/ppb_graphics_3d.idl#n...
ppapi/api/ppb_graphics_3d.idl:178: [out] int32_t[] attrib_list);
On 2012/02/06 22:26:57, noelallen1 wrote:
> inout not just out.  Out implies that we are allocating it.

Changing it to inout did not fix it. I still have an array of pointers.

[alokp, 2012-02-06 23:36:35]

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h
File ppapi/c/ppb_graphics_3d.h (right):

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h#newco...
ppapi/c/ppb_graphics_3d.h:95: * would share resources. If
<code>share_context</code> is not 0, then all
On 2012/02/06 21:58:11, dmichael wrote:
> nit: would->will?

Done.

http://codereview.chromium.org/9340003/diff/1/ppapi/c/ppb_graphics_3d.h#newco...
ppapi/c/ppb_graphics_3d.h:274: * @param[in] callback The callback that will
executed when
On 2012/02/06 21:58:11, dmichael wrote:
> nit: will "be" executed?

Done.

[noelallen1, 2012-02-06 23:46:58]

The generation IDL and generation now LGTM

[alokp, 2012-02-07 04:52:04]

http://codereview.chromium.org/9340003/diff/5001/ppapi/c/ppb_graphics_3d.h
File ppapi/c/ppb_graphics_3d.h (left):

http://codereview.chromium.org/9340003/diff/5001/ppapi/c/ppb_graphics_3d.h#ol...
ppapi/c/ppb_graphics_3d.h:8: #include "ppapi/c/pp_graphics_3d.h"
Noel: Is there a way to force-include this file? It is not required, but we may
break existing code if we don't.

[alokp, 2012-02-09 17:56:43]

+brettw (for question about revving the interface)

http://codereview.chromium.org/9340003/diff/13001/ppapi/c/ppb_graphics_3d.h
File ppapi/c/ppb_graphics_3d.h (right):

http://codereview.chromium.org/9340003/diff/13001/ppapi/c/ppb_graphics_3d.h#n...
ppapi/c/ppb_graphics_3d.h:207: int32_t (*SetAttribs)(PP_Resource context, const
int32_t attrib_list[]);
We are changing the function signature in this case. Although the change does
not affect binary compatibility, does it still warrant a rev change?

[brettw, 2012-02-09 18:15:29]

http://codereview.chromium.org/9340003/diff/13001/ppapi/c/ppb_graphics_3d.h
File ppapi/c/ppb_graphics_3d.h (right):

http://codereview.chromium.org/9340003/diff/13001/ppapi/c/ppb_graphics_3d.h#n...
ppapi/c/ppb_graphics_3d.h:207: int32_t (*SetAttribs)(PP_Resource context, const
int32_t attrib_list[]);
I'd say don't bother revving the interface if binary compat is the same. It will
be a big pain to update everything.

[dmichael (off chromium), 2012-02-09 18:27:49]

On Thu, Feb 9, 2012 at 11:15 AM, <brettw@chromium.org> wrote:

>
> http://codereview.chromium.**org/9340003/diff/13001/ppapi/**
>
c/ppb_graphics_3d.h<http://codereview.chromium.org/9340003/diff/13001/ppapi/c/ppb_graphics_3d.h>
> File ppapi/c/ppb_graphics_3d.h (right):
>
> http://codereview.chromium.**org/9340003/diff/13001/ppapi/**
>
c/ppb_graphics_3d.h#newcode207<http://codereview.chromium.org/9340003/diff/13001/ppapi/c/ppb_graphics_3d.h#newcode207>
> ppapi/c/ppb_graphics_3d.h:207: int32_t (*SetAttribs)(PP_Resource
> context, const int32_t attrib_list[]);
> I'd say don't bother revving the interface if binary compat is the same.
> It will be a big pain to update everything.
>
I had suggested revving it to Alok because it does potentially break source
compatibility. Though I think most (probably all) programs in the wild will
compile just fine and won't notice. So if you want to take the informed
risk, that's fine with me.

>
>
http://codereview.chromium.**org/9340003/<http://codereview.chromium.org/9340...
>

[commit-bot: I haz the power, 2012-02-09 22:23:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alokp@chromium.org/9340003/15008

[commit-bot: I haz the power, 2012-02-09 23:29:19]

Try job failure for 9340003-15008 (retry) on linux_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-02-10 06:49:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alokp@chromium.org/9340003/15008

[commit-bot: I haz the power, 2012-02-10 09:39:57]

Change committed as 121419