[multi-part] Eliminate 1k selection limit
* Implement ring buffer to cope the challenge of deleting file clips
* Fix a bug to allow concurrent read on the same clip file
Change-Id: I53450d94ee881966d5631f0dc6edcb0fdd8ff9d5
diff --git a/packages/DocumentsUI/src/com/android/documentsui/ClipStorage.java b/packages/DocumentsUI/src/com/android/documentsui/ClipStorage.java
index 5102718..3f0427f 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/ClipStorage.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/ClipStorage.java
@@ -16,9 +16,12 @@
package com.android.documentsui;
+import android.content.SharedPreferences;
import android.net.Uri;
import android.os.AsyncTask;
import android.support.annotation.VisibleForTesting;
+import android.system.ErrnoException;
+import android.system.Os;
import android.util.Log;
import java.io.Closeable;
@@ -27,62 +30,157 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.channels.FileLock;
+import java.util.HashMap;
+import java.util.Map;
import java.util.Scanner;
+import java.util.concurrent.TimeUnit;
/**
* Provides support for storing lists of documents identified by Uri.
*
- * <li>Access to this object *must* be synchronized externally.
- * <li>All calls to this class are I/O intensive and must be wrapped in an AsyncTask.
+ * This class uses a ring buffer to recycle clip file slots, to mitigate the issue of clip file
+ * deletions.
*/
public final class ClipStorage {
+ public static final int NO_SELECTION_TAG = -1;
+
+ static final String PREF_NAME = "ClipStoragePref";
+
+ @VisibleForTesting
+ static final int NUM_OF_SLOTS = 20;
+
private static final String TAG = "ClipStorage";
+ private static final long STALENESS_THRESHOLD = TimeUnit.DAYS.toMillis(2);
+
+ private static final String NEXT_POS_TAG = "NextPosTag";
+ private static final String PRIMARY_DATA_FILE_NAME = "primary";
+
private static final byte[] LINE_SEPARATOR = System.lineSeparator().getBytes();
- public static final long NO_SELECTION_TAG = -1;
private final File mOutDir;
+ private final SharedPreferences mPref;
+
+ private final File[] mSlots = new File[NUM_OF_SLOTS];
+ private int mNextPos;
/**
* @param outDir see {@link #prepareStorage(File)}.
*/
- public ClipStorage(File outDir) {
+ public ClipStorage(File outDir, SharedPreferences pref) {
assert(outDir.isDirectory());
mOutDir = outDir;
+ mPref = pref;
+
+ mNextPos = mPref.getInt(NEXT_POS_TAG, 0);
}
/**
- * Creates a clip tag.
+ * Tries to get the next available clip slot. It's guaranteed to return one. If none of
+ * slots is available, it returns the next slot of the most recently returned slot by this
+ * method.
*
- * NOTE: this tag doesn't guarantee perfect uniqueness, but should work well unless user creates
- * clips more than hundreds of times per second.
+ * <p>This is not a perfect solution, but should be enough for most regular use. There are
+ * several situations this method may not work:
+ * <ul>
+ * <li>Making {@link #NUM_OF_SLOTS} - 1 times of large drag and drop or moveTo/copyTo/delete
+ * operations after cutting a primary clip, then the primary clip is overwritten.</li>
+ * <li>Having more than {@link #NUM_OF_SLOTS} queued jumbo file operations, one or more clip
+ * file may be overwritten.</li>
+ * </ul>
*/
- public long createTag() {
- return System.currentTimeMillis();
+ public synchronized int claimStorageSlot() {
+ int curPos = mNextPos;
+ for (int i = 0; i < NUM_OF_SLOTS; ++i, curPos = (curPos + 1) % NUM_OF_SLOTS) {
+ createSlotFile(curPos);
+
+ if (!mSlots[curPos].exists()) {
+ break;
+ }
+
+ // No file or only primary file exists, we deem it available.
+ if (mSlots[curPos].list().length <= 1) {
+ break;
+ }
+ // This slot doesn't seem available, but still need to check if it's a legacy of
+ // service being killed or a service crash etc. If it's stale, it's available.
+ else if(checkStaleFiles(curPos)) {
+ break;
+ }
+ }
+
+ prepareSlot(curPos);
+
+ mNextPos = (curPos + 1) % NUM_OF_SLOTS;
+ mPref.edit().putInt(NEXT_POS_TAG, mNextPos).commit();
+ return curPos;
+ }
+
+ private boolean checkStaleFiles(int pos) {
+ File slotData = toSlotDataFile(pos);
+
+ // No need to check if the file exists. File.lastModified() returns 0L if the file doesn't
+ // exist.
+ return slotData.lastModified() + STALENESS_THRESHOLD <= System.currentTimeMillis();
+ }
+
+ private void prepareSlot(int pos) {
+ assert(mSlots[pos] != null);
+
+ Files.deleteRecursively(mSlots[pos]);
+ mSlots[pos].mkdir();
+ assert(mSlots[pos].isDirectory());
}
/**
* Returns a writer. Callers must close the writer when finished.
*/
- public Writer createWriter(long tag) throws IOException {
- File file = toTagFile(tag);
+ private Writer createWriter(int tag) throws IOException {
+ File file = toSlotDataFile(tag);
return new Writer(file);
}
- @VisibleForTesting
- public Reader createReader(long tag) throws IOException {
- File file = toTagFile(tag);
+ /**
+ * Gets a {@link File} instance given a tag.
+ *
+ * This method creates a symbolic link in the slot folder to the data file as a reference
+ * counting method. When someone is done using this symlink, it's responsible to delete it.
+ * Therefore we can have a neat way to track how many things are still using this slot.
+ */
+ public File getFile(int tag) throws IOException {
+ createSlotFile(tag);
+
+ File primary = toSlotDataFile(tag);
+
+ String linkFileName = Integer.toString(mSlots[tag].list().length);
+ File link = new File(mSlots[tag], linkFileName);
+
+ try {
+ Os.symlink(primary.getAbsolutePath(), link.getAbsolutePath());
+ } catch (ErrnoException e) {
+ e.rethrowAsIOException();
+ }
+ return link;
+ }
+
+ /**
+ * Returns a Reader. Callers must close the reader when finished.
+ */
+ public Reader createReader(File file) throws IOException {
+ assert(file.getParentFile().getParentFile().equals(mOutDir));
return new Reader(file);
}
- @VisibleForTesting
- public void delete(long tag) throws IOException {
- toTagFile(tag).delete();
+ private File toSlotDataFile(int pos) {
+ assert(mSlots[pos] != null);
+ return new File(mSlots[pos], PRIMARY_DATA_FILE_NAME);
}
- private File toTagFile(long tag) {
- return new File(mOutDir, String.valueOf(tag));
+ private void createSlotFile(int pos) {
+ if (mSlots[pos] == null) {
+ mSlots[pos] = new File(mOutDir, Integer.toString(pos));
+ }
}
/**
@@ -96,27 +194,39 @@
return clipDir;
}
- public static boolean hasDocList(long tag) {
- return tag != NO_SELECTION_TAG;
- }
-
private static File getClipDir(File cacheDir) {
return new File(cacheDir, "clippings");
}
static final class Reader implements Iterable<Uri>, Closeable {
+ /**
+ * FileLock can't be held multiple times in a single JVM, but it's possible to have multiple
+ * readers reading the same clip file. Share the FileLock here so that it can be released
+ * when it's not needed.
+ */
+ private static final Map<String, FileLockEntry> sLocks = new HashMap<>();
+
+ private final String mCanonicalPath;
private final Scanner mScanner;
- private final FileLock mLock;
private Reader(File file) throws IOException {
FileInputStream inStream = new FileInputStream(file);
-
- // Lock the file here so it won't pass this line until the corresponding writer is done
- // writing.
- mLock = inStream.getChannel().lock(0L, Long.MAX_VALUE, true);
-
mScanner = new Scanner(inStream);
+
+ mCanonicalPath = file.getCanonicalPath(); // Resolve symlink
+ synchronized (sLocks) {
+ if (sLocks.containsKey(mCanonicalPath)) {
+ // Read lock is already held by someone in this JVM, just increment the ref
+ // count.
+ sLocks.get(mCanonicalPath).mCount++;
+ } else {
+ // No map entry, need to lock the file so it won't pass this line until the
+ // corresponding writer is done writing.
+ FileLock lock = inStream.getChannel().lock(0L, Long.MAX_VALUE, true);
+ sLocks.put(mCanonicalPath, new FileLockEntry(1, lock, mScanner));
+ }
+ }
}
@Override
@@ -126,12 +236,21 @@
@Override
public void close() throws IOException {
- if (mLock != null) {
- mLock.release();
- }
+ synchronized (sLocks) {
+ FileLockEntry ref = sLocks.get(mCanonicalPath);
- if (mScanner != null) {
- mScanner.close();
+ assert(ref.mCount > 0);
+ if (--ref.mCount == 0) {
+ // If ref count is 0 now, then there is no one who needs to hold the read lock.
+ // Release the lock, and remove the entry.
+ ref.mLock.release();
+ ref.mScanner.close();
+ sLocks.remove(mCanonicalPath);
+ }
+
+ if (mScanner != ref.mScanner) {
+ mScanner.close();
+ }
}
}
}
@@ -155,12 +274,28 @@
}
}
+ private static final class FileLockEntry {
+ private int mCount;
+ private FileLock mLock;
+ // We need to keep this scanner here because if the scanner is closed, the file lock is
+ // closed too.
+ private Scanner mScanner;
+
+ private FileLockEntry(int count, FileLock lock, Scanner scanner) {
+ mCount = count;
+ mLock = lock;
+ mScanner = scanner;
+ }
+ }
+
private static final class Writer implements Closeable {
private final FileOutputStream mOut;
private final FileLock mLock;
private Writer(File file) throws IOException {
+ assert(!file.exists());
+
mOut = new FileOutputStream(file);
// Lock the file here so copy tasks would wait until everything is flushed to disk
@@ -192,9 +327,9 @@
private final ClipStorage mClipStorage;
private final Iterable<Uri> mUris;
- private final long mTag;
+ private final int mTag;
- PersistTask(ClipStorage clipStorage, Iterable<Uri> uris, long tag) {
+ PersistTask(ClipStorage clipStorage, Iterable<Uri> uris, int tag) {
mClipStorage = clipStorage;
mUris = uris;
mTag = tag;
@@ -202,7 +337,7 @@
@Override
protected Void doInBackground(Void... params) {
- try (ClipStorage.Writer writer = mClipStorage.createWriter(mTag)) {
+ try(Writer writer = mClipStorage.createWriter(mTag)){
for (Uri uri: mUris) {
assert(uri != null);
writer.write(uri);
diff --git a/packages/DocumentsUI/src/com/android/documentsui/DocumentClipper.java b/packages/DocumentsUI/src/com/android/documentsui/DocumentClipper.java
index 72413bd..c95ffd1 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/DocumentClipper.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/DocumentClipper.java
@@ -21,9 +21,7 @@
import android.content.ClipboardManager;
import android.content.ContentResolver;
import android.content.Context;
-import android.content.SharedPreferences;
import android.net.Uri;
-import android.os.BaseBundle;
import android.os.PersistableBundle;
import android.provider.DocumentsContract;
import android.support.annotation.Nullable;
@@ -48,7 +46,7 @@
* ClipboardManager wrapper class providing higher level logical
* support for dealing with Documents.
*/
-public final class DocumentClipper implements ClipboardManager.OnPrimaryClipChangedListener {
+public final class DocumentClipper {
private static final String TAG = "DocumentClipper";
@@ -57,34 +55,14 @@
static final String OP_JUMBO_SELECTION_SIZE = "jumboSelection-size";
static final String OP_JUMBO_SELECTION_TAG = "jumboSelection-tag";
- // Use shared preference to store last seen primary clip tag, so that we can delete the file
- // when we realize primary clip has been changed when we're not running.
- private static final String PREF_NAME = "DocumentClipperPref";
- private static final String LAST_PRIMARY_CLIP_TAG = "lastPrimaryClipTag";
-
private final Context mContext;
private final ClipStorage mClipStorage;
private final ClipboardManager mClipboard;
- // Here we're tracking the last clipped tag ids so we can delete them later.
- private long mLastDragClipTag = ClipStorage.NO_SELECTION_TAG;
- private long mLastUnusedPrimaryClipTag = ClipStorage.NO_SELECTION_TAG;
-
- private final SharedPreferences mPref;
-
DocumentClipper(Context context, ClipStorage storage) {
mContext = context;
mClipStorage = storage;
mClipboard = context.getSystemService(ClipboardManager.class);
-
- mClipboard.addPrimaryClipChangedListener(this);
-
- // Primary clips may be changed when we're not running, now it's time to clean up the
- // remnant.
- mPref = context.getSharedPreferences(PREF_NAME, 0);
- mLastUnusedPrimaryClipTag =
- mPref.getLong(LAST_PRIMARY_CLIP_TAG, ClipStorage.NO_SELECTION_TAG);
- deleteLastUnusedPrimaryClip();
}
public boolean hasItemsToPaste() {
@@ -112,24 +90,8 @@
/**
* Returns {@link ClipData} representing the selection, or null if selection is empty,
* or cannot be converted.
- *
- * This is specialized for drag and drop so that we know which file to delete if nobody accepts
- * the drop.
*/
- public @Nullable ClipData getClipDataForDrag(
- Function<String, Uri> uriBuilder, Selection selection, @OpType int opType) {
- ClipData data = getClipDataForDocuments(uriBuilder, selection, opType);
-
- mLastDragClipTag = getTag(data);
-
- return data;
- }
-
- /**
- * Returns {@link ClipData} representing the selection, or null if selection is empty,
- * or cannot be converted.
- */
- private @Nullable ClipData getClipDataForDocuments(
+ public ClipData getClipDataForDocuments(
Function<String, Uri> uriBuilder, Selection selection, @OpType int opType) {
assert(selection != null);
@@ -147,7 +109,7 @@
/**
* Returns ClipData representing the selection.
*/
- private @Nullable ClipData createStandardClipData(
+ private ClipData createStandardClipData(
Function<String, Uri> uriBuilder, Selection selection, @OpType int opType) {
assert(!selection.isEmpty());
@@ -178,7 +140,7 @@
/**
* Returns ClipData representing the list of docs
*/
- private @Nullable ClipData createJumboClipData(
+ private ClipData createJumboClipData(
Function<String, Uri> uriBuilder, Selection selection, @OpType int opType) {
assert(!selection.isEmpty());
@@ -210,8 +172,8 @@
bundle.putInt(OP_JUMBO_SELECTION_SIZE, selection.size());
// Creates a clip tag
- long tag = mClipStorage.createTag();
- bundle.putLong(OP_JUMBO_SELECTION_TAG, tag);
+ int tag = mClipStorage.claimStorageSlot();
+ bundle.putInt(OP_JUMBO_SELECTION_TAG, tag);
ClipDescription description = new ClipDescription(
"", // Currently "label" is not displayed anywhere in the UI.
@@ -232,7 +194,7 @@
getClipDataForDocuments(uriBuilder, selection, FileOperationService.OPERATION_COPY);
assert(data != null);
- setPrimaryClip(data);
+ mClipboard.setPrimaryClip(data);
}
/**
@@ -250,68 +212,10 @@
PersistableBundle bundle = data.getDescription().getExtras();
bundle.putString(SRC_PARENT_KEY, parent.derivedUri.toString());
- setPrimaryClip(data);
- }
-
- private void setPrimaryClip(ClipData data) {
- deleteLastPrimaryClip();
-
- long tag = getTag(data);
- setLastUnusedPrimaryClipTag(tag);
-
mClipboard.setPrimaryClip(data);
}
/**
- * Sets this primary tag to both class variable and shared preference.
- */
- private void setLastUnusedPrimaryClipTag(long tag) {
- mLastUnusedPrimaryClipTag = tag;
- mPref.edit().putLong(LAST_PRIMARY_CLIP_TAG, tag).commit();
- }
-
- /**
- * This is a good chance for us to remove previous clip file for cut/copy because we know a new
- * primary clip is set.
- */
- @Override
- public void onPrimaryClipChanged() {
- deleteLastUnusedPrimaryClip();
- }
-
- private void deleteLastUnusedPrimaryClip() {
- ClipData primary = mClipboard.getPrimaryClip();
- long primaryTag = getTag(primary);
-
- // onPrimaryClipChanged is also called after we call setPrimaryClip(), so make sure we don't
- // delete the clip file we just created.
- if (mLastUnusedPrimaryClipTag != primaryTag) {
- deleteLastPrimaryClip();
- }
- }
-
- private void deleteLastPrimaryClip() {
- deleteClip(mLastUnusedPrimaryClipTag);
- setLastUnusedPrimaryClipTag(ClipStorage.NO_SELECTION_TAG);
- }
-
- /**
- * Deletes the last seen drag clip file.
- */
- public void deleteDragClip() {
- deleteClip(mLastDragClipTag);
- mLastDragClipTag = ClipStorage.NO_SELECTION_TAG;
- }
-
- private void deleteClip(long tag) {
- try {
- mClipStorage.delete(tag);
- } catch (IOException e) {
- Log.w(TAG, "Error deleting clip file with tag: " + tag, e);
- }
- }
-
- /**
* Copies documents from clipboard. It's the same as {@link #copyFromClipData} with clipData
* returned from {@link ClipboardManager#getPrimaryClip()}.
*
@@ -324,10 +228,6 @@
DocumentStack docStack,
FileOperations.Callback callback) {
- // The primary clip has been claimed by a file operation. It's now the operation's duty
- // to make sure the clip file is deleted after use.
- setLastUnusedPrimaryClipTag(ClipStorage.NO_SELECTION_TAG);
-
copyFromClipData(destination, docStack, mClipboard.getPrimaryClip(), callback);
}
@@ -352,33 +252,38 @@
PersistableBundle bundle = clipData.getDescription().getExtras();
@OpType int opType = getOpType(bundle);
- UrisSupplier uris = UrisSupplier.create(clipData);
+ try {
+ UrisSupplier uris = UrisSupplier.create(clipData, mContext);
+ if (!canCopy(destination)) {
+ callback.onOperationResult(
+ FileOperations.Callback.STATUS_REJECTED, opType, 0);
+ return;
+ }
- if (!canCopy(destination)) {
- callback.onOperationResult(
- FileOperations.Callback.STATUS_REJECTED, opType, 0);
+ if (uris.getItemCount() == 0) {
+ callback.onOperationResult(
+ FileOperations.Callback.STATUS_ACCEPTED, opType, 0);
+ return;
+ }
+
+ DocumentStack dstStack = new DocumentStack(docStack, destination);
+
+ String srcParentString = bundle.getString(SRC_PARENT_KEY);
+ Uri srcParent = srcParentString == null ? null : Uri.parse(srcParentString);
+
+ FileOperation operation = new FileOperation.Builder()
+ .withOpType(opType)
+ .withSrcParent(srcParent)
+ .withDestination(dstStack)
+ .withSrcs(uris)
+ .build();
+
+ FileOperations.start(mContext, operation, callback);
+ } catch(IOException e) {
+ Log.e(TAG, "Cannot create uris supplier.", e);
+ callback.onOperationResult(FileOperations.Callback.STATUS_REJECTED, opType, 0);
return;
}
-
- if (uris.getItemCount() == 0) {
- callback.onOperationResult(
- FileOperations.Callback.STATUS_ACCEPTED, opType, 0);
- return;
- }
-
- DocumentStack dstStack = new DocumentStack(docStack, destination);
-
- String srcParentString = bundle.getString(SRC_PARENT_KEY);
- Uri srcParent = srcParentString == null ? null : Uri.parse(srcParentString);
-
- FileOperation operation = new FileOperation.Builder()
- .withOpType(opType)
- .withSrcParent(srcParent)
- .withDestination(dstStack)
- .withSrcs(uris)
- .build();
-
- FileOperations.start(mContext, operation, callback);
}
/**
@@ -397,28 +302,6 @@
return true;
}
- /**
- * Obtains tag from {@link ClipData}. Returns {@link ClipStorage#NO_SELECTION_TAG}
- * if it's not a jumbo clip.
- */
- private static long getTag(@Nullable ClipData data) {
- if (data == null) {
- return ClipStorage.NO_SELECTION_TAG;
- }
-
- ClipDescription description = data.getDescription();
- if (description == null) {
- return ClipStorage.NO_SELECTION_TAG;
- }
-
- BaseBundle bundle = description.getExtras();
- if (bundle == null) {
- return ClipStorage.NO_SELECTION_TAG;
- }
-
- return bundle.getLong(OP_JUMBO_SELECTION_TAG, ClipStorage.NO_SELECTION_TAG);
- }
-
public static @OpType int getOpType(ClipData data) {
PersistableBundle bundle = data.getDescription().getExtras();
return getOpType(bundle);
diff --git a/packages/DocumentsUI/src/com/android/documentsui/DocumentsApplication.java b/packages/DocumentsUI/src/com/android/documentsui/DocumentsApplication.java
index 3d3902d..93eb527 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/DocumentsApplication.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/DocumentsApplication.java
@@ -77,7 +77,9 @@
mThumbnailCache = new ThumbnailCache(memoryClassBytes / 4);
- mClipStorage = new ClipStorage(ClipStorage.prepareStorage(getCacheDir()));
+ mClipStorage = new ClipStorage(
+ ClipStorage.prepareStorage(getCacheDir()),
+ getSharedPreferences(ClipStorage.PREF_NAME, 0));
mClipper = new DocumentClipper(this, mClipStorage);
final IntentFilter packageFilter = new IntentFilter();
diff --git a/packages/DocumentsUI/src/com/android/documentsui/Files.java b/packages/DocumentsUI/src/com/android/documentsui/Files.java
index 38f98be..009fecb4 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/Files.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/Files.java
@@ -26,13 +26,11 @@
private Files() {} // no initialization for utility classes.
public static void deleteRecursively(File file) {
- if (file.exists()) {
- if (file.isDirectory()) {
- for (File child : file.listFiles()) {
- deleteRecursively(child);
- }
+ if (file.isDirectory()) {
+ for (File child : file.listFiles()) {
+ deleteRecursively(child);
}
- file.delete();
}
+ file.delete();
}
}
diff --git a/packages/DocumentsUI/src/com/android/documentsui/UrisSupplier.java b/packages/DocumentsUI/src/com/android/documentsui/UrisSupplier.java
index c5d30aa..e4a11ef 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/UrisSupplier.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/UrisSupplier.java
@@ -31,6 +31,7 @@
import com.android.documentsui.dirlist.MultiSelectManager.Selection;
import com.android.documentsui.services.FileOperation;
+import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
@@ -54,31 +55,25 @@
*
* @param context We need context to obtain {@link ClipStorage}. It can't be sent in a parcel.
*/
- public Iterable<Uri> getDocs(Context context) throws IOException {
- return getDocs(DocumentsApplication.getClipStorage(context));
+ public Iterable<Uri> getUris(Context context) throws IOException {
+ return getUris(DocumentsApplication.getClipStorage(context));
}
@VisibleForTesting
- abstract Iterable<Uri> getDocs(ClipStorage storage) throws IOException;
+ abstract Iterable<Uri> getUris(ClipStorage storage) throws IOException;
- public void dispose(Context context) {
- ClipStorage storage = DocumentsApplication.getClipStorage(context);
- dispose(storage);
- }
-
- @VisibleForTesting
- void dispose(ClipStorage storage) {}
+ public void dispose() {}
@Override
public int describeContents() {
return 0;
}
- public static UrisSupplier create(ClipData clipData) {
+ public static UrisSupplier create(ClipData clipData, Context context) throws IOException {
UrisSupplier uris;
PersistableBundle bundle = clipData.getDescription().getExtras();
if (bundle.containsKey(OP_JUMBO_SELECTION_TAG)) {
- uris = new JumboUrisSupplier(clipData);
+ uris = new JumboUrisSupplier(clipData, context);
} else {
uris = new StandardUrisSupplier(clipData);
}
@@ -87,7 +82,9 @@
}
public static UrisSupplier create(
- Selection selection, Function<String, Uri> uriBuilder, Context context) {
+ Selection selection, Function<String, Uri> uriBuilder, Context context)
+ throws IOException {
+
ClipStorage storage = DocumentsApplication.getClipStorage(context);
List<Uri> uris = new ArrayList<>(selection.size());
@@ -99,7 +96,7 @@
}
@VisibleForTesting
- static UrisSupplier create(List<Uri> uris, ClipStorage storage) {
+ static UrisSupplier create(List<Uri> uris, ClipStorage storage) throws IOException {
UrisSupplier urisSupplier = (uris.size() > Shared.MAX_DOCS_IN_INTENT)
? new JumboUrisSupplier(uris, storage)
: new StandardUrisSupplier(uris);
@@ -110,24 +107,32 @@
private static class JumboUrisSupplier extends UrisSupplier {
private static final String TAG = "JumboUrisSupplier";
- private final long mSelectionTag;
+ private final File mFile;
private final int mSelectionSize;
private final transient AtomicReference<ClipStorage.Reader> mReader =
new AtomicReference<>();
- private JumboUrisSupplier(ClipData clipData) {
+ private JumboUrisSupplier(ClipData clipData, Context context) throws IOException {
PersistableBundle bundle = clipData.getDescription().getExtras();
- mSelectionTag = bundle.getLong(OP_JUMBO_SELECTION_TAG, ClipStorage.NO_SELECTION_TAG);
- assert(mSelectionTag != ClipStorage.NO_SELECTION_TAG);
+ final int tag = bundle.getInt(OP_JUMBO_SELECTION_TAG, ClipStorage.NO_SELECTION_TAG);
+ assert(tag != ClipStorage.NO_SELECTION_TAG);
+ mFile = DocumentsApplication.getClipStorage(context).getFile(tag);
+ assert(mFile.exists());
mSelectionSize = bundle.getInt(OP_JUMBO_SELECTION_SIZE);
assert(mSelectionSize > Shared.MAX_DOCS_IN_INTENT);
}
- private JumboUrisSupplier(Collection<Uri> uris, ClipStorage storage) {
- mSelectionTag = storage.createTag();
- new ClipStorage.PersistTask(storage, uris, mSelectionTag).execute();
+ private JumboUrisSupplier(Collection<Uri> uris, ClipStorage storage) throws IOException {
+ final int tag = storage.claimStorageSlot();
+ new ClipStorage.PersistTask(storage, uris, tag).execute();
+
+ // There is a tiny race condition here. A job may starts to read before persist task
+ // starts to write, but it has to beat an IPC and background task schedule, which is
+ // pretty rare. Creating a symlink doesn't need that file to exist, but we can't assert
+ // on its existence.
+ mFile = storage.getFile(tag);
mSelectionSize = uris.size();
}
@@ -137,8 +142,8 @@
}
@Override
- Iterable<Uri> getDocs(ClipStorage storage) throws IOException {
- ClipStorage.Reader reader = mReader.getAndSet(storage.createReader(mSelectionTag));
+ Iterable<Uri> getUris(ClipStorage storage) throws IOException {
+ ClipStorage.Reader reader = mReader.getAndSet(storage.createReader(mFile));
if (reader != null) {
reader.close();
mReader.get().close();
@@ -149,7 +154,7 @@
}
@Override
- void dispose(ClipStorage storage) {
+ public void dispose() {
try {
ClipStorage.Reader reader = mReader.get();
if (reader != null) {
@@ -158,18 +163,18 @@
} catch (IOException e) {
Log.w(TAG, "Failed to close the reader.", e);
}
- try {
- storage.delete(mSelectionTag);
- } catch(IOException e) {
- Log.w(TAG, "Failed to delete clip with tag: " + mSelectionTag + ".", e);
- }
+
+ // mFile is a symlink to the actual data file. Delete the symlink here so that we know
+ // there is one fewer referrer that needs the data file. The actual data file will be
+ // cleaned up during file slot rotation. See ClipStorage for more details.
+ mFile.delete();
}
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("JumboUrisSupplier{");
- builder.append("selectionTag=").append(mSelectionTag);
+ builder.append("file=").append(mFile.getAbsolutePath());
builder.append(", selectionSize=").append(mSelectionSize);
builder.append("}");
return builder.toString();
@@ -177,12 +182,12 @@
@Override
public void writeToParcel(Parcel dest, int flags) {
- dest.writeLong(mSelectionTag);
+ dest.writeString(mFile.getAbsolutePath());
dest.writeInt(mSelectionSize);
}
private JumboUrisSupplier(Parcel in) {
- mSelectionTag = in.readLong();
+ mFile = new File(in.readString());
mSelectionSize = in.readInt();
}
@@ -236,7 +241,7 @@
}
@Override
- Iterable<Uri> getDocs(ClipStorage storage) {
+ Iterable<Uri> getUris(ClipStorage storage) {
return mDocs;
}
diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
index 21f772e..ce4e993 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
@@ -99,6 +99,7 @@
import com.android.documentsui.services.FileOperationService.OpType;
import com.android.documentsui.services.FileOperations;
+import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
@@ -409,7 +410,7 @@
if (resultCode == Activity.RESULT_CANCELED || data == null) {
// User pressed the back button or otherwise cancelled the destination pick. Don't
// proceed with the copy.
- operation.dispose(getContext());
+ operation.dispose();
return;
}
@@ -494,26 +495,6 @@
state.dirState.put(mStateKey, container);
}
- void dragStarted() {
- // When files are selected for dragging, ActionMode is started. This obscures the breadcrumb
- // with an ActionBar. In order to make drag and drop to the breadcrumb possible, we first
- // end ActionMode so the breadcrumb is visible to the user.
- if (mActionMode != null) {
- mActionMode.finish();
- }
- }
-
- void dragStopped(boolean result) {
- if (result) {
- clearSelection();
- } else {
- // When drag starts we might write a new clip file to disk.
- // No drop event happens, remove clip file here. This may be called multiple times,
- // but it should be OK because deletion is idempotent and cheap.
- deleteDragClipFile();
- }
- }
-
public void onDisplayStateChanged() {
updateDisplayState();
}
@@ -1005,10 +986,15 @@
Log.w(TAG, "Action mode is null before deleting documents.");
}
- UrisSupplier srcs = UrisSupplier.create(
- selected,
- mModel::getItemUri,
- getContext());
+ UrisSupplier srcs;
+ try {
+ srcs = UrisSupplier.create(
+ selected,
+ mModel::getItemUri,
+ getContext());
+ } catch(IOException e) {
+ throw new RuntimeException("Failed to create uri supplier.", e);
+ }
FileOperation operation = new FileOperation.Builder()
.withOpType(FileOperationService.OPERATION_DELETE)
@@ -1041,8 +1027,12 @@
getActivity(),
DocumentsActivity.class);
- UrisSupplier srcs =
- UrisSupplier.create(selected, mModel::getItemUri, getContext());
+ UrisSupplier srcs;
+ try {
+ srcs = UrisSupplier.create(selected, mModel::getItemUri, getContext());
+ } catch(IOException e) {
+ throw new RuntimeException("Failed to create uri supplier.", e);
+ }
Uri srcParent = getDisplayState().stack.peek().derivedUri;
mPendingOperation = new FileOperation.Builder()
@@ -1276,8 +1266,19 @@
}
}
- public void clearSelection() {
- mSelectionManager.clearSelection();
+ void dragStarted() {
+ // When files are selected for dragging, ActionMode is started. This obscures the breadcrumb
+ // with an ActionBar. In order to make drag and drop to the breadcrumb possible, we first
+ // end ActionMode so the breadcrumb is visible to the user.
+ if (mActionMode != null) {
+ mActionMode.finish();
+ }
+ }
+
+ void dragStopped(boolean result) {
+ if (result) {
+ mSelectionManager.clearSelection();
+ }
}
@Override
@@ -1300,10 +1301,6 @@
activity.setRootsDrawerOpen(false);
}
- private void deleteDragClipFile() {
- mClipper.deleteDragClip();
- }
-
boolean handleDropEvent(View v, DragEvent event) {
BaseActivity activity = (BaseActivity) getActivity();
activity.setRootsDrawerOpen(false);
@@ -1508,7 +1505,7 @@
// the current code layout and framework assumptions don't support
// this. So for now, we could end up doing a bunch of i/o on main thread.
v.startDragAndDrop(
- mClipper.getClipDataForDrag(
+ mClipper.getClipDataForDocuments(
mModel::getItemUri,
selection,
FileOperationService.OPERATION_COPY),
diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java
index 54ccc2a..7497a83 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/services/CopyJob.java
@@ -272,7 +272,7 @@
private void buildDocumentList() throws ResourceException {
try {
final ContentResolver resolver = appContext.getContentResolver();
- final Iterable<Uri> uris = srcs.getDocs(appContext);
+ final Iterable<Uri> uris = srcs.getUris(appContext);
for (Uri uri : uris) {
DocumentInfo doc = DocumentInfo.fromUri(resolver, uri);
if (canCopy(doc, stack.root)) {
diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java b/packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java
index f6202c5..92440a2 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/services/DeleteJob.java
@@ -97,7 +97,7 @@
try {
final List<DocumentInfo> srcs = new ArrayList<>(this.srcs.getItemCount());
- final Iterable<Uri> uris = this.srcs.getDocs(appContext);
+ final Iterable<Uri> uris = this.srcs.getUris(appContext);
final ContentResolver resolver = appContext.getContentResolver();
final DocumentInfo srcParent = DocumentInfo.fromUri(resolver, mSrcParent);
diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/FileOperation.java b/packages/DocumentsUI/src/com/android/documentsui/services/FileOperation.java
index ce63864..c905a35 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/services/FileOperation.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/services/FileOperation.java
@@ -71,8 +71,8 @@
mDestination = destination;
}
- public void dispose(Context context) {
- mSrcs.dispose(context);
+ public void dispose() {
+ mSrcs.dispose();
}
abstract Job createJob(Context service, Job.Listener listener, String id);
diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/Job.java b/packages/DocumentsUI/src/com/android/documentsui/services/Job.java
index 29e0210..f9f49ca 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/services/Job.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/services/Job.java
@@ -151,7 +151,7 @@
// NOTE: If this details is a JumboClipDetails, and it's still referred in primary clip
// at this point, user won't be able to paste it to anywhere else because the underlying
- srcs.dispose(appContext);
+ srcs.dispose();
}
}
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/ClipStorageTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/ClipStorageTest.java
index 851000b..788a5b3 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/ClipStorageTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/ClipStorageTest.java
@@ -16,17 +16,20 @@
package com.android.documentsui;
+import static com.android.documentsui.ClipStorage.NUM_OF_SLOTS;
+
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import android.content.SharedPreferences;
import android.net.Uri;
import android.os.AsyncTask;
+import android.support.test.InstrumentationRegistry;
import android.support.test.filters.SmallTest;
import android.support.test.runner.AndroidJUnit4;
import com.android.documentsui.ClipStorage.Reader;
-import com.android.documentsui.dirlist.TestModel;
import com.android.documentsui.testing.TestScheduledExecutorService;
import org.junit.AfterClass;
@@ -37,13 +40,14 @@
import org.junit.runner.RunWith;
import java.io.File;
-import java.io.IOException;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.List;
@RunWith(AndroidJUnit4.class)
@SmallTest
public class ClipStorageTest {
+ private static final String PREF_NAME = "pref";
private static final List<Uri> TEST_URIS = createList(
"content://ham/fancy",
"content://poodle/monkey/giraffe");
@@ -51,22 +55,22 @@
@Rule
public TemporaryFolder folder = new TemporaryFolder();
+ private SharedPreferences mPref;
private TestScheduledExecutorService mExecutor;
-
private ClipStorage mStorage;
- private TestModel mModel;
- private long mTag;
+ private int mTag;
@Before
public void setUp() {
+ mPref = InstrumentationRegistry.getContext().getSharedPreferences(PREF_NAME, 0);
File clipDir = ClipStorage.prepareStorage(folder.getRoot());
- mStorage = new ClipStorage(clipDir);
+ mStorage = new ClipStorage(clipDir, mPref);
mExecutor = new TestScheduledExecutorService();
AsyncTask.setDefaultExecutor(mExecutor);
- mTag = mStorage.createTag();
+ mTag = mStorage.claimStorageSlot();
}
@AfterClass
@@ -83,7 +87,9 @@
public void testRead() throws Exception {
writeAll(mTag, TEST_URIS);
List<Uri> uris = new ArrayList<>();
- try(Reader provider = mStorage.createReader(mTag)) {
+
+ File copy = mStorage.getFile(mTag);
+ try(Reader provider = mStorage.createReader(copy)) {
for (Uri uri : provider) {
uris.add(uri);
}
@@ -92,12 +98,43 @@
}
@Test
- public void testDelete() throws Exception {
+ public void testGetTag_NoAvailableSlot() throws Exception {
+ int firstTag = mStorage.claimStorageSlot();
+ writeAll(firstTag, TEST_URIS);
+ mStorage.getFile(firstTag);
+ for (int i = 0; i < NUM_OF_SLOTS - 1; ++i) {
+ int tag = mStorage.claimStorageSlot();
+ writeAll(tag, TEST_URIS);
+ mStorage.getFile(tag);
+ }
+
+ assertEquals(firstTag, mStorage.claimStorageSlot());
+ }
+
+ @Test
+ public void testReadConcurrently() throws Exception {
writeAll(mTag, TEST_URIS);
- mStorage.delete(mTag);
- try {
- mStorage.createReader(mTag);
- } catch (IOException expected) {}
+ List<Uri> uris = new ArrayList<>();
+ List<Uri> uris2 = new ArrayList<>();
+
+ File copy = mStorage.getFile(mTag);
+ File copy2 = mStorage.getFile(mTag);
+ try(Reader reader = mStorage.createReader(copy)) {
+ try(Reader reader2 = mStorage.createReader(copy2)){
+ Iterator<Uri> iter = reader.iterator();
+ Iterator<Uri> iter2 = reader2.iterator();
+
+ while (iter.hasNext() && iter2.hasNext()) {
+ uris.add(iter.next());
+ uris2.add(iter2.next());
+ }
+
+ assertFalse(iter.hasNext());
+ assertFalse(iter2.hasNext());
+ }
+ }
+ assertEquals(TEST_URIS, uris);
+ assertEquals(TEST_URIS, uris2);
}
@Test
@@ -108,7 +145,7 @@
assertFalse(clipDir.equals(folder.getRoot()));
}
- private void writeAll(long tag, List<Uri> uris) {
+ private void writeAll(int tag, List<Uri> uris) {
new ClipStorage.PersistTask(mStorage, uris, tag).execute();
mExecutor.runAll();
}
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/UrisSupplierTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/UrisSupplierTest.java
index 719f0e2..2622322 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/UrisSupplierTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/UrisSupplierTest.java
@@ -19,9 +19,11 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import android.content.SharedPreferences;
import android.net.Uri;
import android.os.AsyncTask;
import android.provider.DocumentsContract;
+import android.support.test.InstrumentationRegistry;
import android.support.test.filters.MediumTest;
import android.support.test.runner.AndroidJUnit4;
@@ -42,6 +44,7 @@
@MediumTest
public class UrisSupplierTest {
+ private static final String PREF_NAME = "pref";
private static final String AUTHORITY = "foo";
private static final List<Uri> SHORT_URI_LIST = createList(3);
private static final List<Uri> LONG_URI_LIST = createList(Shared.MAX_DOCS_IN_INTENT + 5);
@@ -49,6 +52,7 @@
@Rule
public TemporaryFolder folder = new TemporaryFolder();
+ private SharedPreferences mPref;
private TestScheduledExecutorService mExecutor;
private ClipStorage mStorage;
@@ -57,7 +61,8 @@
mExecutor = new TestScheduledExecutorService();
AsyncTask.setDefaultExecutor(mExecutor);
- mStorage = new ClipStorage(folder.getRoot());
+ mPref = InstrumentationRegistry.getContext().getSharedPreferences(PREF_NAME, 0);
+ mStorage = new ClipStorage(folder.getRoot(), mPref);
}
@AfterClass
@@ -66,14 +71,14 @@
}
@Test
- public void testItemCountEquals_shortList() {
+ public void testItemCountEquals_shortList() throws Exception {
UrisSupplier uris = createWithShortList();
assertEquals(SHORT_URI_LIST.size(), uris.getItemCount());
}
@Test
- public void testItemCountEquals_longList() {
+ public void testItemCountEquals_longList() throws Exception {
UrisSupplier uris = createWithLongList();
assertEquals(LONG_URI_LIST.size(), uris.getItemCount());
@@ -83,35 +88,35 @@
public void testGetDocsEquals_shortList() throws Exception {
UrisSupplier uris = createWithShortList();
- assertIterableEquals(SHORT_URI_LIST, uris.getDocs(mStorage));
+ assertIterableEquals(SHORT_URI_LIST, uris.getUris(mStorage));
}
@Test
public void testGetDocsEquals_longList() throws Exception {
UrisSupplier uris = createWithLongList();
- assertIterableEquals(LONG_URI_LIST, uris.getDocs(mStorage));
+ assertIterableEquals(LONG_URI_LIST, uris.getUris(mStorage));
}
@Test
public void testDispose_shortList() throws Exception {
UrisSupplier uris = createWithShortList();
- uris.dispose(mStorage);
+ uris.dispose();
}
@Test
public void testDispose_longList() throws Exception {
UrisSupplier uris = createWithLongList();
- uris.dispose(mStorage);
+ uris.dispose();
}
- private UrisSupplier createWithShortList() {
+ private UrisSupplier createWithShortList() throws Exception {
return UrisSupplier.create(SHORT_URI_LIST, mStorage);
}
- private UrisSupplier createWithLongList() {
+ private UrisSupplier createWithLongList() throws Exception {
UrisSupplier uris =
UrisSupplier.create(LONG_URI_LIST, mStorage);