Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8167ebe4 authored by Garfield Tan's avatar Garfield Tan
Browse files

Stick FileOperationService to foreground.

Note there is still a small window between service is started and our
service enters foreground mode because originally we only show
notifications once job has started. Therefore our service is still a
background service before the first job starts to set up.

Bug: 34512210
Test: Automatic tests pass. Some manual tests.
Change-Id: I33987c1da861bb7583d12a21547f44c77de860d8
(cherry picked from commit 0bed4ce4)
parent 05376628
Loading
Loading
Loading
Loading
+129 −41
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.documentsui.services;
import static com.android.documentsui.base.Shared.DEBUG;

import android.annotation.IntDef;
import android.app.Notification;
import android.app.NotificationManager;
import android.app.Service;
import android.content.Intent;
@@ -37,12 +38,14 @@ import java.util.Map;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicReference;

import javax.annotation.concurrent.GuardedBy;

public class FileOperationService extends Service implements Job.Listener {

    private static final int POOL_SIZE = 2;  // "pool size", not *max* "pool size".

    private static final int NOTIFICATION_ID_PROGRESS = 0;
    private static final int NOTIFICATION_ID_FAILURE = 1;
    private static final int NOTIFICATION_ID_WARNING = 2;
@@ -91,12 +94,20 @@ public class FileOperationService extends Service implements Job.Listener {
    // Use a handler to schedule monitor tasks.
    @VisibleForTesting Handler handler;

    @GuardedBy("mRunning")
    private final Map<String, JobRecord> mRunning = new HashMap<>();
    // Use a foreground manager to change foreground state of this service.
    @VisibleForTesting ForegroundManager foregroundManager;

    // Use a notification manager to post and cancel notifications for jobs.
    @VisibleForTesting NotificationManager notificationManager;

    @GuardedBy("mJobs")
    private final Map<String, JobRecord> mJobs = new HashMap<>();

    // The job whose notification is used to keep the service in foreground mode.
    private final AtomicReference<Job> mForegroundJob = new AtomicReference<>();

    private PowerManager mPowerManager;
    private PowerManager.WakeLock mWakeLock;  // the wake lock, if held.
    private NotificationManager mNotificationManager;

    private int mLastServiceId;

@@ -116,9 +127,16 @@ public class FileOperationService extends Service implements Job.Listener {
            handler = new Handler();
        }

        if (foregroundManager == null) {
            foregroundManager = createForegroundManager(this);
        }

        if (notificationManager == null) {
            notificationManager = getSystemService(NotificationManager.class);
        }

        if (DEBUG) Log.d(TAG, "Created.");
        mPowerManager = getSystemService(PowerManager.class);
        mNotificationManager = getSystemService(NotificationManager.class);
    }

    @Override
@@ -166,12 +184,12 @@ public class FileOperationService extends Service implements Job.Listener {
    }

    private void handleOperation(String jobId, FileOperation operation) {
        synchronized (mRunning) {
        synchronized (mJobs) {
            if (mWakeLock == null) {
                mWakeLock = mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
            }

            if (mRunning.containsKey(jobId)) {
            if (mJobs.containsKey(jobId)) {
                Log.w(TAG, "Duplicate job id: " + jobId
                        + ". Ignoring job request for operation: " + operation + ".");
                return;
@@ -186,10 +204,10 @@ public class FileOperationService extends Service implements Job.Listener {
            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));
            mJobs.put(jobId, new JobRecord(job, future));

            // Acquire wake lock to keep CPU running until we finish all jobs. Acquire wake lock
            // after we create a job and put it in mRunning to avoid potential leaking of wake lock
            // after we create a job and put it in mJobs to avoid potential leaking of wake lock
            // in case where job creation fails.
            mWakeLock.acquire();
        }
@@ -208,12 +226,12 @@ public class FileOperationService extends Service implements Job.Listener {

        if (DEBUG) Log.d(TAG, "handleCancel: " + jobId);

        synchronized (mRunning) {
        synchronized (mJobs) {
            // Do nothing if the cancelled ID doesn't match the current job ID. This prevents racey
            // cancellation requests from affecting unrelated copy jobs.  However, if the current job ID
            // is null, the service most likely crashed and was revived by the incoming cancel intent.
            // In that case, always allow the cancellation to proceed.
            JobRecord record = mRunning.get(jobId);
            JobRecord record = mJobs.get(jobId);
            if (record != null) {
                record.job.cancel();
            }
@@ -223,7 +241,7 @@ public class FileOperationService extends Service implements Job.Listener {
        // interactivity for the user in case the copy loop is stalled.
        // Try to cancel it even if we don't have a job id...in case there is some sad
        // orphan notification.
        mNotificationManager.cancel(jobId, NOTIFICATION_ID_PROGRESS);
        notificationManager.cancel(jobId, NOTIFICATION_ID_PROGRESS);

        // TODO: Guarantee the job is being finalized
    }
@@ -240,7 +258,7 @@ public class FileOperationService extends Service implements Job.Listener {
        }
    }

    @GuardedBy("mRunning")
    @GuardedBy("mJobs")
    private void deleteJob(Job job) {
        if (DEBUG) Log.d(TAG, "deleteJob: " + job.id);

@@ -250,13 +268,12 @@ public class FileOperationService extends Service implements Job.Listener {
            mWakeLock = null;
        }

        JobRecord record = mRunning.remove(job.id);
        JobRecord record = mJobs.remove(job.id);
        assert(record != null);
        record.job.cleanup();

        if (mRunning.isEmpty()) {
            shutdown();
        }
        // Delay the shutdown until we've cleaned up all notifications. shutdown() is now posted in
        // onFinished(Job job) to main thread.
    }

    /**
@@ -286,12 +303,20 @@ public class FileOperationService extends Service implements Job.Listener {
    public void onStart(Job job) {
        if (DEBUG) Log.d(TAG, "onStart: " + job.id);

        Notification notification = job.getSetupNotification();
        // If there is no foreground job yet, set this job to foreground job.
        if (mForegroundJob.compareAndSet(null, job)) {
            if (DEBUG) Log.d(TAG, "Set foreground job to " + job.id);
            foregroundManager.startForeground(NOTIFICATION_ID_PROGRESS, notification);
        }

        // Show start up notification
        mNotificationManager.notify(
                job.id, NOTIFICATION_ID_PROGRESS, job.getSetupNotification());
        if (DEBUG) Log.d(TAG, "Posting notification for " + job.id);
        notificationManager.notify(
                job.id, NOTIFICATION_ID_PROGRESS, notification);

        // Set up related monitor
        JobMonitor monitor = new JobMonitor(job, mNotificationManager, handler);
        JobMonitor monitor = new JobMonitor(job, notificationManager, handler);
        monitor.start();
    }

@@ -300,11 +325,59 @@ public class FileOperationService extends Service implements Job.Listener {
        assert(job.isFinished());
        if (DEBUG) Log.d(TAG, "onFinished: " + job.id);

        synchronized (mJobs) {
            // Delete the job from mJobs first to avoid this job being selected as the foreground
            // task again if we need to swap the foreground job.
            deleteJob(job);

            // Update foreground state before cleaning up notification. If the finishing job is the
            // foreground job, we would need to switch to another one or go to background before
            // we can clean up notifications.
            updateForegroundState(job);

            // Use the same thread of monitors to tackle notifications to avoid race conditions.
            // Otherwise we may fail to dismiss progress notification.
        handler.post(() -> {
            handler.post(() -> cleanUpNotification(job));

            // Post the shutdown message to main thread after cleanUpNotification() to give it a
            // chance to run. Otherwise this process may be torn down by Android before we've
            // cleaned up the notifications of the last job.
            if (mJobs.isEmpty()) {
                handler.post(this::shutdown);
            }
        }
    }

    @GuardedBy("mJobs")
    private void updateForegroundState(Job job) {
        Job candidate = mJobs.isEmpty() ? null : mJobs.values().iterator().next().job;

        // If foreground job is retiring and there is still work to do, we need to set it to a new
        // job.
        if (mForegroundJob.compareAndSet(job, candidate)) {
            if (candidate == null) {
                if (DEBUG) Log.d(TAG, "Stop foreground");
                // Remove the notification here just in case we're torn down before we have the
                // chance to clean up notifications.
                foregroundManager.stopForeground(true);
            } else {
                if (DEBUG) Log.d(TAG, "Switch foreground job to " + candidate.id);

                Notification notification = (candidate.getState() == Job.STATE_STARTED)
                        ? candidate.getSetupNotification()
                        : candidate.getProgressNotification();
                foregroundManager.startForeground(NOTIFICATION_ID_PROGRESS, notification);
                notificationManager.notify(candidate.id, NOTIFICATION_ID_PROGRESS,
                        notification);
            }
        }
    }

    private void cleanUpNotification(Job job) {

        if (DEBUG) Log.d(TAG, "Canceling notification for " + job.id);
        // Dismiss the ongoing copy notification when the copy is done.
            mNotificationManager.cancel(job.id, NOTIFICATION_ID_PROGRESS);
        notificationManager.cancel(job.id, NOTIFICATION_ID_PROGRESS);

        if (job.hasFailures()) {
            if (!job.failedUris.isEmpty()) {
@@ -313,20 +386,15 @@ public class FileOperationService extends Service implements Job.Listener {
            if (!job.failedDocs.isEmpty()) {
                Log.e(TAG, "Job failed to process docs: " + job.failedDocs + ".");
            }
                mNotificationManager.notify(
            notificationManager.notify(
                    job.id, NOTIFICATION_ID_FAILURE, job.getFailureNotification());
        }

        if (job.hasWarnings()) {
            if (DEBUG) Log.d(TAG, "Job finished with warnings.");
                mNotificationManager.notify(
            notificationManager.notify(
                    job.id, NOTIFICATION_ID_WARNING, job.getWarningNotification());
        }
        });

        synchronized (mRunning) {
            deleteJob(job);
        }
    }

    private static final class JobRecord {
@@ -385,4 +453,24 @@ public class FileOperationService extends Service implements Job.Listener {
    public IBinder onBind(Intent intent) {
        return null;  // Boilerplate. See super#onBind
    }

    private static ForegroundManager createForegroundManager(final Service service) {
        return new ForegroundManager() {
            @Override
            public void startForeground(int id, Notification notification) {
                service.startForeground(id, notification);
            }

            @Override
            public void stopForeground(boolean removeNotification) {
                service.stopForeground(removeNotification);
            }
        };
    }

    @VisibleForTesting
    interface ForegroundManager {
        void startForeground(int id, Notification notification);
        void stopForeground(boolean removeNotification);
    }
}
+56 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2017 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.documentsui.services;

import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertNull;

import android.app.Notification;

class TestForegroundManager implements FileOperationService.ForegroundManager {

    private int mForegroundId = -1;
    private Notification mForegroundNotification;

    @Override
    public void startForeground(int id, Notification notification) {
        mForegroundId = id;
        mForegroundNotification = notification;
    }

    @Override
    public void stopForeground(boolean cancelNotification) {
        mForegroundId = -1;
        mForegroundNotification = null;
    }

    void assertInForeground() {
        assertNotNull(mForegroundNotification);
    }

    void assertInBackground() {
        assertNull(mForegroundNotification);
    }

    int getForegroundId() {
        return mForegroundId;
    }

    Notification getForegroundNotification() {
        return mForegroundNotification;
    }
}
+103 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2017 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.documentsui.services;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertTrue;

import android.app.Notification;
import android.app.NotificationManager;
import android.util.SparseArray;

import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;

import java.util.HashMap;

class TestNotificationManager {

    private final TestForegroundManager mForegroundManager;
    private final SparseArray<HashMap<String, Notification>> mNotifications = new SparseArray<>();
    private final Answer<Void> mAnswer = this::invoke;

    TestNotificationManager(TestForegroundManager foregroundManager) {
        assert(foregroundManager != null);
        mForegroundManager = foregroundManager;
    }

    private void notify(String tag, int id, Notification notification) {
        if (notification == mForegroundManager.getForegroundNotification()
                && id != mForegroundManager.getForegroundId()) {
            throw new IllegalStateException("Mismatching ID and notification.");
        }

        if (mNotifications.get(id) == null) {
            mNotifications.put(id, new HashMap<>());
        }

        mNotifications.get(id).put(tag, notification);
    }

    private void cancel(String tag, int id) {
        final HashMap<String, Notification> idMap = mNotifications.get(id);
        if (idMap != null && idMap.containsKey(tag)) {
            final Notification notification = idMap.get(tag);
            // Only cancel non-foreground notification
            if (mForegroundManager.getForegroundNotification() != notification) {
                idMap.remove(tag);
            }
        }
    }

    private Void invoke(InvocationOnMock invocation) {
        Object[] args = invocation.getArguments();
        switch (invocation.getMethod().getName()) {
            case "notify":
                if (args.length == 2) {
                    notify(null, (Integer) args[0], (Notification) args[1]);
                }
                if (args.length == 3) {
                    notify((String) args[0], (Integer) args[1], (Notification) args[2]);
                }
                break;
            case "cancel":
                if (args.length == 1) {
                    cancel(null, (Integer) args[0]);
                }
                if (args.length == 2) {
                    cancel((String) args[0], (Integer) args[1]);
                }
                break;
        }
        return null;
    }

    NotificationManager createNotificationManager() {
        return Mockito.mock(NotificationManager.class, mAnswer);
    }

    void assertNumberOfNotifications(int expect) {
        int count = 0;
        for (int i = 0; i < mNotifications.size(); ++i) {
            count += mNotifications.valueAt(i).size();
        }

        assertEquals(expect, count);
    }
}
+19 −2
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.documentsui.testing;
import android.os.Handler;
import android.os.Looper;
import android.os.Message;
import android.os.SystemClock;

import java.util.TimerTask;

@@ -28,6 +29,14 @@ import java.util.TimerTask;
public class TestHandler extends Handler {
    private TestTimer mTimer = new TestTimer();

    // Handler uses SystemClock.uptimeMillis() when scheduling task to get current time, but
    // TestTimer has its own warped time for us to "fast forward" into the future. Therefore after
    // we "fast forwarded" TestTimer once Handler may schedule tasks running in the "past" relative
    // to the fast-forwarded TestTimer and cause problems. This value is used to track how much we
    // fast-forward into the future to make sure we schedule tasks in the future of TestTimer as
    // well.
    private long mTimeDelta = 0;

    public TestHandler() {
        // Use main looper to trick underlying handler, we're not using it at all.
        super(Looper.getMainLooper());
@@ -39,19 +48,27 @@ public class TestHandler extends Handler {

    public void dispatchNextMessage() {
        mTimer.fastForwardToNextTask();

        mTimeDelta = mTimer.getNow() - SystemClock.uptimeMillis();
    }

    public void dispatchAllMessages() {
    public void dispatchAllScheduledMessages() {
        while (hasScheduledMessage()) {
            dispatchNextMessage();
        }
    }

    public void dispatchAllMessages() {
        while (hasScheduledMessage()) {
            dispatchAllScheduledMessages();
        }
    }

    @Override
    public boolean sendMessageAtTime(Message msg, long uptimeMillis) {
        msg.setTarget(this);
        TimerTask task = new MessageTimerTask(msg);
        mTimer.scheduleAtTime(new TestTimer.Task(task), uptimeMillis);
        mTimer.scheduleAtTime(new TestTimer.Task(task), uptimeMillis + mTimeDelta);
        return true;
    }

+2 −0
Original line number Diff line number Diff line
@@ -150,6 +150,8 @@ public class TestScheduledExecutorService implements ScheduledExecutorService {

    public void run(int taskIndex) {
        scheduled.get(taskIndex).runnable.run();

        scheduled.remove(taskIndex);
    }

    public void assertAlive() {
Loading