diff options
author | scroggo <scroggo@google.com> | 2015-01-21 12:09:53 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-01-21 12:09:53 -0800 |
commit | a1193e4b0e34a7e4e1bd33e9708d7341679f8321 (patch) | |
tree | e63e73eeadfd6d979e81596804e34e7af8fa0390 /tests | |
parent | fd1ad48d4d9073b90f58e60219637046a8446267 (diff) |
Make SkStream *not* ref counted.
SkStream is a stateful object, so it does not make sense for it to have
multiple owners. Make SkStream inherit directly from SkNoncopyable.
Update methods which previously called SkStream::ref() (e.g.
SkImageDecoder::buildTileIndex() and SkFrontBufferedStream::Create(),
which required the existing owners to call SkStream::unref()) to take
ownership of their SkStream parameters and delete when done (including
on failure).
Switch all SkAutoTUnref<SkStream>s to SkAutoTDelete<SkStream>s. In some
cases this means heap allocating streams that were previously stack
allocated.
Respect ownership rules of SkTypeface::CreateFromStream() and
SkImageDecoder::buildTileIndex().
Update the comments for exceptional methods which do not affect the
ownership of their SkStream parameters (e.g.
SkPicture::CreateFromStream() and SkTypeface::Deserialize()) to be
explicit about ownership.
Remove test_stream_life, which tested that buildTileIndex() behaved
correctly when SkStream was a ref counted object. The test does not
make sense now that it is not.
In SkPDFStream, remove the SkMemoryStream member. Instead of using it,
create a new SkMemoryStream to pass to fDataStream (which is now an
SkAutoTDelete).
Make other pdf rasterizers behave like SkPDFDocumentToBitmap.
SkPDFDocumentToBitmap delete the SkStream, so do the same in the
following pdf rasterizers:
SkPopplerRasterizePDF
SkNativeRasterizePDF
SkNoRasterizePDF
Requires a change to Android, which currently treats SkStreams as ref
counted objects.
Review URL: https://codereview.chromium.org/849103004
Diffstat (limited to 'tests')
-rw-r--r-- | tests/FontHostStreamTest.cpp | 4 | ||||
-rw-r--r-- | tests/FrontBufferedStreamTest.cpp | 45 | ||||
-rw-r--r-- | tests/ImageDecodingTest.cpp | 70 | ||||
-rw-r--r-- | tests/KtxTest.cpp | 4 | ||||
-rw-r--r-- | tests/PDFPrimitivesTest.cpp | 2 | ||||
-rw-r--r-- | tests/SerializationTest.cpp | 2 | ||||
-rw-r--r-- | tests/StreamTest.cpp | 14 |
7 files changed, 40 insertions, 101 deletions
diff --git a/tests/FontHostStreamTest.cpp b/tests/FontHostStreamTest.cpp index 18c74605bf..affda98cf6 100644 --- a/tests/FontHostStreamTest.cpp +++ b/tests/FontHostStreamTest.cpp @@ -96,8 +96,8 @@ DEF_TEST(FontHostStream, reporter) { } int ttcIndex; - SkAutoTUnref<SkStream> fontData(origTypeface->openStream(&ttcIndex)); - SkTypeface* streamTypeface = SkTypeface::CreateFromStream(fontData); + SkAutoTDelete<SkStream> fontData(origTypeface->openStream(&ttcIndex)); + SkTypeface* streamTypeface = SkTypeface::CreateFromStream(fontData.detach()); SkFontDescriptor desc; bool isLocalStream = false; diff --git a/tests/FrontBufferedStreamTest.cpp b/tests/FrontBufferedStreamTest.cpp index 167d1a20e8..69dade04b9 100644 --- a/tests/FrontBufferedStreamTest.cpp +++ b/tests/FrontBufferedStreamTest.cpp @@ -51,10 +51,13 @@ const char gAbcs[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdef // Tests reading the stream across boundaries of what has been buffered so far and what // the total buffer size is. static void test_incremental_buffering(skiatest::Reporter* reporter, size_t bufferSize) { - SkMemoryStream memStream(gAbcs, strlen(gAbcs), false); + // NOTE: For this and other tests in this file, we cheat and continue to refer to the + // wrapped stream, but that's okay because we know the wrapping stream has not been + // deleted yet (and we only call const methods in it). + SkMemoryStream* memStream = SkNEW_ARGS(SkMemoryStream, (gAbcs, strlen(gAbcs), false)); - SkAutoTUnref<SkStream> bufferedStream(SkFrontBufferedStream::Create(&memStream, bufferSize)); - test_hasLength(reporter, *bufferedStream.get(), memStream); + SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(memStream, bufferSize)); + test_hasLength(reporter, *bufferedStream.get(), *memStream); // First, test reading less than the max buffer size. test_read(reporter, bufferedStream, gAbcs, bufferSize / 2); @@ -79,9 +82,9 @@ static void test_incremental_buffering(skiatest::Reporter* reporter, size_t buff } static void test_perfectly_sized_buffer(skiatest::Reporter* reporter, size_t bufferSize) { - SkMemoryStream memStream(gAbcs, strlen(gAbcs), false); - SkAutoTUnref<SkStream> bufferedStream(SkFrontBufferedStream::Create(&memStream, bufferSize)); - test_hasLength(reporter, *bufferedStream.get(), memStream); + SkMemoryStream* memStream = SkNEW_ARGS(SkMemoryStream, (gAbcs, strlen(gAbcs), false)); + SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(memStream, bufferSize)); + test_hasLength(reporter, *bufferedStream.get(), *memStream); // Read exactly the amount that fits in the buffer. test_read(reporter, bufferedStream, gAbcs, bufferSize); @@ -98,9 +101,9 @@ static void test_perfectly_sized_buffer(skiatest::Reporter* reporter, size_t buf } static void test_skipping(skiatest::Reporter* reporter, size_t bufferSize) { - SkMemoryStream memStream(gAbcs, strlen(gAbcs), false); - SkAutoTUnref<SkStream> bufferedStream(SkFrontBufferedStream::Create(&memStream, bufferSize)); - test_hasLength(reporter, *bufferedStream.get(), memStream); + SkMemoryStream* memStream = SkNEW_ARGS(SkMemoryStream, (gAbcs, strlen(gAbcs), false)); + SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(memStream, bufferSize)); + test_hasLength(reporter, *bufferedStream.get(), *memStream); // Skip half the buffer. bufferedStream->skip(bufferSize / 2); @@ -149,11 +152,11 @@ private: // does not invalidate the buffer. static void test_read_beyond_buffer(skiatest::Reporter* reporter, size_t bufferSize) { // Use a stream that behaves like Android's stream. - AndroidLikeMemoryStream memStream((void*)gAbcs, bufferSize, false); + AndroidLikeMemoryStream* memStream = SkNEW_ARGS(AndroidLikeMemoryStream, ((void*)gAbcs, bufferSize, false)); // Create a buffer that matches the length of the stream. - SkAutoTUnref<SkStream> bufferedStream(SkFrontBufferedStream::Create(&memStream, bufferSize)); - test_hasLength(reporter, *bufferedStream.get(), memStream); + SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(memStream, bufferSize)); + test_hasLength(reporter, *bufferedStream.get(), *memStream); // Attempt to read one more than the bufferSize test_read(reporter, bufferedStream.get(), gAbcs, bufferSize + 1); @@ -197,22 +200,22 @@ private: static void test_length_combos(skiatest::Reporter* reporter, size_t bufferSize) { for (int hasLen = 0; hasLen <= 1; hasLen++) { for (int hasPos = 0; hasPos <= 1; hasPos++) { - LengthOptionalStream stream(SkToBool(hasLen), SkToBool(hasPos)); - SkAutoTUnref<SkStream> buffered(SkFrontBufferedStream::Create(&stream, bufferSize)); - test_hasLength(reporter, *buffered.get(), stream); + LengthOptionalStream* stream = SkNEW_ARGS(LengthOptionalStream, (SkToBool(hasLen), SkToBool(hasPos))); + SkAutoTDelete<SkStream> buffered(SkFrontBufferedStream::Create(stream, bufferSize)); + test_hasLength(reporter, *buffered.get(), *stream); } } } // Test using a stream with an initial offset. static void test_initial_offset(skiatest::Reporter* reporter, size_t bufferSize) { - SkMemoryStream memStream(gAbcs, strlen(gAbcs), false); + SkMemoryStream* memStream = SkNEW_ARGS(SkMemoryStream, (gAbcs, strlen(gAbcs), false)); // Skip a few characters into the memStream, so that bufferedStream represents an offset into // the stream it wraps. const size_t arbitraryOffset = 17; - memStream.skip(arbitraryOffset); - SkAutoTUnref<SkStream> bufferedStream(SkFrontBufferedStream::Create(&memStream, bufferSize)); + memStream->skip(arbitraryOffset); + SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(memStream, bufferSize)); // Since SkMemoryStream has a length and a position, bufferedStream must also. REPORTER_ASSERT(reporter, bufferedStream->hasLength()); @@ -283,13 +286,13 @@ private: }; DEF_TEST(ShortFrontBufferedStream, reporter) { - FailingStream failingStream; - SkAutoTUnref<SkStreamRewindable> stream(SkFrontBufferedStream::Create(&failingStream, 64)); + FailingStream* failingStream = SkNEW(FailingStream); + SkAutoTDelete<SkStreamRewindable> stream(SkFrontBufferedStream::Create(failingStream, 64)); SkBitmap bm; // The return value of DecodeStream is not important. We are just using DecodeStream because // it simulates a bug. DecodeStream will read the stream, then rewind, then attempt to read // again. FrontBufferedStream::read should not continue to read its underlying stream beyond // its end. SkImageDecoder::DecodeStream(stream, &bm); - REPORTER_ASSERT(reporter, !failingStream.readAfterEnd()); + REPORTER_ASSERT(reporter, !failingStream->readAfterEnd()); } diff --git a/tests/ImageDecodingTest.cpp b/tests/ImageDecodingTest.cpp index 78d6b6c35f..b94e29c11d 100644 --- a/tests/ImageDecodingTest.cpp +++ b/tests/ImageDecodingTest.cpp @@ -324,76 +324,13 @@ DEF_TEST(ImageDecoding_unpremul, reporter) { #endif // SK_BUILD_FOR_UNIX/ANDROID skbug.com/2388 #ifdef SK_DEBUG -// Create a stream containing a bitmap encoded to Type type. -static SkMemoryStream* create_image_stream(SkImageEncoder::Type type) { - SkBitmap bm; - const int size = 50; - bm.allocN32Pixels(size, size); - SkCanvas canvas(bm); - SkPoint points[2] = { - { SkIntToScalar(0), SkIntToScalar(0) }, - { SkIntToScalar(size), SkIntToScalar(size) } - }; - SkColor colors[2] = { SK_ColorWHITE, SK_ColorBLUE }; - SkShader* shader = SkGradientShader::CreateLinear(points, colors, NULL, - SK_ARRAY_COUNT(colors), - SkShader::kClamp_TileMode); - SkPaint paint; - paint.setShader(shader)->unref(); - canvas.drawPaint(paint); - // Now encode it to a stream. - SkAutoTUnref<SkData> data(SkImageEncoder::EncodeData(bm, type, 100)); - if (NULL == data.get()) { - return NULL; - } - return SkNEW_ARGS(SkMemoryStream, (data.get())); -} - -// For every format that supports tile based decoding, ensure that -// calling decodeSubset will not fail if the caller has unreffed the -// stream provided in buildTileIndex. -// Only runs in debug mode since we are testing for a crash. -static void test_stream_life() { - const SkImageEncoder::Type gTypes[] = { -#ifdef SK_BUILD_FOR_ANDROID - SkImageEncoder::kJPEG_Type, - SkImageEncoder::kPNG_Type, -#endif - SkImageEncoder::kWEBP_Type, - }; - for (size_t i = 0; i < SK_ARRAY_COUNT(gTypes); ++i) { - // SkDebugf("encoding to %i\n", i); - SkAutoTUnref<SkMemoryStream> stream(create_image_stream(gTypes[i])); - if (NULL == stream.get()) { - SkDebugf("no stream\n"); - continue; - } - SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(stream)); - if (NULL == decoder.get()) { - SkDebugf("no decoder\n"); - continue; - } - int width, height; - if (!decoder->buildTileIndex(stream.get(), &width, &height)) { - SkDebugf("could not build a tile index\n"); - continue; - } - // Now unref the stream to make sure it survives - stream.reset(NULL); - SkBitmap bm; - decoder->decodeSubset(&bm, SkIRect::MakeWH(width, height), kN32_SkColorType); - } -} - // Test inside SkScaledBitmapSampler.cpp extern void test_row_proc_choice(); - #endif // SK_DEBUG DEF_TEST(ImageDecoding, reporter) { test_unpremul(reporter); #ifdef SK_DEBUG - test_stream_life(); test_row_proc_choice(); #endif } @@ -488,7 +425,6 @@ static SkPixelRef* install_pixel_ref(SkBitmap* bitmap, SkASSERT(bitmap != NULL); SkASSERT(stream != NULL); SkASSERT(stream->rewind()); - SkASSERT(stream->unique()); SkColorType colorType = bitmap->colorType(); SkDecodingImageGenerator::Options opts(sampleSize, ditherImage, colorType); if (SkInstallDiscardablePixelRef( @@ -503,7 +439,7 @@ static SkPixelRef* install_pixel_ref(SkBitmap* bitmap, */ DEF_TEST(ImprovedBitmapFactory, reporter) { SkString pngFilename = GetResourcePath("randPixels.png"); - SkAutoTUnref<SkStreamRewindable> stream(SkStream::NewFromFile(pngFilename.c_str())); + SkAutoTDelete<SkStreamRewindable> stream(SkStream::NewFromFile(pngFilename.c_str())); if (sk_exists(pngFilename.c_str())) { SkBitmap bm; SkAssertResult(bm.setInfo(SkImageInfo::MakeN32Premul(1, 1))); @@ -698,7 +634,7 @@ DEF_TEST(ImageDecoderOptions, reporter) { SkAutoDataUnref encodedData(SkData::NewFromFileName(path.c_str())); REPORTER_ASSERT(reporter, encodedData.get() != NULL); - SkAutoTUnref<SkStreamRewindable> encodedStream( + SkAutoTDelete<SkStreamRewindable> encodedStream( SkStream::NewFromFile(path.c_str())); REPORTER_ASSERT(reporter, encodedStream.get() != NULL); @@ -782,7 +718,7 @@ private: DEF_TEST(ImageDecoding_JpegOverwrite, r) { SkString resourceDir = GetResourcePath(); SkString path = SkOSPath::Join(resourceDir.c_str(), "randPixels.jpg"); - SkAutoTUnref<SkStreamAsset> stream( + SkAutoTDelete<SkStreamAsset> stream( SkStream::NewFromFile(path.c_str())); if (!stream.get()) { SkDebugf("\nPath '%s' missing.\n", path.c_str()); diff --git a/tests/KtxTest.cpp b/tests/KtxTest.cpp index 23c1d70900..53caabdd91 100644 --- a/tests/KtxTest.cpp +++ b/tests/KtxTest.cpp @@ -55,7 +55,7 @@ DEF_TEST(KtxReadWrite, reporter) { SkAutoDataUnref encodedData(SkImageEncoder::EncodeData(bm8888, SkImageEncoder::kKTX_Type, 0)); REPORTER_ASSERT(reporter, encodedData); - SkAutoTUnref<SkMemoryStream> stream(SkNEW_ARGS(SkMemoryStream, (encodedData))); + SkAutoTDelete<SkMemoryStream> stream(SkNEW_ARGS(SkMemoryStream, (encodedData))); REPORTER_ASSERT(reporter, stream); SkBitmap decodedBitmap; @@ -107,7 +107,7 @@ DEF_TEST(KtxReadUnpremul, reporter) { 0xFF, 0xFF, 0xFF, 0x80, // Pixel 3 0xFF, 0xFF, 0xFF, 0x80};// Pixel 4 - SkAutoTUnref<SkMemoryStream> stream( + SkAutoTDelete<SkMemoryStream> stream( SkNEW_ARGS(SkMemoryStream, (kHalfWhiteKTX, sizeof(kHalfWhiteKTX)))); REPORTER_ASSERT(reporter, stream); diff --git a/tests/PDFPrimitivesTest.cpp b/tests/PDFPrimitivesTest.cpp index 3a4c6f6ab0..3610dd9581 100644 --- a/tests/PDFPrimitivesTest.cpp +++ b/tests/PDFPrimitivesTest.cpp @@ -119,7 +119,7 @@ static void SimpleCheckObjectOutput(skiatest::Reporter* reporter, static void TestPDFStream(skiatest::Reporter* reporter) { char streamBytes[] = "Test\nFoo\tBar"; - SkAutoTUnref<SkMemoryStream> streamData(new SkMemoryStream( + SkAutoTDelete<SkMemoryStream> streamData(new SkMemoryStream( streamBytes, strlen(streamBytes), true)); SkAutoTUnref<SkPDFStream> stream(new SkPDFStream(streamData.get())); SimpleCheckObjectOutput( diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp index cd5bc60574..f6a8a7d5fc 100644 --- a/tests/SerializationTest.cpp +++ b/tests/SerializationTest.cpp @@ -350,7 +350,7 @@ static void TestPictureTypefaceSerialization(skiatest::Reporter* reporter) { // Serlialize picture and create its clone from stream. SkDynamicMemoryWStream stream; picture->serialize(&stream); - SkAutoTUnref<SkStream> inputStream(stream.detachAsStream()); + SkAutoTDelete<SkStream> inputStream(stream.detachAsStream()); SkAutoTUnref<SkPicture> loadedPicture(SkPicture::CreateFromStream(inputStream.get())); // Draw both original and clone picture and compare bitmaps -- they should be identical. diff --git a/tests/StreamTest.cpp b/tests/StreamTest.cpp index ab0af14088..ff22ecf554 100644 --- a/tests/StreamTest.cpp +++ b/tests/StreamTest.cpp @@ -58,7 +58,7 @@ static void test_filestreams(skiatest::Reporter* reporter, const char* tmpDir) { REPORTER_ASSERT(reporter, stream.isValid()); test_loop_stream(reporter, &stream, s, 26, 100); - SkAutoTUnref<SkStreamAsset> stream2(stream.duplicate()); + SkAutoTDelete<SkStreamAsset> stream2(stream.duplicate()); test_loop_stream(reporter, stream2.get(), s, 26, 100); } @@ -68,7 +68,7 @@ static void test_filestreams(skiatest::Reporter* reporter, const char* tmpDir) { REPORTER_ASSERT(reporter, stream.isValid()); test_loop_stream(reporter, &stream, s, 26, 100); - SkAutoTUnref<SkStreamAsset> stream2(stream.duplicate()); + SkAutoTDelete<SkStreamAsset> stream2(stream.duplicate()); test_loop_stream(reporter, stream2.get(), s, 26, 100); } } @@ -91,15 +91,15 @@ static void TestWStream(skiatest::Reporter* reporter) { } { - SkAutoTUnref<SkStreamAsset> stream(ds.detachAsStream()); + SkAutoTDelete<SkStreamAsset> stream(ds.detachAsStream()); REPORTER_ASSERT(reporter, 100 * 26 == stream->getLength()); REPORTER_ASSERT(reporter, ds.getOffset() == 0); test_loop_stream(reporter, stream.get(), s, 26, 100); - SkAutoTUnref<SkStreamAsset> stream2(stream->duplicate()); + SkAutoTDelete<SkStreamAsset> stream2(stream->duplicate()); test_loop_stream(reporter, stream2.get(), s, 26, 100); - SkAutoTUnref<SkStreamAsset> stream3(stream->fork()); + SkAutoTDelete<SkStreamAsset> stream3(stream->fork()); REPORTER_ASSERT(reporter, stream3->isAtEnd()); char tmp; size_t bytes = stream->read(&tmp, 1); @@ -121,11 +121,11 @@ static void TestWStream(skiatest::Reporter* reporter) { { // Test that this works after a copyToData. - SkAutoTUnref<SkStreamAsset> stream(ds.detachAsStream()); + SkAutoTDelete<SkStreamAsset> stream(ds.detachAsStream()); REPORTER_ASSERT(reporter, ds.getOffset() == 0); test_loop_stream(reporter, stream.get(), s, 26, 100); - SkAutoTUnref<SkStreamAsset> stream2(stream->duplicate()); + SkAutoTDelete<SkStreamAsset> stream2(stream->duplicate()); test_loop_stream(reporter, stream2.get(), s, 26, 100); } delete[] dst; |