Switch SkPDFStream's internal storage from SkStream to SkData

src/pdf/SkPDFStream.cpp

Index: src/pdf/SkPDFStream.cpp
diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp
index e570976403382cac79dfcb327030391fa9a08253..f490c2b75b1328ef05ba9fea71992c8667976467 100644
--- a/src/pdf/SkPDFStream.cpp
+++ b/src/pdf/SkPDFStream.cpp
@@ -12,6 +12,7 @@
 #include "SkPDFCatalog.h"
 #include "SkPDFStream.h"
 #include "SkStream.h"
+#include "SkStreamHelpers.h"  // CopyStreamToData
 
 static bool skip_compression(SkPDFCatalog* catalog) {
     return SkToBool(catalog->getDocumentFlags() &
@@ -19,17 +20,17 @@ static bool skip_compression(SkPDFCatalog* catalog) {
 }
 
 SkPDFStream::SkPDFStream(SkStream* stream) : fState(kUnused_State) {
-    setData(stream);
+    this->setData(stream);
 }
 
 SkPDFStream::SkPDFStream(SkData* data) : fState(kUnused_State) {
-    setData(data);
+    this->setData(data);
 }
 
 SkPDFStream::SkPDFStream(const SkPDFStream& pdfStream)
         : SkPDFDict(),
           fState(kUnused_State) {
-    setData(pdfStream.fData.get());
+    this->setData(pdfStream.fData.get());
     bool removeLength = true;
     // Don't uncompress an already compressed stream, but we could.
     if (pdfStream.fState == kCompressed_State) {
@@ -61,8 +62,9 @@ void SkPDFStream::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
 
     this->INHERITED::emitObject(stream, catalog, false);
     stream->writeText(" stream\n");
-    stream->writeStream(fData.get(), fData->getLength());
-    fData->rewind();
+    if (fData.get()) {
+        stream->write(fData->data(), fData->size());
+    }
     stream->writeText("\nendstream");
 }
 
@@ -75,44 +77,46 @@ size_t SkPDFStream::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
     }
 
     return this->INHERITED::getOutputSize(catalog, false) +
-        strlen(" stream\n\nendstream") + fData->getLength();
+        strlen(" stream\n\nendstream") + this->dataSize();
 }
 
 SkPDFStream::SkPDFStream() : fState(kUnused_State) {}
 
 void SkPDFStream::setData(SkData* data) {
-    SkMemoryStream* stream = new SkMemoryStream;
-    stream->setData(data);
-    fData.reset(stream);  // Transfer ownership.
+    fData.reset(SkSafeRef(data));
 }
 
 void SkPDFStream::setData(SkStream* stream) {
     // Code assumes that the stream starts at the beginning and is rewindable.
     if (stream) {
         SkASSERT(stream->getPosition() == 0);
+        fData.reset(CopyStreamToData(stream));
         SkASSERT(stream->rewind());
+    } else {
+        fData.reset(NULL);
     }
-    fData.reset(stream);
-    SkSafeRef(stream);
+}
+
+size_t SkPDFStream::dataSize() {
+    return fData.get() ? fData->size() : 0;
 }
 
 bool SkPDFStream::populate(SkPDFCatalog* catalog) {
+    SkAutoMutexAcquire lock(fMutex);  // multiple threads could be calling emit

[mtklein, 2014/06/26 13:21:59]

Why not lock in emitObject then?

I'd recommend you put all thoughts of performance to the side for now.  We want
correctness, and preferably obvious correctness.  We can improve things
performance-wise from there, if we care.  Just lock the hell out of this if it's
at all uncertain.

[hal.canary, 2014/06/26 18:42:02]

Done.

     if (fState == kUnused_State) {
         if (!skip_compression(catalog) && SkFlate::HaveFlate()) {
             SkDynamicMemoryWStream compressedData;
 
             SkAssertResult(SkFlate::Deflate(fData.get(), &compressedData));
-            if (compressedData.getOffset() < fData->getLength()) {
-                SkMemoryStream* stream = new SkMemoryStream;
-                stream->setData(compressedData.copyToData())->unref();
-                fData.reset(stream);  // Transfer ownership.
+            if (compressedData.getOffset() < this->dataSize()) {
+                fData.reset(compressedData.copyToData());
                 insertName("Filter", "FlateDecode");
             }
             fState = kCompressed_State;
         } else {
             fState = kNoCompression_State;
         }
-        insertInt("Length", fData->getLength());
+        insertInt("Length", this->dataSize());
     } else if (fState == kNoCompression_State && !skip_compression(catalog) &&
                SkFlate::HaveFlate()) {
         if (!fSubstitute.get()) {