Handle broken archives correctly. (40cec2ae) · Commits · e / os / android_packages_apps_DocumentsUI

res/values/strings.xml

+3 −0

Original line number	Diff line number	Diff line
		@@ -265,4 +265,7 @@
		<string name="documents_shortcut_label">Documents</string>
		<!-- Shortcut label of Downloads root folder -->
		<string name="downloads_shortcut_label">Downloads</string>

		<!-- Error message shown when an archive fails to load -->
		<string name="archive_loading_failed">Unable to open archive for browsing. File is either corrupt, or an unsupported format.</string>
		</resources>

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

+32 −15

Original line number	Diff line number	Diff line
		@@ -35,6 +35,7 @@ import android.support.annotation.Nullable;
		import android.util.Log;
		import android.util.LruCache;

		import com.android.documentsui.R;
		import com.android.internal.annotations.GuardedBy;
		import com.android.internal.util.Preconditions;

		@@ -70,8 +71,6 @@ public class ArchivesProvider extends DocumentsProvider implements Closeable {
		oldValue.getWriteLock().lock();
		try {
		oldValue.get().close();
		} catch (FileNotFoundException e) {
		Log.e(TAG, "Failed to close an archive as it no longer exists.");
		} finally {
		oldValue.getWriteLock().unlock();
		}
		@@ -96,20 +95,34 @@ public class ArchivesProvider extends DocumentsProvider implements Closeable {
		Loader loader = null;
		try {
		loader = obtainInstance(documentId);
		if (loader.mArchive == null) {
		final int status = loader.getStatus();
		// If already loaded, then forward the request to the archive.
		if (status == Loader.STATUS_OPENED) {
		return loader.get().queryChildDocuments(documentId, projection, sortOrder);
		}

		final MatrixCursor cursor = new MatrixCursor(
		projection != null ? projection : Archive.DEFAULT_PROJECTION);
		final Bundle bundle = new Bundle();

		switch (status) {
		case Loader.STATUS_OPENING:
		bundle.putBoolean(DocumentsContract.EXTRA_LOADING, true);
		break;

		case Loader.STATUS_FAILED:
		// Return an empty cursor with EXTRA_LOADING, which shows spinner
		// in DocumentsUI. Once the archive is loaded, the notification will
		// be sent, and the directory reloaded.
		final Bundle bundle = new Bundle();
		bundle.putBoolean(DocumentsContract.EXTRA_LOADING, true);
		bundle.putString(DocumentsContract.EXTRA_ERROR,
		getContext().getString(R.string.archive_loading_failed));
		break;
		}

		cursor.setExtras(bundle);
		cursor.setNotificationUri(getContext().getContentResolver(),
		buildUriForArchive(archiveId.mArchiveUri));
		return cursor;
		}
		return loader.get().queryChildDocuments(documentId, projection, sortOrder);
		} finally {
		releaseInstance(loader);
		}
		@@ -259,6 +272,10 @@ public class ArchivesProvider extends DocumentsProvider implements Closeable {

		final Cursor cursor = getContext().getContentResolver().query(
		id.mArchiveUri, new String[] { Document.COLUMN_MIME_TYPE }, null, null, null);
		if (cursor == null \|\| cursor.getCount() == 0) {
		throw new FileNotFoundException("File not found." + id.mArchiveUri);
		}

		cursor.moveToFirst();
		final String mimeType = cursor.getString(cursor.getColumnIndex(
		Document.COLUMN_MIME_TYPE));

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

+39 −15

Original line number	Diff line number	Diff line
		@@ -16,8 +16,11 @@

		package com.android.documentsui.archives;

		import com.android.internal.annotations.GuardedBy;

		import android.content.Context;
		import android.net.Uri;
		import android.util.Log;

		import java.io.File;
		import java.io.FileNotFoundException;
		@@ -31,13 +34,21 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
		* Loads an instance of Archive lazily.
		*/
		public class Loader {
		private static final String TAG = "Loader";

		public static final int STATUS_OPENING = 0;
		public static final int STATUS_OPENED = 1;
		public static final int STATUS_FAILED = 2;

		private final Context mContext;
		private final Uri mArchiveUri;
		private final Uri mNotificationUri;
		private final ReentrantReadWriteLock mLock = new ReentrantReadWriteLock();
		private final ExecutorService mExecutor = Executors.newSingleThreadExecutor();
		private Exception mFailureException = null;
		public Archive mArchive = null;
		private final Object mStatusLock = new Object();
		@GuardedBy("mStatusLock")
		private int mStatus = STATUS_OPENING;
		private Archive mArchive = null;

		Loader(Context context, Uri archiveUri, Uri notificationUri) {
		this.mContext = context;
		@@ -48,18 +59,21 @@ public class Loader {
		mExecutor.submit(this::get);
		}

		synchronized Archive get() throws FileNotFoundException {
		if (mArchive != null) {
		synchronized Archive get() {
		synchronized (mStatusLock) {
		if (mStatus == STATUS_OPENED) {
		return mArchive;
		}
		}

		// Once loading the archive failed, do not to retry opening it until the
		// archive file has changed (the loader is deleted once we receive
		// a notification about the archive file being changed).
		if (mFailureException != null) {
		synchronized (mStatusLock) {
		if (mStatus == STATUS_FAILED) {
		throw new IllegalStateException(
		"Trying to perform an operation on an archive which failed to load.",
		mFailureException);
		"Trying to perform an operation on an archive which failed to load.");
		}
		}

		try {
		@@ -68,12 +82,15 @@ public class Loader {
		mContext.getContentResolver().openFileDescriptor(
		mArchiveUri, "r", null /* signal */),
		mArchiveUri, mNotificationUri);
		} catch (IOException e) {
		mFailureException = e;
		throw new IllegalStateException(e);
		} catch (RuntimeException e) {
		mFailureException = e;
		throw e;
		synchronized (mStatusLock) {
		mStatus = STATUS_OPENED;
		}
		} catch (IOException \| RuntimeException e) {
		Log.e(TAG, "Failed to open the archive.", e);
		synchronized (mStatusLock) {
		mStatus = STATUS_FAILED;
		}
		throw new IllegalStateException("Failed to open the archive.", e);
		} finally {
		// Notify observers that the root directory is loaded (or failed)
		// so clients reload it.
		@@ -81,9 +98,16 @@ public class Loader {
		ArchivesProvider.buildUriForArchive(mArchiveUri),
		null /* observer /, false / syncToNetwork */);
		}

		return mArchive;
		}

		int getStatus() {
		synchronized (mStatusLock) {
		return mStatus;
		}
		}

		Lock getReadLock() {
		return mLock.readLock();
		}

tests/Android.mk

+3 −1

Original line number	Diff line number	Diff line
		@@ -7,8 +7,10 @@ LOCAL_SRC_FILES := $(call all-java-files-under, common) \
		$(call all-java-files-under, unit) \
		$(call all-java-files-under, functional)

		# For testing ZIP files.
		# For testing ZIP files. Include testing ZIP files as uncompresseed raw
		# resources.
		LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res
		LOCAL_AAPT_FLAGS += -0 .zip

		LOCAL_JAVA_LIBRARIES := android.test.runner
		LOCAL_STATIC_JAVA_LIBRARIES := mockito-target ub-uiautomator espresso-core guava

tests/AndroidManifest.xml

+12 −0

Original line number	Diff line number	Diff line
		@@ -29,6 +29,18 @@
		<action android:name="android.content.action.DOCUMENTS_PROVIDER" />
		</intent-filter>
		</provider>
		<!-- Provider for testing archives. -->
		<provider
		android:name="com.android.documentsui.archives.ResourcesProvider"
		android:authorities="com.android.documentsui.archives.resourcesprovider"
		android:exported="true"
		android:grantUriPermissions="true"
		android:permission="android.permission.MANAGE_DOCUMENTS"
		android:enabled="true">
		<intent-filter>
		<action android:name="android.content.action.DOCUMENTS_PROVIDER" />
		</intent-filter>
		</provider>
		</application>

		<instrumentation android:name="android.support.test.runner.AndroidJUnitRunner"