Add SkTLList, linked list class implemented on top of the internal llist class.

Add SkTLList, linked list class implemented on top of the internal llist class.

R=robertphillips@google.com
Committed: https://code.google.com/p/skia/source/detail?r=6644
Committed: https://code.google.com/p/skia/source/detail?r=6647

[robertphillips, 2012-12-03 16:04:48]

LGTM + suggestions/questions

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h
File src/core/SkTLList.h (right):

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode12
src/core/SkTLList.h:12: the list creates the objects and they are deleted upon
removal. */
// This class in intended to add block allocation and linked list functionality
to an arbitrary class

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode19
src/core/SkTLList.h:19: SK_DECLARE_INTERNAL_LLIST_INTERFACE(Node);
// back pointer to owning block

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode40
src/core/SkTLList.h:40: if (0 == --block->fNodesInUse) {
Move the sk_free after the ~Node calls?

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode104
src/core/SkTLList.h:104: int count() const { return fCount; }
0 == fCount instead?

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode116
src/core/SkTLList.h:116: a.next(), b.next()) {
Turn this into: "if (NULL == b.get()) { return false; }"?

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode128
src/core/SkTLList.h:128: private:
Can this not go with the other private stuff at the bottom?

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode137
src/core/SkTLList.h:137: 
I thought we weren't calling INHERITED()?

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode174
src/core/SkTLList.h:174: Node* createNode() {
Node* node = fFreeList.head();
if (NULL != node) {
?

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode199
src/core/SkTLList.h:199: if (0 == --node->fBlock->fNodesInUse) {
// Opportunistically delete unused blocks to reduce memory consumption

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp
File tests/LListTest.cpp (right):

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp#newcode126
tests/LListTest.cpp:126: 
"to the tail"

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp#newcode156
tests/LListTest.cpp:156: #endif
Maybe "REPORTER_ASSERT(reporter, !list1.isEmpty() && !list2.isEmpty());" here?

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp#newcode174
tests/LListTest.cpp:174: } else {
Do we want to checkin this brkpt code?

[bsalomon, 2012-12-03 16:17:55]

Add SkTLList, linked list class implemented on top of the internal llist class.

R=robertphillips@google.com

[bsalomon, 2012-12-03 18:03:11]

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h
File src/core/SkTLList.h (right):

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode12
src/core/SkTLList.h:12: the list creates the objects and they are deleted upon
removal. */
On 2012/12/03 16:04:48, robertphillips wrote:
> // This class in intended to add block allocation and linked list
functionality
> to an arbitrary class

Done.

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode19
src/core/SkTLList.h:19: SK_DECLARE_INTERNAL_LLIST_INTERFACE(Node);
On 2012/12/03 16:04:48, robertphillips wrote:
> // back pointer to owning block

Done.

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode40
src/core/SkTLList.h:40: if (0 == --block->fNodesInUse) {
On 2012/12/03 16:04:48, robertphillips wrote:
> Move the sk_free after the ~Node calls?

Done.

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode104
src/core/SkTLList.h:104: int count() const { return fCount; }
On 2012/12/03 16:04:48, robertphillips wrote:
> 0 == fCount instead?

Done.

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode116
src/core/SkTLList.h:116: a.next(), b.next()) {
On 2012/12/03 16:04:48, robertphillips wrote:
> Turn this into: "if (NULL == b.get()) { return false; }"?

b should never be NULL because we first check if the counts match and we already
checked that a is not NULL. Added comment.

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode128
src/core/SkTLList.h:128: private:
On 2012/12/03 16:04:48, robertphillips wrote:
> Can this not go with the other private stuff at the bottom?
I'm using INHERITED::IterStart before private

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode137
src/core/SkTLList.h:137: 
On 2012/12/03 16:04:48, robertphillips wrote:
> I thought we weren't calling INHERITED()?

Done.

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode174
src/core/SkTLList.h:174: Node* createNode() {
On 2012/12/03 16:04:48, robertphillips wrote:
> Node* node = fFreeList.head();
> if (NULL != node) {
> ?

Done.

https://codereview.appspot.com/6869049/diff/1/src/core/SkTLList.h#newcode199
src/core/SkTLList.h:199: if (0 == --node->fBlock->fNodesInUse) {
On 2012/12/03 16:04:48, robertphillips wrote:
> // Opportunistically delete unused blocks to reduce memory consumption

Done.

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp
File tests/LListTest.cpp (right):

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp#newcode126
tests/LListTest.cpp:126: 
On 2012/12/03 16:04:48, robertphillips wrote:
> "to the tail"

Done.

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp#newcode156
tests/LListTest.cpp:156: #endif
On 2012/12/03 16:04:48, robertphillips wrote:
> Maybe "REPORTER_ASSERT(reporter, !list1.isEmpty() && !list2.isEmpty());" here?

Done.

https://codereview.appspot.com/6869049/diff/1/tests/LListTest.cpp#newcode174
tests/LListTest.cpp:174: } else {
On 2012/12/03 16:04:48, robertphillips wrote:
> Do we want to checkin this brkpt code?

Done.