New documentation for resource.h, core.h, and common.h

New documentation for resource.h, core.h, and common.h
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88562

[dmichael (off chromium), 2011-06-07 05:01:23]

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h
File ppapi/cpp/core.h (right):

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode26
ppapi/cpp/core.h:26: /// A function that adds a reference to a resource.
'Ref' is really short for ref count.  I would say something like:
AddRefResource increments the reference count for the given resource.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode33
ppapi/cpp/core.h:33: /// A function that removes a reference from a resource.
ReleaseResource decrements the reference count for the given resource.  If the
reference count reaches zero, the resource will be deallocated.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode71
ppapi/cpp/core.h:71: /// of this, it will not run change if the user changes
their computer clock.
remove 'run'

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode80
ppapi/cpp/core.h:80: /// A function that schedules work to be executed on the
main module thread
module thread->pepper thread?  This should be consistent with what IsMainThread
says, and I'm leaning towards 'pepper thread', though either is right.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode105
ppapi/cpp/core.h:105: /// A function that returns true if thecurrent thread is
the main pepper
thecurrent->the current

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h
File ppapi/cpp/resource.h (right):

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode22
ppapi/cpp/resource.h:22: /// A constructor accepting a reference to a Resource.
You can just say 'The copy constructor.'

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode36
ppapi/cpp/resource.h:36: /// This functions determiens if this resource is
invalid or
I liked the original wording a little better.  if you leave it this way, fix
'determiens'

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode44
ppapi/cpp/resource.h:44: /// This function releases ownership of this reource
and returns it to the
reource->resource

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode54
ppapi/cpp/resource.h:54: /// A constructer used when a PP_Resource is provided
as a return value
constructer->constructor

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode55
ppapi/cpp/resource.h:55: /// that we need to provide a reference.
change to 'whose reference count we need to increment.'

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode61
ppapi/cpp/resource.h:61: /// Resource with a PP_Resource that has already
received a reference using
'received a reference'->'had its reference count incremented'

[jond, 2011-06-07 16:41:00]

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h
File ppapi/cpp/core.h (right):

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode26
ppapi/cpp/core.h:26: /// A function that adds a reference to a resource.
On 2011/06/07 05:01:23, dmichael1 wrote:
> 'Ref' is really short for ref count.  I would say something like:
> AddRefResource increments the reference count for the given resource.

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode33
ppapi/cpp/core.h:33: /// A function that removes a reference from a resource.
On 2011/06/07 05:01:23, dmichael1 wrote:
> ReleaseResource decrements the reference count for the given resource.  If the
> reference count reaches zero, the resource will be deallocated.

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode71
ppapi/cpp/core.h:71: /// of this, it will not run change if the user changes
their computer clock.
On 2011/06/07 05:01:23, dmichael1 wrote:
> remove 'run'

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode71
ppapi/cpp/core.h:71: /// of this, it will not run change if the user changes
their computer clock.
On 2011/06/07 05:01:23, dmichael1 wrote:
> remove 'run'

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode80
ppapi/cpp/core.h:80: /// A function that schedules work to be executed on the
main module thread
On 2011/06/07 05:01:23, dmichael1 wrote:
> module thread->pepper thread?  This should be consistent with what
IsMainThread
> says, and I'm leaning towards 'pepper thread', though either is right.

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode105
ppapi/cpp/core.h:105: /// A function that returns true if thecurrent thread is
the main pepper
On 2011/06/07 05:01:23, dmichael1 wrote:
> thecurrent->the current

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/core.h#newcode105
ppapi/cpp/core.h:105: /// A function that returns true if thecurrent thread is
the main pepper
On 2011/06/07 05:01:23, dmichael1 wrote:
> thecurrent->the current

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h
File ppapi/cpp/resource.h (right):

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode22
ppapi/cpp/resource.h:22: /// A constructor accepting a reference to a Resource.
On 2011/06/07 05:01:23, dmichael1 wrote:
> You can just say 'The copy constructor.'

I changed this to "A constructor fo copying a resource" only because in other
places, I start this sentence "A constructor..." and I didn't want to have to
rewrite all of them.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode36
ppapi/cpp/resource.h:36: /// This functions determiens if this resource is
invalid or
I'm trying to be consistent and all these sentences start out "This function..."

- Jon

On 2011/06/07 05:01:23, dmichael1 wrote:
> I liked the original wording a little better.  if you leave it this way, fix
> 'determiens'

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode44
ppapi/cpp/resource.h:44: /// This function releases ownership of this reource
and returns it to the
On 2011/06/07 05:01:23, dmichael1 wrote:
> reource->resource

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode44
ppapi/cpp/resource.h:44: /// This function releases ownership of this reource
and returns it to the
On 2011/06/07 05:01:23, dmichael1 wrote:
> reource->resource

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode54
ppapi/cpp/resource.h:54: /// A constructer used when a PP_Resource is provided
as a return value
On 2011/06/07 05:01:23, dmichael1 wrote:
> constructer->constructor

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode55
ppapi/cpp/resource.h:55: /// that we need to provide a reference.
On 2011/06/07 05:01:23, dmichael1 wrote:
> change to 'whose reference count we need to increment.'

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode55
ppapi/cpp/resource.h:55: /// that we need to provide a reference.
On 2011/06/07 05:01:23, dmichael1 wrote:
> change to 'whose reference count we need to increment.'

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode61
ppapi/cpp/resource.h:61: /// Resource with a PP_Resource that has already
received a reference using
On 2011/06/07 05:01:23, dmichael1 wrote:
> 'received a reference'->'had its reference count incremented'

Done.

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode61
ppapi/cpp/resource.h:61: /// Resource with a PP_Resource that has already
received a reference using
On 2011/06/07 05:01:23, dmichael1 wrote:
> 'received a reference'->'had its reference count incremented'

Done.

[dmichael (off chromium), 2011-06-07 20:45:26]

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h
File ppapi/cpp/resource.h (right):

http://codereview.chromium.org/7054060/diff/1/ppapi/cpp/resource.h#newcode22
ppapi/cpp/resource.h:22: /// A constructor accepting a reference to a Resource.
On 2011/06/07 16:41:00, jond wrote:
> On 2011/06/07 05:01:23, dmichael1 wrote:
> > You can just say 'The copy constructor.'
> 
> I changed this to "A constructor fo copying a resource" only because in other
> places, I start this sentence "A constructor..." and I didn't want to have to
> rewrite all of them.
Above it says 'The default constructor.'  The copy constructor is special as
well, and is called 'The copy constructor'.  I think wording it otherwise is
more confusing than helpful.

http://codereview.chromium.org/7054060/diff/5002/ppapi/cpp/resource.h#newcode62
ppapi/cpp/resource.h:62: /// Core::AddRefResource. It also assumes this object
has no current resource.
'incremented' -> 'incremented by'

[jond, 2011-06-08 16:30:11]

http://codereview.chromium.org/7054060/diff/5002/ppapi/cpp/resource.h
File ppapi/cpp/resource.h (right):

http://codereview.chromium.org/7054060/diff/5002/ppapi/cpp/resource.h#newcode62
ppapi/cpp/resource.h:62: /// incremented Core::AddRefResource. It also assumes
this object has no
On 2011/06/07 20:45:26, dmichael1 wrote:
> 'incremented' -> 'incremented by'

Done.

[dmichael (off chromium), 2011-06-08 19:17:27]

LGTM