diff options
7 files changed, 291 insertions, 66 deletions
diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/DocumentLoader.java b/packages/MtpDocumentsProvider/src/com/android/mtp/DocumentLoader.java index 90b5c096de20..712dbcb3463d 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/DocumentLoader.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/DocumentLoader.java @@ -16,6 +16,8 @@ package com.android.mtp; +import android.annotation.Nullable; +import android.annotation.WorkerThread; import android.content.ContentResolver; import android.database.Cursor; import android.mtp.MtpObjectInfo; @@ -25,6 +27,8 @@ import android.os.Process; import android.provider.DocumentsContract; import android.util.Log; +import com.android.internal.util.Preconditions; + import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; @@ -38,60 +42,46 @@ import java.util.LinkedList; * background thread to load the rest documents and caches its result for next requests. * TODO: Rename this class to ObjectInfoLoader */ -class DocumentLoader { +class DocumentLoader implements AutoCloseable { static final int NUM_INITIAL_ENTRIES = 10; static final int NUM_LOADING_ENTRIES = 20; static final int NOTIFY_PERIOD_MS = 500; + private final int mDeviceId; private final MtpManager mMtpManager; private final ContentResolver mResolver; private final MtpDatabase mDatabase; private final TaskList mTaskList = new TaskList(); - private boolean mHasBackgroundThread = false; + private Thread mBackgroundThread; - DocumentLoader(MtpManager mtpManager, ContentResolver resolver, MtpDatabase database) { + DocumentLoader(int deviceId, MtpManager mtpManager, ContentResolver resolver, + MtpDatabase database) { + mDeviceId = deviceId; mMtpManager = mtpManager; mResolver = resolver; mDatabase = database; } - private static MtpObjectInfo[] loadDocuments(MtpManager manager, int deviceId, int[] handles) - throws IOException { - final ArrayList<MtpObjectInfo> objects = new ArrayList<>(); - for (int i = 0; i < handles.length; i++) { - final MtpObjectInfo info = manager.getObjectInfo(deviceId, handles[i]); - if (info == null) { - Log.e(MtpDocumentsProvider.TAG, - "Failed to obtain object info handle=" + handles[i]); - continue; - } - objects.add(info); - } - return objects.toArray(new MtpObjectInfo[objects.size()]); - } - + /** + * Queries the child documents of given parent. + * It loads the first NUM_INITIAL_ENTRIES of object info, then launches the background thread + * to load the rest. + */ synchronized Cursor queryChildDocuments(String[] columnNames, Identifier parent) throws IOException { + Preconditions.checkArgument(parent.mDeviceId == mDeviceId); LoaderTask task = mTaskList.findTask(parent); if (task == null) { if (parent.mDocumentId == null) { throw new FileNotFoundException("Parent not found."); } - - int parentHandle = parent.mObjectHandle; - // Need to pass the special value MtpManager.OBJECT_HANDLE_ROOT_CHILDREN to - // getObjectHandles if we would like to obtain children under the root. - if (parent.mDocumentType == MtpDatabaseConstants.DOCUMENT_TYPE_STORAGE) { - parentHandle = MtpManager.OBJECT_HANDLE_ROOT_CHILDREN; - } // TODO: Handle nit race around here. // 1. getObjectHandles. // 2. putNewDocument. // 3. startAddingChildDocuemnts. // 4. stopAddingChildDocuments - It removes the new document added at the step 2, // because it is not updated between start/stopAddingChildDocuments. - task = new LoaderTask(mDatabase, parent, mMtpManager.getObjectHandles( - parent.mDeviceId, parent.mStorageId, parentHandle)); + task = LoaderTask.create(mDatabase, mMtpManager, parent); task.fillDocuments(loadDocuments( mMtpManager, parent.mDeviceId, @@ -102,15 +92,72 @@ class DocumentLoader { } mTaskList.addFirst(task); - if (task.getState() == LoaderTask.STATE_LOADING && !mHasBackgroundThread) { - mHasBackgroundThread = true; - new BackgroundLoaderThread().start(); + if (task.getState() == LoaderTask.STATE_LOADING) { + resume(); } return task.createCursor(mResolver, columnNames); } - synchronized void clearTasks() { - mTaskList.clear(); + /** + * Resumes a background thread. + */ + synchronized void resume() { + if (mBackgroundThread == null) { + mBackgroundThread = new BackgroundLoaderThread(); + mBackgroundThread.start(); + } + } + + /** + * Obtains next task to be run in background thread, or release the reference to background + * thread. + * + * Worker thread that receives null task needs to exit. + */ + @WorkerThread + synchronized @Nullable LoaderTask getNextTaskOrReleaseBackgroundThread() { + Preconditions.checkState(mBackgroundThread != null); + + final LoaderTask task = mTaskList.findRunningTask(); + if (task != null) { + return task; + } + + final Identifier identifier = mDatabase.getUnmappedDocumentsParent(mDeviceId); + if (identifier != null) { + final LoaderTask existingTask = mTaskList.findTask(identifier); + if (existingTask != null) { + Preconditions.checkState(existingTask.getState() != LoaderTask.STATE_LOADING); + mTaskList.remove(existingTask); + } + try { + final LoaderTask newTask = LoaderTask.create(mDatabase, mMtpManager, identifier); + mTaskList.addFirst(newTask); + return newTask; + } catch (IOException exception) { + Log.e(MtpDocumentsProvider.TAG, "Failed to create a task for mapping", exception); + // Continue to release the background thread. + } + } + + mBackgroundThread = null; + return null; + } + + /** + * Terminates background thread. + */ + @Override + public void close() throws InterruptedException { + final Thread thread; + synchronized (this) { + mTaskList.clear(); + thread = mBackgroundThread; + } + if (thread != null) { + thread.interrupt(); + thread.join(); + } } synchronized void clearCompletedTasks() { @@ -121,27 +168,45 @@ class DocumentLoader { mTaskList.clearTask(parentIdentifier); } + /** + * Helper method to loads multiple object info. + */ + private static MtpObjectInfo[] loadDocuments(MtpManager manager, int deviceId, int[] handles) + throws IOException { + final ArrayList<MtpObjectInfo> objects = new ArrayList<>(); + for (int i = 0; i < handles.length; i++) { + final MtpObjectInfo info = manager.getObjectInfo(deviceId, handles[i]); + if (info == null) { + Log.e(MtpDocumentsProvider.TAG, + "Failed to obtain object info handle=" + handles[i]); + continue; + } + objects.add(info); + } + return objects.toArray(new MtpObjectInfo[objects.size()]); + } + + /** + * Background thread to fetch object info. + */ private class BackgroundLoaderThread extends Thread { + /** + * Finds task that needs to be processed, then loads NUM_LOADING_ENTRIES of object info and + * store them to the database. If it does not find a task, exits the thread. + */ @Override public void run() { Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); - while (true) { - LoaderTask task; - int deviceId; - int[] handles; - synchronized (DocumentLoader.this) { - task = mTaskList.findRunningTask(); - if (task == null) { - mHasBackgroundThread = false; - return; - } - deviceId = task.mIdentifier.mDeviceId; - handles = task.getUnloadedObjectHandles(NUM_LOADING_ENTRIES); + while (!Thread.interrupted()) { + final LoaderTask task = getNextTaskOrReleaseBackgroundThread(); + if (task == null) { + return; } - try { - final MtpObjectInfo[] objectInfos = - loadDocuments(mMtpManager, deviceId, handles); + final MtpObjectInfo[] objectInfos = loadDocuments( + mMtpManager, + task.mIdentifier.mDeviceId, + task.getUnloadedObjectHandles(NUM_LOADING_ENTRIES)); task.fillDocuments(objectInfos); final boolean shouldNotify = task.mLastNotified.getTime() < @@ -157,6 +222,9 @@ class DocumentLoader { } } + /** + * Task list that has helper methods to search/clear tasks. + */ private static class TaskList extends LinkedList<LoaderTask> { LoaderTask findTask(Identifier parent) { for (int i = 0; i < size(); i++) { @@ -197,6 +265,10 @@ class DocumentLoader { } } + /** + * Loader task. + * Each task is responsible for fetching child documents for the given parent document. + */ private static class LoaderTask { static final int STATE_LOADING = 0; static final int STATE_COMPLETED = 1; @@ -217,6 +289,11 @@ class DocumentLoader { mLastNotified = new Date(); } + /** + * Returns a cursor that traverses the child document of the parent document handled by the + * task. + * The returned task may have a EXTRA_LOADING flag. + */ Cursor createCursor(ContentResolver resolver, String[] columnNames) throws IOException { final Bundle extras = new Bundle(); switch (getState()) { @@ -235,6 +312,9 @@ class DocumentLoader { return cursor; } + /** + * Returns a state of the task. + */ int getState() { if (mError != null) { return STATE_ERROR; @@ -245,6 +325,9 @@ class DocumentLoader { } } + /** + * Obtains object handles that have not been loaded yet. + */ int[] getUnloadedObjectHandles(int count) { return Arrays.copyOfRange( mObjectHandles, @@ -252,11 +335,17 @@ class DocumentLoader { Math.min(mNumLoaded + count, mObjectHandles.length)); } + /** + * Notifies a change of child list of the document. + */ void notify(ContentResolver resolver) { resolver.notifyChange(createUri(), null, false); mLastNotified = new Date(); } + /** + * Stores object information into database. + */ void fillDocuments(MtpObjectInfo[] objectInfoList) { if (objectInfoList.length == 0 || getState() != STATE_LOADING) { return; @@ -276,6 +365,9 @@ class DocumentLoader { } } + /** + * Marks the loading task as error. + */ void setError(Exception error) { final int lastState = getState(); setErrorInternal(error); @@ -298,5 +390,20 @@ class DocumentLoader { return DocumentsContract.buildChildDocumentsUri( MtpDocumentsProvider.AUTHORITY, mIdentifier.mDocumentId); } + + /** + * Creates a LoaderTask that loads children of the given document. + */ + static LoaderTask create(MtpDatabase database, MtpManager manager, Identifier parent) + throws IOException { + int parentHandle = parent.mObjectHandle; + // Need to pass the special value MtpManager.OBJECT_HANDLE_ROOT_CHILDREN to + // getObjectHandles if we would like to obtain children under the root. + if (parent.mDocumentType == MtpDatabaseConstants.DOCUMENT_TYPE_STORAGE) { + parentHandle = MtpManager.OBJECT_HANDLE_ROOT_CHILDREN; + } + return new LoaderTask(database, parent, manager.getObjectHandles( + parent.mDeviceId, parent.mStorageId, parentHandle)); + } } } diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/Mapper.java b/packages/MtpDocumentsProvider/src/com/android/mtp/Mapper.java index 5e3417aa91f5..17aae335048d 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/Mapper.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/Mapper.java @@ -404,12 +404,7 @@ class Mapper { return null; } try { - final Identifier identifier = mDatabase.createIdentifier(parentId); - if (mDatabase.getRowState(parentId) == ROW_STATE_DISCONNECTED) { - throw new FileNotFoundException( - "document: " + parentId + " is in disconnected device."); - } - return identifier; + return mDatabase.createIdentifier(parentId); } catch (FileNotFoundException error) { mInMappingIds.remove(parentId); throw error; diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java index ca5c79974d4d..e14109abfe54 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java @@ -40,6 +40,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.Preconditions; import java.io.FileNotFoundException; +import java.io.IOException; import java.util.Objects; /** @@ -406,15 +407,15 @@ class MtpDatabase { COLUMN_STORAGE_ID, COLUMN_OBJECT_HANDLE, COLUMN_DOCUMENT_TYPE), - SELECTION_DOCUMENT_ID, - strings(documentId), + SELECTION_DOCUMENT_ID + " AND " + COLUMN_ROW_STATE + " IN (?, ?)", + strings(documentId, ROW_STATE_VALID, ROW_STATE_INVALIDATED), null, null, null, "1"); try { if (cursor.getCount() == 0) { - throw new FileNotFoundException("ID is not found."); + throw new FileNotFoundException("ID \"" + documentId + "\" is not found."); } else { cursor.moveToNext(); return new Identifier( @@ -598,6 +599,48 @@ class MtpDatabase { } } + /** + * Obtains a document that has already mapped but has unmapped children. + * @param deviceId Device to find documents. + * @return Identifier of found document or null. + */ + public @Nullable Identifier getUnmappedDocumentsParent(int deviceId) { + final String fromClosure = + TABLE_DOCUMENTS + " AS child INNER JOIN " + + TABLE_DOCUMENTS + " AS parent ON " + + "child." + COLUMN_PARENT_DOCUMENT_ID + " = " + + "parent." + Document.COLUMN_DOCUMENT_ID; + final String whereClosure = + "parent." + COLUMN_DEVICE_ID + " = ? AND " + + "parent." + COLUMN_ROW_STATE + " IN (?, ?) AND " + + "child." + COLUMN_ROW_STATE + " = ?"; + try (final Cursor cursor = mDatabase.query( + fromClosure, + strings("parent." + COLUMN_DEVICE_ID, + "parent." + COLUMN_STORAGE_ID, + "parent." + COLUMN_OBJECT_HANDLE, + "parent." + Document.COLUMN_DOCUMENT_ID, + "parent." + COLUMN_DOCUMENT_TYPE), + whereClosure, + strings(deviceId, ROW_STATE_VALID, ROW_STATE_INVALIDATED, + ROW_STATE_DISCONNECTED), + null, + null, + null, + "1")) { + if (cursor.getCount() == 0) { + return null; + } + cursor.moveToNext(); + return new Identifier( + cursor.getInt(0), + cursor.getInt(1), + cursor.getInt(2), + cursor.getString(3), + cursor.getInt(4)); + } + } + private static class OpenHelper extends SQLiteOpenHelper { public OpenHelper(Context context, int flags) { super(context, diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java index d329e3cdd375..48499787c3b9 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java @@ -324,14 +324,18 @@ public class MtpDocumentsProvider extends DocumentsProvider { Log.d(TAG, "Open device " + deviceId); } mMtpManager.openDevice(deviceId); - mDeviceToolkits.put( - deviceId, new DeviceToolkit(mMtpManager, mResolver, mDatabase)); + final DeviceToolkit toolkit = + new DeviceToolkit(deviceId, mMtpManager, mResolver, mDatabase); + mDeviceToolkits.put(deviceId, toolkit); mIntentSender.sendUpdateNotificationIntent(); try { mRootScanner.resume().await(); } catch (InterruptedException error) { Log.e(TAG, "openDevice", error); } + // Resume document loader to remap disconnected document ID. Must be invoked after the + // root scanner resumes. + toolkit.mDocumentLoader.resume(); } } @@ -425,7 +429,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { if (DEBUG) { Log.d(TAG, "Close device " + deviceId); } - getDeviceToolkit(deviceId).mDocumentLoader.clearTasks(); + getDeviceToolkit(deviceId).mDocumentLoader.close(); mDeviceToolkits.remove(deviceId); mMtpManager.closeDevice(deviceId); if (getOpenedDeviceIds().length == 0) { @@ -485,9 +489,10 @@ public class MtpDocumentsProvider extends DocumentsProvider { public final PipeManager mPipeManager; public final DocumentLoader mDocumentLoader; - public DeviceToolkit(MtpManager manager, ContentResolver resolver, MtpDatabase database) { + public DeviceToolkit( + int deviceId, MtpManager manager, ContentResolver resolver, MtpDatabase database) { mPipeManager = new PipeManager(database); - mDocumentLoader = new DocumentLoader(manager, resolver, database); + mDocumentLoader = new DocumentLoader(deviceId, manager, resolver, database); } } diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java index 474da072e36d..b75a9e63a17e 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java @@ -44,7 +44,7 @@ public class DocumentLoaderTest extends AndroidTestCase { mDatabase.getMapper().startAddingDocuments(null); mDatabase.getMapper().putDeviceDocument( - new MtpDeviceRecord(1, "Device", null, true, new MtpRoot[0], null, null)); + new MtpDeviceRecord(0, "Device", null, true, new MtpRoot[0], null, null)); mDatabase.getMapper().stopAddingDocuments(null); mDatabase.getMapper().startAddingDocuments("1"); @@ -55,11 +55,12 @@ public class DocumentLoaderTest extends AndroidTestCase { mManager = new BlockableTestMtpManager(getContext()); mResolver = new TestContentResolver(); - mLoader = new DocumentLoader(mManager, mResolver, mDatabase); + mLoader = new DocumentLoader(0, mManager, mResolver, mDatabase); } @Override - public void tearDown() { + public void tearDown() throws Exception { + mLoader.close(); mDatabase.close(); } diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java index 5eda9b2b8bba..884d132d2b42 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java @@ -22,6 +22,7 @@ import android.mtp.MtpObjectInfo; import android.net.Uri; import android.os.ParcelFileDescriptor; import android.os.storage.StorageManager; +import android.provider.DocumentsContract.Document; import android.provider.DocumentsContract.Root; import android.system.Os; import android.system.OsConstants; @@ -587,6 +588,79 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } } + public void testMappingDisconnectedDocuments() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); + mMtpManager.addValidDevice(new MtpDeviceRecord( + 0, + "Device A", + "device key", + true /* unopened */, + new MtpRoot[] { + new MtpRoot( + 0 /* deviceId */, + 1 /* storageId */, + "Storage A" /* volume description */, + 1024 /* free space */, + 2048 /* total space */, + "" /* no volume identifier */) + }, + null, + null)); + + final String[] names = strings("Directory A", "Directory B", "Directory C"); + final int objectHandleOffset = 100; + for (int i = 0; i < names.length; i++) { + final int parentHandle = i == 0 ? + MtpManager.OBJECT_HANDLE_ROOT_CHILDREN : objectHandleOffset + i - 1; + final int objectHandle = i + objectHandleOffset; + mMtpManager.setObjectHandles(0, 1, parentHandle, new int[] { objectHandle }); + mMtpManager.setObjectInfo( + 0, + new MtpObjectInfo.Builder() + .setName(names[i]) + .setObjectHandle(objectHandle) + .setFormat(MtpConstants.FORMAT_ASSOCIATION) + .setStorageId(1) + .build()); + } + + mProvider.resumeRootScanner(); + mResolver.waitForNotification(ROOTS_URI, 1); + + final int documentIdOffset = 2; + for (int i = 0; i < names.length; i++) { + try (final Cursor cursor = mProvider.queryChildDocuments( + String.valueOf(documentIdOffset + i), + strings(Document.COLUMN_DOCUMENT_ID, Document.COLUMN_DISPLAY_NAME), + null)) { + assertEquals(1, cursor.getCount()); + cursor.moveToNext(); + assertEquals(String.valueOf(documentIdOffset + i + 1), cursor.getString(0)); + assertEquals(names[i], cursor.getString(1)); + } + } + + mProvider.closeDevice(0); + mResolver.waitForNotification(ROOTS_URI, 2); + + mProvider.openDevice(0); + mResolver.waitForNotification(ROOTS_URI, 3); + + for (int i = 0; i < names.length; i++) { + mResolver.waitForNotification(DocumentsContract.buildChildDocumentsUri( + MtpDocumentsProvider.AUTHORITY, + String.valueOf(documentIdOffset + i)), 1); + try (final Cursor cursor = mProvider.queryChildDocuments( + String.valueOf(documentIdOffset + i), + strings(Document.COLUMN_DOCUMENT_ID), + null)) { + assertEquals(1, cursor.getCount()); + cursor.moveToNext(); + assertEquals(String.valueOf(documentIdOffset + i + 1), cursor.getString(0)); + } + } + } + private void setupProvider(int flag) { mDatabase = new MtpDatabase(getContext(), flag); mProvider = new MtpDocumentsProvider(); diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestMtpManager.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestMtpManager.java index 1b46f3c62aa1..3043ce896a7e 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestMtpManager.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestMtpManager.java @@ -86,7 +86,7 @@ public class TestMtpManager extends MtpManager { @Override void openDevice(int deviceId) throws IOException { final MtpDeviceRecord device = mDevices.get(deviceId); - if (device == null || device.opened) { + if (device == null) { throw new IOException(); } mDevices.put( @@ -99,7 +99,7 @@ public class TestMtpManager extends MtpManager { @Override void closeDevice(int deviceId) throws IOException { final MtpDeviceRecord device = mDevices.get(deviceId); - if (device == null || !device.opened) { + if (device == null) { throw new IOException(); } mDevices.put( |