Fixes for SkRWBuffer

src/core/SkRWBuffer.cpp

Index: src/core/SkRWBuffer.cpp
diff --git a/src/core/SkRWBuffer.cpp b/src/core/SkRWBuffer.cpp
index c0a93bdf53c037ce768a0699983f1d7cf1ce30d5..5b3f5bf3475d9643703501f8f9790037f0ddfffc 100644
--- a/src/core/SkRWBuffer.cpp
+++ b/src/core/SkRWBuffer.cpp
@@ -40,6 +40,8 @@ struct SkBufferBlock {
         return amount;
     }
 
+    // Do not call in the reader thread, since the writer may be updating fUsed.
+    // (The assertion is still true, but TSAN still may complain about its raciness.)
     void validate() const {
 #ifdef SK_DEBUG
         SkASSERT(fCapacity > 0);
@@ -94,7 +96,7 @@ struct SkBufferHead {
         }
     }
 
-    void validate(size_t minUsed, SkBufferBlock* tail = nullptr) const {
+    void validate(size_t minUsed, const SkBufferBlock* tail = nullptr) const {
 #ifdef SK_DEBUG
         SkASSERT(fRefCnt > 0);
         size_t totalUsed = 0;
@@ -114,19 +116,26 @@ struct SkBufferHead {
     }
 };
 
-SkROBuffer::SkROBuffer(const SkBufferHead* head, size_t used) : fHead(head), fUsed(used) {
+SkROBuffer::SkROBuffer(const SkBufferHead* head, size_t used, const SkBufferBlock* tail)
+    : fHead(head)
+    , fUsed(used)
+    , fTail(tail)
+    , fTailUsed(tail ? tail->fUsed : 0)
+{
     if (head) {
         fHead->ref();
         SkASSERT(used > 0);
-        head->validate(used);
+        SkASSERT(tail);
+        head->validate(used, tail);
     } else {
         SkASSERT(0 == used);
+        SkASSERT(!tail);
+        SkASSERT(0 == fTailUsed);
     }
 }
 
 SkROBuffer::~SkROBuffer() {
     if (fHead) {
-        fHead->validate(fUsed);
         fHead->unref();
     }
 }
@@ -136,7 +145,13 @@ SkROBuffer::Iter::Iter(const SkROBuffer* buffer) {
 }
 
 void SkROBuffer::Iter::reset(const SkROBuffer* buffer) {
+    if (!buffer->fUsed) {
+        SkASSERT(!buffer->fHead);
+        buffer = nullptr;
+    }
+    fBuffer = buffer;
     if (buffer) {
+        SkASSERT(buffer->fHead);
         fBlock = &buffer->fHead->fBlock;
         fRemaining = buffer->fUsed;
     } else {
@@ -153,13 +168,29 @@ size_t SkROBuffer::Iter::size() const {
     if (!fBlock) {
         return 0;
     }
-    return SkTMin(fBlock->fUsed, fRemaining);
+
+    if (fBuffer->fTail == fBlock) {
+        return fBuffer->fTailUsed;
+    }
+
+    // There is another block that this object knows about, with data that is
+    // accounted for in fRemaining.
+    // Further, since there are more blocks, it is safe to read fUsed, which will
+    // not be updated anymore.
+    SkASSERT(fRemaining > fBlock->fUsed);
+    return fBlock->fUsed;
 }
 
 bool SkROBuffer::Iter::next() {
     if (fRemaining) {
         fRemaining -= this->size();
-        fBlock = fBlock->fNext;
+        if (fBuffer->fTail == fBlock) {
+            // There are more blocks, but fBuffer does not know about them.
+            SkASSERT(fRemaining == 0);
+            fBlock = nullptr;
+        } else {
+            fBlock = fBlock->fNext;
+        }
     }
     return fRemaining != 0;
 }
@@ -234,7 +265,9 @@ void SkRWBuffer::validate() const {
 }
 #endif
 
-SkROBuffer* SkRWBuffer::newRBufferSnapshot() const { return new SkROBuffer(fHead, fTotalUsed); }
+SkROBuffer* SkRWBuffer::newRBufferSnapshot() const {
+    return new SkROBuffer(fHead, fTotalUsed, fTail);
+}
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////