Reland "Make SkPngCodec only read as much of the stream as necessary"

(Originally uploaded as 13900.)

Previously, SkPngCodec assumed that the stream only contained one
image, which ended at the end of the stream. It read the stream in
arbitrarily-sized chunks, and then passed that data to libpng for
processing.

If a stream contains more than one image, this may result in reading
beyond the end of the image, making future reads read the wrong data.

Now, SkPngCodec starts by reading 8 bytes at a time. After the
signature, 8 bytes is enough to know which chunk is next and how many
bytes are in the chunk.

When decoding the size, we stop when we reach IDAT, and when decoding
the image, we stop when we reach IEND.

This manual parsing is necessary to support APNG, which is planned in
the future. It also allows us to remove the SK_GOOGLE3_PNG_HACK, which
was a workaround for reading more than necessary at the beginning of
the image.

Add a test that simulates the issue, by decoding a special stream that
reports an error if the codec attempts to read beyond the end.

Temporarily disable the partial decoding tests for png. A larger change
will be necessary to get those working again, and no clients are
currently relying on incrementally decoding PNGs (i.e. decode part of
an image, then decode further with more data).

Include a workaround for older versions of libpng (e.g. 1.2 in
Google3). In older versions, if the row callback is null when the
IDAT header is processed, reading the image will fail. When we see the
IDAT, we save the length and process a recreated IDAT header later,
after the row callback has been set.

Bug: skia:5368
Bug:b/34073812
Test: Existing tests, plus a new test in dm.

Change-Id: I293a4ddc013b82669a8b735062228b26d0bce933
Reviewed-on: https://skia-review.googlesource.com/13984
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>

1 files changed, 5 insertions, 22 deletions

diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index 09231f16bd..4809723db6 100644
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -16,13 +16,6 @@
 #include "SkRefCnt.h"
 #include "SkSwizzler.h"
 
-// FIXME (scroggo): GOOGLE3 is currently using an outdated version of libpng,
-// so we need to work around the lack of the method png_process_data_pause.
-// This code will be unnecessary once we update GOOGLE3. It would make more
-// sense to condition this on the version of libpng being used, but we do not
-// know that here because png.h is only included by the cpp file.
-#define SK_GOOGLE3_PNG_HACK
-
 class SkStream;
 
 class SkPngCodec : public SkCodec {
@@ -32,6 +25,9 @@ public:
     // Assume IsPng was called and returned true.
     static SkCodec* NewFromStream(SkStream*, SkPngChunkReader* = NULL);
 
+    // FIXME (scroggo): Temporarily needed by AutoCleanPng.
+    void setIdatLength(size_t len) { fIdatLength = len; }
+
     ~SkPngCodec() override;
 
 protected:
@@ -76,18 +72,6 @@ protected:
      */
     void processData();
 
-#ifdef SK_GOOGLE3_PNG_HACK
-    // In libpng 1.2.56, png_process_data_pause does not exist, so when we wanted to
-    // read the header, we may have read too far. In that case, we need to delete the
-    // png_ptr and info_ptr and recreate them. This method does that (and attaches the
-    // chunk reader.
-    bool rereadHeaderIfNecessary();
-
-    // This method sets up the new png_ptr/info_ptr (created in rereadHeaderIfNecessary)
-    // the way we set up the old one the first time in AutoCleanPng.decodeBounds's callback.
-    void rereadInfoCallback();
-#endif
-
     Result onStartIncrementalDecode(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes,
             const SkCodec::Options&,
             SkPMColor* ctable, int* ctableCount) override;
@@ -134,9 +118,8 @@ private:
     SkAlphaType                    fXformAlphaType;
     int                            fXformWidth;
 
-#ifdef SK_GOOGLE3_PNG_HACK
-    bool        fNeedsToRereadHeader;
-#endif
+    size_t                         fIdatLength;
+    bool                           fDecodedIdat;
 
     typedef SkCodec INHERITED;
 };