add protected method for internal_dispose overrides to jam fRefCnt before

add protected method for internal_dispose overrides to jam fRefCnt before
calling destructor.

move SkTRefArray to actually inherit from SkRefCnt
Committed: https://code.google.com/p/skia/source/detail?r=4719

[reed1, 2012-07-22 22:36:49]

adding culprit in friending GrTexture inside SkRefCnt

[robertphillips, 2012-07-23 14:05:17]

Unfortunately I don't think this will allow us to un-friend GrTexture from
SkRefCnt. GrTexture still needs to call SkRefCnt's internal_dispose method. We
should alter GrTexture to use the new method however so there will be one less
thing keeping that friending around.

http://codereview.appspot.com/6422057/diff/4001/include/core/SkRefCnt.h
File include/core/SkRefCnt.h (right):

http://codereview.appspot.com/6422057/diff/4001/include/core/SkRefCnt.h#newco...
include/core/SkRefCnt.h:76: // for debugging.
Should this be camel caps?

http://codereview.appspot.com/6422057/diff/4001/include/core/SkRefCnt.h#newco...
include/core/SkRefCnt.h:88: virtual void internal_dispose() const {
dispose?

http://codereview.appspot.com/6422057/diff/4001/src/core/SkTRefArray.h
File src/core/SkTRefArray.h (right):

http://codereview.appspot.com/6422057/diff/4001/src/core/SkTRefArray.h#newcode23
src/core/SkTRefArray.h:23: 
NEW macro?

http://codereview.appspot.com/6422057/diff/4001/src/core/SkTRefArray.h#newcode33
src/core/SkTRefArray.h:33: for (int i = 0; i < count; ++i) {
NEW macro?

http://codereview.appspot.com/6422057/diff/4001/src/core/SkTRefArray.h#newcode75
src/core/SkTRefArray.h:75: this->~SkTRefArray<T>();
Macro?

[bungeman, 2012-07-23 14:10:00]

The ref counting part looks ok, I don't see anything which would cause threading
issues.

https://codereview.appspot.com/6422057/diff/4001/include/core/SkRefCnt.h
File include/core/SkRefCnt.h (right):

https://codereview.appspot.com/6422057/diff/4001/include/core/SkRefCnt.h#newc...
include/core/SkRefCnt.h:77: void internal_displose_setrefcnt_to_1() const {
The name says what it does, but not why. Maybe something like
internal_dispose__pre_destruct or something like that?

https://codereview.appspot.com/6422057/diff/4001/include/core/SkRefCnt.h#newc...
include/core/SkRefCnt.h:88: virtual void internal_dispose() const {
Maybe a comment about how if you override this it is up to you to call
'internal_displose_setrefcnt_to_1' (or whatever it ends up being called) before
the destructor is run.

https://codereview.appspot.com/6422057/diff/4001/src/core/SkTRefArray.h
File src/core/SkTRefArray.h (right):

https://codereview.appspot.com/6422057/diff/4001/src/core/SkTRefArray.h#newco...
src/core/SkTRefArray.h:20: static SkTRefArray<T>* Alloc(int count) {
This guy is private (by default) but maybe it would be nice to be explicit about
it?

https://codereview.appspot.com/6422057/diff/4001/src/core/SkTRefArray.h#newco...
src/core/SkTRefArray.h:24: new (obj) SkTRefArray<T>;
Use SkNEW_PLACEMENT. (This may help with debugging later.)

[reed1, 2012-07-23 14:36:19]

tried to address all comments

[reed1, 2012-07-23 14:45:39]

fixed spelling of displose

[robertphillips, 2012-07-23 14:47:33]

LGTM w/ two nits

https://codereview.appspot.com/6422057/diff/3002/include/core/SkRefCnt.h
File include/core/SkRefCnt.h (right):

https://codereview.appspot.com/6422057/diff/3002/include/core/SkRefCnt.h#newc...
include/core/SkRefCnt.h:77: *  be called right before calling through to
inherited internal_dispose
ore -> or

https://codereview.appspot.com/6422057/diff/3002/src/core/SkTRefArray.h
File src/core/SkTRefArray.h (right):

https://codereview.appspot.com/6422057/diff/3002/src/core/SkTRefArray.h#newco...
src/core/SkTRefArray.h:50: /**
provide -> provided

[bungeman, 2012-07-23 14:56:18]

LGTM