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
This commit is contained in:
rmistry 2014-06-26 14:31:06 -07:00 committed by Commit bot
parent c1dfa14b64
commit e1c55869f3
7 changed files with 63 additions and 86 deletions

View File

@ -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).

View File

@ -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;

View File

@ -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);

View File

@ -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);

View File

@ -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();
}

View File

@ -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()) {

View File

@ -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;