From e0282dd7d409dce7738173162e766ce9317ef804 Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Thu, 26 Nov 2015 15:20:08 +0900 Subject: [PATCH] Reopen MTP devices when the provider is created. When the provider is created, it reopens MTP devices that are recorded in the database. It enables the provider to resume MTP session after the provider process is killed by the system. BUG=25704854 Change-Id: I58ae80fdb2e02cb0f045c63c4ade8943e2baae06 --- .../src/com/android/mtp/MtpDatabase.java | 33 +++++++ .../com/android/mtp/MtpDatabaseConstants.java | 2 +- .../com/android/mtp/MtpDocumentsProvider.java | 95 +++++++++++-------- .../com/android/mtp/MtpDocumentsService.java | 34 +++---- .../src/com/android/mtp/MtpManager.java | 1 + .../android/mtp/MtpDocumentsProviderTest.java | 51 +++++++++- 6 files changed, 150 insertions(+), 66 deletions(-) diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java index 4afffeacd2de5..00fe7a7e3debf 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabase.java @@ -22,6 +22,7 @@ import android.content.ContentValues; import android.content.Context; import android.content.res.Resources; import android.database.Cursor; +import android.database.DatabaseUtils; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteOpenHelper; import android.database.sqlite.SQLiteQueryBuilder; @@ -34,6 +35,7 @@ import android.provider.DocumentsContract.Root; import com.android.internal.annotations.VisibleForTesting; +import java.io.File; import java.io.FileNotFoundException; import java.util.Objects; @@ -305,6 +307,32 @@ class MtpDatabase { } } + /** + * Returns the set of device ID stored in the database. + */ + int[] getDeviceIds() { + final Cursor cursor = mDatabase.query( + true, + TABLE_DOCUMENTS, + strings(COLUMN_DEVICE_ID), + null, + null, + null, + null, + null, + null); + try { + final int[] ids = new int[cursor.getCount()]; + for (int i = 0; i < ids.length; i++) { + cursor.moveToNext(); + ids[i] = cursor.getInt(0); + } + return ids; + } finally { + cursor.close(); + } + } + private boolean deleteDocumentsAndRoots(String selection, String[] args) { mDatabase.beginTransaction(); try { @@ -351,6 +379,11 @@ class MtpDatabase { } } + @VisibleForTesting + static void deleteDatabase(Context context) { + context.deleteDatabase(DATABASE_NAME); + } + /** * Gets {@link ContentValues} for the given root. * @param values {@link ContentValues} that receives values. diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabaseConstants.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabaseConstants.java index b0286aa7fce88..0ead2d5331b74 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabaseConstants.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabaseConstants.java @@ -24,7 +24,7 @@ import android.provider.DocumentsContract.Root; */ class MtpDatabaseConstants { static final int DATABASE_VERSION = 1; - static final String DATABASE_NAME = null; + static final String DATABASE_NAME = "database"; static final int FLAG_DATABASE_IN_MEMORY = 1; static final int FLAG_DATABASE_IN_FILE = 0; diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java index 743f5837cc710..9511e15f21c80 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java @@ -20,7 +20,6 @@ import android.content.ContentResolver; import android.content.res.AssetFileDescriptor; import android.content.res.Resources; import android.database.Cursor; -import android.database.MatrixCursor; import android.graphics.Point; import android.media.MediaFile; import android.mtp.MtpConstants; @@ -58,11 +57,13 @@ public class MtpDocumentsProvider extends DocumentsProvider { Document.COLUMN_FLAGS, Document.COLUMN_SIZE, }; + private final Object mDeviceListLock = new Object(); + private static MtpDocumentsProvider sSingleton; private MtpManager mMtpManager; private ContentResolver mResolver; - @GuardedBy("mDeviceToolkits") + @GuardedBy("mDeviceListLock") private Map mDeviceToolkits; private RootScanner mRootScanner; private Resources mResources; @@ -84,6 +85,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { mDeviceToolkits = new HashMap(); mDatabase = new MtpDatabase(getContext(), MtpDatabaseConstants.FLAG_DATABASE_IN_FILE); mRootScanner = new RootScanner(mResolver, mResources, mMtpManager, mDatabase); + resume(); return true; } @@ -99,6 +101,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { mDeviceToolkits = new HashMap(); mDatabase = database; mRootScanner = new RootScanner(mResolver, mResources, mMtpManager, mDatabase); + resume(); } @Override @@ -196,7 +199,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { @Override public void onTrimMemory(int level) { - synchronized (mDeviceToolkits) { + synchronized (mDeviceListLock) { for (final DeviceToolkit toolkit : mDeviceToolkits.values()) { toolkit.mDocumentLoader.clearCompletedTasks(); } @@ -234,47 +237,26 @@ public class MtpDocumentsProvider extends DocumentsProvider { } void openDevice(int deviceId) throws IOException { - synchronized (mDeviceToolkits) { + synchronized (mDeviceListLock) { mMtpManager.openDevice(deviceId); - mDeviceToolkits.put(deviceId, new DeviceToolkit(mMtpManager, mResolver, mDatabase)); + mDeviceToolkits.put( + deviceId, new DeviceToolkit(mMtpManager, mResolver, mDatabase)); } mRootScanner.resume(); } void closeDevice(int deviceId) throws IOException, InterruptedException { - // TODO: Flush the device before closing (if not closed externally). - synchronized (mDeviceToolkits) { - getDeviceToolkit(deviceId).mDocumentLoader.clearTasks(); - mDeviceToolkits.remove(deviceId); + synchronized (mDeviceListLock) { + closeDeviceInternal(deviceId); mDatabase.removeDeviceRows(deviceId); - mMtpManager.closeDevice(deviceId); } mRootScanner.notifyChange(); - if (!hasOpenedDevices()) { - mRootScanner.pause(); - } - } - - synchronized void closeAllDevices() throws InterruptedException { - boolean closed = false; - for (int deviceId : mMtpManager.getOpenedDeviceIds()) { - try { - mDatabase.removeDeviceRows(deviceId); - mMtpManager.closeDevice(deviceId); - getDeviceToolkit(deviceId).mDocumentLoader.clearTasks(); - closed = true; - } catch (IOException d) { - Log.d(TAG, "Failed to close the MTP device: " + deviceId); - } - } - if (closed) { - mRootScanner.notifyChange(); - mRootScanner.pause(); - } } boolean hasOpenedDevices() { - return mMtpManager.getOpenedDeviceIds().length != 0; + synchronized (mDeviceListLock) { + return mMtpManager.getOpenedDeviceIds().length != 0; + } } /** @@ -282,14 +264,18 @@ public class MtpDocumentsProvider extends DocumentsProvider { */ @Override public void shutdown() { - try { - closeAllDevices(); - } catch (InterruptedException e) { - // It should fail unit tests by throwing runtime exception. - throw new RuntimeException(e.getMessage()); - } finally { - mDatabase.close(); - super.shutdown(); + synchronized (mDeviceListLock) { + try { + for (final int id : mMtpManager.getOpenedDeviceIds()) { + closeDeviceInternal(id); + } + } catch (InterruptedException|IOException e) { + // It should fail unit tests by throwing runtime exception. + throw new RuntimeException(e); + } finally { + mDatabase.close(); + super.shutdown(); + } } } @@ -300,8 +286,35 @@ public class MtpDocumentsProvider extends DocumentsProvider { false); } + /** + * Reopens MTP devices based on database state. + */ + private void resume() { + synchronized (mDeviceListLock) { + mDatabase.getMapper().clearMapping(); + final int[] ids = mDatabase.getDeviceIds(); + for (final int id : ids) { + try { + openDevice(id); + } catch (IOException exception) { + mDatabase.removeDeviceRows(id); + } + } + } + } + + private void closeDeviceInternal(int deviceId) throws IOException, InterruptedException { + // TODO: Flush the device before closing (if not closed externally). + getDeviceToolkit(deviceId).mDocumentLoader.clearTasks(); + mDeviceToolkits.remove(deviceId); + mMtpManager.closeDevice(deviceId); + if (!hasOpenedDevices()) { + mRootScanner.pause(); + } + } + private DeviceToolkit getDeviceToolkit(int deviceId) throws FileNotFoundException { - synchronized (mDeviceToolkits) { + synchronized (mDeviceListLock) { final DeviceToolkit toolkit = mDeviceToolkits.get(deviceId); if (toolkit == null) { throw new FileNotFoundException(); diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java index 723dc149c15db..9b3c20fdefbe6 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java @@ -55,22 +55,20 @@ public class MtpDocumentsService extends Service { @Override public int onStartCommand(Intent intent, int flags, int startId) { - if (intent == null) { - // If intent is null, the service was restarted. - // TODO: Recover opened devices here. - return START_STICKY; - } - if (intent.getAction().equals(ACTION_OPEN_DEVICE)) { - final UsbDevice device = intent.getParcelableExtra(EXTRA_DEVICE); - try { - final MtpDocumentsProvider provider = MtpDocumentsProvider.getInstance(); - provider.openDevice(device.getDeviceId()); - return START_STICKY; - } catch (IOException error) { - Log.e(MtpDocumentsProvider.TAG, error.getMessage()); + // If intent is null, the service was restarted. + if (intent != null) { + if (intent.getAction().equals(ACTION_OPEN_DEVICE)) { + final UsbDevice device = intent.getParcelableExtra(EXTRA_DEVICE); + try { + final MtpDocumentsProvider provider = MtpDocumentsProvider.getInstance(); + provider.openDevice(device.getDeviceId()); + return START_STICKY; + } catch (IOException error) { + Log.e(MtpDocumentsProvider.TAG, error.getMessage()); + } + } else { + Log.e(MtpDocumentsProvider.TAG, "Received unknown intent action."); } - } else { - Log.w(MtpDocumentsProvider.TAG, "Received unknown intent action."); } stopSelfIfNeeded(); return Service.START_NOT_STICKY; @@ -78,14 +76,8 @@ public class MtpDocumentsService extends Service { @Override public void onDestroy() { - final MtpDocumentsProvider provider = MtpDocumentsProvider.getInstance(); unregisterReceiver(mReceiver); mReceiver = null; - try { - provider.closeAllDevices(); - } catch (InterruptedException e) { - Log.e(MtpDocumentsProvider.TAG, e.getMessage()); - } super.onDestroy(); } diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java index 714936dff1983..cd52f31bf0467 100644 --- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java +++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpManager.java @@ -32,6 +32,7 @@ import com.android.internal.annotations.VisibleForTesting; import java.io.FileNotFoundException; import java.io.IOException; +import java.util.ArrayList; /** * The model wrapping android.mtp API. diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java index 7a53a90481e05..b20b3bb134d57 100644 --- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java +++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java @@ -23,6 +23,7 @@ import android.net.Uri; import android.provider.DocumentsContract.Root; import android.provider.DocumentsContract; import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.MediumTest; import android.test.suitebuilder.annotation.SmallTest; import java.io.FileNotFoundException; @@ -45,17 +46,16 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { public void setUp() throws IOException { mResolver = new TestContentResolver(); mMtpManager = new TestMtpManager(getContext()); - mProvider = new MtpDocumentsProvider(); - mDatabase = new MtpDatabase(getContext(), MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); - mProvider.onCreateForTesting(mResources, mMtpManager, mResolver, mDatabase); } @Override public void tearDown() { mProvider.shutdown(); + MtpDatabase.deleteDatabase(getContext()); } public void testOpenAndCloseDevice() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mMtpManager.setRoots(0, new MtpRoot[] { new MtpRoot( @@ -76,6 +76,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testOpenAndCloseErrorDevice() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); try { mProvider.openDevice(1); fail(); @@ -107,6 +108,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testQueryRoots() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mMtpManager.addValidDevice(1); mMtpManager.setRoots(0, new MtpRoot[] { @@ -163,6 +165,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testQueryRoots_error() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mMtpManager.addValidDevice(1); // Not set roots for device 0 so that MtpManagerMock#getRoots throws IOException. @@ -195,6 +198,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testQueryDocument() throws IOException, InterruptedException, TimeoutException { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); @@ -234,6 +238,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { public void testQueryDocument_directory() throws IOException, InterruptedException, TimeoutException { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); @@ -271,6 +276,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { public void testQueryDocument_forRoot() throws IOException, InterruptedException, TimeoutException { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); @@ -297,6 +303,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testQueryChildDocuments() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); @@ -332,6 +339,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testQueryChildDocuments_cursorError() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); try { @@ -343,6 +351,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testQueryChildDocuments_documentError() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); setupRoots(0, new MtpRoot[] { new MtpRoot(0, 0, "Device", "Storage", 1000, 1000, "") }); @@ -356,6 +365,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { } public void testDeleteDocument() throws IOException, InterruptedException, TimeoutException { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); setupRoots(0, new MtpRoot[] { @@ -377,6 +387,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { public void testDeleteDocument_error() throws IOException, InterruptedException, TimeoutException { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_MEMORY); mMtpManager.addValidDevice(0); mProvider.openDevice(0); setupRoots(0, new MtpRoot[] { @@ -400,6 +411,40 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { MtpDocumentsProvider.AUTHORITY, "1"))); } + @MediumTest + public void testPauseAndResume() throws Exception { + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_FILE); + mMtpManager.addValidDevice(0); + mProvider.openDevice(0); + setupRoots(0, new MtpRoot[] { new MtpRoot(0, 0, "Device", "Storage", 0, 0, "")}); + + { + final Cursor cursor = mProvider.queryRoots( + strings(DocumentsContract.Root.COLUMN_ROOT_ID)); + cursor.moveToNext(); + assertEquals(1, cursor.getInt(0)); + } + + mProvider.shutdown(); + setupProvider(MtpDatabaseConstants.FLAG_DATABASE_IN_FILE); + + { + // We can still fetch roots after relaunching the provider. + final Cursor cursor = mProvider.queryRoots( + strings(DocumentsContract.Root.COLUMN_ROOT_ID)); + assertEquals(1, cursor.getCount()); + cursor.moveToNext(); + assertEquals(1, cursor.getInt(0)); + assertEquals(1, mMtpManager.getOpenedDeviceIds().length); + } + } + + private void setupProvider(int flag) { + mDatabase = new MtpDatabase(getContext(), flag); + mProvider = new MtpDocumentsProvider(); + mProvider.onCreateForTesting(mResources, mMtpManager, mResolver, mDatabase); + } + private String[] getStrings(Cursor cursor) { try { final String[] results = new String[cursor.getCount()];