Don't try to delete files twice.

Also, apply delete in reverse order to placate RecyclerView.
TODO: Refactor this whole stack...needs some love.

Bug: 25091323, 24749296
Change-Id: I84e21c16423048bd50cd988eb1e2a36dc62b9d16
This commit is contained in:
Steve McKay
2015-10-21 14:38:54 -07:00
parent 9991e5f382
commit edd0575b46
3 changed files with 109 additions and 95 deletions

View File

@@ -98,10 +98,12 @@ import com.android.documentsui.RecentsProvider.StateColumns;
import com.android.documentsui.model.DocumentInfo;
import com.android.documentsui.model.DocumentStack;
import com.android.documentsui.model.RootInfo;
import com.android.internal.annotations.GuardedBy;
import com.google.common.collect.Lists;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -1743,6 +1745,9 @@ public class DirectoryFragment extends Fragment {
private Context mContext;
private int mCursorCount;
private boolean mIsLoading;
@GuardedBy("mPendingDelete")
private Boolean mPendingDelete = false;
@GuardedBy("mPendingDelete")
private SparseBooleanArray mMarkedForDeletion = new SparseBooleanArray();
private UpdateListener mUpdateListener;
@Nullable private Cursor mCursor;
@@ -1787,35 +1792,39 @@ public class DirectoryFragment extends Fragment {
}
int getItemCount() {
return mCursorCount - mMarkedForDeletion.size();
synchronized(mPendingDelete) {
return mCursorCount - mMarkedForDeletion.size();
}
}
Cursor getItem(int position) {
// Items marked for deletion are masked out of the UI. To do this, for every marked
// item whose position is less than the requested item position, advance the requested
// position by 1.
final int originalPos = position;
final int size = mMarkedForDeletion.size();
for (int i = 0; i < size; ++i) {
// It'd be more concise, but less efficient, to iterate over positions while calling
// mMarkedForDeletion.get. Instead, iterate over deleted entries.
if (mMarkedForDeletion.keyAt(i) <= position && mMarkedForDeletion.valueAt(i)) {
++position;
synchronized(mPendingDelete) {
// Items marked for deletion are masked out of the UI. To do this, for every marked
// item whose position is less than the requested item position, advance the requested
// position by 1.
final int originalPos = position;
final int size = mMarkedForDeletion.size();
for (int i = 0; i < size; ++i) {
// It'd be more concise, but less efficient, to iterate over positions while calling
// mMarkedForDeletion.get. Instead, iterate over deleted entries.
if (mMarkedForDeletion.keyAt(i) <= position && mMarkedForDeletion.valueAt(i)) {
++position;
}
}
}
if (DEBUG && position != originalPos) {
Log.d(TAG, "Item position adjusted for deletion. Original: " + originalPos
+ " Adjusted: " + position);
}
if (DEBUG && position != originalPos) {
Log.d(TAG, "Item position adjusted for deletion. Original: " + originalPos
+ " Adjusted: " + position);
}
if (position >= mCursorCount) {
throw new IndexOutOfBoundsException("Attempt to retrieve " + position + " of " +
mCursorCount + " items");
}
if (position >= mCursorCount) {
throw new IndexOutOfBoundsException("Attempt to retrieve " + position + " of " +
mCursorCount + " items");
}
mCursor.moveToPosition(position);
return mCursor;
mCursor.moveToPosition(position);
return mCursor;
}
}
private boolean isEmpty() {
@@ -1848,17 +1857,19 @@ public class DirectoryFragment extends Fragment {
}
List<DocumentInfo> getDocumentsMarkedForDeletion() {
final int size = mMarkedForDeletion.size();
List<DocumentInfo> docs = new ArrayList<>(size);
synchronized (mPendingDelete) {
final int size = mMarkedForDeletion.size();
List<DocumentInfo> docs = new ArrayList<>(size);
for (int i = 0; i < size; ++i) {
final int position = mMarkedForDeletion.keyAt(i);
checkState(position < mCursorCount);
mCursor.moveToPosition(position);
final DocumentInfo doc = DocumentInfo.fromDirectoryCursor(mCursor);
docs.add(doc);
for (int i = 0; i < size; ++i) {
final int position = mMarkedForDeletion.keyAt(i);
checkState(position < mCursorCount);
mCursor.moveToPosition(position);
final DocumentInfo doc = DocumentInfo.fromDirectoryCursor(mCursor);
docs.add(doc);
}
return docs;
}
return docs;
}
/**
@@ -1869,17 +1880,23 @@ public class DirectoryFragment extends Fragment {
* @param selected A selection representing the files to delete.
*/
void markForDeletion(Selection selected) {
// Only one deletion operation at a time.
checkState(mMarkedForDeletion.size() == 0);
// There should never be more to delete than what exists.
checkState(mCursorCount >= selected.size());
synchronized (mPendingDelete) {
mPendingDelete = true;
// Only one deletion operation at a time.
checkState(mMarkedForDeletion.size() == 0);
// There should never be more to delete than what exists.
checkState(mCursorCount >= selected.size());
final int size = selected.size();
for (int i = 0; i < size; ++i) {
int position = selected.get(i);
if (DEBUG) Log.d(TAG, "Marked position " + position + " for deletion");
mMarkedForDeletion.append(position, true);
mViewAdapter.notifyItemRemoved(position);
int[] positions = selected.getAll();
Arrays.sort(positions);
// Walk backwards through the set, since we're removing positions.
// Otherwise, positions would change after the first modification.
for (int p = positions.length - 1; p >= 0; p--) {
mMarkedForDeletion.append(positions[p], true);
mViewAdapter.notifyItemRemoved(positions[p]);
if (DEBUG) Log.d(TAG, "Scheduled " + positions[p] + " for delete.");
}
}
}
@@ -1888,17 +1905,24 @@ public class DirectoryFragment extends Fragment {
* unmarked, and restored in the UI. See {@link #markForDeletion(Selection)}.
*/
void undoDeletion() {
// Iterate over deleted items, temporarily marking them false in the deletion list, and
// re-adding them to the UI.
final int size = mMarkedForDeletion.size();
for (int i = 0; i < size; ++i) {
final int position = mMarkedForDeletion.keyAt(i);
mMarkedForDeletion.put(position, false);
mViewAdapter.notifyItemInserted(position);
synchronized (mPendingDelete) {
// Iterate over deleted items, temporarily marking them false in the deletion list, and
// re-adding them to the UI.
final int size = mMarkedForDeletion.size();
for (int i = 0; i < size; ++i) {
final int position = mMarkedForDeletion.keyAt(i);
mMarkedForDeletion.put(position, false);
mViewAdapter.notifyItemInserted(position);
}
resetDeleteData();
}
}
// Then, clear the deletion list.
mMarkedForDeletion.clear();
private void resetDeleteData() {
synchronized (mPendingDelete) {
mPendingDelete = false;
mMarkedForDeletion.clear();
}
}
/**
@@ -1909,9 +1933,16 @@ public class DirectoryFragment extends Fragment {
* snackbars) for errors, info, etc.
*/
void finalizeDeletion(DeletionListener listener) {
final ContentResolver resolver = mContext.getContentResolver();
DeleteFilesTask task = new DeleteFilesTask(resolver, listener);
task.execute();
synchronized (mPendingDelete) {
if (mPendingDelete) {
// Necessary to avoid b/25072545. Even when that's resolved, this
// is a nice safe thing to day.
mPendingDelete = false;
final ContentResolver resolver = mContext.getContentResolver();
DeleteFilesTask task = new DeleteFilesTask(resolver, listener);
task.execute();
}
}
}
/**
@@ -1968,7 +1999,7 @@ public class DirectoryFragment extends Fragment {
} else {
if (DEBUG) Log.d(TAG, "Deletion task completed successfully.");
}
mMarkedForDeletion.clear();
resetDeleteData();
mListener.onCompletion();
}

View File

@@ -601,6 +601,14 @@ public final class MultiSelectManager implements View.OnKeyListener {
mTotalSelection = new SparseBooleanArray();
}
@VisibleForTesting
public Selection(int... positions) {
this();
for (int i = 0; i < positions.length; i++) {
add(positions[i]);
}
}
/**
* @param position
* @return true if the position is currently selected.
@@ -623,6 +631,18 @@ public final class MultiSelectManager implements View.OnKeyListener {
return mTotalSelection.keyAt(index);
}
/**
* Returns an unordered array of selected positions.
*/
public int[] getAll() {
final int size = size();
int[] positions = new int[size];
for (int i = 0; i < size; i++) {
positions[i] = get(i);
}
return positions;
}
/**
* @return size of the selection.
*/

View File

@@ -32,10 +32,6 @@ import com.android.documentsui.MultiSelectManager.Selection;
import com.android.documentsui.model.DocumentInfo;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
public class DirectoryFragmentModelTest extends AndroidTestCase {
@@ -117,15 +113,6 @@ public class DirectoryFragmentModelTest extends AndroidTestCase {
assertEquals("0", docs.get(0).documentId);
assertEquals("1", docs.get(1).documentId);
assertEquals("4", docs.get(2).documentId);
TestDeletionListener testListener = new TestDeletionListener();
model.finalizeDeletion(testListener);
testListener.waitForDone();
docs = getDocumentInfo(0, 1, 2);
assertEquals("0", docs.get(0).documentId);
assertEquals("1", docs.get(1).documentId);
assertEquals("2", docs.get(2).documentId);
}
// Tests that Model.getItem returns the right items after a deletion is undone.
@@ -149,20 +136,12 @@ public class DirectoryFragmentModelTest extends AndroidTestCase {
};
}
private void delete(int... items) {
Selection sel = new Selection();
for (int item: items) {
sel.add(item);
}
model.markForDeletion(sel);
private void delete(int... positions) {
model.markForDeletion(new Selection(positions));
}
private List<DocumentInfo> getDocumentInfo(int... items) {
Selection sel = new Selection();
for (int item: items) {
sel.add(item);
}
return model.getDocuments(sel);
private List<DocumentInfo> getDocumentInfo(int... positions) {
return model.getDocuments(new Selection(positions));
}
private static class DummyListener extends Model.UpdateListener {
@@ -177,20 +156,4 @@ public class DirectoryFragmentModelTest extends AndroidTestCase {
return null;
}
}
private static class TestDeletionListener extends Model.DeletionListener {
final CountDownLatch mSignal = new CountDownLatch(1);
@Override
public void onCompletion() {
mSignal.countDown();
}
public void waitForDone() {
try {
boolean timeout = mSignal.await(10, TimeUnit.SECONDS);
assertTrue("Timed out waiting for deletion completion", timeout);
} catch (InterruptedException e) {}
}
}
}