diff options
7 files changed, 223 insertions, 68 deletions
diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java index bcdfb714d241..3a98a588c548 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java @@ -57,6 +57,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { private ContentResolver mResolver; private PipeManager mPipeManager; private DocumentLoader mDocumentLoader; + private RootScanner mRootScanner; /** * Provides singleton instance to MtpDocumentsService. @@ -72,6 +73,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { mResolver = getContext().getContentResolver(); mPipeManager = new PipeManager(); mDocumentLoader = new DocumentLoader(mMtpManager, mResolver); + mRootScanner = new RootScanner(mResolver, mMtpManager); return true; } @@ -80,6 +82,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { mMtpManager = mtpManager; mResolver = resolver; mDocumentLoader = new DocumentLoader(mMtpManager, mResolver); + mRootScanner = new RootScanner(mResolver, mMtpManager); } @Override @@ -88,25 +91,17 @@ public class MtpDocumentsProvider extends DocumentsProvider { projection = MtpDocumentsProvider.DEFAULT_ROOT_PROJECTION; } final MatrixCursor cursor = new MatrixCursor(projection); - for (final int deviceId : mMtpManager.getOpenedDeviceIds()) { - try { - final MtpRoot[] roots = mMtpManager.getRoots(deviceId); - // TODO: Add retry logic here. - - for (final MtpRoot root : roots) { - final Identifier rootIdentifier = new Identifier(deviceId, root.mStorageId); - final MatrixCursor.RowBuilder builder = cursor.newRow(); - builder.add(Root.COLUMN_ROOT_ID, rootIdentifier.toRootId()); - builder.add(Root.COLUMN_FLAGS, Root.FLAG_SUPPORTS_IS_CHILD); - builder.add(Root.COLUMN_TITLE, root.mDescription); - builder.add( - Root.COLUMN_DOCUMENT_ID, - rootIdentifier.toDocumentId()); - builder.add(Root.COLUMN_AVAILABLE_BYTES , root.mFreeSpace); - } - } catch (IOException error) { - Log.d(TAG, error.getMessage()); - } + final MtpRoot[] roots = mRootScanner.getRoots(); + for (final MtpRoot root : roots) { + final Identifier rootIdentifier = new Identifier(root.mDeviceId, root.mStorageId); + final MatrixCursor.RowBuilder builder = cursor.newRow(); + builder.add(Root.COLUMN_ROOT_ID, rootIdentifier.toRootId()); + builder.add(Root.COLUMN_FLAGS, Root.FLAG_SUPPORTS_IS_CHILD); + builder.add(Root.COLUMN_TITLE, root.mDescription); + builder.add( + Root.COLUMN_DOCUMENT_ID, + rootIdentifier.toDocumentId()); + builder.add(Root.COLUMN_AVAILABLE_BYTES , root.mFreeSpace); } cursor.setNotificationUri( mResolver, DocumentsContract.buildRootsUri(MtpDocumentsProvider.AUTHORITY)); @@ -221,13 +216,13 @@ public class MtpDocumentsProvider extends DocumentsProvider { void openDevice(int deviceId) throws IOException { mMtpManager.openDevice(deviceId); - notifyRootsChange(); + mRootScanner.scanNow(); } void closeDevice(int deviceId) throws IOException { mMtpManager.closeDevice(deviceId); mDocumentLoader.clearCache(deviceId); - notifyRootsChange(); + mRootScanner.scanNow(); } void closeAllDevices() { @@ -242,7 +237,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { } } if (closed) { - notifyRootsChange(); + mRootScanner.scanNow(); } } @@ -250,11 +245,6 @@ public class MtpDocumentsProvider extends DocumentsProvider { return mMtpManager.getOpenedDeviceIds().length != 0; } - private void notifyRootsChange() { - mResolver.notifyChange( - DocumentsContract.buildRootsUri(MtpDocumentsProvider.AUTHORITY), null, false); - } - private void notifyChildDocumentsChange(String parentDocumentId) { mResolver.notifyChange( DocumentsContract.buildChildDocumentsUri(AUTHORITY, parentDocumentId), diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java index 141d118b3486..088315787e8f 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java @@ -72,7 +72,7 @@ class MtpManager { // Handle devices that fail to obtain storages just after opening a MTP session. final int[] storageIds = device.getStorageIds(); - if (storageIds == null || storageIds.length == 0) { + if (storageIds == null) { throw new IOException("Not found MTP storages in the device."); } @@ -97,7 +97,7 @@ class MtpManager { final int[] storageIds = device.getStorageIds(); final MtpRoot[] results = new MtpRoot[storageIds.length]; for (int i = 0; i < storageIds.length; i++) { - results[i] = new MtpRoot(device.getStorageInfo(storageIds[i])); + results[i] = new MtpRoot(deviceId, device.getStorageInfo(storageIds[i])); } return results; } diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpRoot.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpRoot.java index fb160aa1e0c9..9dd53c0fc6c9 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpRoot.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpRoot.java @@ -21,6 +21,7 @@ import android.mtp.MtpStorageInfo; import com.android.internal.annotations.VisibleForTesting; class MtpRoot { + final int mDeviceId; final int mStorageId; final String mDescription; final long mFreeSpace; @@ -28,27 +29,45 @@ class MtpRoot { final String mVolumeIdentifier; @VisibleForTesting - MtpRoot(int storageId, + MtpRoot(int deviceId, + int storageId, String description, long freeSpace, long maxCapacity, String volumeIdentifier) { - this.mStorageId = storageId; - this.mDescription = description; - this.mFreeSpace = freeSpace; - this.mMaxCapacity = maxCapacity; - this.mVolumeIdentifier = volumeIdentifier; + mDeviceId = deviceId; + mStorageId = storageId; + mDescription = description; + mFreeSpace = freeSpace; + mMaxCapacity = maxCapacity; + mVolumeIdentifier = volumeIdentifier; } - MtpRoot(MtpStorageInfo storageInfo) { + MtpRoot(int deviceId, MtpStorageInfo storageInfo) { + mDeviceId = deviceId; mStorageId = storageInfo.getStorageId(); mDescription = storageInfo.getDescription(); mFreeSpace = storageInfo.getFreeSpace(); mMaxCapacity = storageInfo.getMaxCapacity(); - if (!storageInfo.getVolumeIdentifier().equals("")) { - mVolumeIdentifier = storageInfo.getVolumeIdentifier(); - } else { - mVolumeIdentifier = null; - } + mVolumeIdentifier = storageInfo.getVolumeIdentifier(); + } + + @Override + public boolean equals(Object object) { + if (!(object instanceof MtpRoot)) + return false; + final MtpRoot other = (MtpRoot) object; + return mDeviceId == other.mDeviceId && + mStorageId == other.mStorageId && + mDescription.equals(other.mDescription) && + mFreeSpace == other.mFreeSpace && + mMaxCapacity == other.mMaxCapacity && + mVolumeIdentifier.equals(other.mVolumeIdentifier); + } + + @Override + public int hashCode() { + return mDeviceId ^ mStorageId ^ mDescription.hashCode() ^ ((int) mFreeSpace) ^ + ((int) mMaxCapacity) ^ mVolumeIdentifier.hashCode(); } } diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java b/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java new file mode 100644 index 000000000000..ffab1766705a --- /dev/null +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java @@ -0,0 +1,106 @@ +package com.android.mtp; + +import android.content.ContentResolver; +import android.net.Uri; +import android.os.Process; +import android.provider.DocumentsContract; +import android.util.Log; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; + +final class RootScanner { + /** + * Polling interval in milliseconds used for first SHORT_POLLING_TIMES because it is more + * likely to add new root just after the device is added. + */ + private final static long SHORT_POLLING_INTERVAL = 2000; + + /** + * Polling interval in milliseconds for low priority polling, when changes are not expected. + */ + private final static long LONG_POLLING_INTERVAL = 30 * 1000; + + /** + * @see #SHORT_POLLING_INTERVAL + */ + private final static long SHORT_POLLING_TIMES = 10; + + final ContentResolver mResolver; + final MtpManager mManager; + MtpRoot[] mLastRoots = new MtpRoot[0]; + int mPollingCount; + boolean mHasBackgroundTask = false; + + RootScanner(ContentResolver resolver, MtpManager manager) { + mResolver = resolver; + mManager = manager; + } + + synchronized MtpRoot[] getRoots() { + return mLastRoots; + } + + /** + * Starts to check new changes right away. + * If the background thread has already gone, it restarts another background thread. + */ + synchronized void scanNow() { + mPollingCount = 0; + if (!mHasBackgroundTask) { + mHasBackgroundTask = true; + new BackgroundLoaderThread().start(); + } else { + notify(); + } + } + + private MtpRoot[] createRoots(int[] deviceIds) { + final ArrayList<MtpRoot> roots = new ArrayList<>(); + for (final int deviceId : deviceIds) { + try { + roots.addAll(Arrays.asList(mManager.getRoots(deviceId))); + } catch (IOException error) { + // Skip device that return error. + Log.d(MtpDocumentsProvider.TAG, error.getMessage()); + } + } + return roots.toArray(new MtpRoot[roots.size()]); + } + + private final class BackgroundLoaderThread extends Thread { + @Override + public void run() { + Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); + synchronized (RootScanner.this) { + while (true) { + final int[] deviceIds = mManager.getOpenedDeviceIds(); + final MtpRoot[] newRoots = createRoots(deviceIds); + if (!Arrays.equals(mLastRoots, newRoots)) { + final Uri uri = + DocumentsContract.buildRootsUri(MtpDocumentsProvider.AUTHORITY); + mResolver.notifyChange(uri, null, false); + mLastRoots = newRoots; + } + if (deviceIds.length == 0) { + break; + } + mPollingCount++; + try { + // Use SHORT_POLLING_PERIOD for the first SHORT_POLLING_TIMES because it is + // more likely to add new root just after the device is added. + // TODO: Use short interval only for a device that is just added. + RootScanner.this.wait( + mPollingCount > SHORT_POLLING_TIMES ? + LONG_POLLING_INTERVAL : SHORT_POLLING_INTERVAL); + } catch (InterruptedException exception) { + break; + } + } + + mHasBackgroundTask = false; + } + } + } +} diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java index 49fcddd4b604..55041478f105 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/DocumentLoaderTest.java @@ -43,7 +43,7 @@ public class DocumentLoaderTest extends AndroidTestCase { mLoader = new DocumentLoader(mManager, mResolver); } - public void testBasic() throws IOException, InterruptedException { + public void testBasic() throws Exception { final Uri uri = DocumentsContract.buildChildDocumentsUri( MtpDocumentsProvider.AUTHORITY, mParentIdentifier.toDocumentId()); setUpDocument(mManager, 40); diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java index e0e3ce643bb1..c1da59f5cb43 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java @@ -29,6 +29,8 @@ import java.util.Date; @SmallTest public class MtpDocumentsProviderTest extends AndroidTestCase { + private final static Uri ROOTS_URI = + DocumentsContract.buildRootsUri(MtpDocumentsProvider.AUTHORITY); private TestContentResolver mResolver; private MtpDocumentsProvider mProvider; private TestMtpManager mMtpManager; @@ -42,46 +44,71 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testOpenAndCloseDevice() throws Exception { - final Uri uri = DocumentsContract.buildRootsUri(MtpDocumentsProvider.AUTHORITY); - mMtpManager.addValidDevice(0); - assertEquals(0, mResolver.getChangeCount(uri)); + mMtpManager.setRoots(0, new MtpRoot[] { + new MtpRoot( + 0 /* deviceId */, + 1 /* storageId */, + "Storage A" /* volume description */, + 1024 /* free space */, + 2048 /* total space */, + "" /* no volume identifier */) + }); mProvider.openDevice(0); - assertEquals(1, mResolver.getChangeCount(uri)); + mResolver.waitForNotification(ROOTS_URI, 1); mProvider.closeDevice(0); - assertEquals(2, mResolver.getChangeCount(uri)); + mResolver.waitForNotification(ROOTS_URI, 2); + } - int exceptionCounter = 0; + public void testOpenAndCloseErrorDevice() throws Exception { try { mProvider.openDevice(1); - } catch (IOException error) { - exceptionCounter++; + fail(); + } catch (Throwable error) { + assertTrue(error instanceof IOException); } - assertEquals(2, mResolver.getChangeCount(uri)); + try { mProvider.closeDevice(1); - } catch (IOException error) { - exceptionCounter++; + fail(); + } catch (Throwable error) { + assertTrue(error instanceof IOException); } - assertEquals(2, mResolver.getChangeCount(uri)); - assertEquals(2, exceptionCounter); - } - - public void testCloseAllDevices() throws IOException { - final Uri uri = DocumentsContract.buildRootsUri(MtpDocumentsProvider.AUTHORITY); + // Check if the following notification is the first one or not. mMtpManager.addValidDevice(0); + mMtpManager.setRoots(0, new MtpRoot[] { + new MtpRoot( + 0 /* deviceId */, + 1 /* storageId */, + "Storage A" /* volume description */, + 1024 /* free space */, + 2048 /* total space */, + "" /* no volume identifier */) + }); + mProvider.openDevice(0); + mResolver.waitForNotification(ROOTS_URI, 1); + } + public void testCloseAllDevices() throws Exception { + mMtpManager.addValidDevice(0); + mMtpManager.setRoots(0, new MtpRoot[] { + new MtpRoot( + 0 /* deviceId */, + 1 /* storageId */, + "Storage A" /* volume description */, + 1024 /* free space */, + 2048 /* total space */, + "" /* no volume identifier */) + }); mProvider.closeAllDevices(); - assertEquals(0, mResolver.getChangeCount(uri)); - mProvider.openDevice(0); - assertEquals(1, mResolver.getChangeCount(uri)); + mResolver.waitForNotification(ROOTS_URI, 1); mProvider.closeAllDevices(); - assertEquals(2, mResolver.getChangeCount(uri)); + mResolver.waitForNotification(ROOTS_URI, 2); } public void testQueryRoots() throws Exception { @@ -89,6 +116,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { mMtpManager.addValidDevice(1); mMtpManager.setRoots(0, new MtpRoot[] { new MtpRoot( + 0 /* deviceId */, 1 /* storageId */, "Storage A" /* volume description */, 1024 /* free space */, @@ -97,16 +125,17 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { }); mMtpManager.setRoots(1, new MtpRoot[] { new MtpRoot( + 1 /* deviceId */, 1 /* storageId */, "Storage B" /* volume description */, 2048 /* free space */, 4096 /* total space */, "Identifier B" /* no volume identifier */) }); - assertEquals(0, mProvider.queryRoots(null).getCount()); { mProvider.openDevice(0); + mResolver.waitForNotification(ROOTS_URI, 1); final Cursor cursor = mProvider.queryRoots(null); assertEquals(1, cursor.getCount()); cursor.moveToNext(); @@ -121,6 +150,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { { mProvider.openDevice(1); + mResolver.waitForNotification(ROOTS_URI, 2); final Cursor cursor = mProvider.queryRoots(null); assertEquals(2, cursor.getCount()); cursor.moveToNext(); @@ -136,17 +166,19 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { { mProvider.closeAllDevices(); + mResolver.waitForNotification(ROOTS_URI, 3); final Cursor cursor = mProvider.queryRoots(null); assertEquals(0, cursor.getCount()); } } - public void testQueryRoots_error() throws IOException { + public void testQueryRoots_error() throws Exception { mMtpManager.addValidDevice(0); mMtpManager.addValidDevice(1); // Not set roots for device 0 so that MtpManagerMock#getRoots throws IOException. mMtpManager.setRoots(1, new MtpRoot[] { new MtpRoot( + 1 /* deviceId */, 1 /* storageId */, "Storage B" /* volume description */, 2048 /* free space */, @@ -156,6 +188,8 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { { mProvider.openDevice(0); mProvider.openDevice(1); + mResolver.waitForNotification(ROOTS_URI, 1); + final Cursor cursor = mProvider.queryRoots(null); assertEquals(1, cursor.getCount()); cursor.moveToNext(); @@ -195,6 +229,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { public void testQueryDocument_forRoot() throws IOException { mMtpManager.setRoots(0, new MtpRoot[] { new MtpRoot( + 0 /* deviceId */, 1 /* storageId */, "Storage A" /* volume description */, 1024 /* free space */, diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestContentResolver.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestContentResolver.java index 33e559f6c1d2..7e772c36d103 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestContentResolver.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/TestContentResolver.java @@ -25,17 +25,22 @@ import junit.framework.Assert; import java.util.HashMap; import java.util.Map; import java.util.concurrent.Phaser; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; class TestContentResolver extends MockContentResolver { - final private Map<Uri, Phaser> mPhasers = new HashMap<>(); + private static final int TIMEOUT_PERIOD_MS = 3000; + private final Map<Uri, Phaser> mPhasers = new HashMap<>(); @Override public void notifyChange(Uri uri, ContentObserver observer, boolean syncToNetwork) { getPhaser(uri).arrive(); } - void waitForNotification(Uri uri, int count) { - Assert.assertEquals(count, getPhaser(uri).awaitAdvance(count - 1)); + + void waitForNotification(Uri uri, int count) throws InterruptedException, TimeoutException { + Assert.assertEquals(count, getPhaser(uri).awaitAdvanceInterruptibly( + count - 1, TIMEOUT_PERIOD_MS, TimeUnit.MILLISECONDS)); } int getChangeCount(Uri uri) { |