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

Commit 1202dae1 authored by François Degros's avatar François Degros
Browse files

Fix error handling in CompressJob 

If a new archive cannot be created, CompressJob.setUp() now returns
false instead of throwing a NullPointerException.

Bug: 439736151
Flag: EXEMPT fixing existing bugs
Test: atest DocumentsUIGoogleTests:com.android.documentsui.services

Change-Id: Id1e196edc9546db2cbffb3c59d5a6d3a5401444f
parent 17b81e69

src/com/android/documentsui/archives/ArchiveId.java

+4 −2
Original line number Diff line number Diff line
@@ -18,14 +18,16 @@ package com.android.documentsui.archives; 


import android.net.Uri;



import androidx.annotation.NonNull;



public class ArchiveId {

    private final static char DELIMITER = '#';



    public final Uri mArchiveUri;

    public final @NonNull Uri mArchiveUri;

    public final int mAccessMode;

    public final String mPath;



    public ArchiveId(Uri archiveUri, int accessMode, String path) {

    public ArchiveId(@NonNull Uri archiveUri, int accessMode, String path) {

        assert(archiveUri.toString().indexOf(DELIMITER) == -1);

        assert(!path.isEmpty());

src/com/android/documentsui/archives/ArchivesProvider.java

+2 −1
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import android.provider.DocumentsProvider; 
import android.util.Log;



import androidx.annotation.GuardedBy;

import androidx.annotation.NonNull;

import androidx.annotation.Nullable;



import com.android.documentsui.R;
@@ -247,7 +248,7 @@ public class ArchivesProvider extends DocumentsProvider { 
     * @see ParcelFileDescriptor#MODE_READ

     * @see ParcelFileDescriptor#MODE_WRITE

     */

    public static Uri buildUriForArchive(Uri externalUri, int accessMode) {

    public static Uri buildUriForArchive(@NonNull Uri externalUri, int accessMode) {

        return DocumentsContract.buildDocumentUri(AUTHORITY,

                new ArchiveId(externalUri, accessMode, "/").toDocumentId());

    }

src/com/android/documentsui/services/CompressJob.java

+25 −33
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ package com.android.documentsui.services; 


import static android.content.ContentResolver.wrap;



import static com.android.documentsui.base.SharedMinimal.DEBUG;

import static com.android.documentsui.base.SharedMinimal.redact;

import static com.android.documentsui.services.FileOperationService.OPERATION_COMPRESS;

import static com.android.documentsui.util.Material3Config.getRes;
@@ -28,7 +30,6 @@ import android.content.Context; 
import android.net.Uri;

import android.os.Messenger;

import android.os.ParcelFileDescriptor;

import android.os.RemoteException;

import android.provider.DocumentsContract;

import android.util.Log;
@@ -40,8 +41,6 @@ import com.android.documentsui.base.Features; 
import com.android.documentsui.base.UserId;

import com.android.documentsui.clipping.UrisSupplier;



import java.io.FileNotFoundException;



// TODO: Stop extending CopyJob.

final class CompressJob extends CopyJob {
@@ -99,10 +98,6 @@ final class CompressJob extends CopyJob { 
            return false;

        }



        final ContentResolver resolver = appContext.getContentResolver();



        // TODO: Move this to DocumentsProvider.



        String displayName;

        if (mResolvedDocs.size() == 1) {

            displayName = mResolvedDocs.get(0).displayName + NEW_ARCHIVE_EXTENSION;
@@ -113,44 +108,41 @@ final class CompressJob extends CopyJob { 
        }



        try {

            mArchiveUri = DocumentsContract.createDocument(

                    resolver, mDstInfo.derivedUri, "application/zip", displayName);

        } catch (Exception e) {

            mArchiveUri = null;

        }

            final ContentResolver resolver = appContext.getContentResolver();

            mArchiveUri = DocumentsContract.createDocument(resolver, mDstInfo.derivedUri,

                    "application/zip", displayName);

            if (DEBUG) Log.d(TAG, "Created archive " + redact(mArchiveUri));



        try {

            mDstInfo = DocumentInfo.fromUri(resolver, ArchivesProvider.buildUriForArchive(

                    mArchiveUri, ParcelFileDescriptor.MODE_WRITE_ONLY), UserId.DEFAULT_USER);

            mDstInfo = DocumentInfo.fromUri(resolver,

                    ArchivesProvider.buildUriForArchive(mArchiveUri,

                            ParcelFileDescriptor.MODE_WRITE_ONLY), UserId.DEFAULT_USER);

            ArchivesProvider.acquireArchive(getClient(mDstInfo), mDstInfo.derivedUri);

        } catch (FileNotFoundException e) {

            Log.e(TAG, "Cannot create document info", e);

            failureCount = mResourceUris.getItemCount();

            return false;

        } catch (RemoteException e) {

            Log.e(TAG, "Cannot acquire archive", e);

            failureCount = mResourceUris.getItemCount();

            return true;

        } catch (Exception e) {

            Log.e(TAG, "Cannot create archive '" + redact(displayName) + "' in " + redact(mDstInfo),

                    e);

            onFileFailed(mResolvedDocs);

            return false;

        }



        return true;

    }



    @Override

    void finish() {

        if (mDstInfo != null) {

            try {

                ArchivesProvider.releaseArchive(getClient(mDstInfo), mDstInfo.derivedUri);

        } catch (RemoteException e) {

            Log.e(TAG, "Cannot release archive", e);

            } catch (Exception e) {

                Log.e(TAG, "Cannot release archive " + redact(mDstInfo), e);

            }

        }



        // Remove the archive file in case of an error.

        if (mArchiveUri != null && (!isFinished() || isCanceled())) {

            try {

            if (!isFinished() || isCanceled()) {

                DocumentsContract.deleteDocument(wrap(getClient(mArchiveUri)), mArchiveUri);

            } catch (Exception e) {

                Log.e(TAG, "Cannot remove partial archive " + redact(mArchiveUri), e);

            }

        } catch (RemoteException | FileNotFoundException e) {

            Log.w(TAG, "Cannot clean up after compress error: " + mDstInfo.toString(), e);

        }



        super.finish();

src/com/android/documentsui/services/Job.java

+18 −0
Original line number Diff line number Diff line
@@ -65,6 +65,7 @@ import java.io.FileNotFoundException; 
import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.util.ArrayList;

import java.util.Collection;

import java.util.HashMap;

import java.util.Locale;

import java.util.Map;
@@ -291,17 +292,34 @@ abstract public class Job implements Runnable { 
        return service.getContentResolver();

    }



    @SuppressWarnings("NonAtomicVolatileUpdate")

    void onFileFailed(DocumentInfo file) {

        // Non-atomic operation is Ok since failureCount is only modified in one thread.

        // noinspection NonAtomicOperationOnVolatileField

        failureCount++;

        failedDocs.add(file);

    }



    @SuppressWarnings("NonAtomicVolatileUpdate")

    void onFileFailed(@NonNull Collection<? extends DocumentInfo> files) {

        // Non-atomic operation is Ok since failureCount is only modified in one thread.

        // noinspection NonAtomicOperationOnVolatileField

        failureCount += files.size();

        failedDocs.addAll(files);

    }



    @SuppressWarnings("NonAtomicVolatileUpdate")

    void onResolveFailed(Uri uri) {

        // Non-atomic operation is Ok since failureCount is only modified in one thread.

        // noinspection NonAtomicOperationOnVolatileField

        failureCount++;

        failedUris.add(uri);

    }



    @SuppressWarnings("NonAtomicVolatileUpdate")

    void onPathFailed(@NonNull String path) {

        // Non-atomic operation is Ok since failureCount is only modified in one thread.

        // noinspection NonAtomicOperationOnVolatileField

        failureCount++;

        failedPaths.add(path);

    }