Merge "Fix crash when deleting multiple files." into nyc-dev

This commit is contained in:
Daichi Hirono
2016-04-11 07:52:02 +00:00
committed by Android (Google) Code Review
4 changed files with 100 additions and 30 deletions

View File

@@ -33,7 +33,6 @@ import com.android.internal.util.Preconditions;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.LinkedList;
@@ -118,9 +117,10 @@ class DocumentLoader implements AutoCloseable {
synchronized @Nullable LoaderTask getNextTaskOrReleaseBackgroundThread() {
Preconditions.checkState(mBackgroundThread != null);
final LoaderTask task = mTaskList.findRunningTask();
if (task != null) {
return task;
for (final LoaderTask task : mTaskList) {
if (task.getState() == LoaderTask.STATE_LOADING) {
return task;
}
}
final Identifier identifier = mDatabase.getUnmappedDocumentsParent(mDevice.deviceId);
@@ -161,8 +161,21 @@ class DocumentLoader implements AutoCloseable {
mTaskList.clearCompletedTasks();
}
synchronized void clearTask(Identifier parentIdentifier) {
mTaskList.clearTask(parentIdentifier);
/**
* Cancels the task for |parentIdentifier|.
*
* Task is removed from the cached list and it will create new task when |parentIdentifier|'s
* children are queried next.
*/
void cancelTask(Identifier parentIdentifier) {
final LoaderTask task;
synchronized (this) {
task = mTaskList.findTask(parentIdentifier);
}
if (task != null) {
task.cancel();
mTaskList.remove(task);
}
}
/**
@@ -205,14 +218,6 @@ class DocumentLoader implements AutoCloseable {
return null;
}
LoaderTask findRunningTask() {
for (int i = 0; i < size(); i++) {
if (get(i).getState() == LoaderTask.STATE_LOADING)
return get(i);
}
return null;
}
void clearCompletedTasks() {
int i = 0;
while (i < size()) {
@@ -223,17 +228,6 @@ class DocumentLoader implements AutoCloseable {
}
}
}
void clearTask(Identifier parentIdentifier) {
for (int i = 0; i < size(); i++) {
final LoaderTask task = get(i);
if (task.mIdentifier.mDeviceId == parentIdentifier.mDeviceId &&
task.mIdentifier.mObjectHandle == parentIdentifier.mObjectHandle) {
remove(i);
return;
}
}
}
}
/**
@@ -245,6 +239,7 @@ class DocumentLoader implements AutoCloseable {
static final int STATE_LOADING = 1;
static final int STATE_COMPLETED = 2;
static final int STATE_ERROR = 3;
static final int STATE_CANCELLED = 4;
final MtpManager mManager;
final MtpDatabase mDatabase;
@@ -272,6 +267,7 @@ class DocumentLoader implements AutoCloseable {
synchronized void loadObjectHandles() {
assert mState == STATE_START;
mPosition = 0;
int parentHandle = mIdentifier.mObjectHandle;
// Need to pass the special value MtpManager.OBJECT_HANDLE_ROOT_CHILDREN to
// getObjectHandles if we would like to obtain children under the root.
@@ -303,12 +299,10 @@ class DocumentLoader implements AutoCloseable {
case STATE_ERROR:
throw mError;
}
final Cursor cursor =
mDatabase.queryChildDocuments(columnNames, mIdentifier.mDocumentId);
cursor.setExtras(extras);
cursor.setNotificationUri(resolver, createUri());
cursor.respond(extras);
return cursor;
}
@@ -374,6 +368,10 @@ class DocumentLoader implements AutoCloseable {
}
}
synchronized (this) {
// Check if the task is cancelled or not.
if (mState != STATE_LOADING) {
return;
}
try {
mDatabase.getMapper().putChildDocuments(
mIdentifier.mDeviceId,
@@ -402,6 +400,14 @@ class DocumentLoader implements AutoCloseable {
}
}
/**
* Cancels the task.
*/
synchronized void cancel() {
mDatabase.getMapper().cancelAddingDocuments(mIdentifier.mDocumentId);
mState = STATE_CANCELLED;
}
/**
* Returns a state of the task.
*/

View File

@@ -362,6 +362,41 @@ class Mapper {
}
}
/**
* Cancels adding documents.
* @param parentId
*/
void cancelAddingDocuments(@Nullable String parentId) {
final String selection;
final String[] args;
if (parentId != null) {
selection = COLUMN_PARENT_DOCUMENT_ID + " = ?";
args = strings(parentId);
} else {
selection = COLUMN_PARENT_DOCUMENT_ID + " IS NULL";
args = EMPTY_ARGS;
}
final SQLiteDatabase database = mDatabase.getSQLiteDatabase();
database.beginTransaction();
try {
if (!mInMappingIds.contains(parentId)) {
return;
}
mInMappingIds.remove(parentId);
final ContentValues values = new ContentValues();
values.put(COLUMN_ROW_STATE, ROW_STATE_VALID);
mDatabase.getSQLiteDatabase().update(
TABLE_DOCUMENTS,
values,
selection + " AND " + COLUMN_ROW_STATE + " = ?",
DatabaseUtils.appendSelectionArgs(args, strings(ROW_STATE_INVALIDATED)));
database.setTransactionSuccessful();
} finally {
database.endTransaction();
}
}
/**
* Queries candidate for each mappingKey, and returns the first cursor that includes a
* candidate.

View File

@@ -308,7 +308,7 @@ public class MtpDocumentsProvider extends DocumentsProvider {
final Identifier parentIdentifier = mDatabase.getParentIdentifier(documentId);
mMtpManager.deleteDocument(identifier.mDeviceId, identifier.mObjectHandle);
mDatabase.deleteDocument(documentId);
getDocumentLoader(parentIdentifier).clearTask(parentIdentifier);
getDocumentLoader(parentIdentifier).cancelTask(parentIdentifier);
notifyChildDocumentsChange(parentIdentifier.mDocumentId);
if (parentIdentifier.mDocumentType == MtpDatabaseConstants.DOCUMENT_TYPE_STORAGE) {
// If the parent is storage, the object might be appeared as child of device because
@@ -402,7 +402,7 @@ public class MtpDocumentsProvider extends DocumentsProvider {
final String documentId = mDatabase.putNewDocument(
parentId.mDeviceId, parentDocumentId, record.operationsSupported,
infoWithHandle, 0l);
getDocumentLoader(parentId).clearTask(parentId);
getDocumentLoader(parentId).cancelTask(parentId);
notifyChildDocumentsChange(parentDocumentId);
return documentId;
} catch (FileNotFoundException | RuntimeException error) {

View File

@@ -21,6 +21,7 @@ import android.database.Cursor;
import android.mtp.MtpObjectInfo;
import android.net.Uri;
import android.provider.DocumentsContract;
import android.provider.DocumentsContract.Document;
import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.MediumTest;
@@ -28,6 +29,7 @@ import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeoutException;
@MediumTest
public class DocumentLoaderTest extends AndroidTestCase {
@@ -141,6 +143,33 @@ public class DocumentLoaderTest extends AndroidTestCase {
}
}
public void testCancelTask() throws IOException, InterruptedException {
setUpDocument(mManager,
DocumentLoader.NUM_INITIAL_ENTRIES + DocumentLoader.NUM_LOADING_ENTRIES + 1);
// Block the first iteration in the background thread.
mManager.blockDocument(
0, DocumentLoader.NUM_INITIAL_ENTRIES + 1);
setUpLoader();
try (final Cursor cursor = mLoader.queryChildDocuments(
MtpDocumentsProvider.DEFAULT_DOCUMENT_PROJECTION, mParentIdentifier)) {
assertTrue(cursor.getExtras().getBoolean(DocumentsContract.EXTRA_LOADING));
}
Thread.sleep(DocumentLoader.NOTIFY_PERIOD_MS);
// Clear task while the first iteration is being blocked.
mManager.unblockDocument(
0, DocumentLoader.NUM_INITIAL_ENTRIES + 1);
mLoader.cancelTask(mParentIdentifier);
Thread.sleep(DocumentLoader.NOTIFY_PERIOD_MS * 2);
// Check if it's OK to query invalidated task.
try (final Cursor cursor = mLoader.queryChildDocuments(
MtpDocumentsProvider.DEFAULT_DOCUMENT_PROJECTION, mParentIdentifier)) {
}
}
private void setUpLoader() {
mLoader = new DocumentLoader(
new MtpDeviceRecord(