upgrade SkReadBuffer to always validate

Bug: skia:
Change-Id: I054560b66c6cde346d939015326d8547879d2c4b
Reviewed-on: https://skia-review.googlesource.com/81160
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>

8 files changed, 196 insertions, 451 deletions

diff --git a/src/core/SkFlattenable.cpp b/src/core/SkFlattenable.cpp
index e89dae5c76..c975023367 100644
--- a/src/core/SkFlattenable.cpp
+++ b/src/core/SkFlattenable.cpp
@@ -121,3 +121,26 @@ const char* SkFlattenable::FactoryToName(Factory fact) {
     }
     return nullptr;
 }
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+sk_sp<SkData> SkFlattenable::serialize(const SkSerialProcs* procs) const {
+    SkBinaryWriteBuffer writer;
+    if (procs) {
+        writer.setSerialProcs(*procs);
+    }
+    writer.writeFlattenable(this);
+    size_t size = writer.bytesWritten();
+    auto data = SkData::MakeUninitialized(size);
+    writer.writeToMemory(data->writable_data());
+    return data;
+}
+
+sk_sp<SkFlattenable> SkFlattenable::Deserialize(SkFlattenable::Type type, const void* data,
+                                                size_t size, const SkDeserialProcs* procs) {
+    SkReadBuffer buffer(data, size);
+    if (procs) {
+        buffer.setDeserialProcs(*procs);
+    }
+    return sk_sp<SkFlattenable>(buffer.readFlattenable(type));
+}
diff --git a/src/core/SkFlattenableSerialization.cpp b/src/core/SkFlattenableSerialization.cpp
index e72b4c5348..3976010fb2 100644
--- a/src/core/SkFlattenableSerialization.cpp
+++ b/src/core/SkFlattenableSerialization.cpp
@@ -6,27 +6,19 @@
  */
 
 #include "SkFlattenableSerialization.h"
-
 #include "SkData.h"
-#include "SkValidatingReadBuffer.h"
-#include "SkWriteBuffer.h"
+#include "SkImageFilter.h"
 
 SkData* SkValidatingSerializeFlattenable(SkFlattenable* flattenable) {
-    SkBinaryWriteBuffer writer;
-    writer.writeFlattenable(flattenable);
-    size_t size = writer.bytesWritten();
-    auto data = SkData::MakeUninitialized(size);
-    writer.writeToMemory(data->writable_data());
-    return data.release();
+    return flattenable->serialize().release();
 }
 
 SkFlattenable* SkValidatingDeserializeFlattenable(const void* data, size_t size,
                                                   SkFlattenable::Type type) {
-    SkValidatingReadBuffer buffer(data, size);
-    return buffer.readFlattenable(type);
+    return SkFlattenable::Deserialize(type, data, size).release();
 }
 
 sk_sp<SkImageFilter> SkValidatingDeserializeImageFilter(const void* data, size_t size) {
-    return sk_sp<SkImageFilter>((SkImageFilter*)SkValidatingDeserializeFlattenable(
-                                data, size, SkImageFilter::GetFlattenableType()));
+    auto flat = SkFlattenable::Deserialize(SkFlattenable::kSkImageFilter_Type, data, size);
+    return sk_sp<SkImageFilter>(static_cast<SkImageFilter*>(flat.release()));
 }
diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index 24e161f191..a86aee2fa0 100644
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -448,7 +448,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
                 return false;
             }
 
-            /* Should we use SkValidatingReadBuffer instead? */
             SkReadBuffer buffer(storage.get(), size);
             buffer.setFlags(pictInfoFlagsToReadBufferFlags(fInfo.fFlags));
             buffer.setVersion(fInfo.getVersion());
diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp
index b1d7808dcb..b6d0d14f48 100644
--- a/src/core/SkReadBuffer.cpp
+++ b/src/core/SkReadBuffer.cpp
@@ -11,6 +11,7 @@
 #include "SkImageDeserializer.h"
 #include "SkImageGenerator.h"
 #include "SkMakeUnique.h"
+#include "SkMatrixPriv.h"
 #include "SkReadBuffer.h"
 #include "SkStream.h"
 #include "SkTypeface.h"
@@ -97,55 +98,101 @@ SkReadBuffer::~SkReadBuffer() {
     sk_free(fMemoryPtr);
 }
 
+void SkReadBuffer::setInvalid() {
+    if (!fError) {
+        // When an error is found, send the read cursor to the end of the stream
+        fReader.skip(fReader.available());
+        fError = true;
+    }
+}
+
+const void* SkReadBuffer::skip(size_t size) {
+    size_t inc = SkAlign4(size);
+    this->validate(inc >= size);
+    const void* addr = fReader.peek();
+    this->validate(IsPtrAlign4(addr) && fReader.isAvailable(inc));
+    if (fError) {
+        return nullptr;
+    }
+
+    fReader.skip(size);
+    return addr;
+}
+
 void SkReadBuffer::setDeserialProcs(const SkDeserialProcs& procs) {
     fProcs = procs;
 }
 
 bool SkReadBuffer::readBool() {
-    return fReader.readBool();
+    uint32_t value = this->readInt();
+    // Boolean value should be either 0 or 1
+    this->validate(!(value & ~1));
+    return value != 0;
 }
 
 SkColor SkReadBuffer::readColor() {
-    return fReader.readInt();
+    return this->readInt();
 }
 
 int32_t SkReadBuffer::readInt() {
-    return fReader.readInt();
+    const size_t inc = sizeof(int32_t);
+    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
+    return fError ? 0 : fReader.readInt();
 }
 
 SkScalar SkReadBuffer::readScalar() {
-    return fReader.readScalar();
+    const size_t inc = sizeof(SkScalar);
+    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
+    return fError ? 0 : fReader.readScalar();
 }
 
 uint32_t SkReadBuffer::readUInt() {
-    return fReader.readU32();
+    return this->readInt();
 }
 
 int32_t SkReadBuffer::read32() {
-    return fReader.readInt();
+    return this->readInt();
+}
+
+uint8_t SkReadBuffer::peekByte() {
+    if (fReader.available() <= 0) {
+        fError = true;
+        return 0;
+    }
+    return *((uint8_t*) fReader.peek());
 }
 
 bool SkReadBuffer::readPad32(void* buffer, size_t bytes) {
-    if (!fReader.isAvailable(bytes)) {
+    if (!this->validate(fReader.isAvailable(bytes))) {
         return false;
     }
     fReader.read(buffer, bytes);
     return true;
 }
 
-uint8_t SkReadBuffer::peekByte() {
-    SkASSERT(fReader.available() > 0);
-    return *((uint8_t*) fReader.peek());
-}
-
 void SkReadBuffer::readString(SkString* string) {
-    size_t len;
-    const char* strContents = fReader.readString(&len);
-    string->set(strContents, len);
+    const size_t len = this->readUInt();
+    const void* ptr = fReader.peek();
+    const char* cptr = (const char*)ptr;
+
+    // skip over the string + '\0' and then pad to a multiple of 4
+    const size_t alignedSize = SkAlign4(len + 1);
+    this->skip(alignedSize);
+    if (!fError) {
+        this->validate(cptr[len] == '\0');
+    }
+    if (!fError) {
+        string->set(cptr, len);
+    }
 }
 
 void SkReadBuffer::readColor4f(SkColor4f* color) {
-    memcpy(color, fReader.skip(sizeof(SkColor4f)), sizeof(SkColor4f));
+    const void* ptr = this->skip(sizeof(SkColor4f));
+    if (!fError) {
+        memcpy(color, ptr, sizeof(SkColor4f));
+    } else {
+        *color = {0, 0, 0, 0};
+    }
 }
 
 void SkReadBuffer::readPoint(SkPoint* point) {
@@ -160,39 +207,74 @@ void SkReadBuffer::readPoint3(SkPoint3* point) {
 }
 
 void SkReadBuffer::readMatrix(SkMatrix* matrix) {
-    fReader.readMatrix(matrix);
+    size_t size = 0;
+    if (!fError) {
+        size = SkMatrixPriv::ReadFromMemory(matrix, fReader.peek(), fReader.available());
+        this->validate((SkAlign4(size) == size) && (0 != size));
+    }
+    if (!fError) {
+        (void)this->skip(size);
+    }
 }
 
 void SkReadBuffer::readIRect(SkIRect* rect) {
-    memcpy(rect, fReader.skip(sizeof(SkIRect)), sizeof(SkIRect));
+    const void* ptr = this->skip(sizeof(SkIRect));
+    if (!fError) {
+        memcpy(rect, ptr, sizeof(SkIRect));
+    } else {
+        rect->setEmpty();
+    }
 }
 
 void SkReadBuffer::readRect(SkRect* rect) {
-    memcpy(rect, fReader.skip(sizeof(SkRect)), sizeof(SkRect));
+    const void* ptr = this->skip(sizeof(SkRect));
+    if (!fError) {
+        memcpy(rect, ptr, sizeof(SkRect));
+    } else {
+        rect->setEmpty();
+    }
 }
 
 void SkReadBuffer::readRRect(SkRRect* rrect) {
-    fReader.readRRect(rrect);
+    if (!this->validate(fReader.readRRect(rrect))) {
+        rrect->setEmpty();
+    }
 }
 
 void SkReadBuffer::readRegion(SkRegion* region) {
-    fReader.readRegion(region);
+    size_t size = 0;
+    if (!fError) {
+        size = region->readFromMemory(fReader.peek(), fReader.available());
+        this->validate((SkAlign4(size) == size) && (0 != size));
+    }
+    if (!fError) {
+        (void)this->skip(size);
+    }
 }
 
 void SkReadBuffer::readPath(SkPath* path) {
-    fReader.readPath(path);
+    size_t size = 0;
+    if (!fError) {
+        size = path->readFromMemory(fReader.peek(), fReader.available());
+        this->validate((SkAlign4(size) == size) && (0 != size));
+    }
+    if (!fError) {
+        (void)this->skip(size);
+    }
 }
 
 bool SkReadBuffer::readArray(void* value, size_t size, size_t elementSize) {
-    const size_t count = this->getArrayCount();
-    if (count == size) {
-        (void)fReader.skip(sizeof(uint32_t)); // Skip array count
-        const size_t byteLength = count * elementSize;
-        memcpy(value, fReader.skip(SkAlign4(byteLength)), byteLength);
+    const uint32_t count = this->getArrayCount();
+    this->validate(size == count);
+    (void)this->skip(sizeof(uint32_t)); // Skip array count
+    const uint64_t byteLength64 = sk_64_mul(count, elementSize);
+    const size_t byteLength = count * elementSize;
+    this->validate(byteLength == byteLength64);
+    const void* ptr = this->skip(SkAlign4(byteLength));
+    if (!fError) {
+        memcpy(value, ptr, byteLength);
         return true;
     }
-    SkASSERT(false);
-    fReader.skip(fReader.available());
     return false;
 }
 
@@ -221,7 +303,9 @@ bool SkReadBuffer::readScalarArray(SkScalar* values, size_t size) {
 }
 
 uint32_t SkReadBuffer::getArrayCount() {
-    return *(uint32_t*)fReader.peek();
+    const size_t inc = sizeof(uint32_t);
+    fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
+    return fError ? 0 : *(uint32_t*)fReader.peek();
 }
 
 sk_sp<SkImage> SkReadBuffer::readImage() {
@@ -312,8 +396,8 @@ SkFlattenable* SkReadBuffer::readFlattenable(SkFlattenable::Type ft) {
             return nullptr;
         }
     } else if (fFactoryCount > 0) {
-        int32_t index = fReader.readU32();
-        if (0 == index) {
+        int32_t index = this->read32();
+        if (0 == index || !this->isValid()) {
             return nullptr; // writer failed to give us the flattenable
         }
         index -= 1;     // we stored the index-base-1
diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h
index ae3fde6818..78038aa7af 100644
--- a/src/core/SkReadBuffer.h
+++ b/src/core/SkReadBuffer.h
@@ -115,37 +115,37 @@ public:
     size_t size() { return fReader.size(); }
     size_t offset() { return fReader.offset(); }
     bool eof() { return fReader.eof(); }
-    virtual const void* skip(size_t size) { return fReader.skip(size); }
+    const void* skip(size_t size);
 
     // primitives
-    virtual bool readBool();
-    virtual SkColor readColor();
-    virtual int32_t readInt();
-    virtual SkScalar readScalar();
-    virtual uint32_t readUInt();
-    virtual int32_t read32();
+    bool readBool();
+    SkColor readColor();
+    int32_t readInt();
+    SkScalar readScalar();
+    uint32_t readUInt();
+    int32_t read32();
 
     // peek
-    virtual uint8_t peekByte();
+    uint8_t peekByte();
 
     // strings -- the caller is responsible for freeing the string contents
-    virtual void readString(SkString* string);
+    void readString(SkString* string);
 
     // common data structures
-    virtual void readColor4f(SkColor4f* color);
-    virtual void readPoint(SkPoint* point);
+    void readColor4f(SkColor4f* color);
+    void readPoint(SkPoint* point);
     SkPoint readPoint() { SkPoint p; this->readPoint(&p); return p; }
-    virtual void readPoint3(SkPoint3* point);
-    virtual void readMatrix(SkMatrix* matrix);
-    virtual void readIRect(SkIRect* rect);
-    virtual void readRect(SkRect* rect);
-    virtual void readRRect(SkRRect* rrect);
-    virtual void readRegion(SkRegion* region);
-
-    virtual void readPath(SkPath* path);
+    void readPoint3(SkPoint3* point);
+    void readMatrix(SkMatrix* matrix);
+    void readIRect(SkIRect* rect);
+    void readRect(SkRect* rect);
+    void readRRect(SkRRect* rrect);
+    void readRegion(SkRegion* region);
+
+    void readPath(SkPath* path);
     virtual void readPaint(SkPaint* paint) { paint->unflatten(*this); }
 
-    virtual SkFlattenable* readFlattenable(SkFlattenable::Type);
+    SkFlattenable* readFlattenable(SkFlattenable::Type);
     template <typename T> sk_sp<T> readFlattenable() {
         return sk_sp<T>((T*)this->readFlattenable(T::GetFlattenableType()));
     }
@@ -158,15 +158,15 @@ public:
     sk_sp<SkShader> readShader() { return this->readFlattenable<SkShaderBase>(); }
 
     // Reads SkAlign4(bytes), but will only copy bytes into the buffer.
-    virtual bool readPad32(void* buffer, size_t bytes);
+    bool readPad32(void* buffer, size_t bytes);
 
     // binary data and arrays
-    virtual bool readByteArray(void* value, size_t size);
-    virtual bool readColorArray(SkColor* colors, size_t size);
-    virtual bool readColor4fArray(SkColor4f* colors, size_t size);
-    virtual bool readIntArray(int32_t* values, size_t size);
-    virtual bool readPointArray(SkPoint* points, size_t size);
-    virtual bool readScalarArray(SkScalar* values, size_t size);
+    bool readByteArray(void* value, size_t size);
+    bool readColorArray(SkColor* colors, size_t size);
+    bool readColor4fArray(SkColor4f* colors, size_t size);
+    bool readIntArray(int32_t* values, size_t size);
+    bool readPointArray(SkPoint* points, size_t size);
+    bool readScalarArray(SkScalar* values, size_t size);
 
     sk_sp<SkData> readByteArrayAsData() {
         size_t len = this->getArrayCount();
@@ -179,7 +179,7 @@ public:
     }
 
     // helpers to get info about arrays and binary data
-    virtual uint32_t getArrayCount();
+    uint32_t getArrayCount();
 
     // If there is a real error (e.g. data is corrupted) this returns null. If the image cannot
     // be created (e.g. it was not originally encoded) then this returns an image that doesn't
@@ -221,9 +221,17 @@ public:
     void setImageDeserializer(SkImageDeserializer* factory);
 #endif
 
-    // Default impelementations don't check anything.
-    virtual bool validate(bool isValid) { return isValid; }
-    virtual bool isValid() const { return true; }
+    /**
+     *  If isValid is false, sets the buffer to be "invalid". Returns true if the buffer
+     *  is still valid.
+     */
+    bool validate(bool isValid) {
+        if (!isValid) {
+            this->setInvalid();
+        }
+        return !fError;
+    }
+    bool isValid() const { return !fError; }
     bool validateIndex(int index, int count) {
         return this->validate(index >= 0 && index < count);
     }
@@ -266,6 +274,7 @@ protected:
     SkTHashMap<uint32_t, SkString> fFlattenableDict;
 
 private:
+    void setInvalid();
     bool readArray(void* value, size_t size, size_t elementSize);
 
     uint32_t fFlags;
@@ -291,6 +300,12 @@ private:
 #endif // DEBUG_NON_DETERMINISTIC_ASSERT
 
     SkInflator* fInflator = nullptr;
+
+    static bool IsPtrAlign4(const void* ptr) {
+        return SkIsAlign4((uintptr_t)ptr);
+    }
+
+    bool fError = false;
 };
 
 #endif // SkReadBuffer_DEFINED
diff --git a/src/core/SkTextBlob.cpp b/src/core/SkTextBlob.cpp
index 3879c1e78e..193c82de66 100644
--- a/src/core/SkTextBlob.cpp
+++ b/src/core/SkTextBlob.cpp
@@ -7,9 +7,9 @@
 
 #include "SkTextBlobRunIterator.h"
 
+#include "SkReadBuffer.h"
 #include "SkSafeMath.h"
 #include "SkTypeface.h"
-#include "SkValidatingReadBuffer.h"
 #include "SkWriteBuffer.h"
 
 #include <limits>
@@ -895,11 +895,11 @@ sk_sp<SkData> SkTextBlob::serialize(SkTypefaceCatalogerProc proc, void* ctx) con
     return data;
 }
 
-class SkTypefaceResolverReadBuffer : public SkValidatingReadBuffer {
+class SkTypefaceResolverReadBuffer : public SkReadBuffer {
 public:
     SkTypefaceResolverReadBuffer(const void* data, size_t size, SkTypefaceResolverProc proc,
                                  void* ctx)
-        : SkValidatingReadBuffer(data, size)
+        : SkReadBuffer(data, size)
         , fResolverProc(proc)
         , fResolverCtx(ctx)
     {}
diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp
deleted file mode 100644
index e13cde5da6..0000000000
--- a/src/core/SkValidatingReadBuffer.cpp
+++ /dev/null
@@ -1,306 +0,0 @@
-/*
- * Copyright 2013 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkBitmap.h"
-#include "SkMatrixPriv.h"
-#include "SkValidatingReadBuffer.h"
-#include "SkStream.h"
-#include "SkTypeface.h"
-
-SkValidatingReadBuffer::SkValidatingReadBuffer(const void* data, size_t size) :
-    fError(false) {
-    this->setMemory(data, size);
-    this->setFlags(SkReadBuffer::kValidation_Flag);
-}
-
-SkValidatingReadBuffer::~SkValidatingReadBuffer() {
-}
-
-bool SkValidatingReadBuffer::validate(bool isValid) {
-    if (!fError && !isValid) {
-        // When an error is found, send the read cursor to the end of the stream
-        fReader.skip(fReader.available());
-        fError = true;
-    }
-    return !fError;
-}
-
-bool SkValidatingReadBuffer::isValid() const {
-    return !fError;
-}
-
-void SkValidatingReadBuffer::setMemory(const void* data, size_t size) {
-    this->validate(IsPtrAlign4(data) && (SkAlign4(size) == size));
-    if (!fError) {
-        fReader.setMemory(data, size);
-    }
-}
-
-const void* SkValidatingReadBuffer::skip(size_t size) {
-    size_t inc = SkAlign4(size);
-    this->validate(inc >= size);
-    const void* addr = fReader.peek();
-    this->validate(IsPtrAlign4(addr) && fReader.isAvailable(inc));
-    if (fError) {
-        return nullptr;
-    }
-
-    fReader.skip(size);
-    return addr;
-}
-
-// All the methods in this file funnel down into either readInt(), readScalar() or skip(),
-// followed by a memcpy. So we've got all our validation in readInt(), readScalar() and skip();
-// if they fail they'll return a zero value or skip nothing, respectively, and set fError to
-// true, which the caller should check to see if an error occurred during the read operation.
-
-bool SkValidatingReadBuffer::readBool() {
-    uint32_t value = this->readInt();
-    // Boolean value should be either 0 or 1
-    this->validate(!(value & ~1));
-    return value != 0;
-}
-
-SkColor SkValidatingReadBuffer::readColor() {
-    return this->readInt();
-}
-
-int32_t SkValidatingReadBuffer::readInt() {
-    const size_t inc = sizeof(int32_t);
-    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
-    return fError ? 0 : fReader.readInt();
-}
-
-SkScalar SkValidatingReadBuffer::readScalar() {
-    const size_t inc = sizeof(SkScalar);
-    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
-    return fError ? 0 : fReader.readScalar();
-}
-
-uint32_t SkValidatingReadBuffer::readUInt() {
-    return this->readInt();
-}
-
-int32_t SkValidatingReadBuffer::read32() {
-    return this->readInt();
-}
-
-uint8_t SkValidatingReadBuffer::peekByte() {
-    if (fReader.available() <= 0) {
-        fError = true;
-        return 0;
-    }
-    return *((uint8_t*) fReader.peek());
-}
-
-void SkValidatingReadBuffer::readString(SkString* string) {
-    const size_t len = this->readUInt();
-    const void* ptr = fReader.peek();
-    const char* cptr = (const char*)ptr;
-
-    // skip over the string + '\0' and then pad to a multiple of 4
-    const size_t alignedSize = SkAlign4(len + 1);
-    this->skip(alignedSize);
-    if (!fError) {
-        this->validate(cptr[len] == '\0');
-    }
-    if (!fError) {
-        string->set(cptr, len);
-    }
-}
-
-void SkValidatingReadBuffer::readColor4f(SkColor4f* color) {
-    const void* ptr = this->skip(sizeof(SkColor4f));
-    if (!fError) {
-        memcpy(color, ptr, sizeof(SkColor4f));
-    } else {
-        *color = SkColor4f::FromColor(SK_ColorBLACK);
-    }
-}
-
-void SkValidatingReadBuffer::readPoint(SkPoint* point) {
-    point->fX = this->readScalar();
-    point->fY = this->readScalar();
-}
-
-void SkValidatingReadBuffer::readPoint3(SkPoint3* point) {
-    point->fX = this->readScalar();
-    point->fY = this->readScalar();
-    point->fZ = this->readScalar();
-}
-
-void SkValidatingReadBuffer::readMatrix(SkMatrix* matrix) {
-    size_t size = 0;
-    if (!fError) {
-        size = SkMatrixPriv::ReadFromMemory(matrix, fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) == size) && (0 != size));
-    }
-    if (!fError) {
-        (void)this->skip(size);
-    }
-}
-
-void SkValidatingReadBuffer::readIRect(SkIRect* rect) {
-    const void* ptr = this->skip(sizeof(SkIRect));
-    if (!fError) {
-        memcpy(rect, ptr, sizeof(SkIRect));
-    } else {
-        rect->setEmpty();
-    }
-}
-
-void SkValidatingReadBuffer::readRect(SkRect* rect) {
-    const void* ptr = this->skip(sizeof(SkRect));
-    if (!fError) {
-        memcpy(rect, ptr, sizeof(SkRect));
-    } else {
-        rect->setEmpty();
-    }
-}
-
-void SkValidatingReadBuffer::readRRect(SkRRect* rrect) {
-    const void* ptr = this->skip(sizeof(SkRRect));
-    if (!fError) {
-        memcpy(rrect, ptr, sizeof(SkRRect));
-        this->validate(rrect->isValid());
-    }
-
-    if (fError) {
-        rrect->setEmpty();
-    }
-}
-
-void SkValidatingReadBuffer::readRegion(SkRegion* region) {
-    size_t size = 0;
-    if (!fError) {
-        size = region->readFromMemory(fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) == size) && (0 != size));
-    }
-    if (!fError) {
-        (void)this->skip(size);
-    }
-}
-
-void SkValidatingReadBuffer::readPath(SkPath* path) {
-    size_t size = 0;
-    if (!fError) {
-        size = path->readFromMemory(fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) == size) && (0 != size));
-    }
-    if (!fError) {
-        (void)this->skip(size);
-    }
-}
-
-bool SkValidatingReadBuffer::readArray(void* value, size_t size, size_t elementSize) {
-    const uint32_t count = this->getArrayCount();
-    this->validate(size == count);
-    (void)this->skip(sizeof(uint32_t)); // Skip array count
-    const uint64_t byteLength64 = sk_64_mul(count, elementSize);
-    const size_t byteLength = count * elementSize;
-    this->validate(byteLength == byteLength64);
-    const void* ptr = this->skip(SkAlign4(byteLength));
-    if (!fError) {
-        memcpy(value, ptr, byteLength);
-        return true;
-    }
-    return false;
-}
-
-bool SkValidatingReadBuffer::readByteArray(void* value, size_t size) {
-    return this->readArray(static_cast<unsigned char*>(value), size, sizeof(unsigned char));
-}
-
-bool SkValidatingReadBuffer::readColorArray(SkColor* colors, size_t size) {
-    return this->readArray(colors, size, sizeof(SkColor));
-}
-
-bool SkValidatingReadBuffer::readColor4fArray(SkColor4f* colors, size_t size) {
-    return this->readArray(colors, size, sizeof(SkColor4f));
-}
-
-bool SkValidatingReadBuffer::readIntArray(int32_t* values, size_t size) {
-    return this->readArray(values, size, sizeof(int32_t));
-}
-
-bool SkValidatingReadBuffer::readPointArray(SkPoint* points, size_t size) {
-    return this->readArray(points, size, sizeof(SkPoint));
-}
-
-bool SkValidatingReadBuffer::readScalarArray(SkScalar* values, size_t size) {
-    return this->readArray(values, size, sizeof(SkScalar));
-}
-
-uint32_t SkValidatingReadBuffer::getArrayCount() {
-    const size_t inc = sizeof(uint32_t);
-    fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
-    return fError ? 0 : *(uint32_t*)fReader.peek();
-}
-
-SkFlattenable* SkValidatingReadBuffer::readFlattenable(SkFlattenable::Type type) {
-    // The validating read buffer always uses strings and string-indices for unflattening.
-    SkASSERT(0 == this->factoryCount());
-
-    uint8_t firstByte = this->peekByte();
-    if (fError) {
-        return nullptr;
-    }
-
-    SkString name;
-    if (firstByte) {
-        // If the first byte is non-zero, the flattenable is specified by a string.
-        this->readString(&name);
-        if (fError) {
-            return nullptr;
-        }
-
-        // Add the string to the dictionary.
-        fFlattenableDict.set(fFlattenableDict.count() + 1, name);
-    } else {
-        // Read the index.  We are guaranteed that the first byte
-        // is zeroed, so we must shift down a byte.
-        uint32_t index = this->readUInt() >> 8;
-        if (0 == index) {
-            return nullptr; // writer failed to give us the flattenable
-        }
-
-        SkString* namePtr = fFlattenableDict.find(index);
-        if (!namePtr) {
-            return nullptr;
-        }
-        name = *namePtr;
-    }
-
-    // Is this the type we wanted ?
-    const char* cname = name.c_str();
-    SkFlattenable::Type baseType;
-    if (!SkFlattenable::NameToType(cname, &baseType) || (baseType != type)) {
-        return nullptr;
-    }
-
-    // Get the factory for this flattenable.
-    SkFlattenable::Factory factory = this->getCustomFactory(name);
-    if (!factory) {
-        factory = SkFlattenable::NameToFactory(cname);
-        if (!factory) {
-            return nullptr; // writer failed to give us the flattenable
-        }
-    }
-
-    // If we get here, the factory is non-null.
-    sk_sp<SkFlattenable> obj;
-    uint32_t sizeRecorded = this->readUInt();
-    size_t offset = fReader.offset();
-    obj = (*factory)(*this);
-    // check that we read the amount we expected
-    size_t sizeRead = fReader.offset() - offset;
-    this->validate(sizeRecorded == sizeRead);
-    if (fError) {
-        obj = nullptr;
-    }
-    return obj.release();
-}
diff --git a/src/core/SkValidatingReadBuffer.h b/src/core/SkValidatingReadBuffer.h
index 3203ebac55..ccf66d4528 100644
--- a/src/core/SkValidatingReadBuffer.h
+++ b/src/core/SkValidatingReadBuffer.h
@@ -17,69 +17,7 @@
 
 class SkBitmap;
 
-class SkValidatingReadBuffer : public SkReadBuffer {
-public:
-    SkValidatingReadBuffer(const void* data, size_t size);
-    ~SkValidatingReadBuffer() override;
+// DEPRECATED -- just use SkReadBuffer (so we can delete this header)
+typedef SkReadBuffer SkValidatingReadBuffer;
 
-    SkReadBuffer* clone(const void* data, size_t size) const override {
-        return new SkValidatingReadBuffer(data, size);
-    }
-
-    const void* skip(size_t size) override;
-
-    // primitives
-    bool readBool() override;
-    SkColor readColor() override;
-    int32_t readInt() override;
-    SkScalar readScalar() override;
-    uint32_t readUInt() override;
-    int32_t read32() override;
-
-    // peek
-    uint8_t peekByte() override;
-
-    // strings -- the caller is responsible for freeing the string contents
-    void readString(SkString* string) override;
-
-    // common data structures
-    SkFlattenable* readFlattenable(SkFlattenable::Type type) override;
-    void readColor4f(SkColor4f* color) override;
-    void readPoint(SkPoint* point) override;
-    void readPoint3(SkPoint3* point) override;
-    void readMatrix(SkMatrix* matrix) override;
-    void readIRect(SkIRect* rect) override;
-    void readRect(SkRect* rect) override;
-    void readRRect(SkRRect* rrect) override;
-    void readRegion(SkRegion* region) override;
-    void readPath(SkPath* path) override;
-
-    // binary data and arrays
-    bool readByteArray(void* value, size_t size) override;
-    bool readColorArray(SkColor* colors, size_t size) override;
-    bool readColor4fArray(SkColor4f* colors, size_t size) override;
-    bool readIntArray(int32_t* values, size_t size) override;
-    bool readPointArray(SkPoint* points, size_t size) override;
-    bool readScalarArray(SkScalar* values, size_t size) override;
-
-    // helpers to get info about arrays and binary data
-    uint32_t getArrayCount() override;
-
-    bool validate(bool isValid) override;
-    bool isValid() const override;
-
-private:
-    bool readArray(void* value, size_t size, size_t elementSize);
-
-    void setMemory(const void* data, size_t size);
-
-    static bool IsPtrAlign4(const void* ptr) {
-        return SkIsAlign4((uintptr_t)ptr);
-    }
-
-    bool fError;
-
-    typedef SkReadBuffer INHERITED;
-};
-
-#endif // SkValidatingReadBuffer_DEFINED
+#endif