commit 3eab82e7e22205c21431fffda067b98b61cc82ad
author Garfield Tan <xutan@google.com> Fri Jan 13 16:51:36 2017 -0800
committer Garfield Tan <xutan@google.com> Tue Jan 17 19:15:55 2017 +0000
tree 3e86fc8cb07ba7aae03e7b24066e0d95bd70bb14
parent e77419c6b4aecf4eb71b49898d6b63034a0771e4

Fix the use of WakeLock.

WakeLock is reference counted, so we need to call release() once for
each acquire(). Otherwise when WakeLock is GC'ed it kills DocumentsUI
by Log.wtf(). Unit tests will then fail to run on eng build.

Test: Tests pass.
Change-Id: I98889e1d99c9b814abf0746de1cfbc59737f05d2
(cherry picked from commit 8b98ab7dba92ec3c9bfe293e7ba24359abd6ddcb)

src/com/android/documentsui/services/FileOperationService.java

diff --git a/src/com/android/documentsui/services/FileOperationService.java b/src/com/android/documentsui/services/FileOperationService.java
index d8485ba..3b0e5d2 100644
--- a/src/com/android/documentsui/services/FileOperationService.java
+++ b/src/com/android/documentsui/services/FileOperationService.java
@@ -183,12 +183,12 @@
                 return;
             }
 
-            mWakeLock.acquire();
-
             assert (job != null);
             if (DEBUG) Log.d(TAG, "Scheduling job " + job.id + ".");
             Future<?> future = getExecutorService(operation.getOpType()).submit(job);
             mRunning.put(jobId, new JobRecord(job, future));
+
+            mWakeLock.acquire();
         }
     }
 
@@ -245,6 +245,11 @@
         assert(record != null);
         record.job.cleanup();
 
+        mWakeLock.release();
+        if (!mWakeLock.isHeld()) {
+            mWakeLock = null;
+        }
+
         if (mRunning.isEmpty()) {
             shutdown();
         }
@@ -256,8 +261,6 @@
      */
     private void shutdown() {
         if (DEBUG) Log.d(TAG, "Shutting down. Last serviceId was " + mLastServiceId);
-        mWakeLock.release();
-        mWakeLock = null;
 
         // Turns out, for us, stopSelfResult always returns false in tests,
         // so we can't guard executor shutdown. For this reason we move

tests/common/com/android/documentsui/testing/TestScheduledExecutorService.java

diff --git a/tests/common/com/android/documentsui/testing/TestScheduledExecutorService.java b/tests/common/com/android/documentsui/testing/TestScheduledExecutorService.java
index f5001ee..ef4f545 100644
--- a/tests/common/com/android/documentsui/testing/TestScheduledExecutorService.java
+++ b/tests/common/com/android/documentsui/testing/TestScheduledExecutorService.java
@@ -21,6 +21,7 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Delayed;
@@ -137,8 +138,11 @@
     }
 
     public void runAll() {
-        for (TestFuture future : scheduled) {
+        final Iterator<TestFuture> iter = scheduled.iterator();
+        while (iter.hasNext()) {
+            TestFuture future = iter.next();
             future.runnable.run();
+            iter.remove();
         }
     }
 

tests/unit/com/android/documentsui/services/FileOperationServiceTest.java

diff --git a/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java b/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
index 3c4d74a..56ddae8 100644
--- a/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
+++ b/tests/unit/com/android/documentsui/services/FileOperationServiceTest.java
@@ -90,9 +90,15 @@
 
     @Override
     protected void tearDown() {
+        // Release all possibly held wake lock here
+        mExecutor.runAll();
+        mDeletionExecutor.runAll();
+
         // There are lots of progress notifications generated in this test case.
         // Dismiss all of them here.
-        mHandler.dispatchAllMessages();
+        while (mHandler.hasScheduledMessage()) {
+            mHandler.dispatchAllMessages();
+        }
     }
 
     public void testRunsCopyJobs() throws Exception {