Handle zero-length encoded images gracefully during deserialization

src/core/SkPictureData.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #include <new>
 #include "SkPictureData.h"
 #include "SkPictureRecord.h"
 #include "SkReadBuffer.h"
+#include "SkSurface.h"
 #include "SkTextBlob.h"
 #include "SkTypeface.h"
 #include "SkWriteBuffer.h"
 
 #if SK_SUPPORT_GPU
 #include "GrContext.h"
 #endif
 
 template <typename T> int SafeCount(const T* obj) {
     return obj ? obj->count() : 0;
@@ skipping 405 matching lines @@
             }
             if (!buffer.isValid()) {
                 return false;
             }
             SkDEBUGCODE(haveBuffer = true;)
         } break;
     }
     return true;    // success
 }
 
+static const SkImage* make_checkerboard_image(int width, int height) {
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(width, height));
+    if (!surface) {
+        return nullptr;
+    }
+
+    SkCanvas* canvas = surface->getCanvas();
+    canvas->clear(SK_ColorWHITE);
+
+    SkPaint paint;
+    paint.setColor(SK_ColorGRAY);
+
+    static const int kSize = 12;
+    for (int y = 0; y < height; y += kSize * 2) {
+        for (int x = 0; x < width; x += kSize * 2) {
+            canvas->drawRect(SkRect::MakeXYWH(x, y, kSize, kSize), paint);
+            canvas->drawRect(SkRect::MakeXYWH(x + kSize, y + kSize, kSize, kSize), paint);
+        }
+    }
+
+    return surface->newImageSnapshot();
+}
+
 static const SkImage* create_image_from_buffer(SkReadBuffer& buffer) {
     int width = buffer.read32();
     int height = buffer.read32();
     if (width <= 0 || height <= 0) {    // SkImage never has a zero dimension
         buffer.validate(false);
         return nullptr;
     }
 
     SkAutoTUnref<SkData> encoded(buffer.readByteArrayAsData());
+    if (encoded->size() == 0) {
+        // The image could not be encoded at serialization time.
+        // Use a checkerboard instead.
+        return make_checkerboard_image(width, height);

[reed1, 2015/09/04 17:26:24]

This seems odd to me, partly because its arbitrary drawing, partly because it
can be arbitrarily big (ram), and partly because it hides the fact that there
was a failure. I was thinking we should just return NULL (and perhaps also
return width/height in case the caller wanted to know that).

[f(malita), 2015/09/04 17:50:41]

Agreed.
That's problematic because the caller (new_array_from_buffer) will immediately
fail if it sees a null value (and delete all previous images too).

In some sense, hiding the failure is exactly what we're trying to accomplish:
should loading such an SKP succeed or not?  If the answer is yes, then we're
effectively hiding the failure at some level, and the question becomes how to
best do that.

One option is to allocate a picture-backed SkImage which draws nothing.  It
saves the RAM and also preserves the SkImage metadata (dimensions).  WDYT?

+    }
+
     int originX = buffer.read32();
     int originY = buffer.read32();
-    if (0 == encoded->size() || originX < 0 || originY < 0) {
+    if (originX < 0 || originY < 0) {
         buffer.validate(false);
         return nullptr;
     }
 
     const SkIRect subset = SkIRect::MakeXYWH(originX, originY, width, height);
     return SkImage::NewFromEncoded(encoded, &subset);
 }
 
 // Need a shallow wrapper to return const SkPicture* to match the other factories,
 // as SkPicture::CreateFromBuffer() returns SkPicture*
@@ skipping 176 matching lines @@
     }
 }
 
 bool SkPictureData::suitableForLayerOptimization() const {
     return fContentInfo.numLayers() > 0;
 }
 #endif
 ///////////////////////////////////////////////////////////////////////////////