Switch SkPDFStream's internal storage from SkStream to SkData

src/pdf/SkPDFFont.cpp

Index: src/pdf/SkPDFFont.cpp
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 014b328030c4e0baa0eacff09150fe510f583264..33b4cff5e8f921282643935d99dfc4fe80bd490d 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -150,8 +150,8 @@ int8_t hexToBin(uint8_t c) {
     return -1;
 }
 
-SkStream* handleType1Stream(SkStream* srcStream, size_t* headerLen,
-                            size_t* dataLen, size_t* trailerLen) {
+static SkData* handleType1Stream(SkStream* srcStream, size_t* headerLen,

[mtklein, 2014/06/25 18:12:31]

-> handle_type1_stream

[hal.canary, 2014/06/25 18:56:26]

Done.

+                                 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,26 +199,28 @@ SkStream* handleType1Stream(SkStream* srcStream, size_t* headerLen,
     SkAutoDataUnref aud(data);
 
     if (parsePFB(src, srcLen, headerLen, dataLen, trailerLen)) {
-        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,
+        const size_t length = *headerLen + *dataLen + *trailerLen;
+        SkASSERT(length > 0);
+        SkAutoMalloc buffer(length);
+        char* const result = static_cast<char*>(buffer.get());
+        memcpy(result, src + 6, *headerLen);

[mtklein, 2014/06/25 18:12:31]

Just pass buffer.get() into each of these?

+        memcpy(result + *headerLen, src + 6 + *headerLen + 6, *dataLen);
+        memcpy(result + (*headerLen + *dataLen),

[mtklein, 2014/06/25 18:12:31]

This whole area is really hard to follow.   Diagram and some structuring to make
it clear that what we're doing here with 6s is intentional?

+               src + 6 + *headerLen + 6 + *dataLen,
                *trailerLen);
-        result->rewind();
-        return result;
+        return SkData::NewFromMalloc(buffer.detach(), length);
     }
 
     // A PFA has to be converted for PDF.
     size_t hexDataLen;
     if (parsePFA((const char*)src, srcLen, headerLen, &hexDataLen, dataLen,
                  trailerLen)) {
-        SkMemoryStream* result =
-            new SkMemoryStream(*headerLen + *dataLen + *trailerLen);
-        memcpy((char*)result->getAtPos(), src, *headerLen);
-        result->seek(*headerLen);
+        const size_t length = *headerLen + *dataLen + *trailerLen;
+        SkASSERT(length > 0);
+        SkAutoMalloc buffer(length);

[mtklein, 2014/06/25 18:12:31]

Just start with SkAutoTMalloc<char> if you want chars?

[hal.canary, 2014/06/25 18:56:26]

SkAutoTMalloc uses SK_MALLOC_TEMP, which is not what we want.

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

Let's ignore that.  SK_MALLOC_TEMP doesn't do anything, and it looks like a
copy-and-paste bug that SkAutoTMalloc sets it.  (SkAutoSTMalloc makes a little
sense that it sets it, but really I think this is a dangerously designed feature
if the implication is we might use alloca instead of malloc...)

+        char* const result = static_cast<char*>(buffer.get());
+        memcpy(result, src, *headerLen);
+        char* const resultData = &(result[*headerLen]);
 
         const uint8_t* hexData = src + *headerLen;
         const uint8_t* trailer = hexData + hexDataLen;
@@ -236,21 +238,19 @@ SkStream* handleType1Stream(SkStream* srcStream, size_t* headerLen,
             } else {
                 dataByte |= curNibble;
                 highNibble = true;
-                ((char *)result->getAtPos())[outputOffset++] = dataByte;
+                resultData[outputOffset++] = dataByte;
             }
         }
         if (!highNibble) {
-            ((char *)result->getAtPos())[outputOffset++] = dataByte;
+            resultData[outputOffset++] = dataByte;
         }
         SkASSERT(outputOffset == *dataLen);
-        result->seek(*headerLen + outputOffset);
 
-        memcpy((char *)result->getAtPos(), src + *headerLen + hexDataLen,
-               *trailerLen);
-        result->rewind();
-        return result;
-    }
+        char* const resultTrailer = &(result[*headerLen + outputOffset]);
+        memcpy(resultTrailer, src + *headerLen + hexDataLen, *trailerLen);
 
+        return SkData::NewFromMalloc(buffer.detach(), length);
+    }
     return NULL;
 }
 
@@ -556,9 +556,8 @@ static SkPDFStream* generate_tounicode_cmap(
     append_cmap_sections(glyphToUnicode, subset, &cmap, multiByteGlyphs,
                          firstGlyphID, lastGlyphID);
     append_cmap_footer(&cmap);
-    SkAutoTUnref<SkMemoryStream> cmapStream(new SkMemoryStream());
-    cmapStream->setData(cmap.copyToData())->unref();
-    return new SkPDFStream(cmapStream.get());
+    SkAutoTUnref<SkData> cmapData(cmap.copyToData());
+    return new SkPDFStream(cmapData.get());

[mtklein, 2014/06/25 18:12:31]

Remind me why this is not

return new SkPDFStream(cmap.copyToData());

?

[hal.canary, 2014/06/25 18:56:26]

The SkPDFStream constructor refs the data.

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

Add a comment?

 }
 
 #if defined (SK_SFNTLY_SUBSETTER)
@@ -574,6 +573,7 @@ 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();
 
@@ -1295,8 +1295,8 @@ 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));
-    SkStream* fontData = handleType1Stream(rawFontData.get(), &header, &data,
-                                           &trailer);
+    SkData* fontData = handleType1Stream(rawFontData.get(), &header, &data,
+                                         &trailer);
     if (fontData == NULL) {
         return false;
     }