Revert of Switch SkPDFStream's internal storage from SkStream to SkData (https://codereview.chromium.org/340783013/)

Reason for revert:
Causes canary failures

Original issue's description:
> Switch SkPDFStream's internal storage from SkStream to SkData
>
> Motivation: This makes SkPDFStream thread-safe for two threads
> serializing it at once, since a SkStream has an internal position.
>
> Updated SkPDFFont, SkPDFGraphicState, and SkPDFPage's use of
> SkPDFStream to use the SkData constructor rather than the SkStream
> constructor (saving a memcpy).
>
> BUG=skia:2683
>
> Committed: https://skia.googlesource.com/skia/+/c1dfa14b645ae274780f026dd86c9b633fbdad06

R=mtklein@google.com, djsollen@google.com, halcanary@google.com
TBR=djsollen@google.com, halcanary@google.com, mtklein@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:2683

Author: rmistry@google.com

Review URL: https://codereview.chromium.org/354043005

7 files changed, 63 insertions, 86 deletions

diff --git a/gyp/pdf.gyp b/gyp/pdf.gyp
index ca4d52e07f..e5923e7483 100644
--- a/gyp/pdf.gyp
+++ b/gyp/pdf.gyp
@@ -18,7 +18,6 @@
         '../src/core', # needed to get SkGlyphCache.h and SkTextFormatParams.h
         '../src/pdf',
         '../src/utils', # needed to get SkBitSet.h
-        '../src/images', # needed to get SkStreamHelpers.h
       ],
       'sources': [
         'pdf.gypi', # Makes the gypi appear in IDEs (but does not modify the build).
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index aa4b4b5534..014b328030 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -150,8 +150,8 @@ int8_t hexToBin(uint8_t c) {
     return -1;
 }
 
-static SkData* handle_type1_stream(SkStream* srcStream, size_t* headerLen,
-                                   size_t* dataLen, size_t* trailerLen) {
+SkStream* handleType1Stream(SkStream* srcStream, size_t* headerLen,
+                            size_t* dataLen, size_t* trailerLen) {
     // srcStream may be backed by a file or a unseekable fd, so we may not be
     // able to use skip(), rewind(), or getMemoryBase().  read()ing through
     // the input only once is doable, but very ugly. Furthermore, it'd be nice
@@ -199,43 +199,26 @@ static SkData* handle_type1_stream(SkStream* srcStream, size_t* headerLen,
     SkAutoDataUnref aud(data);
 
     if (parsePFB(src, srcLen, headerLen, dataLen, trailerLen)) {
-        static const int kPFBSectionHeaderLength = 6;
-        const size_t length = *headerLen + *dataLen + *trailerLen;
-        SkASSERT(length > 0);
-        SkASSERT(length + (2 * kPFBSectionHeaderLength) <= srcLen);
-
-        SkAutoTMalloc<uint8_t> buffer(length);
-
-        const uint8_t* const srcHeader = src + kPFBSectionHeaderLength;
-        // There is a six-byte section header before header and data
-        // (but not trailer) that we're not going to copy.
-        const uint8_t* const srcData
-            = srcHeader + *headerLen + kPFBSectionHeaderLength;
-        const uint8_t* const srcTrailer = srcData + *headerLen;
-
-        uint8_t* const resultHeader = buffer.get();
-        uint8_t* const resultData = resultHeader + *headerLen;
-        uint8_t* const resultTrailer = resultData + *dataLen;
-
-        SkASSERT(resultTrailer + *trailerLen == resultHeader + length);
-
-        memcpy(resultHeader,  srcHeader,  *headerLen);
-        memcpy(resultData,    srcData,    *dataLen);
-        memcpy(resultTrailer, srcTrailer, *trailerLen);
-
-        return SkData::NewFromMalloc(buffer.detach(), length);
+        SkMemoryStream* result =
+            new SkMemoryStream(*headerLen + *dataLen + *trailerLen);
+        memcpy((char*)result->getAtPos(), src + 6, *headerLen);
+        result->seek(*headerLen);
+        memcpy((char*)result->getAtPos(), src + 6 + *headerLen + 6, *dataLen);
+        result->seek(*headerLen + *dataLen);
+        memcpy((char*)result->getAtPos(), src + 6 + *headerLen + 6 + *dataLen,
+               *trailerLen);
+        result->rewind();
+        return result;
     }
 
     // A PFA has to be converted for PDF.
     size_t hexDataLen;
     if (parsePFA((const char*)src, srcLen, headerLen, &hexDataLen, dataLen,
                  trailerLen)) {
-        const size_t length = *headerLen + *dataLen + *trailerLen;
-        SkASSERT(length > 0);
-        SkAutoTMalloc<uint8_t> buffer(length);
-
-        memcpy(buffer.get(), src, *headerLen);
-        uint8_t* const resultData = &(buffer[*headerLen]);
+        SkMemoryStream* result =
+            new SkMemoryStream(*headerLen + *dataLen + *trailerLen);
+        memcpy((char*)result->getAtPos(), src, *headerLen);
+        result->seek(*headerLen);
 
         const uint8_t* hexData = src + *headerLen;
         const uint8_t* trailer = hexData + hexDataLen;
@@ -253,19 +236,21 @@ static SkData* handle_type1_stream(SkStream* srcStream, size_t* headerLen,
             } else {
                 dataByte |= curNibble;
                 highNibble = true;
-                resultData[outputOffset++] = dataByte;
+                ((char *)result->getAtPos())[outputOffset++] = dataByte;
             }
         }
         if (!highNibble) {
-            resultData[outputOffset++] = dataByte;
+            ((char *)result->getAtPos())[outputOffset++] = dataByte;
         }
         SkASSERT(outputOffset == *dataLen);
+        result->seek(*headerLen + outputOffset);
 
-        uint8_t* const resultTrailer = &(buffer[*headerLen + outputOffset]);
-        memcpy(resultTrailer, src + *headerLen + hexDataLen, *trailerLen);
-
-        return SkData::NewFromMalloc(buffer.detach(), length);
+        memcpy((char *)result->getAtPos(), src + *headerLen + hexDataLen,
+               *trailerLen);
+        result->rewind();
+        return result;
     }
+
     return NULL;
 }
 
@@ -571,8 +556,9 @@ static SkPDFStream* generate_tounicode_cmap(
     append_cmap_sections(glyphToUnicode, subset, &cmap, multiByteGlyphs,
                          firstGlyphID, lastGlyphID);
     append_cmap_footer(&cmap);
-    SkAutoTUnref<SkData> cmapData(cmap.copyToData());
-    return new SkPDFStream(cmapData.get());
+    SkAutoTUnref<SkMemoryStream> cmapStream(new SkMemoryStream());
+    cmapStream->setData(cmap.copyToData())->unref();
+    return new SkPDFStream(cmapStream.get());
 }
 
 #if defined (SK_SFNTLY_SUBSETTER)
@@ -588,7 +574,6 @@ static size_t get_subset_font_stream(const char* fontName,
                                      SkPDFStream** fontStream) {
     int ttcIndex;
     SkAutoTUnref<SkStream> fontData(typeface->openStream(&ttcIndex));
-    SkASSERT(fontData.get());
 
     size_t fontSize = fontData->getLength();
 
@@ -1310,7 +1295,7 @@ bool SkPDFType1Font::addFontDescriptor(int16_t defaultWidth) {
     size_t data SK_INIT_TO_AVOID_WARNING;
     size_t trailer SK_INIT_TO_AVOID_WARNING;
     SkAutoTUnref<SkStream> rawFontData(typeface()->openStream(&ttcIndex));
-    SkData* fontData = handle_type1_stream(rawFontData.get(), &header, &data,
+    SkStream* fontData = handleType1Stream(rawFontData.get(), &header, &data,
                                            &trailer);
     if (fontData == NULL) {
         return false;
diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp
index fa3baaf95e..1b495341b9 100644
--- a/src/pdf/SkPDFGraphicState.cpp
+++ b/src/pdf/SkPDFGraphicState.cpp
@@ -5,10 +5,10 @@
  * found in the LICENSE file.
  */
 
-#include "SkData.h"
 #include "SkPDFFormXObject.h"
 #include "SkPDFGraphicState.h"
 #include "SkPDFUtils.h"
+#include "SkStream.h"
 #include "SkTypes.h"
 
 static const char* blend_mode_from_xfermode(SkXfermode::Mode mode) {
@@ -121,9 +121,8 @@ SkPDFObject* SkPDFGraphicState::GetInvertFunction() {
         domainAndRange->appendInt(1);
 
         static const char psInvert[] = "{1 exch sub}";
-        // Do not copy the trailing '\0' into the SkData.
-        SkAutoTUnref<SkData> psInvertStream(
-                SkData::NewWithCopy(psInvert, strlen(psInvert)));
+        SkAutoTUnref<SkMemoryStream> psInvertStream(
+            new SkMemoryStream(&psInvert, strlen(psInvert), true));
 
         invertFunction = new SkPDFStream(psInvertStream.get());
         invertFunction->insertInt("FunctionType", 4);
diff --git a/src/pdf/SkPDFImage.cpp b/src/pdf/SkPDFImage.cpp
index 7e17f98a15..77fd84eff9 100644
--- a/src/pdf/SkPDFImage.cpp
+++ b/src/pdf/SkPDFImage.cpp
@@ -512,7 +512,7 @@ SkPDFImage::SkPDFImage(SkStream* stream,
     }
 
     if (stream != NULL) {
-        this->setData(stream);
+        setData(stream);
         fStreamValid = true;
     } else {
         fStreamValid = false;
@@ -598,11 +598,13 @@ bool SkPDFImage::populate(SkPDFCatalog* catalog) {
             SkAutoTUnref<SkData> data(fEncoder(&pixelRefOffset, subset));
             if (data.get() && data->size() < get_uncompressed_size(fBitmap,
                                                                    fSrcRect)) {
-                this->setData(data.get());
+                SkAutoTUnref<SkStream> stream(SkNEW_ARGS(SkMemoryStream,
+                                                         (data)));
+                setData(stream.get());
 
                 insertName("Filter", "DCTDecode");
                 insertInt("ColorTransform", kNoColorTransform);
-                insertInt("Length", this->dataSize());
+                insertInt("Length", getData()->getLength());
                 setState(kCompressed_State);
                 return true;
             }
@@ -611,7 +613,7 @@ bool SkPDFImage::populate(SkPDFCatalog* catalog) {
         if (!fStreamValid) {
             SkAutoTUnref<SkStream> stream(
                     extract_image_data(fBitmap, fSrcRect, fIsAlpha, NULL));
-            this->setData(stream);
+            setData(stream);
             fStreamValid = true;
         }
         return INHERITED::populate(catalog);
diff --git a/src/pdf/SkPDFPage.cpp b/src/pdf/SkPDFPage.cpp
index cfb6790876..8961d2f3d3 100644
--- a/src/pdf/SkPDFPage.cpp
+++ b/src/pdf/SkPDFPage.cpp
@@ -7,11 +7,11 @@
  */
 
 
-#include "SkData.h"
 #include "SkPDFCatalog.h"
 #include "SkPDFDevice.h"
 #include "SkPDFPage.h"
 #include "SkPDFResourceDict.h"
+#include "SkStream.h"
 
 SkPDFPage::SkPDFPage(SkPDFDevice* content)
     : SkPDFDict("Page"),
@@ -36,7 +36,7 @@ void SkPDFPage::finalizePage(SkPDFCatalog* catalog, bool firstPage,
             }
         }
 
-        SkAutoTUnref<SkData> content(fDevice->copyContentToData());
+        SkAutoTUnref<SkStream> content(fDevice->content());
         fContentStream.reset(new SkPDFStream(content.get()));
         insert("Contents", new SkPDFObjRef(fContentStream.get()))->unref();
     }
diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp
index 319433f7d0..e570976403 100644
--- a/src/pdf/SkPDFStream.cpp
+++ b/src/pdf/SkPDFStream.cpp
@@ -12,7 +12,6 @@
 #include "SkPDFCatalog.h"
 #include "SkPDFStream.h"
 #include "SkStream.h"
-#include "SkStreamHelpers.h"  // CopyStreamToData
 
 static bool skip_compression(SkPDFCatalog* catalog) {
     return SkToBool(catalog->getDocumentFlags() &
@@ -20,17 +19,17 @@ static bool skip_compression(SkPDFCatalog* catalog) {
 }
 
 SkPDFStream::SkPDFStream(SkStream* stream) : fState(kUnused_State) {
-    this->setData(stream);
+    setData(stream);
 }
 
 SkPDFStream::SkPDFStream(SkData* data) : fState(kUnused_State) {
-    this->setData(data);
+    setData(data);
 }
 
 SkPDFStream::SkPDFStream(const SkPDFStream& pdfStream)
         : SkPDFDict(),
           fState(kUnused_State) {
-    this->setData(pdfStream.fData.get());
+    setData(pdfStream.fData.get());
     bool removeLength = true;
     // Don't uncompress an already compressed stream, but we could.
     if (pdfStream.fState == kCompressed_State) {
@@ -56,16 +55,14 @@ void SkPDFStream::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
     if (indirect) {
         return emitIndirectObject(stream, catalog);
     }
-    SkAutoMutexAcquire lock(fMutex);  // multiple threads could be calling emit
     if (!this->populate(catalog)) {
         return fSubstitute->emitObject(stream, catalog, indirect);
     }
 
     this->INHERITED::emitObject(stream, catalog, false);
     stream->writeText(" stream\n");
-    if (fData.get()) {
-        stream->write(fData->data(), fData->size());
-    }
+    stream->writeStream(fData.get(), fData->getLength());
+    fData->rewind();
     stream->writeText("\nendstream");
 }
 
@@ -73,34 +70,30 @@ size_t SkPDFStream::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
     if (indirect) {
         return getIndirectOutputSize(catalog);
     }
-    SkAutoMutexAcquire lock(fMutex);  // multiple threads could be calling emit
     if (!this->populate(catalog)) {
         return fSubstitute->getOutputSize(catalog, indirect);
     }
 
     return this->INHERITED::getOutputSize(catalog, false) +
-        strlen(" stream\n\nendstream") + this->dataSize();
+        strlen(" stream\n\nendstream") + fData->getLength();
 }
 
 SkPDFStream::SkPDFStream() : fState(kUnused_State) {}
 
 void SkPDFStream::setData(SkData* data) {
-    fData.reset(SkSafeRef(data));
+    SkMemoryStream* stream = new SkMemoryStream;
+    stream->setData(data);
+    fData.reset(stream);  // Transfer ownership.
 }
 
 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);
     }
-}
-
-size_t SkPDFStream::dataSize() const {
-    return fData.get() ? fData->size() : 0;
+    fData.reset(stream);
+    SkSafeRef(stream);
 }
 
 bool SkPDFStream::populate(SkPDFCatalog* catalog) {
@@ -109,15 +102,17 @@ bool SkPDFStream::populate(SkPDFCatalog* catalog) {
             SkDynamicMemoryWStream compressedData;
 
             SkAssertResult(SkFlate::Deflate(fData.get(), &compressedData));
-            if (compressedData.getOffset() < this->dataSize()) {
-                fData.reset(compressedData.copyToData());
+            if (compressedData.getOffset() < fData->getLength()) {
+                SkMemoryStream* stream = new SkMemoryStream;
+                stream->setData(compressedData.copyToData())->unref();
+                fData.reset(stream);  // Transfer ownership.
                 insertName("Filter", "FlateDecode");
             }
             fState = kCompressed_State;
         } else {
             fState = kNoCompression_State;
         }
-        insertInt("Length", this->dataSize());
+        insertInt("Length", fData->getLength());
     } else if (fState == kNoCompression_State && !skip_compression(catalog) &&
                SkFlate::HaveFlate()) {
         if (!fSubstitute.get()) {
diff --git a/src/pdf/SkPDFStream.h b/src/pdf/SkPDFStream.h
index f847a2952b..6371bc187d 100644
--- a/src/pdf/SkPDFStream.h
+++ b/src/pdf/SkPDFStream.h
@@ -41,8 +41,7 @@ public:
     explicit SkPDFStream(const SkPDFStream& pdfStream);
     virtual ~SkPDFStream();
 
-    // The SkPDFObject interface.  These two methods use a mutex to
-    // allow multiple threads to call at the same time.
+    // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog,
                             bool indirect);
     virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect);
@@ -68,22 +67,22 @@ protected:
         fSubstitute.reset(stream);
     }
 
-    SkPDFStream* getSubstitute() const {
+    SkPDFStream* getSubstitute() {
         return fSubstitute.get();
     }
 
     void setData(SkData* data);
     void setData(SkStream* stream);
 
-    size_t dataSize() const;
-
-    SkData* getData() const { return fData.get(); }
+    SkStream* getData() {
+        return fData.get();
+    }
 
     void setState(State state) {
         fState = state;
     }
 
-    State getState() const {
+    State getState() {
         return fState;
     }
 
@@ -91,10 +90,8 @@ private:
     // Indicates what form (or if) the stream has been requested.
     State fState;
 
-    // Mutex guards fState, fData, and fSubstitute in public interface.
-    SkMutex fMutex;
-
-    SkAutoTUnref<SkData> fData;
+    // TODO(vandebo): Use SkData (after removing deprecated constructor).
+    SkAutoTUnref<SkStream> fData;
     SkAutoTUnref<SkPDFStream> fSubstitute;
 
     typedef SkPDFDict INHERITED;