Remove position from FrontBufferedStream

When FrontBufferedStream was written, it implemented getPosition()
and returned true for hasPosition(). The CL that introduced it
(crrev.com/23717055) does not have any indication why, but I'm guessing
it was because it was easy to implement. None of our decoders rely on
this (only some tests do).

Now that we have a decoder (SkRawCodec) that expects to be able to seek
a stream if it has a position (which makes sense - SkStream.h
associates that with SkStreamSeekable, and there is no other way to
check to see if a stream is seekable), it is failing because
FrontBufferedStream reports it has a position and the decoder tries to
seek.

Remove FrontBufferedStream::hasPosition() (reverting to the default,
false) and ::getPosition() (so it will return 0).

Fix tests - do not call FrontBufferedStream::getPosition()

Update CodexTest to test using an FrontBufferedStream, like Android
does.

BUG=b/27218441
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1703293002

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

src/utils/SkFrontBufferedStream.cpp

@@ -22,10 +22,6 @@ public:
 
     bool rewind() override;
 
-    bool hasPosition() const override { return true; }
-
-    size_t getPosition() const override { return fOffset; }
-
     bool hasLength() const override { return fHasLength; }
 
     size_t getLength() const override { return fLength; }

tests/CodexTest.cpp

@@ -11,6 +11,7 @@
 #include "SkCodec.h"
 #include "SkCodecImageGenerator.h"
 #include "SkData.h"
+#include "SkFrontBufferedStream.h"
 #include "SkImageDecoder.h"
 #include "SkMD5.h"
 #include "SkRandom.h"
@@ -180,7 +181,7 @@ static void check(skiatest::Reporter* r,
 
     // Test full image decodes with SkCodec
     SkMD5::Digest codecDigest;
-    SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
+    const SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
     SkBitmap bm;
     SkCodec::Result expectedResult = isIncomplete ? SkCodec::kIncompleteInput : SkCodec::kSuccess;
     test_codec(r, codec.get(), bm, info, size, expectedResult, &codecDigest, nullptr);
@@ -313,8 +314,8 @@ static void check(skiatest::Reporter* r,
                    &codecDigest);
     }
 
-    // Test SkCodecImageGenerator
     if (!isIncomplete) {
+        // Test SkCodecImageGenerator
         SkAutoTDelete<SkStream> stream(resource(path));
         SkAutoTUnref<SkData> fullData(SkData::NewFromStream(stream, stream->getLength()));
         SkAutoTDelete<SkImageGenerator> gen(SkCodecImageGenerator::NewFromEncodedCodec(fullData));
@@ -323,6 +324,16 @@ static void check(skiatest::Reporter* r,
         SkAutoLockPixels autoLockPixels(bm);
         REPORTER_ASSERT(r, gen->getPixels(info, bm.getPixels(), bm.rowBytes()));
         compare_to_good_digest(r, codecDigest, bm);
+
+        // Test using SkFrontBufferedStream, as Android does
+        SkStream* bufferedStream = SkFrontBufferedStream::Create(new SkMemoryStream(fullData),
+                SkCodec::MinBufferedBytesNeeded());
+        REPORTER_ASSERT(r, bufferedStream);
+        codec.reset(SkCodec::NewFromStream(bufferedStream));
+        REPORTER_ASSERT(r, codec);
+        if (codec) {
+            test_info(r, codec.get(), info, SkCodec::kSuccess, &codecDigest);
+        }
     }
 
     // If we've just tested incomplete decodes, let's run the same test again on full decodes.

tests/FrontBufferedStreamTest.cpp

@@ -68,7 +68,7 @@ static void test_incremental_buffering(skiatest::Reporter* reporter, size_t buff
     test_read(reporter, bufferedStream, gAbcs, bufferSize / 4);
 
     // Now test reading part of what was buffered, and buffering new data.
-    test_read(reporter, bufferedStream, gAbcs + bufferedStream->getPosition(), bufferSize / 2);
+    test_read(reporter, bufferedStream, gAbcs + bufferSize / 4, bufferSize / 2);
 
     // Now test reading what was buffered, buffering new data, and
     // reading directly from the stream.
@@ -96,7 +96,7 @@ static void test_perfectly_sized_buffer(skiatest::Reporter* reporter, size_t buf
     test_read(reporter, bufferedStream, gAbcs, bufferSize);
 
     // Read past the size of the buffer. At this point, we cannot return.
-    test_read(reporter, bufferedStream, gAbcs + bufferedStream->getPosition(), 1);
+    test_read(reporter, bufferedStream, gAbcs + memStream->getPosition(), 1);
     test_rewind(reporter, bufferedStream, false);
 }
 
@@ -116,7 +116,7 @@ static void test_skipping(skiatest::Reporter* reporter, size_t bufferSize) {
     bufferedStream->skip(bufferSize / 2);
 
     // Test that reading will still work.
-    test_read(reporter, bufferedStream, gAbcs + bufferedStream->getPosition(), bufferSize / 4);
+    test_read(reporter, bufferedStream, gAbcs + memStream->getPosition(), bufferSize / 4);
 
     test_rewind(reporter, bufferedStream, true);
     test_read(reporter, bufferedStream, gAbcs, bufferSize);
@@ -219,13 +219,12 @@ static void test_initial_offset(skiatest::Reporter* reporter, size_t bufferSize)
     memStream->skip(arbitraryOffset);
     SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(memStream, bufferSize));
 
-    // Since SkMemoryStream has a length and a position, bufferedStream must also.
+    // Since SkMemoryStream has a length, bufferedStream must also.
     REPORTER_ASSERT(reporter, bufferedStream->hasLength());
 
     const size_t amountToRead = 10;
     const size_t bufferedLength = bufferedStream->getLength();
-    size_t currentPosition = bufferedStream->getPosition();
-    REPORTER_ASSERT(reporter, 0 == currentPosition);
+    size_t currentPosition = 0;
 
     // Read the stream in chunks. After each read, the position must match currentPosition,
     // which sums the amount attempted to read, unless the end of the stream has been reached.
@@ -235,7 +234,7 @@ static void test_initial_offset(skiatest::Reporter* reporter, size_t bufferSize)
         test_read(reporter, bufferedStream, gAbcs + arbitraryOffset + currentPosition,
                   amountToRead);
         currentPosition = SkTMin(currentPosition + amountToRead, bufferedLength);
-        REPORTER_ASSERT(reporter, bufferedStream->getPosition() == currentPosition);
+        REPORTER_ASSERT(reporter, memStream->getPosition() - arbitraryOffset == currentPosition);
     }
     REPORTER_ASSERT(reporter, bufferedStream->isAtEnd());
     REPORTER_ASSERT(reporter, bufferedLength == currentPosition);

tests/StreamTest.cpp

@@ -266,12 +266,11 @@ static void test_peeking_front_buffered_stream(skiatest::Reporter* r,
     for (size_t start = 0; start <= bufferSize; start++) {
         // Skip to the starting point
         REPORTER_ASSERT(r, bufferedStream->skip(start) == start);
-        REPORTER_ASSERT(r, bufferedStream->getPosition() == start);
 
         const size_t bytesPeeked = bufferedStream->peek(peekStorage.get(), bytesToPeek);
         if (0 == bytesPeeked) {
-            // Peeking should only fail completely if we have read beyond the buffer.
-            REPORTER_ASSERT(r, bufferedStream->getPosition() >= bufferSize);
+            // Peeking should only fail completely if we have read/skipped beyond the buffer.
+            REPORTER_ASSERT(r, start >= bufferSize);
             break;
         }