Revert of add colortable support to imagegenerator (https://codereview.chromium.org/304443003/)

Reason for revert:
failing tests

Original issue's description:
> add colortable support to imagegenerator
>
> BUG=skia:
>
> Committed: http://code.google.com/p/skia/source/detail?r=14916

R=halcanary@google.com, scroggo@google.com
TBR=halcanary@google.com, scroggo@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Author: reed@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@14917 2bbb7eff-a529-9590-31e7-b0007b416f81

9 files changed, 63 insertions, 174 deletions

diff --git a/gyp/images.gyp b/gyp/images.gyp
index 18a18be7e1..0062f24f1d 100644
--- a/gyp/images.gyp
+++ b/gyp/images.gyp
@@ -71,7 +71,6 @@
         '../src/images/SkImageEncoder.cpp',
         '../src/images/SkImageEncoder_Factory.cpp',
         '../src/images/SkImageEncoder_argb.cpp',
-        '../src/images/SkImageGenerator.cpp',
         '../src/images/SkImageRef.cpp',
         '../src/images/SkImageRefPool.cpp',
         '../src/images/SkImageRefPool.h',
diff --git a/gyp/skia_for_chromium_defines.gypi b/gyp/skia_for_chromium_defines.gypi
index a47b391c3c..c114b7142e 100644
--- a/gyp/skia_for_chromium_defines.gypi
+++ b/gyp/skia_for_chromium_defines.gypi
@@ -16,7 +16,6 @@
       'SK_SUPPORT_LEGACY_GETTOPDEVICE',
       'SK_SUPPORT_LEGACY_ASIMAGEINFO',
       'SK_SUPPORT_LEGACY_N32_NAME',
-      'SK_SUPPORT_LEGACY_IMAGEGENERATORAPI',
     ],
   },
 }
diff --git a/include/core/SkImageGenerator.h b/include/core/SkImageGenerator.h
index 21ebcbb3f2..f399ab538e 100644
--- a/include/core/SkImageGenerator.h
+++ b/include/core/SkImageGenerator.h
@@ -9,7 +9,6 @@
 #define SkImageGenerator_DEFINED
 
 #include "SkImageInfo.h"
-#include "SkColor.h"
 
 class SkBitmap;
 class SkData;
@@ -48,13 +47,6 @@ public:
      */
     virtual ~SkImageGenerator() { }
 
-#ifdef SK_SUPPORT_LEGACY_IMAGEGENERATORAPI
-    virtual bool refEncodedData() { return this->onRefEncodedData(); }
-    virtual bool getInfo(SkImageInfo* info) { return this->onGetInfo(info); }
-    virtual bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) {
-        return this->onGetPixels(info, pixels, rowBytes, NULL, NULL);
-    }
-#else
     /**
      *  Return a ref to the encoded (i.e. compressed) representation,
      *  of this data.
@@ -62,7 +54,7 @@ public:
      *  If non-NULL is returned, the caller is responsible for calling
      *  unref() on the data when it is finished.
      */
-    SkData* refEncodedData() { return this->onRefEncodedData(); }
+    virtual SkData* refEncodedData() { return NULL; }
 
     /**
      *  Return some information about the image, allowing the owner of
@@ -73,7 +65,7 @@ public:
      *
      *  @return false if anything goes wrong.
      */
-    bool getInfo(SkImageInfo* info);
+    virtual bool getInfo(SkImageInfo* info) = 0;
 
     /**
      *  Decode into the given pixels, a block of memory of size at
@@ -91,31 +83,12 @@ public:
      *         different output-configs, which the implementation can
      *         decide to support or not.
      *
-     *  If info is kIndex8_SkColorType, then the caller must provide storage for up to 256
-     *  SkPMColor values in ctable. On success the generator must copy N colors into that storage,
-     *  (where N is the logical number of table entries) and set ctableCount to N.
-     *
-     *  If info is not kIndex8_SkColorType, then the last two parameters may be NULL. If ctableCount
-     *  is not null, it will be set to 0.
-     *
      *  @return false if anything goes wrong or if the image info is
      *          unsupported.
      */
-    bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
-                   SkPMColor ctable[], int* ctableCount);
-
-    /**
-     *  Simplified version of getPixels() that asserts that info is NOT kIndex8_SkColorType.
-     */
-    bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes);
-#endif
-
-protected:
-    virtual SkData* onRefEncodedData();
-    virtual bool onGetInfo(SkImageInfo* info);
-    virtual bool onGetPixels(const SkImageInfo& info,
-                             void* pixels, size_t rowBytes,
-                             SkPMColor ctable[], int* ctableCount);
+    virtual bool getPixels(const SkImageInfo& info,
+                           void* pixels,
+                           size_t rowBytes) = 0;
 };
 
 #endif  // SkImageGenerator_DEFINED
diff --git a/src/images/SkDecodingImageGenerator.cpp b/src/images/SkDecodingImageGenerator.cpp
index 1f0f706ede..b3924a7daa 100644
--- a/src/images/SkDecodingImageGenerator.cpp
+++ b/src/images/SkDecodingImageGenerator.cpp
@@ -23,6 +23,12 @@ bool equal_modulo_alpha(const SkImageInfo& a, const SkImageInfo& b) {
 class DecodingImageGenerator : public SkImageGenerator {
 public:
     virtual ~DecodingImageGenerator();
+    virtual SkData* refEncodedData() SK_OVERRIDE;
+    // This implementaion of getInfo() always returns true.
+    virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE;
+    virtual bool getPixels(const SkImageInfo& info,
+                           void* pixels,
+                           size_t rowBytes) SK_OVERRIDE;
 
     SkData*                fData;
     SkStreamRewindable*    fStream;
@@ -35,18 +41,6 @@ public:
                            const SkImageInfo& info,
                            int sampleSize,
                            bool ditherImage);
-
-protected:
-    virtual SkData* onRefEncodedData() SK_OVERRIDE;
-    virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE {
-        *info = fInfo;
-        return true;
-    }
-    virtual bool onGetPixels(const SkImageInfo& info,
-                             void* pixels, size_t rowBytes,
-                             SkPMColor ctable[], int* ctableCount) SK_OVERRIDE;
-
-private:
     typedef SkImageGenerator INHERITED;
 };
 
@@ -129,7 +123,14 @@ DecodingImageGenerator::~DecodingImageGenerator() {
     fStream->unref();
 }
 
-SkData* DecodingImageGenerator::onRefEncodedData() {
+bool DecodingImageGenerator::getInfo(SkImageInfo* info) {
+    if (info != NULL) {
+        *info = fInfo;
+    }
+    return true;
+}
+
+SkData* DecodingImageGenerator::refEncodedData() {
     // This functionality is used in `gm --serialize`
     // Does not encode options.
     if (fData != NULL) {
@@ -150,15 +151,22 @@ SkData* DecodingImageGenerator::onRefEncodedData() {
     return SkSafeRef(fData);
 }
 
-bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info,
-                                         void* pixels, size_t rowBytes,
-                                         SkPMColor ctableEntries[], int* ctableCount) {
+bool DecodingImageGenerator::getPixels(const SkImageInfo& info,
+                                         void* pixels,
+                                         size_t rowBytes) {
+    if (NULL == pixels) {
+        return false;
+    }
     if (fInfo != info) {
         // The caller has specified a different info.  This is an
         // error for this kind of SkImageGenerator.  Use the Options
         // to change the settings.
         return false;
     }
+    if (info.minRowBytes() > rowBytes) {
+        // The caller has specified a bad rowBytes.
+        return false;
+    }
 
     SkAssertResult(fStream->rewind());
     SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(fStream));
@@ -194,20 +202,6 @@ bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info,
     } else {
         SkASSERT(check_alpha(info.alphaType(), bitmap.alphaType()));
     }
-
-    if (kIndex_8_SkColorType == info.colorType()) {
-        if (kIndex_8_SkColorType != bitmap.colorType()) {
-            return false;   // they asked for Index8, but we didn't receive that from decoder
-        }
-        SkColorTable* ctable = bitmap.getColorTable();
-        if (NULL == ctable) {
-            return false;
-        }
-        const int count = ctable->count();
-        memcpy(ctableEntries, ctable->lockColors(), count * sizeof(SkPMColor));
-        ctable->unlockColors();
-        *ctableCount = count;
-    }
     return true;
 }
 
@@ -220,6 +214,11 @@ SkImageGenerator* CreateDecodingImageGenerator(
         const SkDecodingImageGenerator::Options& opts) {
     SkASSERT(stream);
     SkAutoTUnref<SkStreamRewindable> autoStream(stream);  // always unref this.
+    if (opts.fUseRequestedColorType &&
+        (kIndex_8_SkColorType == opts.fRequestedColorType)) {
+        // We do not support indexed color with SkImageGenerators,
+        return NULL;
+    }
     SkAssertResult(autoStream->rewind());
     SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(autoStream));
     if (NULL == decoder.get()) {
@@ -228,16 +227,24 @@ SkImageGenerator* CreateDecodingImageGenerator(
     SkBitmap bitmap;
     decoder->setSampleSize(opts.fSampleSize);
     decoder->setRequireUnpremultipliedColors(opts.fRequireUnpremul);
-    if (!decoder->decode(stream, &bitmap, SkImageDecoder::kDecodeBounds_Mode)) {
+    if (!decoder->decode(stream, &bitmap,
+                         SkImageDecoder::kDecodeBounds_Mode)) {
         return NULL;
     }
-    if (kUnknown_SkColorType == bitmap.colorType()) {
+    if (bitmap.config() == SkBitmap::kNo_Config) {
         return NULL;
     }
 
     SkImageInfo info = bitmap.info();
 
-    if (opts.fUseRequestedColorType && (opts.fRequestedColorType != info.colorType())) {
+    if (!opts.fUseRequestedColorType) {
+        // Use default
+        if (kIndex_8_SkColorType == bitmap.colorType()) {
+            // We don't support kIndex8 because we don't support
+            // colortables in this workflow.
+            info.fColorType = kN32_SkColorType;
+        }
+    } else {
         if (!bitmap.canCopyTo(opts.fRequestedColorType)) {
             SkASSERT(bitmap.colorType() != opts.fRequestedColorType);
             return NULL;  // Can not translate to needed config.
diff --git a/src/images/SkImageGenerator.cpp b/src/images/SkImageGenerator.cpp
deleted file mode 100644
index b898764f0c..0000000000
--- a/src/images/SkImageGenerator.cpp
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * Copyright 2014 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkImageGenerator.h"
-
-#ifndef SK_SUPPORT_LEGACY_IMAGEGENERATORAPI
-bool SkImageGenerator::getInfo(SkImageInfo* info) {
-    SkImageInfo dummy;
-    if (NULL == info) {
-        info = &dummy;
-    }
-    return this->onGetInfo(info);
-}
-
-bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
-                                 SkPMColor ctable[], int* ctableCount) {
-    // Dummy (out-of-range) value to catch subclasses that forgot to respect ctable params.
-    int localCTableCount = -1;
-    int* localCTableCountPtr = NULL;
-
-    if (kIndex_8_SkColorType == info.colorType()) {
-        if (NULL == ctable || NULL == ctableCount) {
-            return false;
-        }
-    } else {
-        if (ctableCount) {
-            *ctableCount = 0;
-        }
-        ctableCount = NULL;
-        ctable = NULL;
-    }
-
-    if (kUnknown_SkColorType == info.colorType()) {
-        return false;
-    }
-    if (NULL == pixels) {
-        return false;
-    }
-    if (rowBytes < info.minRowBytes()) {
-        return false;
-    }
-
-    bool success = this->onGetPixels(info, pixels, rowBytes, ctable, localCTableCountPtr);
-
-    if (success && localCTableCountPtr) {
-        SkASSERT(localCTableCount >= 0 && localCTableCount <= 256);
-        if (ctableCount) {
-            *ctableCount = localCTableCount;
-        }
-    }
-    return success;
-}
-
-bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) {
-    SkASSERT(kIndex_8_SkColorType != info.colorType());
-    if (kIndex_8_SkColorType == info.colorType()) {
-        return false;
-    }
-    return this->getPixels(info, pixels, rowBytes, NULL, NULL);
-}
-#endif
-
-/////////////////////////////////////////////////////////////////////////////////////////////
-
-SkData* SkImageGenerator::onRefEncodedData() {
-    return NULL;
-}
-
-bool SkImageGenerator::onGetInfo(SkImageInfo*) {
-    return false;
-}
-
-bool SkImageGenerator::onGetPixels(const SkImageInfo&, void*, size_t, SkPMColor*, int*) {
-    return false;
-}
diff --git a/src/lazy/SkDiscardablePixelRef.cpp b/src/lazy/SkDiscardablePixelRef.cpp
index b0bbd27e3a..ccf812ce6f 100644
--- a/src/lazy/SkDiscardablePixelRef.cpp
+++ b/src/lazy/SkDiscardablePixelRef.cpp
@@ -60,30 +60,15 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
     }
 
     void* pixels = fDiscardableMemory->data();
-    const SkImageInfo& info = this->info();
-    SkPMColor colors[256];
-    int colorCount = 0;
-
-    if (!fGenerator->getPixels(info, pixels, fRowBytes, colors, &colorCount)) {
+    if (!fGenerator->getPixels(this->info(), pixels, fRowBytes)) {
         fDiscardableMemory->unlock();
         SkDELETE(fDiscardableMemory);
         fDiscardableMemory = NULL;
         return false;
     }
 
-    // Note: our ctable is not purgable, as it is not stored in the discardablememory block.
-    // This is because SkColorTable is refcntable, and therefore our caller could hold onto it
-    // beyond the scope of a lock/unlock. If we change the API/lifecycle for SkColorTable, we
-    // could move it into the block, but then again perhaps it is small enough that this doesn't
-    // really matter.
-    if (colorCount > 0) {
-        fCTable.reset(SkNEW_ARGS(SkColorTable, (colors, colorCount)));
-    } else {
-        fCTable.reset(NULL);
-    }
-
     rec->fPixels = pixels;
-    rec->fColorTable = fCTable.get();
+    rec->fColorTable = NULL;
     rec->fRowBytes = fRowBytes;
     return true;
 }
diff --git a/src/lazy/SkDiscardablePixelRef.h b/src/lazy/SkDiscardablePixelRef.h
index 52a1d6ce37..e5e1c9f0e1 100644
--- a/src/lazy/SkDiscardablePixelRef.h
+++ b/src/lazy/SkDiscardablePixelRef.h
@@ -17,6 +17,10 @@
  *  A PixelRef backed by SkDiscardableMemory, with the ability to
  *  re-generate the pixels (via a SkImageGenerator) if the DM is
  *  purged.
+ *
+ *  Since SkColorTable is reference-counted, we do not support indexed
+ *  color with this class; there would be no way for the discardable
+ *  memory system to unref the color table.
  */
 class SkDiscardablePixelRef : public SkPixelRef {
 public:
@@ -42,7 +46,6 @@ private:
     // PixelRef, since the SkBitmap doesn't expect them to change.
 
     SkDiscardableMemory* fDiscardableMemory;
-    SkAutoTUnref<SkColorTable> fCTable;
 
     /* Takes ownership of SkImageGenerator. */
     SkDiscardablePixelRef(const SkImageInfo&, SkImageGenerator*,
diff --git a/tests/CachedDecodingPixelRefTest.cpp b/tests/CachedDecodingPixelRefTest.cpp
index eff77e215d..10e33ba841 100644
--- a/tests/CachedDecodingPixelRefTest.cpp
+++ b/tests/CachedDecodingPixelRefTest.cpp
@@ -183,9 +183,7 @@ public:
         SkASSERT((fType <= kLast_TestType) && (fType >= 0));
     }
     virtual ~TestImageGenerator() { }
-
-protected:
-    virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE {
+    virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE {
         REPORTER_ASSERT(fReporter, NULL != info);
         if ((NULL == info) || (kFailGetInfo_TestType == fType)) {
             return false;
@@ -196,9 +194,9 @@ protected:
         info->fAlphaType = kOpaque_SkAlphaType;
         return true;
     }
-
-    virtual bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
-                             SkPMColor ctable[], int* ctableCount) SK_OVERRIDE {
+    virtual bool getPixels(const SkImageInfo& info,
+                           void* pixels,
+                           size_t rowBytes) SK_OVERRIDE {
         REPORTER_ASSERT(fReporter, pixels != NULL);
         size_t minRowBytes
             = static_cast<size_t>(info.fWidth * info.bytesPerPixel());
diff --git a/tests/DrawBitmapRectTest.cpp b/tests/DrawBitmapRectTest.cpp
index aa69de4bd4..f294ae7e6e 100644
--- a/tests/DrawBitmapRectTest.cpp
+++ b/tests/DrawBitmapRectTest.cpp
@@ -22,16 +22,20 @@ class FailureImageGenerator : public SkImageGenerator {
 public:
     FailureImageGenerator() { }
     virtual ~FailureImageGenerator() { }
-    
-protected:
-    virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE {
+    virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE {
         info->fWidth = 100;
         info->fHeight = 100;
         info->fColorType = kN32_SkColorType;
         info->fAlphaType = kPremul_SkAlphaType;
         return true;
     }
-    // default onGetPixels() returns false, which is what we want.
+    virtual bool getPixels(const SkImageInfo& info,
+                           void* pixels,
+                           size_t rowBytes) SK_OVERRIDE {
+        // this will deliberately return false if they are asking us
+        // to decode into pixels.
+        return false;
+    }
 };
 
 // crbug.com/295895