Revert "impl SkSerial picture procs"

This reverts commit c8226728541fdd6434a7f1b97678246202b0edc5.

Reason for revert: broke old skps

Original change's description:
> impl SkSerial picture procs
> 
> The picture serialization code is a bit of a mess, with duplicated functions for streams and buffers.
> Could not see how to fix that and land this at the same time, but I will try to circle back and
> simplify if possible afterwards.
> 
> Bug: skia:
> Change-Id: I9053fdc476c60f483df013d021e248258181c199
> Reviewed-on: https://skia-review.googlesource.com/83943
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Commit-Queue: Mike Reed <reed@google.com>

TBR=mtklein@google.com,fmalita@chromium.org,reed@google.com

Change-Id: I68ae019a286691b65cc373cb29c941d6620fd34a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/84460
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>

5 files changed, 25 insertions, 101 deletions

diff --git a/src/core/SkMathPriv.h b/src/core/SkMathPriv.h
index 6c33e36747..ddb231507f 100644
--- a/src/core/SkMathPriv.h
+++ b/src/core/SkMathPriv.h
@@ -49,21 +49,6 @@ static inline unsigned SkClampUMax(unsigned value, unsigned max) {
     return value;
 }
 
-// If a signed int holds min_int (e.g. 0x80000000) it is undefined what happens when
-// we negate it (even though we *know* we're 2's complement and we'll get the same
-// value back). So we create this helper function that casts to size_t (unsigned) first,
-// to avoid the complaint.
-static inline size_t sk_negate_to_size_t(int32_t value) {
-#if defined(_MSC_VER)
-#pragma warning(push)
-#pragma warning(disable : 4146)  // Thanks MSVC, we know what we're negating an unsigned
-#endif
-    return -static_cast<size_t>(value);
-#if defined(_MSC_VER)
-#pragma warning(pop)
-#endif
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 /** Return a*b/255, truncating away any fractional bits. Only valid if both
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index edac1349ed..5442738368 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -8,7 +8,6 @@
 #include "SkAtomics.h"
 #include "SkImageDeserializer.h"
 #include "SkImageGenerator.h"
-#include "SkMathPriv.h"
 #include "SkPicture.h"
 #include "SkPictureCommon.h"
 #include "SkPictureData.h"
@@ -176,25 +175,7 @@ sk_sp<SkPicture> SkPicture::MakeFromStream(SkStream* stream, const SkDeserialPro
 sk_sp<SkPicture> SkPicture::MakeFromStream(SkStream* stream, const SkDeserialProcs& procs,
                                            SkTypefacePlayback* typefaces) {
     SkPictInfo info;
-    if (!InternalOnly_StreamIsSKP(stream, &info)) {
-        return nullptr;
-    }
-
-    // size should be 0, 1, or negative
-    int32_t ssize = stream->readS32();
-    if (ssize < 0) {
-        if (!procs.fPictureProc) {
-            return nullptr;
-        }
-        size_t size = sk_negate_to_size_t(ssize);
-        auto data = SkData::MakeUninitialized(size);
-        if (stream->read(data->writable_data(), size) != size) {
-            return nullptr;
-        }
-        return procs.fPictureProc(data->data(), size, procs.fPictureCtx);
-    }
-    if (ssize != 1) {
-        // 1 is the magic 'size' that means SkPictureData follows
+    if (!InternalOnly_StreamIsSKP(stream, &info) || !stream->readBool()) {
         return nullptr;
     }
     std::unique_ptr<SkPictureData> data(
@@ -204,24 +185,10 @@ sk_sp<SkPicture> SkPicture::MakeFromStream(SkStream* stream, const SkDeserialPro
 
 sk_sp<SkPicture> SkPicture::MakeFromBuffer(SkReadBuffer& buffer) {
     SkPictInfo info;
-    if (!InternalOnly_BufferIsSKP(&buffer, &info)) {
+    if (!InternalOnly_BufferIsSKP(&buffer, &info) || !buffer.readBool()) {
         return nullptr;
     }
-    // size should be 0, 1, or negative
-    int32_t ssize = buffer.read32();
-    if (ssize < 0) {
-        const SkDeserialProcs& procs = buffer.fProcs;
-        if (!procs.fPictureProc) {
-            return nullptr;
-        }
-        size_t size = sk_negate_to_size_t(ssize);
-        return procs.fPictureProc(buffer.skip(size), size, procs.fPictureCtx);
-    }
-    if (ssize != 1) {
-        // 1 is the magic 'size' that means SkPictureData follows
-        return nullptr;
-    }
-   std::unique_ptr<SkPictureData> data(SkPictureData::CreateFromBuffer(buffer, info));
+    std::unique_ptr<SkPictureData> data(SkPictureData::CreateFromBuffer(buffer, info));
     return Forwardport(info, data.get(), &buffer);
 }
 
@@ -255,49 +222,17 @@ sk_sp<SkData> SkPicture::serialize(const SkSerialProcs& procs) const {
     return stream.detachAsData();
 }
 
-static sk_sp<SkData> custom_serialize(const SkPicture* picture, const SkSerialProcs& procs) {
-    if (procs.fPictureProc) {
-        auto data = procs.fPictureProc(const_cast<SkPicture*>(picture), procs.fPictureCtx);
-        if (data) {
-            size_t size = data->size();
-            if (!sk_64_isS32(size) || size <= 1) {
-                return SkData::MakeEmpty();
-            }
-            return data;
-        }
-    }
-    return nullptr;
-}
-
-static bool write_pad32(SkWStream* stream, const void* data, size_t size) {
-    if (!stream->write(data, size)) {
-        return false;
-    }
-    if (size & 3) {
-        uint32_t zero = 0;
-        return stream->write(&zero, 4 - (size & 3));
-    }
-    return true;
-}
-
 void SkPicture::serialize(SkWStream* stream, const SkSerialProcs& procs,
                           SkRefCntSet* typefaceSet) const {
     SkPictInfo info = this->createHeader();
-    stream->write(&info, sizeof(info));
-
-    if (auto custom = custom_serialize(this, procs)) {
-        int32_t size = SkToS32(custom->size());
-        stream->write32(-size);    // negative for custom format
-        write_pad32(stream, custom->data(), size);
-        return;
-    }
-
     std::unique_ptr<SkPictureData> data(this->backport());
+
+    stream->write(&info, sizeof(info));
     if (data) {
-        stream->write32(1); // special size meaning SkPictureData
+        stream->writeBool(true);
         data->serialize(stream, procs, typefaceSet);
     } else {
-        stream->write32(0); // signal no content
+        stream->writeBool(false);
     }
 }
 
@@ -309,19 +244,11 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const {
     buffer.writeUInt(info.getVersion());
     buffer.writeRect(info.fCullRect);
     buffer.writeUInt(info.fFlags);
-
-    if (auto custom = custom_serialize(this, buffer.fProcs)) {
-        int32_t size = SkToS32(custom->size());
-        buffer.write32(-size);    // negative for custom format
-        buffer.writePad32(custom->data(), size);
-        return;
-    }
-
     if (data) {
-        buffer.write32(1); // special size meaning SkPictureData
+        buffer.writeBool(true);
         data->flatten(buffer);
     } else {
-        buffer.write32(0); // signal no content
+        buffer.writeBool(false);
     }
 }
 
diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index 3018b879d2..a86aee2fa0 100644
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -305,9 +305,8 @@ void SkPictureData::serialize(SkWStream* stream, const SkSerialProcs& procs,
         bool write(const void*, size_t size) override { fBytesWritten += size; return true; }
         size_t bytesWritten() const override { return fBytesWritten; }
     } devnull;
-    SkSerialProcs nullProcs;
     for (int i = 0; i < fPictureCount; i++) {
-        fPictureRefs[i]->serialize(&devnull, nullProcs, typefaceSet);
+        fPictureRefs[i]->serialize(&devnull, procs, typefaceSet);
     }
 
     // We need to write factories before we write the buffer.
diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp
index 94bf34af91..c0be4f3490 100644
--- a/src/core/SkReadBuffer.cpp
+++ b/src/core/SkReadBuffer.cpp
@@ -11,13 +11,27 @@
 #include "SkImageDeserializer.h"
 #include "SkImageGenerator.h"
 #include "SkMakeUnique.h"
-#include "SkMathPriv.h"
 #include "SkMatrixPriv.h"
 #include "SkReadBuffer.h"
 #include "SkStream.h"
 #include "SkTypeface.h"
 
 namespace {
+    // If a signed int holds min_int (e.g. 0x80000000) it is undefined what happens when
+    // we negate it (even though we *know* we're 2's complement and we'll get the same
+    // value back). So we create this helper function that casts to size_t (unsigned) first,
+    // to avoid the complaint.
+    size_t sk_negate_to_size_t(int32_t value) {
+#if defined(_MSC_VER)
+#pragma warning(push)
+#pragma warning(disable : 4146)  // Thanks MSVC, we know what we're negating an unsigned
+#endif
+        return -static_cast<size_t>(value);
+#if defined(_MSC_VER)
+#pragma warning(pop)
+#endif
+    }
+
     // This generator intentionally should always fail on all attempts to get its pixels,
     // simulating a bad or empty codec stream.
     class EmptyImageGenerator final : public SkImageGenerator {
diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h
index 4c0ca9bec2..61f1915e19 100644
--- a/src/core/SkReadBuffer.h
+++ b/src/core/SkReadBuffer.h
@@ -292,7 +292,6 @@ private:
     SkTHashMap<SkString, SkFlattenable::Factory> fCustomFactory;
 
     SkDeserialProcs fProcs;
-    friend class SkPicture;
 
 #ifdef DEBUG_NON_DETERMINISTIC_ASSERT
     // Debugging counter to keep track of how many bitmaps we