Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7bc18b20 authored by Philip P. Moltmann's avatar Philip P. Moltmann
Browse files

Deal with finalize() after failed constructor 

If the constructor throws an exception, the object will still be
finalized. Hence we have to make sure to set objects that are cleaned up
to 0/null and clean up everything else. Otherwise the finalizer might
double clean up a failed PdfRenderer.

It also appears that the Pdfium error state is only set if a open-doc
function fail. Hence remove it in every other case. Otherwise successes
might read the error of the last call.

Fixes: 63875707, 37052344
Test: cts-tradefed run singleCommand cts-dev -m Pdf --test=android.graphics.pdf.cts.PdfRendererTest
Change-Id: I6d9fc26be768ba1e338344740e340cf9b56386e9
parent 9f5e9915

core/jni/android/graphics/pdf/PdfEditor.cpp

+1 −40
Original line number Diff line number Diff line
@@ -60,12 +60,7 @@ static jint nativeRemovePage(JNIEnv* env, jclass thiz, jlong documentPtr, jint p 
    FPDF_DOCUMENT document = reinterpret_cast<FPDF_DOCUMENT>(documentPtr);



    FPDFPage_Delete(document, pageIndex);

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, -1)



    int pageCount = FPDF_GetPageCount(document);

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, -1)



    return pageCount;

    return FPDF_GetPageCount(document);

}



struct PdfToFdWriter : FPDF_FILEWRITE {
@@ -110,7 +105,6 @@ static void nativeWrite(JNIEnv* env, jclass thiz, jlong documentPtr, jint fd) { 
        jniThrowExceptionFmt(env, "java/io/IOException",

                "cannot write to fd. Error: %d", errno);

    }

    HANDLE_PDFIUM_ERROR_STATE(env)

}



static void nativeSetTransformAndClip(JNIEnv* env, jclass thiz, jlong documentPtr, jint pageIndex,
@@ -123,7 +117,6 @@ static void nativeSetTransformAndClip(JNIEnv* env, jclass thiz, jlong documentPt 
                "cannot open page");

        return;

    }

    HANDLE_PDFIUM_ERROR_STATE(env);



    double width = 0;

    double height = 0;
@@ -134,11 +127,6 @@ static void nativeSetTransformAndClip(JNIEnv* env, jclass thiz, jlong documentPt 
                    "cannot get page size");

        return;

    }

    bool isExceptionPending = forwardPdfiumError(env);

    if (isExceptionPending) {

        FPDF_ClosePage(page);

        return;

    }



    // PDF's coordinate system origin is left-bottom while in graphics it

    // is the top-left. So, translate the PDF coordinates to ours.
@@ -170,14 +158,8 @@ static void nativeSetTransformAndClip(JNIEnv* env, jclass thiz, jlong documentPt 
    FS_RECTF clip = {(float) clipLeft, (float) clipTop, (float) clipRight, (float) clipBottom};



    FPDFPage_TransFormWithClip(page, &transform, &clip);

    isExceptionPending = forwardPdfiumError(env);

    if (isExceptionPending) {

        FPDF_ClosePage(page);

        return;

    }



    FPDF_ClosePage(page);

    HANDLE_PDFIUM_ERROR_STATE(env);

}



static void nativeGetPageSize(JNIEnv* env, jclass thiz, jlong documentPtr,
@@ -190,7 +172,6 @@ static void nativeGetPageSize(JNIEnv* env, jclass thiz, jlong documentPtr, 
                "cannot open page");

        return;

    }

    HANDLE_PDFIUM_ERROR_STATE(env);



    double width = 0;

    double height = 0;
@@ -201,17 +182,11 @@ static void nativeGetPageSize(JNIEnv* env, jclass thiz, jlong documentPtr, 
                    "cannot get page size");

        return;

    }

    bool isExceptionPending = forwardPdfiumError(env);

    if (isExceptionPending) {

        FPDF_ClosePage(page);

        return;

    }



    env->SetIntField(outSize, gPointClassInfo.x, width);

    env->SetIntField(outSize, gPointClassInfo.y, height);



    FPDF_ClosePage(page);

    HANDLE_PDFIUM_ERROR_STATE(env);

}



static bool nativeGetPageBox(JNIEnv* env, jclass thiz, jlong documentPtr, jint pageIndex,
@@ -224,7 +199,6 @@ static bool nativeGetPageBox(JNIEnv* env, jclass thiz, jlong documentPtr, jint p 
                "cannot open page");

        return false;

    }

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, false);



    float left;

    float top;
@@ -234,14 +208,8 @@ static bool nativeGetPageBox(JNIEnv* env, jclass thiz, jlong documentPtr, jint p 
    const FPDF_BOOL success = (pageBox == PAGE_BOX_MEDIA)

        ? FPDFPage_GetMediaBox(page, &left, &top, &right, &bottom)

        : FPDFPage_GetCropBox(page, &left, &top, &right, &bottom);

    bool isExceptionPending = forwardPdfiumError(env);

    if (isExceptionPending) {

        FPDF_ClosePage(page);

        return false;

    }



    FPDF_ClosePage(page);

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, false);



    if (!success) {

        return false;
@@ -279,7 +247,6 @@ static void nativeSetPageBox(JNIEnv* env, jclass thiz, jlong documentPtr, jint p 
                "cannot open page");

        return;

    }

    HANDLE_PDFIUM_ERROR_STATE(env);



    const int left = env->GetIntField(box, gRectClassInfo.left);

    const int top = env->GetIntField(box, gRectClassInfo.top);
@@ -291,14 +258,8 @@ static void nativeSetPageBox(JNIEnv* env, jclass thiz, jlong documentPtr, jint p 
    } else {

        FPDFPage_SetCropBox(page, left, top, right, bottom);

    }

    bool isExceptionPending = forwardPdfiumError(env);

    if (isExceptionPending) {

        FPDF_ClosePage(page);

        return;

    }



    FPDF_ClosePage(page);

    HANDLE_PDFIUM_ERROR_STATE(env);

}



static void nativeSetPageMediaBox(JNIEnv* env, jclass thiz, jlong documentPtr, jint pageIndex,

core/jni/android/graphics/pdf/PdfRenderer.cpp

+0 −9
Original line number Diff line number Diff line
@@ -50,7 +50,6 @@ static jlong nativeOpenPageAndGetSize(JNIEnv* env, jclass thiz, jlong documentPt 
                "cannot load page");

        return -1;

    }

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, -1)



    double width = 0;

    double height = 0;
@@ -61,7 +60,6 @@ static jlong nativeOpenPageAndGetSize(JNIEnv* env, jclass thiz, jlong documentPt 
                    "cannot get page size");

        return -1;

    }

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, -1)



    env->SetIntField(outSize, gPointClassInfo.x, width);

    env->SetIntField(outSize, gPointClassInfo.y, height);
@@ -72,7 +70,6 @@ static jlong nativeOpenPageAndGetSize(JNIEnv* env, jclass thiz, jlong documentPt 
static void nativeClosePage(JNIEnv* env, jclass thiz, jlong pagePtr) {

    FPDF_PAGE page = reinterpret_cast<FPDF_PAGE>(pagePtr);

    FPDF_ClosePage(page);

    HANDLE_PDFIUM_ERROR_STATE(env)

}



static void nativeRenderPage(JNIEnv* env, jclass thiz, jlong documentPtr, jlong pagePtr,
@@ -87,11 +84,6 @@ static void nativeRenderPage(JNIEnv* env, jclass thiz, jlong documentPtr, jlong 


    FPDF_BITMAP bitmap = FPDFBitmap_CreateEx(skBitmap.width(), skBitmap.height(),

            FPDFBitmap_BGRA, skBitmap.getPixels(), stride);

    bool isExceptionPending = forwardPdfiumError(env);

    if (isExceptionPending || bitmap == NULL) {

        ALOGE("Error creating bitmap");

        return;

    }



    int renderFlags = FPDF_REVERSE_BYTE_ORDER;

    if (renderMode == RENDER_MODE_FOR_DISPLAY) {
@@ -129,7 +121,6 @@ static void nativeRenderPage(JNIEnv* env, jclass thiz, jlong documentPtr, jlong 
    FS_RECTF clip = {(float) clipLeft, (float) clipTop, (float) clipRight, (float) clipBottom};



    FPDF_RenderPageBitmapWithMatrix(bitmap, page, &transform, &clip, renderFlags);

    HANDLE_PDFIUM_ERROR_STATE(env);



    skBitmap.notifyPixelsChanged();

}

core/jni/android/graphics/pdf/PdfUtils.cpp

+4 −20
Original line number Diff line number Diff line
@@ -78,34 +78,24 @@ bool forwardPdfiumError(JNIEnv* env) { 
    return true;

}



static bool initializeLibraryIfNeeded(JNIEnv* env) {

static void initializeLibraryIfNeeded(JNIEnv* env) {

    if (sUnmatchedPdfiumInitRequestCount == 0) {

        FPDF_InitLibrary();



        HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, false);

    }



    sUnmatchedPdfiumInitRequestCount++;

    return true;

}



static void destroyLibraryIfNeeded(JNIEnv* env, bool handleError) {

    if (sUnmatchedPdfiumInitRequestCount == 1) {

        FPDF_DestroyLibrary();



       if (handleError) {

           HANDLE_PDFIUM_ERROR_STATE(env);

       }

    }



    sUnmatchedPdfiumInitRequestCount--;

}



jlong nativeOpen(JNIEnv* env, jclass thiz, jint fd, jlong size) {

    bool isInitialized = initializeLibraryIfNeeded(env);

    if (!isInitialized) {

        return -1;

    }

    initializeLibraryIfNeeded(env);



    FPDF_FILEACCESS loader;

    loader.m_FileLen = size;
@@ -125,7 +115,6 @@ jlong nativeOpen(JNIEnv* env, jclass thiz, jint fd, jlong size) { 
void nativeClose(JNIEnv* env, jclass thiz, jlong documentPtr) {

    FPDF_DOCUMENT document = reinterpret_cast<FPDF_DOCUMENT>(documentPtr);

    FPDF_CloseDocument(document);

    HANDLE_PDFIUM_ERROR_STATE(env)



    destroyLibraryIfNeeded(env, true);

}
@@ -133,17 +122,12 @@ void nativeClose(JNIEnv* env, jclass thiz, jlong documentPtr) { 
jint nativeGetPageCount(JNIEnv* env, jclass thiz, jlong documentPtr) {

    FPDF_DOCUMENT document = reinterpret_cast<FPDF_DOCUMENT>(documentPtr);



    int pageCount = FPDF_GetPageCount(document);

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, -1);



    return pageCount;

    return FPDF_GetPageCount(document);

}



jboolean nativeScaleForPrinting(JNIEnv* env, jclass thiz, jlong documentPtr) {

    FPDF_DOCUMENT document = reinterpret_cast<FPDF_DOCUMENT>(documentPtr);



    FPDF_BOOL printScaling = FPDF_VIEWERREF_GetPrintScaling(document);

    HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, false);



    return printScaling ? JNI_TRUE : JNI_FALSE;

}

core/jni/android/graphics/pdf/PdfUtils.h

+0 −18
Original line number Diff line number Diff line
@@ -24,24 +24,6 @@ namespace android { 
int getBlock(void* param, unsigned long position, unsigned char* outBuffer,

        unsigned long size);



bool forwardPdfiumError(JNIEnv* env);



#define HANDLE_PDFIUM_ERROR_STATE(env)                         \

        {                                                      \

            bool isExceptionPending = forwardPdfiumError(env); \

            if (isExceptionPending) {                          \

                return;                                        \

            }                                                  \

        }



#define HANDLE_PDFIUM_ERROR_STATE_WITH_RET_CODE(env, retCode)  \

        {                                                      \

            bool isExceptionPending = forwardPdfiumError(env); \

            if (isExceptionPending) {                          \

                return retCode;                                \

            }                                                  \

        }



jlong nativeOpen(JNIEnv* env, jclass thiz, jint fd, jlong size);

void nativeClose(JNIEnv* env, jclass thiz, jlong documentPtr);

graphics/java/android/graphics/pdf/PdfEditor.java

+19 −10
Original line number Diff line number Diff line
@@ -39,7 +39,7 @@ public final class PdfEditor { 


    private final CloseGuard mCloseGuard = CloseGuard.get();



    private final long mNativeDocument;

    private long mNativeDocument;



    private int mPageCount;
@@ -77,12 +77,17 @@ public final class PdfEditor { 
        } catch (ErrnoException ee) {

            throw new IllegalArgumentException("file descriptor not seekable");

        }



        mInput = input;



        synchronized (PdfRenderer.sPdfiumLock) {

            mNativeDocument = nativeOpen(mInput.getFd(), size);

            try {

                mPageCount = nativeGetPageCount(mNativeDocument);

            } catch (Throwable t) {

                nativeClose(mNativeDocument);

                mNativeDocument = 0;

                throw t;

            }

        }



        mCloseGuard.open("close");
@@ -274,20 +279,24 @@ public final class PdfEditor { 
                mCloseGuard.warnIfOpen();

            }



            if (mInput != null) {

            doClose();

            }

        } finally {

            super.finalize();

        }

    }



    private void doClose() {

        if (mNativeDocument != 0) {

            synchronized (PdfRenderer.sPdfiumLock) {

                nativeClose(mNativeDocument);

            }

            mNativeDocument = 0;

        }



        if (mInput != null) {

            IoUtils.closeQuietly(mInput);

            mInput = null;

        }

        mCloseGuard.close();

    }