unfinished =
+ new ArrayList<>(unfinishedCopies.size() + unfinishedDeletions.size());
+ unfinished.addAll(unfinishedCopies);
+ unfinished.addAll(unfinishedDeletions);
if (!unfinished.isEmpty()) {
Log.w(TAG, "Shutting down, but executor reports running jobs: " + unfinished);
}
+
executor = null;
+ deletionExecutor = null;
+ handler = null;
if (DEBUG) Log.d(TAG, "Destroyed.");
}
@@ -154,7 +173,6 @@ public class FileOperationService extends Service implements Job.Listener {
// Track the service supplied id so we can stop the service once we're out of work to do.
mLastServiceId = serviceId;
- Job job = null;
synchronized (mRunning) {
if (mWakeLock == null) {
mWakeLock = mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
@@ -164,7 +182,7 @@ public class FileOperationService extends Service implements Job.Listener {
DocumentInfo srcParent = intent.getParcelableExtra(EXTRA_SRC_PARENT);
DocumentStack stack = intent.getParcelableExtra(Shared.EXTRA_STACK);
- job = createJob(operationType, jobId, srcs, srcParent, stack);
+ Job job = createJob(operationType, jobId, srcs, srcParent, stack);
if (job == null) {
return;
@@ -301,40 +319,45 @@ public class FileOperationService extends Service implements Job.Listener {
@Override
public void onStart(Job job) {
if (DEBUG) Log.d(TAG, "onStart: " + job.id);
- mNotificationManager.notify(job.id, NOTIFICATION_ID_PROGRESS, job.getSetupNotification());
+
+ // Show start up notification
+ mNotificationManager.notify(
+ job.id, NOTIFICATION_ID_PROGRESS, job.getSetupNotification());
+
+ // Set up related monitor
+ JobMonitor monitor = new JobMonitor(job, mNotificationManager, handler);
+ monitor.start();
}
@Override
public void onFinished(Job job) {
+ assert(job.isFinished());
if (DEBUG) Log.d(TAG, "onFinished: " + job.id);
- // Dismiss the ongoing copy notification when the copy is done.
- mNotificationManager.cancel(job.id, NOTIFICATION_ID_PROGRESS);
+ // Use the same thread of monitors to tackle notifications to avoid race conditions.
+ // Otherwise we may fail to dismiss progress notification.
+ handler.post(() -> {
+ // Dismiss the ongoing copy notification when the copy is done.
+ mNotificationManager.cancel(job.id, NOTIFICATION_ID_PROGRESS);
- if (job.hasFailures()) {
- Log.e(TAG, "Job failed on files: " + job.failedFiles.size() + ".");
- mNotificationManager.notify(
- job.id, NOTIFICATION_ID_FAILURE, job.getFailureNotification());
- }
+ if (job.hasFailures()) {
+ Log.e(TAG, "Job failed on files: " + job.failedFiles.size() + ".");
+ mNotificationManager.notify(
+ job.id, NOTIFICATION_ID_FAILURE, job.getFailureNotification());
+ }
- if (job.hasWarnings()) {
- if (DEBUG) Log.d(TAG, "Job finished with warnings.");
- mNotificationManager.notify(
- job.id, NOTIFICATION_ID_WARNING, job.getWarningNotification());
- }
+ if (job.hasWarnings()) {
+ if (DEBUG) Log.d(TAG, "Job finished with warnings.");
+ mNotificationManager.notify(
+ job.id, NOTIFICATION_ID_WARNING, job.getWarningNotification());
+ }
+ });
synchronized (mRunning) {
deleteJob(job);
}
}
- @Override
- public void onProgress(CopyJob job) {
- if (DEBUG) Log.d(TAG, "onProgress: " + job.id);
- mNotificationManager.notify(
- job.id, NOTIFICATION_ID_PROGRESS, job.getProgressNotification());
- }
-
private static final class JobRecord {
private final Job job;
private final Future future;
@@ -345,6 +368,47 @@ public class FileOperationService extends Service implements Job.Listener {
}
}
+ /**
+ * A class used to periodically polls state of a job.
+ *
+ * It's possible that jobs hang because underlying document providers stop responding. We
+ * still need to update notifications if jobs hang, so instead of jobs pushing their states,
+ * we poll states of jobs.
+ */
+ private static final class JobMonitor implements Runnable {
+ private static final long INITIAL_PROGRESS_DELAY_MILLIS = 10L;
+ private static final long PROGRESS_INTERVAL_MILLIS = 500L;
+
+ private final Job mJob;
+ private final NotificationManager mNotificationManager;
+ private final Handler mHandler;
+
+ private JobMonitor(Job job, NotificationManager notificationManager, Handler handler) {
+ mJob = job;
+ mNotificationManager = notificationManager;
+ mHandler = handler;
+ }
+
+ private void start() {
+ // Delay the first update to avoid dividing by 0 when calculate speed
+ mHandler.postDelayed(this, INITIAL_PROGRESS_DELAY_MILLIS);
+ }
+
+ @Override
+ public void run() {
+ if (mJob.isFinished()) {
+ // Finish notification is already shown. Progress notification is removed.
+ // Just finish itself.
+ return;
+ }
+
+ mNotificationManager.notify(
+ mJob.id, NOTIFICATION_ID_PROGRESS, mJob.getProgressNotification());
+
+ mHandler.postDelayed(this, PROGRESS_INTERVAL_MILLIS);
+ }
+ }
+
@Override
public IBinder onBind(Intent intent) {
return null; // Boilerplate. See super#onBind
diff --git a/packages/DocumentsUI/src/com/android/documentsui/services/Job.java b/packages/DocumentsUI/src/com/android/documentsui/services/Job.java
index b8f8fba72d62d..fc3a73171add2 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/services/Job.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/services/Job.java
@@ -25,6 +25,7 @@ import static com.android.documentsui.services.FileOperationService.EXTRA_SRC_LI
import static com.android.documentsui.services.FileOperationService.OPERATION_UNKNOWN;
import android.annotation.DrawableRes;
+import android.annotation.IntDef;
import android.annotation.PluralsRes;
import android.app.Notification;
import android.app.Notification.Builder;
@@ -48,6 +49,8 @@ import com.android.documentsui.model.DocumentInfo;
import com.android.documentsui.model.DocumentStack;
import com.android.documentsui.services.FileOperationService.OpType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -60,6 +63,18 @@ import java.util.Map;
abstract public class Job implements Runnable {
private static final String TAG = "Job";
+ @Retention(RetentionPolicy.SOURCE)
+ @IntDef({STATE_CREATED, STATE_STARTED, STATE_COMPLETED, STATE_CANCELED})
+ @interface State {}
+ static final int STATE_CREATED = 0;
+ static final int STATE_STARTED = 1;
+ static final int STATE_COMPLETED = 2;
+ /**
+ * A job is in canceled state as long as {@link #cancel()} is called on it, even after it is
+ * completed.
+ */
+ static final int STATE_CANCELED = 3;
+
static final String INTENT_TAG_WARNING = "warning";
static final String INTENT_TAG_FAILURE = "failure";
static final String INTENT_TAG_PROGRESS = "progress";
@@ -77,7 +92,7 @@ abstract public class Job implements Runnable {
final Notification.Builder mProgressBuilder;
private final Map mClients = new HashMap<>();
- private volatile boolean mCanceled;
+ private volatile @State int mState = STATE_CREATED;
/**
* A simple progressable job, much like an AsyncTask, but with support
@@ -111,6 +126,12 @@ abstract public class Job implements Runnable {
@Override
public final void run() {
+ if (isCanceled()) {
+ // Canceled before running
+ return;
+ }
+
+ mState = STATE_STARTED;
listener.onStart(this);
try {
start();
@@ -120,6 +141,7 @@ abstract public class Job implements Runnable {
Log.e(TAG, "Operation failed due to an unhandled runtime exception.", e);
Metrics.logFileOperationErrors(service, operationType, failedFiles);
} finally {
+ mState = (mState == STATE_STARTED) ? STATE_COMPLETED : mState;
listener.onFinished(this);
}
}
@@ -127,8 +149,7 @@ abstract public class Job implements Runnable {
abstract void start();
abstract Notification getSetupNotification();
- // TODO: Progress notification for deletes.
- // abstract Notification getProgressNotification(long bytesCopied);
+ abstract Notification getProgressNotification();
abstract Notification getFailureNotification();
abstract Notification getWarningNotification();
@@ -158,13 +179,21 @@ abstract public class Job implements Runnable {
}
}
+ final @State int getState() {
+ return mState;
+ }
+
final void cancel() {
- mCanceled = true;
+ mState = STATE_CANCELED;
Metrics.logFileOperationCancelled(service, operationType);
}
final boolean isCanceled() {
- return mCanceled;
+ return mState == STATE_CANCELED;
+ }
+
+ final boolean isFinished() {
+ return mState == STATE_CANCELED || mState == STATE_COMPLETED;
}
final ContentResolver getContentResolver() {
@@ -321,6 +350,5 @@ abstract public class Job implements Runnable {
interface Listener {
void onStart(Job job);
void onFinished(Job job);
- void onProgress(CopyJob job);
}
}
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java
index f385776cad4cb..9d6e1d7e26e84 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/services/FileOperationServiceTest.java
@@ -20,6 +20,7 @@ import static com.android.documentsui.services.FileOperationService.OPERATION_CO
import static com.android.documentsui.services.FileOperationService.OPERATION_DELETE;
import static com.android.documentsui.services.FileOperations.createBaseIntent;
import static com.android.documentsui.services.FileOperations.createJobId;
+
import static com.google.android.collect.Lists.newArrayList;
import android.content.Context;
@@ -31,13 +32,11 @@ import android.test.suitebuilder.annotation.MediumTest;
import com.android.documentsui.model.DocumentInfo;
import com.android.documentsui.model.DocumentStack;
import com.android.documentsui.services.Job.Listener;
+import com.android.documentsui.testing.TestHandler;
import java.util.ArrayList;
import java.util.List;
-/**
- * TODO: Test progress updates.
- */
@MediumTest
public class FileOperationServiceTest extends ServiceTestCase {
@@ -49,6 +48,7 @@ public class FileOperationServiceTest extends ServiceTestCase files, DocumentInfo dest)
@@ -217,10 +245,21 @@ public class FileOperationServiceTest extends ServiceTestCase copyJobs = new ArrayList<>();
- final List deleteJobs = new ArrayList<>();
+ private final List copyJobs = new ArrayList<>();
+ private final List deleteJobs = new ArrayList<>();
+
+ private Runnable mJobRunnable = () -> {
+ // The following statement is executed concurrently to Job.start() in real situation.
+ // Call it in TestJob.start() to mimic this behavior.
+ mHandler.dispatchNextMessage();
+ };
void assertAllCopyJobsStarted() {
for (TestJob job : copyJobs) {
@@ -258,7 +297,8 @@ public class FileOperationServiceTest extends ServiceTestCase iter = mTaskList.listIterator(0);
- while (iter.hasNext()) {
- if (iter.next().mExecuteTime >= executeTime) {
- break;
- }
- }
- iter.add(testTimerTask);
+ scheduleAtTime(task, executeTime);
}
@Override
@@ -106,6 +109,19 @@ public class TestTimer extends Timer {
throw new UnsupportedOperationException();
}
+ public void scheduleAtTime(TimerTask task, long executeTime) {
+ Task testTimerTask = (Task) task;
+ testTimerTask.mExecuteTime = executeTime;
+
+ ListIterator iter = mTaskList.listIterator(0);
+ while (iter.hasNext()) {
+ if (iter.next().mExecuteTime >= executeTime) {
+ break;
+ }
+ }
+ iter.add(testTimerTask);
+ }
+
public static class Task extends TimerTask {
private boolean mIsCancelled;
private long mExecuteTime;