Regard timeout as an error in the MtpDocumentsProvider test.

Previously if DocumentsProvider found timeout when terminatnig
RootScanner's background thread, it just output it in error log. Thus
the timeout is not regarded as an error in MtpDocumentsProviderTest, and
it makes flaky PipeManagerTest which runs just after
MtpDocumentsProviderTest.

The CL

 * lets MtpDocumentsProvider throw TimeoutException for timeout.
 * removes redundant resumeRootScanner calls to avoid timeout of
   RootScanner#pause.

Also the CL did cleanup the logic that pauses RootScanner when we don't
find any devices. Previously the logic was in
MtpDocumentsProvider#closeInternal but it is not efficient because we
invokes RootScanner#resume just after
MtpDocumentsProvider#closeInternal. Now the CL moves the logic to
RootScanner so that it can pause itself automatically.

BUG=27638500

Change-Id: Ic11bca67c099cbb0f46679db2f035988045d67d6

4 files changed, 15 insertions, 11 deletions

diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
index d4d45912aecc..58ff401d5a71 100644
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
@@ -36,7 +36,6 @@ import android.provider.DocumentsContract.Root;
 import android.provider.DocumentsContract;
 import android.provider.DocumentsProvider;
 import android.provider.Settings;
-import android.provider.Settings.SettingNotFoundException;
 import android.util.Log;
 
 import com.android.internal.annotations.GuardedBy;
@@ -48,6 +47,7 @@ import java.io.IOException;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeoutException;
 
 /**
  * DocumentsProvider for MTP devices.
@@ -426,7 +426,7 @@ public class MtpDocumentsProvider extends DocumentsProvider {
                     closeDeviceInternal(id);
                 }
                 mRootScanner.pause();
-            } catch (InterruptedException|IOException e) {
+            } catch (InterruptedException | IOException | TimeoutException e) {
                 // It should fail unit tests by throwing runtime exception.
                 throw new RuntimeException(e);
             } finally {
@@ -464,9 +464,6 @@ public class MtpDocumentsProvider extends DocumentsProvider {
         getDeviceToolkit(deviceId).close();
         mDeviceToolkits.remove(deviceId);
         mMtpManager.closeDevice(deviceId);
-        if (mDeviceToolkits.size() == 0) {
-            mRootScanner.pause();
-        }
     }
 
     private DeviceToolkit getDeviceToolkit(int deviceId) throws FileNotFoundException {
diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java b/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java
index 2f66c5cf17d7..2e9133bcb92f 100644
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java
@@ -27,6 +27,7 @@ import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 final class RootScanner {
     /**
@@ -95,15 +96,19 @@ final class RootScanner {
      * Stops background thread and wait for its termination.
      * @throws InterruptedException
      */
-    synchronized void pause() throws InterruptedException {
+    synchronized void pause() throws InterruptedException, TimeoutException {
         if (mExecutor == null) {
             return;
         }
         mExecutor.shutdownNow();
-        if (!mExecutor.awaitTermination(AWAIT_TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS)) {
-            Log.e(MtpDocumentsProvider.TAG, "Failed to terminate RootScanner's background thread.");
+        try {
+            if (!mExecutor.awaitTermination(AWAIT_TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS)) {
+                throw new TimeoutException(
+                        "Timeout for terminating RootScanner's background thread.");
+            }
+        } finally {
+            mExecutor = null;
         }
-        mExecutor = null;
     }
 
     /**
@@ -173,6 +178,9 @@ final class RootScanner {
                 }
                 mFirstScanCompleted.countDown();
                 pollingCount++;
+                if (devices.length == 0) {
+                    break;
+                }
                 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.
diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
index d6ad0f3c630c..afcb45716c59 100644
--- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
+++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
@@ -262,11 +262,9 @@ public class MtpDocumentsProviderTest extends AndroidTestCase {
                 null));
         {
             mProvider.openDevice(0);
-            mProvider.resumeRootScanner();
             mResolver.waitForNotification(ROOTS_URI, 1);
 
             mProvider.openDevice(1);
-            mProvider.resumeRootScanner();
             mResolver.waitForNotification(ROOTS_URI, 2);
 
             final Cursor cursor = mProvider.queryRoots(null);
diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java
index cdddd8178dc9..a08d9ee39a44 100644
--- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java
+++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java
@@ -48,6 +48,7 @@ public class PipeManagerTest extends AndroidTestCase {
     @Override
     protected void tearDown() throws Exception {
         assertTrue(mPipeManager.close());
+        mDatabase.close();
     }
 
     public void testReadDocument_basic() throws Exception {