Revert of change pixel-serializer to support reencoding existing data (patchset #5 id:80001 of https://codereview.chromium.org/1373683003/ )

Reason for revert:
Need to somehow get access to encoders in chrome -- link error on the roll since SkImageEncoder is not built as part of chrome.

Original issue's description:
> change pixel-serializer to support reencoding existing data
>
> Trying to evolve this interface so it can
> - support rich set of backend-encoders (including ones like ETC1 that can cheaply convert to KXT
> - allow for encoding images as well as bitmaps (e.g. for picture serialization)
> - perhaps replace SkImageEncoder as an API (assuming we create a factory that returns a serializer given a format)
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/13f48dc85aa68a60da66aaf39c93d527d11d1278

TBR=scroggo@google.com,msarett@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

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

11 files changed, 62 insertions, 122 deletions

diff --git a/gyp/core.gypi b/gyp/core.gypi
index 95d44f24ab..58d98994a0 100644
--- a/gyp/core.gypi
+++ b/gyp/core.gypi
@@ -165,7 +165,6 @@
         '<(skia_src_path)/core/SkPictureShader.cpp',
         '<(skia_src_path)/core/SkPictureShader.h',
         '<(skia_src_path)/core/SkPixelRef.cpp',
-        '<(skia_src_path)/core/SkPixelSerializer.cpp',
         '<(skia_src_path)/core/SkPixmap.cpp',
         '<(skia_src_path)/core/SkPoint.cpp',
         '<(skia_src_path)/core/SkPoint3.cpp',
diff --git a/include/core/SkImageEncoder.h b/include/core/SkImageEncoder.h
index 5fa69f7992..fc999dcc18 100644
--- a/include/core/SkImageEncoder.h
+++ b/include/core/SkImageEncoder.h
@@ -13,8 +13,6 @@
 
 class SkBitmap;
 class SkData;
-class SkPixmap;
-class SkPixelSerializer;
 class SkWStream;
 
 class SkImageEncoder {
@@ -62,11 +60,8 @@ public:
      */
     bool encodeStream(SkWStream* stream, const SkBitmap& bm, int quality);
 
-    SkPixelSerializer* refSerializer();
-
     static SkData* EncodeData(const SkImageInfo&, const void* pixels, size_t rowBytes,
                               Type, int quality);
-    static SkData* EncodeData(const SkPixmap&, Type, int quality);
     static SkData* EncodeData(const SkBitmap&, Type, int quality);
 
     static bool EncodeFile(const char file[], const SkBitmap&, Type,
@@ -74,12 +69,6 @@ public:
     static bool EncodeStream(SkWStream*, const SkBitmap&, Type,
                            int quality);
 
-    /**
-     *  If the existing data can be re-encoded into the specified type (efficiently), this
-     *  returns that new data (which the caller must unref()). If not, this returns null.
-     */
-    static SkData* ReencodeData(SkData* encoded, Type);
-
 protected:
     /**
      * Encode bitmap 'bm' in the desired format, writing results to
@@ -89,8 +78,6 @@ protected:
      * This must be overridden by each SkImageEncoder implementation.
      */
     virtual bool onEncode(SkWStream* stream, const SkBitmap& bm, int quality) = 0;
-
-    virtual SkData* onReencodeData(SkData*) { return nullptr; }
 };
 
 // This macro declares a global (i.e., non-class owned) creation entry point
diff --git a/include/core/SkPixelSerializer.h b/include/core/SkPixelSerializer.h
index 4ac5253077..d089209909 100644
--- a/include/core/SkPixelSerializer.h
+++ b/include/core/SkPixelSerializer.h
@@ -8,10 +8,9 @@
 #ifndef SkPixelSerializer_DEFINED
 #define SkPixelSerializer_DEFINED
 
-#include "SkData.h"
-#include "SkPixmap.h"
 #include "SkRefCnt.h"
 
+class SkData;
 struct SkImageInfo;
 
 /**
@@ -19,38 +18,35 @@ struct SkImageInfo;
  */
 class SkPixelSerializer : public SkRefCnt {
 public:
+    virtual ~SkPixelSerializer() {}
+
     /**
-     *  Call to determine if the client wants to serialize the encoded data.
-     *
-     *  If the encoded data is can be re-encoded (or taken as is), this returns a ref to a data
-     *  with the result, which the caller must unref() when they are through. The returned
-     *  data may be the same as the input, or it may be different, but either way the caller is
-     *  responsible for calling unref() on it.
-     *
-     *  If the encoded data is not acceptable to this pixel serializer, this returns NULL.
+     *  Call to determine if the client wants to serialize the encoded data. If
+     *  false, serialize another version (e.g. the result of encodePixels).
      */
-    SkData* reencodeData(SkData* encoded);
+    bool useEncodedData(const void* data, size_t len) {
+        return this->onUseEncodedData(data, len);
+    }
 
     /**
      *  Call to get the client's version of encoding these pixels. If it
      *  returns NULL, serialize the raw pixels.
      */
-    SkData* encodePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes);
+    SkData* encodePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes) {
+        return this->onEncodePixels(info, pixels, rowBytes);
+    }
 
+protected:
     /**
-     *  Call to get the client's version of encoding these pixels. If it
-     *  returns NULL, serialize the raw pixels.
+     *  Return true if you want to serialize the encoded data, false if you want
+     *  another version serialized (e.g. the result of encodePixels).
      */
-    SkData* encodePixels(const SkPixmap& pixmap);
-    
-protected:
-    // DEPRECATED -- this is no longer called, so remove from your subclasses!
-    virtual bool onUseEncodedData(const void*, size_t) { return true; }
-
-    virtual SkData* onReencodeData(SkData* encoded) {
-        return SkRef(encoded);
-    }
+    virtual bool onUseEncodedData(const void* data, size_t len) = 0;
 
+    /**
+     *  If you want to encode these pixels, return the encoded data as an SkData
+     *  Return null if you want to serialize the raw pixels.
+     */
     virtual SkData* onEncodePixels(const SkImageInfo&, const void* pixels, size_t rowBytes) = 0;
 };
 #endif // SkPixelSerializer_DEFINED
diff --git a/src/core/SkPixelSerializer.cpp b/src/core/SkPixelSerializer.cpp
deleted file mode 100644
index a694bbe846..0000000000
--- a/src/core/SkPixelSerializer.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * Copyright 2015 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkPixelSerializer.h"
-
-SkData* SkPixelSerializer::reencodeData(SkData* encoded) {
-    return encoded ? this->onReencodeData(encoded) : nullptr;
-}
-
-SkData* SkPixelSerializer::encodePixels(const SkImageInfo& info, const void* pixels,
-                                        size_t rowBytes) {
-    if (kUnknown_SkColorType == info.colorType() || !pixels) {
-        return nullptr;
-    }
-    return this->onEncodePixels(info, pixels, rowBytes);
-}
-
-SkData* SkPixelSerializer::encodePixels(const SkPixmap& pixmap) {
-    return this->encodePixels(pixmap.info(), pixmap.addr(), pixmap.rowBytes());
-}
diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp
index 1799df1714..1dfe0b3972 100644
--- a/src/core/SkWriteBuffer.cpp
+++ b/src/core/SkWriteBuffer.cpp
@@ -186,15 +186,13 @@ void SkWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
     SkPixelRef* pixelRef = bitmap.pixelRef();
     if (pixelRef) {
         // see if the pixelref already has an encoded version
-        SkAutoDataUnref encodedData(pixelRef->refEncodedData());
-        if (encodedData) {
+        SkAutoDataUnref existingData(pixelRef->refEncodedData());
+        if (existingData.get() != nullptr) {
             // Assumes that if the client did not set a serializer, they are
             // happy to get the encoded data.
-            if (fPixelSerializer) {
-                encodedData.reset(fPixelSerializer->reencodeData(encodedData));
-            }
-            if (encodedData) {
-                write_encoded_bitmap(this, encodedData, bitmap.pixelRefOrigin());
+            if (!fPixelSerializer || fPixelSerializer->useEncodedData(existingData->data(),
+                                                                      existingData->size())) {
+                write_encoded_bitmap(this, existingData, bitmap.pixelRefOrigin());
                 return;
             }
         }
@@ -202,9 +200,12 @@ void SkWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
         // see if the caller wants to manually encode
         SkAutoPixmapUnlock result;
         if (fPixelSerializer && bitmap.requestLock(&result)) {
+            const SkPixmap& pmap = result.pixmap();
             SkASSERT(nullptr == fBitmapHeap);
-            SkAutoDataUnref data(fPixelSerializer->encodePixels(result.pixmap()));
-            if (data) {
+            SkAutoDataUnref data(fPixelSerializer->encodePixels(pmap.info(),
+                                                                pmap.addr(),
+                                                                pmap.rowBytes()));
+            if (data.get() != nullptr) {
                 // if we have to "encode" the bitmap, then we assume there is no
                 // offset to share, since we are effectively creating a new pixelref
                 write_encoded_bitmap(this, data, SkIPoint::Make(0, 0));
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index 240c06e63b..ee76a7ea39 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -74,14 +74,6 @@ SkShader* SkImage::newShader(SkShader::TileMode tileX,
 }
 
 SkData* SkImage::encode(SkImageEncoder::Type type, int quality) const {
-    SkAutoDataUnref encoded(this->refEncoded());
-    if (encoded) {
-        SkAutoDataUnref reencoded(SkImageEncoder::ReencodeData(encoded, type));
-        if (reencoded) {
-            return reencoded.detach();
-        }
-    }
-
     SkBitmap bm;
     if (as_IB(this)->getROPixels(&bm)) {
         return SkImageEncoder::EncodeData(bm, type, quality);
@@ -93,6 +85,10 @@ namespace {
 
 class DefaultSerializer :  public SkPixelSerializer {
 protected:
+    bool onUseEncodedData(const void *data, size_t len) override {
+        return true;
+    }
+
     SkData* onEncodePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes) override {
         return SkImageEncoder::EncodeData(info, pixels, rowBytes, SkImageEncoder::kPNG_Type, 100);
     }
@@ -105,17 +101,15 @@ SkData* SkImage::encode(SkPixelSerializer* serializer) const {
     SkPixelSerializer* effectiveSerializer = serializer ? serializer : &defaultSerializer;
 
     SkAutoTUnref<SkData> encoded(this->refEncoded());
-    if (encoded) {
-        encoded.reset(effectiveSerializer->reencodeData(encoded));
-        if (encoded) {
-            return encoded.detach();
-        }
+    if (encoded && effectiveSerializer->useEncodedData(encoded->data(), encoded->size())) {
+        return encoded.detach();
     }
 
     SkBitmap bm;
     SkAutoPixmapUnlock apu;
     if (as_IB(this)->getROPixels(&bm) && bm.requestLock(&apu)) {
-        return effectiveSerializer->encodePixels(apu.pixmap());
+        const SkPixmap& pmap = apu.pixmap();
+        return effectiveSerializer->encodePixels(pmap.info(), pmap.addr(), pmap.rowBytes());
     }
 
     return nullptr;
diff --git a/src/images/SkImageDecoder_ktx.cpp b/src/images/SkImageDecoder_ktx.cpp
index f903b3fd56..a95ab6f602 100644
--- a/src/images/SkImageDecoder_ktx.cpp
+++ b/src/images/SkImageDecoder_ktx.cpp
@@ -252,40 +252,34 @@ SkImageDecoder::Result SkKTXImageDecoder::onDecode(SkStream* stream, SkBitmap* b
 class SkKTXImageEncoder : public SkImageEncoder {
 protected:
     bool onEncode(SkWStream* stream, const SkBitmap& bm, int quality) override;
-    SkData* onReencodeData(SkData*) override;
 
 private:
     virtual bool encodePKM(SkWStream* stream, const SkData *data);
     typedef SkImageEncoder INHERITED;
 };
 
-SkData* SkKTXImageEncoder::onReencodeData(SkData* encoded) {
-    const uint8_t* bytes = encoded->bytes();
-    if (etc1_pkm_is_valid(bytes)) {
-        SkDynamicMemoryWStream stream;
-        if (this->encodePKM(&stream, encoded)) {
-            return stream.copyToData();
-        }
-    }
-    // Is it a KTX file??
-    if (SkKTXFile::is_ktx(bytes)) {
-        return SkRef(encoded);
-    }
-    return nullptr;
-}
-
 bool SkKTXImageEncoder::onEncode(SkWStream* stream, const SkBitmap& bitmap, int) {
     if (!bitmap.pixelRef()) {
         return false;
     }
+    SkAutoDataUnref data(bitmap.pixelRef()->refEncodedData());
+
+    // Is this even encoded data?
+    if (data) {
+        const uint8_t *bytes = data->bytes();
+        if (etc1_pkm_is_valid(bytes)) {
+            return this->encodePKM(stream, data);
+        }
 
-    SkAutoDataUnref encoded(bitmap.pixelRef()->refEncodedData());
-    if (encoded) {
-        SkAutoDataUnref reencoded(this->onReencodeData(encoded));
-        if (reencoded) {
-            return stream->write(reencoded->bytes(), reencoded->size());
+        // Is it a KTX file??
+        if (SkKTXFile::is_ktx(bytes)) {
+            return stream->write(bytes, data->size());
         }
+        
+        // If it's neither a KTX nor a PKM, then we need to
+        // get at the actual pixels, so fall through and decompress...
     }
+
     return SkKTXFile::WriteBitmapToKTX(stream, bitmap);
 }
 
diff --git a/src/images/SkImageEncoder.cpp b/src/images/SkImageEncoder.cpp
index 4ad12c84cd..cc1b73baa5 100644
--- a/src/images/SkImageEncoder.cpp
+++ b/src/images/SkImageEncoder.cpp
@@ -60,13 +60,3 @@ SkData* SkImageEncoder::EncodeData(const SkImageInfo& info, const void* pixels,
     SkAutoTDelete<SkImageEncoder> enc(SkImageEncoder::Create(t));
     return enc.get() ? enc.get()->encodeData(bm, quality) : nullptr;
 }
-
-SkData* SkImageEncoder::EncodeData(const SkPixmap& pmap, Type t, int quality) {
-    return EncodeData(pmap.info(), pmap.addr(), pmap.rowBytes(), t, quality);
-}
-
-SkData* SkImageEncoder::ReencodeData(SkData* encoded, Type t) {
-    SkAutoTDelete<SkImageEncoder> enc(SkImageEncoder::Create(t));
-    return enc.get() ? enc.get()->onReencodeData(encoded) : nullptr;
-}
-
diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index e0220778a0..0b27af9780 100644
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -119,8 +119,8 @@ public:
     bool didEncode() const { return fDidEncode; }
 
 protected:
-    SkData* onReencodeData(SkData*) override {
-        return nullptr;
+    bool onUseEncodedData(const void*, size_t) override {
+        return false;
     }
 
     SkData* onEncodePixels(const SkImageInfo&, const void*, size_t) override {
diff --git a/tests/KtxTest.cpp b/tests/KtxTest.cpp
index 0ad24969c2..e0d9a27b40 100644
--- a/tests/KtxTest.cpp
+++ b/tests/KtxTest.cpp
@@ -8,7 +8,6 @@
 #include "Resources.h"
 #include "SkBitmap.h"
 #include "SkData.h"
-#include "SkImage.h"
 #include "SkImageGenerator.h"
 #include "SkForceLinking.h"
 #include "SkImageDecoder.h"
@@ -150,11 +149,14 @@ DEF_TEST(KtxReexportPKM, reporter) {
         return;
     }
 
-    SkAutoTUnref<SkImage> image(SkImage::NewFromEncoded(fileData));
-    REPORTER_ASSERT(reporter, image.get());
+    bool installDiscardablePixelRefSuccess =
+        SkDEPRECATED_InstallDiscardablePixelRef(fileData, &etcBitmap);
+    REPORTER_ASSERT(reporter, installDiscardablePixelRefSuccess);
 
-    SkAutoDataUnref newKtxData(image->encode(SkImageEncoder::kKTX_Type, 0));
-    REPORTER_ASSERT(reporter, newKtxData.get());
+    // Write the bitmap out to a KTX file.
+    SkData *ktxDataPtr = SkImageEncoder::EncodeData(etcBitmap, SkImageEncoder::kKTX_Type, 0);
+    SkAutoDataUnref newKtxData(ktxDataPtr);
+    REPORTER_ASSERT(reporter, ktxDataPtr);
 
     // See is this data is identical to data in existing ktx file.
     SkString ktxFilename = GetResourcePath("mandrill_128.ktx");
diff --git a/tools/sk_tool_utils.h b/tools/sk_tool_utils.h
index d8709cc71e..b20c60ba76 100644
--- a/tools/sk_tool_utils.h
+++ b/tools/sk_tool_utils.h
@@ -115,6 +115,7 @@ namespace sk_tool_utils {
     // used.
     class PngPixelSerializer : public SkPixelSerializer {
     public:
+        bool onUseEncodedData(const void*, size_t) override { return true; }
         SkData* onEncodePixels(const SkImageInfo& info, const void* pixels,
                                size_t rowBytes) override {
             return SkImageEncoder::EncodeData(info, pixels, rowBytes,