Support using ImageDecoder with ContentResolver + URI

Bug: 63909536
Test: CTS: I0f36ce34c968fd7fae4d8edebabea3a421859615

Add ImageDecoder.createSource(ContentResolver, URI), allowing a client
to decode images from files, content, resources, etc.

Prefer using a file descriptor to using an InputStream so the input can
be cheaply seeked and rewound if necessary.

Make ImageDecoder implement AutoCloseable to handle closing the input.

Make decodeDrawable/decodeBitmap always return an object or throw an
IOException. Avoid checking for a file in the Source constructor.

Fix a bug where inner Exception classes were not static.

Update JavaInputStreamAdaptor to be usable by ImageDecoder:
- previously it always swallowed exceptions. Allow them to propagate
  (optionally) so that they can be reported back to the client.
- Add refs to the InputStream and byte[]. ImageDecoder returns from
  native and then uses the JavaInputStreamAdaptor again, making the
  local refs go out of scope.
- Hold on to the JavaVM and convert to the JNIEnv when necessary. Pass
  local env pointers to avoid looking it up multiple times in one call.
- If an exception is thrown inside the doRead() loop, return the number
  of bytes successfully read.

Change-Id: I869dad55521cf942efd010c06baf3f44c1c08374

core/jni/android/graphics/CreateJavaOutputStreamAdaptor.cpp

@@ -11,21 +11,62 @@
 static jmethodID    gInputStream_readMethodID;
 static jmethodID    gInputStream_skipMethodID;
 
+// FIXME: Share with ByteBufferStreamAdaptor.cpp?
+static JNIEnv* get_env_or_die(JavaVM* jvm) {
+    JNIEnv* env;
+    if (jvm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK) {
+        LOG_ALWAYS_FATAL("Failed to get JNIEnv for JavaVM: %p", jvm);
+    }
+    return env;
+}
+
 /**
  *  Wrapper for a Java InputStream.
  */
 class JavaInputStreamAdaptor : public SkStream {
+    JavaInputStreamAdaptor(JavaVM* jvm, jobject js, jbyteArray ar, jint capacity,
+                           bool swallowExceptions)
+            : fJvm(jvm)
+            , fJavaInputStream(js)
+            , fJavaByteArray(ar)
+            , fCapacity(capacity)
+            , fBytesRead(0)
+            , fIsAtEnd(false)
+            , fSwallowExceptions(swallowExceptions) {}
+
 public:
-    JavaInputStreamAdaptor(JNIEnv* env, jobject js, jbyteArray ar)
-        : fEnv(env), fJavaInputStream(js), fJavaByteArray(ar) {
-        SkASSERT(ar);
-        fCapacity = env->GetArrayLength(ar);
-        SkASSERT(fCapacity > 0);
-        fBytesRead = 0;
-        fIsAtEnd = false;
+    static JavaInputStreamAdaptor* Create(JNIEnv* env, jobject js, jbyteArray ar,
+                                          bool swallowExceptions) {
+        JavaVM* jvm;
+        LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&jvm) != JNI_OK);
+
+        js = env->NewGlobalRef(js);
+        if (!js) {
+            return nullptr;
+        }
+
+        ar = (jbyteArray) env->NewGlobalRef(ar);
+        if (!ar) {
+            env->DeleteGlobalRef(js);
+            return nullptr;
+        }
+
+        jint capacity = env->GetArrayLength(ar);
+        return new JavaInputStreamAdaptor(jvm, js, ar, capacity, swallowExceptions);
     }
 
-    virtual size_t read(void* buffer, size_t size) {
+    ~JavaInputStreamAdaptor() override {
+        auto* env = get_env_or_die(fJvm);
+        env->DeleteGlobalRef(fJavaInputStream);
+        env->DeleteGlobalRef(fJavaByteArray);
+    }
+
+    size_t read(void* buffer, size_t size) override {
+        auto* env = get_env_or_die(fJvm);
+        if (!fSwallowExceptions && checkException(env)) {
+            // Just in case the caller did not clear from a previous exception.
+            return 0;
+        }
         if (NULL == buffer) {
             if (0 == size) {
                 return 0;
@@ -36,10 +77,10 @@ public:
                  */
                 size_t amountSkipped = 0;
                 do {
-                    size_t amount = this->doSkip(size - amountSkipped);
+                    size_t amount = this->doSkip(size - amountSkipped, env);
                     if (0 == amount) {
                         char tmp;
-                        amount = this->doRead(&tmp, 1);
+                        amount = this->doRead(&tmp, 1, env);
                         if (0 == amount) {
                             // if read returned 0, we're at EOF
                             fIsAtEnd = true;
@@ -51,16 +92,13 @@ public:
                 return amountSkipped;
             }
         }
-        return this->doRead(buffer, size);
+        return this->doRead(buffer, size, env);
     }
 
-    virtual bool isAtEnd() const {
-        return fIsAtEnd;
-    }
+    bool isAtEnd() const override { return fIsAtEnd; }
 
 private:
-    size_t doRead(void* buffer, size_t size) {
-        JNIEnv* env = fEnv;
+    size_t doRead(void* buffer, size_t size, JNIEnv* env) {
         size_t bytesRead = 0;
         // read the bytes
         do {
@@ -75,13 +113,9 @@ private:
 
             jint n = env->CallIntMethod(fJavaInputStream,
                                         gInputStream_readMethodID, fJavaByteArray, 0, requested);
-            if (env->ExceptionCheck()) {
-                env->ExceptionDescribe();
-                env->ExceptionClear();
+            if (checkException(env)) {
                 SkDebugf("---- read threw an exception\n");
-                // Consider the stream to be at the end, since there was an error.
-                fIsAtEnd = true;
-                return 0;
+                return bytesRead;
             }
 
             if (n < 0) { // n == 0 should not be possible, see InputStream read() specifications.
@@ -91,14 +125,9 @@ private:
 
             env->GetByteArrayRegion(fJavaByteArray, 0, n,
                                     reinterpret_cast<jbyte*>(buffer));
-            if (env->ExceptionCheck()) {
-                env->ExceptionDescribe();
-                env->ExceptionClear();
+            if (checkException(env)) {
                 SkDebugf("---- read:GetByteArrayRegion threw an exception\n");
-                // The error was not with the stream itself, but consider it to be at the
-                // end, since we do not have a way to recover.
-                fIsAtEnd = true;
-                return 0;
+                return bytesRead;
             }
 
             buffer = (void*)((char*)buffer + n);
@@ -110,14 +139,10 @@ private:
         return bytesRead;
     }
 
-    size_t doSkip(size_t size) {
-        JNIEnv* env = fEnv;
-
+    size_t doSkip(size_t size, JNIEnv* env) {
         jlong skipped = env->CallLongMethod(fJavaInputStream,
                                             gInputStream_skipMethodID, (jlong)size);
-        if (env->ExceptionCheck()) {
-            env->ExceptionDescribe();
-            env->ExceptionClear();
+        if (checkException(env)) {
             SkDebugf("------- skip threw an exception\n");
             return 0;
         }
@@ -128,20 +153,37 @@ private:
         return (size_t)skipped;
     }
 
-    JNIEnv*     fEnv;
-    jobject     fJavaInputStream;   // the caller owns this object
-    jbyteArray  fJavaByteArray;     // the caller owns this object
-    jint        fCapacity;
+    bool checkException(JNIEnv* env) {
+        if (!env->ExceptionCheck()) {
+            return false;
+        }
+
+        env->ExceptionDescribe();
+        if (fSwallowExceptions) {
+            env->ExceptionClear();
+        }
+
+        // There is no way to recover from the error, so consider the stream
+        // to be at the end.
+        fIsAtEnd = true;
+
+        return true;
+    }
+
+    JavaVM*     fJvm;
+    jobject     fJavaInputStream;
+    jbyteArray  fJavaByteArray;
+    const jint  fCapacity;
     size_t      fBytesRead;
     bool        fIsAtEnd;
+    const bool  fSwallowExceptions;
 };
 
-SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream,
-                                       jbyteArray storage) {
-    return new JavaInputStreamAdaptor(env, stream, storage);
+SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream, jbyteArray storage,
+                                       bool swallowExceptions) {
+    return JavaInputStreamAdaptor::Create(env, stream, storage, swallowExceptions);
 }
 
-
 static SkMemoryStream* adaptor_to_mem_stream(SkStream* stream) {
     SkASSERT(stream != NULL);
     size_t bufferSize = 4096;

core/jni/android/graphics/CreateJavaOutputStreamAdaptor.h

@@ -16,13 +16,16 @@ class SkWStream;
  *  @param stream Pointer to Java InputStream.
  *  @param storage Java byte array for retrieving data from the
  *      Java InputStream.
+ *  @param swallowExceptions Whether to call ExceptionClear() after
+ *      an Exception is thrown. If false, it is up to the client to
+ *      clear or propagate the exception.
  *  @return SkStream Simple subclass of SkStream which supports its
  *      basic methods like reading. Only valid until the calling
  *      function returns, since the Java InputStream is not managed
  *      by the SkStream.
  */
-SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream,
-                                       jbyteArray storage);
+SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream, jbyteArray storage,
+                                       bool swallowExceptions = true);
 
 /**
  *  Copy a Java InputStream. The result will be rewindable.
@@ -33,10 +36,8 @@ SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream,
  *  @return SkStreamRewindable The data in stream will be copied
  *      to a new SkStreamRewindable.
  */
-SkStreamRewindable* CopyJavaInputStream(JNIEnv* env, jobject stream,
-                                        jbyteArray storage);
+SkStreamRewindable* CopyJavaInputStream(JNIEnv* env, jobject stream, jbyteArray storage);
 
-SkWStream* CreateJavaOutputStreamAdaptor(JNIEnv* env, jobject stream,
-                                         jbyteArray storage);
+SkWStream* CreateJavaOutputStreamAdaptor(JNIEnv* env, jobject stream, jbyteArray storage);
 
 #endif  // _ANDROID_GRAPHICS_CREATE_JAVA_OUTPUT_STREAM_ADAPTOR_H_

core/jni/android/graphics/ImageDecoder.cpp

@@ -16,6 +16,7 @@
 
 #include "Bitmap.h"
 #include "ByteBufferStreamAdaptor.h"
+#include "CreateJavaOutputStreamAdaptor.h"
 #include "GraphicsJNI.h"
 #include "NinePatchPeeker.h"
 #include "Utils.h"
@@ -26,10 +27,12 @@
 
 #include <SkAndroidCodec.h>
 #include <SkEncodedImageFormat.h>
+#include <SkFrontBufferedStream.h>
 #include <SkStream.h>
 
 #include <androidfw/Asset.h>
 #include <jni.h>
+#include <sys/stat.h>
 
 using namespace android;
 
@@ -69,15 +72,15 @@ struct ImageDecoder {
 
 static jobject native_create(JNIEnv* env, std::unique_ptr<SkStream> stream) {
     if (!stream.get()) {
-        return nullObjectReturn("Failed to create a stream");
+        doThrowIOE(env, "Failed to create a stream");
+        return nullptr;
     }
     std::unique_ptr<ImageDecoder> decoder(new ImageDecoder);
     decoder->mCodec = SkAndroidCodec::MakeFromStream(std::move(stream), &decoder->mPeeker);
     if (!decoder->mCodec.get()) {
-        // FIXME: Add an error code to SkAndroidCodec::MakeFromStream, like
-        // SkCodec? Then this can print a more informative error message.
-        // (Or we can print one from within SkCodec.)
-        ALOGE("Failed to create an SkCodec");
+        // FIXME: (b/71578461) Use the error message from
+        // SkCodec::MakeFromStream to report a more informative error message.
+        doThrowIOE(env, "Failed to create an SkCodec");
         return nullptr;
     }
 
@@ -88,7 +91,52 @@ static jobject native_create(JNIEnv* env, std::unique_ptr<SkStream> stream) {
                           reinterpret_cast<jlong>(decoder.release()), width, height);
 }
 
-static jobject ImageDecoder_nCreate(JNIEnv* env, jobject /*clazz*/, jlong assetPtr) {
+static jobject ImageDecoder_nCreateFd(JNIEnv* env, jobject /*clazz*/,
+        jobject fileDescriptor) {
+    int descriptor = jniGetFDFromFileDescriptor(env, fileDescriptor);
+
+    struct stat fdStat;
+    if (fstat(descriptor, &fdStat) == -1) {
+        doThrowIOE(env, "broken file descriptor; fstat returned -1");
+        return nullptr;
+    }
+
+    int dupDescriptor = dup(descriptor);
+    FILE* file = fdopen(dupDescriptor, "r");
+    if (file == NULL) {
+        close(dupDescriptor);
+        doThrowIOE(env, "Could not open file");
+        return nullptr;
+    }
+    std::unique_ptr<SkFILEStream> fileStream(new SkFILEStream(file));
+
+    if (::lseek(descriptor, 0, SEEK_CUR) == 0) {
+        return native_create(env, std::move(fileStream));
+    }
+
+    // FIXME: This allows us to pretend the current location is the beginning,
+    // but it would be better if SkFILEStream allowed treating its starting
+    // point as the beginning.
+    std::unique_ptr<SkStream> stream(SkFrontBufferedStream::Make(std::move(fileStream),
+                SkCodec::MinBufferedBytesNeeded()));
+    return native_create(env, std::move(stream));
+}
+
+static jobject ImageDecoder_nCreateInputStream(JNIEnv* env, jobject /*clazz*/,
+        jobject is, jbyteArray storage) {
+    std::unique_ptr<SkStream> stream(CreateJavaInputStreamAdaptor(env, is, storage, false));
+
+    if (!stream.get()) {
+        doThrowIOE(env, "Failed to create stream!");
+        return nullptr;
+    }
+    std::unique_ptr<SkStream> bufferedStream(
+        SkFrontBufferedStream::Make(std::move(stream),
+        SkCodec::MinBufferedBytesNeeded()));
+    return native_create(env, std::move(bufferedStream));
+}
+
+static jobject ImageDecoder_nCreateAsset(JNIEnv* env, jobject /*clazz*/, jlong assetPtr) {
     Asset* asset = reinterpret_cast<Asset*>(assetPtr);
     std::unique_ptr<SkStream> stream(new AssetStreamAdaptor(asset));
     return native_create(env, std::move(stream));
@@ -99,6 +147,7 @@ static jobject ImageDecoder_nCreateByteBuffer(JNIEnv* env, jobject /*clazz*/, jo
     std::unique_ptr<SkStream> stream = CreateByteBufferStreamAdaptor(env, jbyteBuffer,
                                                                      initialPosition, limit);
     if (!stream) {
+        doThrowIOE(env, "Failed to read ByteBuffer");
         return nullptr;
     }
     return native_create(env, std::move(stream));
@@ -114,6 +163,7 @@ static bool supports_any_down_scale(const SkAndroidCodec* codec) {
     return codec->getEncodedFormat() == SkEncodedImageFormat::kWEBP;
 }
 
+// This method should never return null. Instead, it should throw an exception.
 static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong nativePtr,
                                           jobject jcallback, jobject jpostProcess,
                                           jint desiredWidth, jint desiredHeight, jobject jsubset,
@@ -165,7 +215,8 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
         case kOpaque_SkAlphaType:
             break;
         case kUnknown_SkAlphaType:
-            return nullObjectReturn("Unknown alpha type");
+            doThrowIOE(env, "Unknown alpha type");
+            return nullptr;
     }
 
     SkColorType colorType = kN32_SkColorType;
@@ -200,7 +251,8 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
         bitmapInfo = bitmapInfo.makeColorType(kAlpha_8_SkColorType);
     }
     if (!bm.setInfo(bitmapInfo)) {
-        return nullObjectReturn("Failed to setInfo properly");
+        doThrowIOE(env, "Failed to setInfo properly");
+        return nullptr;
     }
 
     sk_sp<Bitmap> nativeBitmap;
@@ -213,35 +265,44 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
         nativeBitmap = Bitmap::allocateHeapBitmap(&bm);
     }
     if (!nativeBitmap) {
-        ALOGE("OOM allocating Bitmap with dimensions %i x %i",
-              decodeInfo.width(), decodeInfo.height());
-        doThrowOOME(env);
+        SkString msg;
+        msg.printf("OOM allocating Bitmap with dimensions %i x %i",
+                decodeInfo.width(), decodeInfo.height());
+        doThrowOOME(env, msg.c_str());
         return nullptr;
     }
 
-    jobject jexception = nullptr;
     SkAndroidCodec::AndroidOptions options;
     options.fSampleSize = sampleSize;
     auto result = codec->getAndroidPixels(decodeInfo, bm.getPixels(), bm.rowBytes(), &options);
+    jobject jexception = env->ExceptionOccurred();
+    if (jexception) {
+        env->ExceptionClear();
+    }
     switch (result) {
         case SkCodec::kSuccess:
+            // Ignore the exception, since the decode was successful anyway.
+            jexception = nullptr;
             break;
         case SkCodec::kIncompleteInput:
-            if (jcallback) {
+            if (jcallback && !jexception) {
                 jexception = env->NewObject(gIncomplete_class, gIncomplete_constructorMethodID);
             }
             break;
         case SkCodec::kErrorInInput:
-            if (jcallback) {
+            if (jcallback && !jexception) {
                 jexception = env->NewObject(gCorrupt_class, gCorrupt_constructorMethodID);
             }
             break;
         default:
-            ALOGE("getPixels failed with error %i", result);
+            SkString msg;
+            msg.printf("getPixels failed with error %i", result);
+            doThrowIOE(env, msg.c_str());
             return nullptr;
     }
 
     if (jexception) {
+        // FIXME: Do not provide a way for the client to force the method to return null.
         if (!env->CallBooleanMethod(jcallback, gCallback_onExceptionMethodID, jexception) ||
             env->ExceptionCheck()) {
             return nullptr;
@@ -268,7 +329,8 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
             size_t ninePatchArraySize = decoder->mPeeker.mPatch->serializedSize();
             ninePatchChunk = env->NewByteArray(ninePatchArraySize);
             if (ninePatchChunk == nullptr) {
-                return nullObjectReturn("ninePatchChunk == null");
+                doThrowOOME(env, "Failed to allocate nine patch chunk.");
+                return nullptr;
             }
 
             env->SetByteArrayRegion(ninePatchChunk, 0, decoder->mPeeker.mPatchSize,
@@ -278,7 +340,8 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
         if (decoder->mPeeker.mHasInsets) {
             ninePatchInsets = decoder->mPeeker.createNinePatchInsets(env, 1.0f);
             if (ninePatchInsets == nullptr) {
-                return nullObjectReturn("nine patch insets == null");
+                doThrowOOME(env, "Failed to allocate nine patch insets.");
+                return nullptr;
             }
         }
     }
@@ -303,7 +366,8 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
         SkImageInfo scaledInfo = bitmapInfo.makeWH(desiredWidth, desiredHeight);
         SkBitmap scaledBm;
         if (!scaledBm.setInfo(scaledInfo)) {
-            nullObjectReturn("Failed scaled setInfo");
+            doThrowIOE(env, "Failed scaled setInfo");
+            return nullptr;
         }
 
         sk_sp<Bitmap> scaledPixelRef;
@@ -313,9 +377,10 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
             scaledPixelRef = Bitmap::allocateHeapBitmap(&scaledBm);
         }
         if (!scaledPixelRef) {
-            ALOGE("OOM allocating scaled Bitmap with dimensions %i x %i",
-                  desiredWidth, desiredHeight);
-            doThrowOOME(env);
+            SkString msg;
+            msg.printf("OOM allocating scaled Bitmap with dimensions %i x %i",
+                    desiredWidth, desiredHeight);
+            doThrowOOME(env, msg.c_str());
             return nullptr;
         }
 
@@ -334,13 +399,11 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
 
     if (jpostProcess) {
         std::unique_ptr<Canvas> canvas(Canvas::create_canvas(bm));
-        if (!canvas) {
-            return nullObjectReturn("Failed to create Canvas for PostProcess!");
-        }
         jobject jcanvas = env->NewObject(gCanvas_class, gCanvas_constructorMethodID,
                                          reinterpret_cast<jlong>(canvas.get()));
         if (!jcanvas) {
-            return nullObjectReturn("Failed to create Java Canvas for PostProcess!");
+            doThrowOOME(env, "Failed to create Java Canvas for PostProcess!");
+            return nullptr;
         }
         // jcanvas will now own canvas.
         canvas.release();
@@ -368,15 +431,17 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
                 newAlphaType = kOpaque_SkAlphaType;
                 break;
             default:
-                ALOGE("invalid return from postProcess: %i", pixelFormat);
-                doThrowIAE(env);
+                SkString msg;
+                msg.printf("invalid return from postProcess: %i", pixelFormat);
+                doThrowIAE(env, msg.c_str());
                 return nullptr;
         }
 
         if (newAlphaType != bm.alphaType()) {
             if (!bm.setAlphaType(newAlphaType)) {
-                ALOGE("incompatible return from postProcess: %i", pixelFormat);
-                doThrowIAE(env);
+                SkString msg;
+                msg.printf("incompatible return from postProcess: %i", pixelFormat);
+                doThrowIAE(env, msg.c_str());
                 return nullptr;
             }
             nativeBitmap->setAlphaType(newAlphaType);
@@ -405,7 +470,8 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong
                                             ninePatchChunk, ninePatchInsets);
             }
             if (allocator == ImageDecoder::kHardware_Allocator) {
-                return nullObjectReturn("failed to allocate hardware Bitmap!");
+                doThrowOOME(env, "failed to allocate hardware Bitmap!");
+                return nullptr;
             }
             // If we failed to create a hardware bitmap, go ahead and create a
             // software one.
@@ -430,19 +496,21 @@ static void ImageDecoder_nGetPadding(JNIEnv* env, jobject /*clazz*/, jlong nativ
     decoder->mPeeker.getPadding(env, outPadding);
 }
 
-static void ImageDecoder_nRecycle(JNIEnv* /*env*/, jobject /*clazz*/, jlong nativePtr) {
+static void ImageDecoder_nClose(JNIEnv* /*env*/, jobject /*clazz*/, jlong nativePtr) {
     delete reinterpret_cast<ImageDecoder*>(nativePtr);
 }
 
 static const JNINativeMethod gImageDecoderMethods[] = {
-    { "nCreate",        "(J)Landroid/graphics/ImageDecoder;",    (void*) ImageDecoder_nCreate },
+    { "nCreate",        "(J)Landroid/graphics/ImageDecoder;",    (void*) ImageDecoder_nCreateAsset },
     { "nCreate",        "(Ljava/nio/ByteBuffer;II)Landroid/graphics/ImageDecoder;", (void*) ImageDecoder_nCreateByteBuffer },
     { "nCreate",        "([BII)Landroid/graphics/ImageDecoder;", (void*) ImageDecoder_nCreateByteArray },
+    { "nCreate",        "(Ljava/io/InputStream;[B)Landroid/graphics/ImageDecoder;", (void*) ImageDecoder_nCreateInputStream },
+    { "nCreate",        "(Ljava/io/FileDescriptor;)Landroid/graphics/ImageDecoder;", (void*) ImageDecoder_nCreateFd },
     { "nDecodeBitmap",  "(JLandroid/graphics/ImageDecoder$OnExceptionListener;Landroid/graphics/PostProcess;IILandroid/graphics/Rect;ZIZZZ)Landroid/graphics/Bitmap;",
                                                                  (void*) ImageDecoder_nDecodeBitmap },
     { "nGetSampledSize","(JI)Landroid/graphics/Point;",          (void*) ImageDecoder_nGetSampledSize },
     { "nGetPadding",    "(JLandroid/graphics/Rect;)V",           (void*) ImageDecoder_nGetPadding },
-    { "nRecycle",       "(J)V",                                  (void*) ImageDecoder_nRecycle},
+    { "nClose",         "(J)V",                                  (void*) ImageDecoder_nClose},
 };
 
 int register_android_graphics_ImageDecoder(JNIEnv* env) {
@@ -459,7 +527,7 @@ int register_android_graphics_ImageDecoder(JNIEnv* env) {
     gCorrupt_constructorMethodID = GetMethodIDOrDie(env, gCorrupt_class, "<init>", "()V");
 
     jclass callback_class = FindClassOrDie(env, "android/graphics/ImageDecoder$OnExceptionListener");
-    gCallback_onExceptionMethodID = GetMethodIDOrDie(env, callback_class, "onException", "(Ljava/lang/Exception;)Z");
+    gCallback_onExceptionMethodID = GetMethodIDOrDie(env, callback_class, "onException", "(Ljava/io/IOException;)Z");
 
     jclass postProcess_class = FindClassOrDie(env, "android/graphics/PostProcess");
     gPostProcess_postProcessMethodID = GetMethodIDOrDie(env, postProcess_class, "postProcess", "(Landroid/graphics/Canvas;II)I");

graphics/java/android/graphics/ImageDecoder.java

@@ -16,29 +16,44 @@
 
 package android.graphics;
 
+import static android.system.OsConstants.SEEK_SET;
+
 import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.annotation.RawRes;
+import android.content.ContentResolver;
+import android.content.res.AssetFileDescriptor;
 import android.content.res.AssetManager;
 import android.content.res.Resources;
 import android.graphics.drawable.Drawable;
 import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.NinePatchDrawable;
+import android.net.Uri;
+import android.system.ErrnoException;
+import android.system.Os;
+
+import libcore.io.IoUtils;
+import dalvik.system.CloseGuard;
 
 import java.nio.ByteBuffer;
+import java.io.FileDescriptor;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.ArrayIndexOutOfBoundsException;
+import java.lang.AutoCloseable;
 import java.lang.NullPointerException;
 import java.lang.RuntimeException;
 import java.lang.annotation.Retention;
 import static java.lang.annotation.RetentionPolicy.SOURCE;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  *  Class for decoding images as {@link Bitmap}s or {@link Drawable}s.
  *  @hide
  */
-public final class ImageDecoder {
+public final class ImageDecoder implements AutoCloseable {
     /**
      *  Source of the encoded image data.
      */
@@ -47,10 +62,7 @@ public final class ImageDecoder {
         Resources getResources() { return null; }
 
         /* @hide */
-        void close() {}
-
-        /* @hide */
-        abstract ImageDecoder createImageDecoder();
+        abstract ImageDecoder createImageDecoder() throws IOException;
     };
 
     private static class ByteArraySource extends Source {
@@ -64,7 +76,7 @@ public final class ImageDecoder {
         private final int    mLength;
 
         @Override
-        public ImageDecoder createImageDecoder() {
+        public ImageDecoder createImageDecoder() throws IOException {
             return nCreate(mData, mOffset, mLength);
         }
     }
@@ -76,7 +88,7 @@ public final class ImageDecoder {
         private final ByteBuffer mBuffer;
 
         @Override
-        public ImageDecoder createImageDecoder() {
+        public ImageDecoder createImageDecoder() throws IOException {
             if (!mBuffer.isDirect() && mBuffer.hasArray()) {
                 int offset = mBuffer.arrayOffset() + mBuffer.position();
                 int length = mBuffer.limit() - mBuffer.position();
@@ -86,61 +98,110 @@ public final class ImageDecoder {
         }
     }
 
-    private static class ResourceSource extends Source {
-        ResourceSource(Resources res, int resId)
-                throws Resources.NotFoundException {
-            // Test that the resource can be found.
-            InputStream is = null;
+    private static class ContentResolverSource extends Source {
+        ContentResolverSource(ContentResolver resolver, Uri uri) {
+            mResolver = resolver;
+            mUri = uri;
+        }
+
+        private final ContentResolver mResolver;
+        private final Uri mUri;
+
+        @Override
+        public ImageDecoder createImageDecoder() throws IOException {
+            AssetFileDescriptor assetFd = null;
             try {
-                is = res.openRawResource(resId);
-            } finally {
-                if (is != null) {
-                    try {
-                        is.close();
-                    } catch (IOException e) {
-                    }
+                if (mUri.getScheme() == ContentResolver.SCHEME_CONTENT) {
+                    assetFd = mResolver.openTypedAssetFileDescriptor(mUri,
+                            "image/*", null);
+                } else {
+                    assetFd = mResolver.openAssetFileDescriptor(mUri, "r");
                 }
+            } catch (FileNotFoundException e) {
+                // Some images cannot be opened as AssetFileDescriptors (e.g.
+                // bmp, ico). Open them as InputStreams.
+                InputStream is = mResolver.openInputStream(mUri);
+                if (is == null) {
+                    throw new FileNotFoundException(mUri.toString());
+                }
+
+                return createFromStream(is);
             }
 
+            final FileDescriptor fd = assetFd.getFileDescriptor();
+            final long offset = assetFd.getStartOffset();
+
+            ImageDecoder decoder = null;
+            try {
+                try {
+                    Os.lseek(fd, offset, SEEK_SET);
+                    decoder = nCreate(fd);
+                } catch (ErrnoException e) {
+                    decoder = createFromStream(new FileInputStream(fd));
+                }
+            } finally {
+                if (decoder == null) {
+                    IoUtils.closeQuietly(assetFd);
+                } else {
+                    decoder.mAssetFd = assetFd;
+                }
+            }
+            return decoder;
+        }
+    }
+
+    private static ImageDecoder createFromStream(InputStream is) throws IOException {
+        // Arbitrary size matches BitmapFactory.
+        byte[] storage = new byte[16 * 1024];
+        ImageDecoder decoder = null;
+        try {
+            decoder = nCreate(is, storage);
+        } finally {
+            if (decoder == null) {
+                IoUtils.closeQuietly(is);
+            } else {
+                decoder.mInputStream = is;
+                decoder.mTempStorage = storage;
+            }
+        }
+
+        return decoder;
+    }
+
+    private static class ResourceSource extends Source {
+        ResourceSource(Resources res, int resId) {
             mResources = res;
             mResId = resId;
         }
 
         final Resources mResources;
         final int       mResId;
-        // This is just stored here in order to keep the underlying Asset
-        // alive. FIXME: Can I access the Asset (and keep it alive) without
-        // this object?
-        InputStream mInputStream;
 
         @Override
         public Resources getResources() { return mResources; }
 
         @Override
-        public ImageDecoder createImageDecoder() {
-            // FIXME: Can I bypass creating the stream?
+        public ImageDecoder createImageDecoder() throws IOException {
+            // This is just used in order to access the underlying Asset and
+            // keep it alive. FIXME: Can we skip creating this object?
+            InputStream is = null;
+            ImageDecoder decoder = null;
             try {
-                mInputStream = mResources.openRawResource(mResId);
-            } catch (Resources.NotFoundException e) {
-                // This should never happen, since we already tested in the
-                // constructor.
-            }
-            if (!(mInputStream instanceof AssetManager.AssetInputStream)) {
-                // This should never happen.
-                throw new RuntimeException("Resource is not an asset?");
-            }
-            long asset = ((AssetManager.AssetInputStream) mInputStream).getNativeAsset();
-            return nCreate(asset);
-        }
-
-        @Override
-        public void close() {
-            try {
-                mInputStream.close();
-            } catch (IOException e) {
+                is = mResources.openRawResource(mResId);
+                if (!(is instanceof AssetManager.AssetInputStream)) {
+                    // This should never happen.
+                    throw new RuntimeException("Resource is not an asset?");
+                }
+                long asset = ((AssetManager.AssetInputStream) is).getNativeAsset();
+                decoder = nCreate(asset);
             } finally {
-                mInputStream = null;
+                if (decoder == null) {
+                    IoUtils.closeQuietly(is);
+                } else {
+                    decoder.mInputStream = is;
+                }
             }
+            return decoder;
         }
     }
 
@@ -159,18 +220,23 @@ public final class ImageDecoder {
     };
 
     /**
-     *  Used if the provided data is incomplete.
+     *  Supplied to onException if the provided data is incomplete.
+     *
+     *  Will never be thrown by ImageDecoder.
      *
      *  There may be a partial image to display.
      */
-    public class IncompleteException extends Exception {};
+    public static class IncompleteException extends IOException {};
 
     /**
      *  Used if the provided data is corrupt.
      *
-     *  There may be a partial image to display.
+     *  May be thrown if there is nothing to display.
+     *
+     *  If supplied to onException, there may be a correct partial image to
+     *  display.
      */
-    public class CorruptException extends Exception {};
+    public static class CorruptException extends IOException {};
 
     /**
      *  Optional listener supplied to {@link #decodeDrawable} or
@@ -193,15 +259,14 @@ public final class ImageDecoder {
     public static interface OnExceptionListener {
         /**
          *  Called when there is a problem in the stream or in the data.
-         *  FIXME: Or do not allow streams?
          *  FIXME: Report how much of the image has been decoded?
          *
-         *  @param e Exception containing information about the error.
+         *  @param e IOException containing information about the error.
          *  @return True to create and return a {@link Drawable}/
          *      {@link Bitmap} with partial data. False to return
          *      {@code null}. True is the default.
          */
-        public boolean onException(Exception e);
+        public boolean onException(IOException e);
     };
 
     // Fields
@@ -221,9 +286,15 @@ public final class ImageDecoder {
     private PostProcess         mPostProcess;
     private OnExceptionListener mOnExceptionListener;
 
+    // Objects for interacting with the input.
+    private InputStream         mInputStream;
+    private byte[]              mTempStorage;
+    private AssetFileDescriptor mAssetFd;
+    private final AtomicBoolean mClosed = new AtomicBoolean();
+    private final CloseGuard    mCloseGuard = CloseGuard.get();
 
     /**
-     * Private constructor called by JNI. {@link #recycle} must be
+     * Private constructor called by JNI. {@link #close} must be
      * called after decoding to delete native resources.
      */
     @SuppressWarnings("unused")
@@ -233,6 +304,20 @@ public final class ImageDecoder {
         mHeight = height;
         mDesiredWidth = width;
         mDesiredHeight = height;
+        mCloseGuard.open("close");
+    }
+
+    @Override
+    protected void finalize() throws Throwable {
+        try {
+            if (mCloseGuard != null) {
+                mCloseGuard.warnIfOpen();
+            }
+
+            close();
+        } finally {
+            super.finalize();
+        }
     }
 
     /**
@@ -243,13 +328,27 @@ public final class ImageDecoder {
      *      // FIXME: Can be an @DrawableRes?
      * @return a new Source object, which can be passed to
      *      {@link #decodeDrawable} or {@link #decodeBitmap}.
-     * @throws Resources.NotFoundException if the asset does not exist.
      */
+    @NonNull
     public static Source createSource(@NonNull Resources res, @RawRes int resId)
-            throws Resources.NotFoundException {
+    {
         return new ResourceSource(res, resId);
     }
 
+    /**
+     * Create a new {@link Source} from a {@link android.net.Uri}.
+     *
+     * @param cr to retrieve from.
+     * @param uri of the image file.
+     * @return a new Source object, which can be passed to
+     *      {@link #decodeDrawable} or {@link #decodeBitmap}.
+     */
+    @NonNull
+    public static Source createSource(@NonNull ContentResolver cr,
+            @NonNull Uri uri) {
+        return new ContentResolverSource(cr, uri);
+    }
+
     /**
      * Create a new {@link Source} from a byte array.
      * @param data byte array of compressed image data.
@@ -307,7 +406,7 @@ public final class ImageDecoder {
                     + "provided " + sampleSize);
         }
         if (mNativePtr == 0) {
-            throw new IllegalStateException("ImageDecoder is recycled!");
+            throw new IllegalStateException("ImageDecoder is closed!");
         }
 
         return nGetSampledSize(mNativePtr, sampleSize);
@@ -500,24 +599,26 @@ public final class ImageDecoder {
         mAsAlphaMask = true;
     }
 
-    /**
-     *  Clean up resources.
-     *
-     *  ImageDecoder has a private constructor, and will always be recycled
-     *  by decodeDrawable or decodeBitmap which creates it, so there is no
-     *  need for a finalizer.
-     */
-    private void recycle() {
-        if (mNativePtr == 0) {
+    @Override
+    public void close() {
+        mCloseGuard.close();
+        if (!mClosed.compareAndSet(false, true)) {
             return;
         }
-        nRecycle(mNativePtr);
+        nClose(mNativePtr);
         mNativePtr = 0;
+
+        IoUtils.closeQuietly(mInputStream);
+        IoUtils.closeQuietly(mAssetFd);
+
+        mInputStream = null;
+        mAssetFd = null;
+        mTempStorage = null;
     }
 
     private void checkState() {
         if (mNativePtr == 0) {
-            throw new IllegalStateException("Cannot reuse ImageDecoder.Source!");
+            throw new IllegalStateException("Cannot use closed ImageDecoder!");
         }
 
         checkSubset(mDesiredWidth, mDesiredHeight, mCropRect);
@@ -548,44 +649,47 @@ public final class ImageDecoder {
 
     /**
      *  Create a {@link Drawable}.
+     *  @throws IOException if {@code src} is not found, is an unsupported
+     *      format, or cannot be decoded for any reason.
      */
-    public static Drawable decodeDrawable(Source src, OnHeaderDecodedListener listener) {
-        ImageDecoder decoder = src.createImageDecoder();
-        if (decoder == null) {
-            return null;
-        }
+    @NonNull
+    public static Drawable decodeDrawable(Source src, OnHeaderDecodedListener listener)
+            throws IOException {
+        try (ImageDecoder decoder = src.createImageDecoder()) {
+            if (listener != null) {
+                ImageInfo info = new ImageInfo(decoder.mWidth, decoder.mHeight);
+                listener.onHeaderDecoded(info, decoder);
+            }
 
-        if (listener != null) {
-            ImageInfo info = new ImageInfo(decoder.mWidth, decoder.mHeight);
-            listener.onHeaderDecoded(info, decoder);
-        }
+            decoder.checkState();
 
-        decoder.checkState();
+            if (decoder.mRequireUnpremultiplied) {
+                // Though this could be supported (ignored) for opaque images,
+                // it seems better to always report this error.
+                throw new IllegalStateException("Cannot decode a Drawable " +
+                                                "with unpremultiplied pixels!");
+            }
 
-        if (decoder.mRequireUnpremultiplied) {
-            // Though this could be supported (ignored) for opaque images, it
-            // seems better to always report this error.
-            throw new IllegalStateException("Cannot decode a Drawable with" +
-                                            " unpremultiplied pixels!");
-        }
+            if (decoder.mMutable) {
+                throw new IllegalStateException("Cannot decode a mutable " +
+                                                "Drawable!");
+            }
 
-        if (decoder.mMutable) {
-            throw new IllegalStateException("Cannot decode a mutable Drawable!");
-        }
-
-        try {
             Bitmap bm = nDecodeBitmap(decoder.mNativePtr,
                                       decoder.mOnExceptionListener,
                                       decoder.mPostProcess,
-                                      decoder.mDesiredWidth, decoder.mDesiredHeight,
+                                      decoder.mDesiredWidth,
+                                      decoder.mDesiredHeight,
                                       decoder.mCropRect,
-                                      false,    // decoder.mMutable
+                                      false,    // mMutable
                                       decoder.mAllocator,
-                                      false,    // decoder.mRequireUnpremultiplied
+                                      false,    // mRequireUnpremultiplied
                                       decoder.mPreferRamOverQuality,
-                                      decoder.mAsAlphaMask
-                                      );
+                                      decoder.mAsAlphaMask);
             if (bm == null) {
+                // FIXME: bm should never be null. Currently a return value
+                // of false from onException will result in bm being null. What
+                // is the right API to choose to discard partial Bitmaps?
                 return null;
             }
 
@@ -606,60 +710,58 @@ public final class ImageDecoder {
 
             // TODO: Handle animation.
             return new BitmapDrawable(res, bm);
-        } finally {
-            decoder.recycle();
-            src.close();
         }
     }
 
     /**
-     * Create a {@link Bitmap}.
+     *  Create a {@link Bitmap}.
+     *  @throws IOException if {@code src} is not found, is an unsupported
+     *      format, or cannot be decoded for any reason.
      */
-    public static Bitmap decodeBitmap(Source src, OnHeaderDecodedListener listener) {
-        ImageDecoder decoder = src.createImageDecoder();
-        if (decoder == null) {
-            return null;
-        }
+    @NonNull
+    public static Bitmap decodeBitmap(Source src, OnHeaderDecodedListener listener)
+            throws IOException {
+        try (ImageDecoder decoder = src.createImageDecoder()) {
+            if (listener != null) {
+                ImageInfo info = new ImageInfo(decoder.mWidth, decoder.mHeight);
+                listener.onHeaderDecoded(info, decoder);
+            }
 
-        if (listener != null) {
-            ImageInfo info = new ImageInfo(decoder.mWidth, decoder.mHeight);
-            listener.onHeaderDecoded(info, decoder);
-        }
+            decoder.checkState();
 
-        decoder.checkState();
-
-        try {
             return nDecodeBitmap(decoder.mNativePtr,
                                  decoder.mOnExceptionListener,
                                  decoder.mPostProcess,
-                                 decoder.mDesiredWidth, decoder.mDesiredHeight,
+                                 decoder.mDesiredWidth,
+                                 decoder.mDesiredHeight,
                                  decoder.mCropRect,
                                  decoder.mMutable,
                                  decoder.mAllocator,
                                  decoder.mRequireUnpremultiplied,
                                  decoder.mPreferRamOverQuality,
                                  decoder.mAsAlphaMask);
-        } finally {
-            decoder.recycle();
-            src.close();
         }
     }
 
-    private static native ImageDecoder nCreate(long asset);
+    private static native ImageDecoder nCreate(long asset) throws IOException;
     private static native ImageDecoder nCreate(ByteBuffer buffer,
                                                int position,
-                                               int limit);
+                                               int limit) throws IOException;
     private static native ImageDecoder nCreate(byte[] data, int offset,
-                                               int length);
+                                               int length) throws IOException;
+    private static native ImageDecoder nCreate(InputStream is, byte[] storage);
+    private static native ImageDecoder nCreate(FileDescriptor fd) throws IOException;
+    @NonNull
     private static native Bitmap nDecodeBitmap(long nativePtr,
             OnExceptionListener listener,
             PostProcess postProcess,
             int width, int height,
             Rect cropRect, boolean mutable,
             int allocator, boolean requireUnpremul,
-            boolean preferRamOverQuality, boolean asAlphaMask);
+            boolean preferRamOverQuality, boolean asAlphaMask)
+        throws IOException;
     private static native Point nGetSampledSize(long nativePtr,
                                                 int sampleSize);
     private static native void nGetPadding(long nativePtr, Rect outRect);
-    private static native void nRecycle(long nativePtr);
+    private static native void nClose(long nativePtr);
 }