Mostly minor changes such as adding <code></code> around elements, a little rewriting, and a few ...

Mostly minor changes such as adding <code></code> around elements, a little rewriting, and a few new documentation bits to pp_var.h. Mostly trying to ensure consistency across all docs and doing cleanup.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91221

[dmichael (off chromium), 2011-06-27 15:45:41]

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

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_completion_callback....
ppapi/c/pp_completion_callback.h:31: * information such as bytes read.
I know this is not your wording here, but I don't think it's right.  By the time
you get the callback, I think the operation has already completed (or not), and
negative values are an indication of error.  The wording sounds closer to the
meaning of the return value for a function that accepts a PP_CompletionCallback
parameter.  Here's a swing at rewording, though you may want to check with
Polina:

If result is 0 (PP_OK), the operation succeeded.  Negative values indicate error
and are specified in pp_errors.h.  Positive values for result usually indicate
success and have some operation-dependent meaning (such as bytes read).

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_completion_callback....
ppapi/c/pp_completion_callback.h:80: * This function is used to create a
<code>PP_CompletionCallback</code>.
I was fine with the old wording of 'PP_MakeCompletionCallback()'. Is 'This
function' a new convention?  I'm happy with it either way; I just want to make
sure you're not changing them on my account. My beef was with 'Func is a pointer
to a function'; mostly the 'is a pointer to a function' part.

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

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_input_event.h#newcod...
ppapi/c/pp_input_event.h:156: /** A combination of the
<code>EVENT_MODIFIER</code> flags. */
EVENT_MODIFIER->PP_InputEvent_Modifier

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_input_event.h#newcod...
ppapi/c/pp_input_event.h:183: * This value is a bit field combination of the
<code>EVENT_MODIFIER</code>
EVENT_MODIFIER->PP_InputEvent_Modifier

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

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode65
ppapi/c/pp_var.h:65: * from modules, although it is used internally for some
tasks.
I think this is one place where we could legitimately say 'Native Client
modules'.  I would also get rid of 'currently'; I don't think we'll ever add
support back in.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode106
ppapi/c/pp_var.h:106: * This value ensures <code>value</code> is aligned on an
8-byte boundary
I think this says 'value' one too many times.  Maybe just say:
'<code>padding</code> ensures <code>value</code>...'

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode114
ppapi/c/pp_var.h:114: * double, or id.
This comment is not accurate.  |value| really does contain the value for the
PP_Var; the type of the PP_Var is indicated by the field |type| above.  It's
more like:
"<code>value</code> represents the contents of the PP_Var. At most one of the
fields of <code>value</code> is valid at a time, based upon <code>type</code>."

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode120
ppapi/c/pp_var.h:120: */
Something like:
'If <code>type</code> is PP_VARTYPE_BOOL, <code>as_bool</code> represents the
value of this PP_Var as a PP_Bool.'

And similarly for the fields below.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode136
ppapi/c/pp_var.h:136: * Internal ID for strings objects, arrays, and
dictionaries. The
please add a comma after strings since you're in the neighborhood.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode137
ppapi/c/pp_var.h:137: * identifier is an opaque handle assigned by the browser
to the module. It
I think we should just take out 'to the module'; I think it's slightly
confusing.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode165
ppapi/c/pp_var.h:165: * This function function is used to wrap a null value into
a
If you stick with this wording, remove the extra 'function'

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode175
ppapi/c/pp_var.h:175: * This function isused to wrap a boolean value into a
isused->is used

[jond, 2011-06-29 21:14:37]

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

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_completion_callback....
ppapi/c/pp_completion_callback.h:31: * information such as bytes read.
Polinia mentioned that -1 isn't really a an error, so I mentioned it.

On 2011/06/27 15:45:41, dmichael1 wrote:
> I know this is not your wording here, but I don't think it's right.  By the
time
> you get the callback, I think the operation has already completed (or not),
and
> negative values are an indication of error.  The wording sounds closer to the
> meaning of the return value for a function that accepts a
PP_CompletionCallback
> parameter.  Here's a swing at rewording, though you may want to check with
> Polina:
> 
> If result is 0 (PP_OK), the operation succeeded.  Negative values indicate
error
> and are specified in pp_errors.h.  Positive values for result usually indicate
> success and have some operation-dependent meaning (such as bytes read).

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_completion_callback....
ppapi/c/pp_completion_callback.h:80: * This function is used to create a
<code>PP_CompletionCallback</code>.
On 2011/06/27 15:45:41, dmichael1 wrote:
> I was fine with the old wording of 'PP_MakeCompletionCallback()'. Is 'This
> function' a new convention?  I'm happy with it either way; I just want to make
> sure you're not changing them on my account. My beef was with 'Func is a
pointer
> to a function'; mostly the 'is a pointer to a function' part.

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_completion_callback....
ppapi/c/pp_completion_callback.h:80: * This function is used to create a
<code>PP_CompletionCallback</code>.
On 2011/06/27 15:45:41, dmichael1 wrote:
> I was fine with the old wording of 'PP_MakeCompletionCallback()'. Is 'This
> function' a new convention?  I'm happy with it either way; I just want to make
> sure you're not changing them on my account. My beef was with 'Func is a
pointer
> to a function'; mostly the 'is a pointer to a function' part.

Done.

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

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_input_event.h#newcod...
ppapi/c/pp_input_event.h:156: /** A combination of the
<code>EVENT_MODIFIER</code> flags. */
On 2011/06/27 15:45:41, dmichael1 wrote:
> EVENT_MODIFIER->PP_InputEvent_Modifier

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_input_event.h#newcod...
ppapi/c/pp_input_event.h:156: /** A combination of the
<code>EVENT_MODIFIER</code> flags. */
On 2011/06/27 15:45:41, dmichael1 wrote:
> EVENT_MODIFIER->PP_InputEvent_Modifier

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_input_event.h#newcod...
ppapi/c/pp_input_event.h:183: * This value is a bit field combination of the
<code>EVENT_MODIFIER</code>
On 2011/06/27 15:45:41, dmichael1 wrote:
> EVENT_MODIFIER->PP_InputEvent_Modifier

Done.

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

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode106
ppapi/c/pp_var.h:106: * This value ensures <code>value</code> is aligned on an
8-byte boundary
Had to put "The <code>padding</code>" because padding is lower case and I
couldn't start the sentence with that.

On 2011/06/27 15:45:41, dmichael1 wrote:
> I think this says 'value' one too many times.  Maybe just say:
> '<code>padding</code> ensures <code>value</code>...'

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode114
ppapi/c/pp_var.h:114: * double, or id.
On 2011/06/27 15:45:41, dmichael1 wrote:
> This comment is not accurate.  |value| really does contain the value for the
> PP_Var; the type of the PP_Var is indicated by the field |type| above.  It's
> more like:
> "<code>value</code> represents the contents of the PP_Var. At most one of the
> fields of <code>value</code> is valid at a time, based upon
<code>type</code>."

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode114
ppapi/c/pp_var.h:114: * double, or id.
On 2011/06/27 15:45:41, dmichael1 wrote:
> This comment is not accurate.  |value| really does contain the value for the
> PP_Var; the type of the PP_Var is indicated by the field |type| above.  It's
> more like:
> "<code>value</code> represents the contents of the PP_Var. At most one of the
> fields of <code>value</code> is valid at a time, based upon
<code>type</code>."

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode120
ppapi/c/pp_var.h:120: */
Can you double-check the as_id description now...

On 2011/06/27 15:45:41, dmichael1 wrote:
> Something like:
> 'If <code>type</code> is PP_VARTYPE_BOOL, <code>as_bool</code> represents the
> value of this PP_Var as a PP_Bool.'
> 
> And similarly for the fields below.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode136
ppapi/c/pp_var.h:136: * Internal ID for strings objects, arrays, and
dictionaries. The
On 2011/06/27 15:45:41, dmichael1 wrote:
> please add a comma after strings since you're in the neighborhood.

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode137
ppapi/c/pp_var.h:137: * identifier is an opaque handle assigned by the browser
to the module. It
On 2011/06/27 15:45:41, dmichael1 wrote:
> I think we should just take out 'to the module'; I think it's slightly
> confusing.

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode137
ppapi/c/pp_var.h:137: * identifier is an opaque handle assigned by the browser
to the module. It
On 2011/06/27 15:45:41, dmichael1 wrote:
> I think we should just take out 'to the module'; I think it's slightly
> confusing.

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode165
ppapi/c/pp_var.h:165: * This function function is used to wrap a null value into
a
On 2011/06/27 15:45:41, dmichael1 wrote:
> If you stick with this wording, remove the extra 'function'

Done.

http://codereview.chromium.org/7241024/diff/1/ppapi/c/pp_var.h#newcode175
ppapi/c/pp_var.h:175: * This function isused to wrap a boolean value into a
On 2011/06/27 15:45:41, dmichael1 wrote:
> isused->is used

Done.

[dmichael (off chromium), 2011-06-30 10:03:48]

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_completion_callb...
File ppapi/c/pp_completion_callback.h (right):

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_completion_callb...
ppapi/c/pp_completion_callback.h:29: * values, other than -1, indicate error and
are specified in pp_errors.h.
nit: Those commas don't seem necessary to me. Parens might be more fitting.

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_var.h
File ppapi/c/pp_var.h (right):

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_var.h#newcode121
ppapi/c/pp_var.h:121: * <code>as_bool</code> represents the value of this
<code>PP_VAR</code> as
PP_VAR->PP_Var, here and for the rest

[jond, 2011-06-30 16:24:16]

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_completion_callb...
File ppapi/c/pp_completion_callback.h (right):

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_completion_callb...
ppapi/c/pp_completion_callback.h:29: * values, other than -1, indicate error and
are specified in pp_errors.h.
On 2011/06/30 10:03:48, dmichael1 wrote:
> nit: Those commas don't seem necessary to me. Parens might be more fitting.

Done.

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_completion_callb...
ppapi/c/pp_completion_callback.h:29: * values, other than -1, indicate error and
are specified in pp_errors.h.
On 2011/06/30 10:03:48, dmichael1 wrote:
> nit: Those commas don't seem necessary to me. Parens might be more fitting.

Done.

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_var.h
File ppapi/c/pp_var.h (right):

http://codereview.chromium.org/7241024/diff/10001/ppapi/c/pp_var.h#newcode121
ppapi/c/pp_var.h:121: * <code>as_bool</code> represents the value of this
<code>PP_VAR</code> as
On 2011/06/30 10:03:48, dmichael1 wrote:
> PP_VAR->PP_Var, here and for the rest

Changed throughout document

[dmichael (off chromium), 2011-06-30 17:35:12]

LGTM, thanks!