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

Commit 99cca9db authored by Rhed Jao's avatar Rhed Jao Committed by Android (Google) Code Review
Browse files

Merge "Fix FileOperationService starts foreground service failed."

parents 9e46bc77 674b0a9e

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

+49 −35
Original line number Diff line number Diff line
@@ -37,13 +37,12 @@ import com.android.documentsui.base.Features; 
import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.util.ArrayList;

import java.util.HashMap;

import java.util.LinkedHashMap;

import java.util.List;

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;
@@ -99,9 +98,9 @@ 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;

    @VisibleForTesting static final int NOTIFICATION_ID_PROGRESS = 1;

    private static final int NOTIFICATION_ID_FAILURE = 2;

    private static final int NOTIFICATION_ID_WARNING = 3;



    // The executor and job factory are visible for testing and non-final

    // so we'll have a way to inject test doubles from the test. It's
@@ -124,10 +123,11 @@ public class FileOperationService extends Service implements Job.Listener { 
    @VisibleForTesting Features features;



    @GuardedBy("mJobs")

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

    private final Map<String, JobRecord> mJobs = new LinkedHashMap<>();



    // The job whose notification is used to keep the service in foreground mode.

    private final AtomicReference<Job> mForegroundJob = new AtomicReference<>();

    @GuardedBy("mJobs")

    private Job mForegroundJob;



    private PowerManager mPowerManager;

    private PowerManager.WakeLock mWakeLock;  // the wake lock, if held.
@@ -270,6 +270,7 @@ public class FileOperationService extends Service implements Job.Listener { 
            JobRecord record = mJobs.get(jobId);

            if (record != null) {

                record.job.cancel();

                updateForegroundState(record.job);

            }

        }
@@ -343,18 +344,23 @@ public class FileOperationService extends Service implements Job.Listener { 


        Notification notification = job.getSetupNotification();

        // If there is no foreground job yet, set this job to foreground job.

        if (mForegroundJob.compareAndSet(null, job)) {

        synchronized (mJobs) {

            if (mForegroundJob == null) {

                if (DEBUG) Log.d(TAG, "Set foreground job to " + job.id);

                mForegroundJob = job;

                foregroundManager.startForeground(NOTIFICATION_ID_PROGRESS, notification);

        }



            } else {

                // Show start up notification

                if (DEBUG) Log.d(TAG, "Posting notification for " + job.id);

                notificationManager.notify(

                job.id, NOTIFICATION_ID_PROGRESS, notification);

                        mForegroundJob == job ? null : job.id,

                        NOTIFICATION_ID_PROGRESS,

                        notification);

            }

        }



        // Set up related monitor

        JobMonitor monitor = new JobMonitor(job, notificationManager, handler, mJobs);

        JobMonitor monitor = new JobMonitor(job);

        monitor.start();

    }
@@ -388,11 +394,12 @@ public class FileOperationService extends Service implements Job.Listener { 


    @GuardedBy("mJobs")

    private void updateForegroundState(Job job) {

        Job candidate = mJobs.isEmpty() ? null : mJobs.values().iterator().next().job;

        Job candidate = getCandidateForegroundJob();



        // 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 (mForegroundJob == job) {

            mForegroundJob = 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
@@ -401,12 +408,11 @@ public class FileOperationService extends Service implements Job.Listener { 
            } else {

                if (DEBUG) Log.d(TAG, "Switch foreground job to " + candidate.id);



                notificationManager.cancel(candidate.id, NOTIFICATION_ID_PROGRESS);

                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);

                notificationManager.notify(NOTIFICATION_ID_PROGRESS, notification);

            }

        }

    }
@@ -435,6 +441,19 @@ public class FileOperationService extends Service implements Job.Listener { 
        }

    }



    @GuardedBy("mJobs")

    private Job getCandidateForegroundJob() {

        if (mJobs.isEmpty()) {

            return null;

        }

        for (JobRecord rec : mJobs.values()) {

            if (!rec.job.isFinished()) {

                return rec.job;

            }

        }

        return null;

    }



    private static final class JobRecord {

        private final Job job;

        private final Future<?> future;
@@ -452,29 +471,22 @@ public class FileOperationService extends Service implements Job.Listener { 
     * 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 final class JobMonitor implements Runnable {

        private static final long PROGRESS_INTERVAL_MILLIS = 500L;



        private final Job mJob;

        private final NotificationManager mNotificationManager;

        private final Handler mHandler;

        private final Object mJobsLock;



        private JobMonitor(Job job, NotificationManager notificationManager, Handler handler,

                Object jobsLock) {

        private JobMonitor(Job job) {

            mJob = job;

            mNotificationManager = notificationManager;

            mHandler = handler;

            mJobsLock = jobsLock;

        }



        private void start() {

            mHandler.post(this);

            handler.post(this);

        }



        @Override

        public void run() {

            synchronized (mJobsLock) {

            synchronized (mJobs) {

                if (mJob.isFinished()) {

                    // Finish notification is already shown. Progress notification is removed.

                    // Just finish itself.
@@ -483,11 +495,13 @@ public class FileOperationService extends Service implements Job.Listener { 


                // Only job in set up state has progress bar

                if (mJob.getState() == Job.STATE_SET_UP) {

                    mNotificationManager.notify(

                            mJob.id, NOTIFICATION_ID_PROGRESS, mJob.getProgressNotification());

                    notificationManager.notify(

                            mForegroundJob == mJob ? null : mJob.id,

                            NOTIFICATION_ID_PROGRESS,

                            mJob.getProgressNotification());

                }



                mHandler.postDelayed(this, PROGRESS_INTERVAL_MILLIS);

                handler.postDelayed(this, PROGRESS_INTERVAL_MILLIS);

            }

        }

    }

tests/common/com/android/documentsui/services/TestForegroundManager.java

+8 −8
Original line number Diff line number Diff line
@@ -23,17 +23,25 @@ import android.app.Notification; 


class TestForegroundManager implements FileOperationService.ForegroundManager {



    private final TestNotificationManager mNotificationManager;

    private int mForegroundId = -1;

    private Notification mForegroundNotification;



    TestForegroundManager(TestNotificationManager notificationManager) {

        assert(notificationManager != null);

        mNotificationManager = notificationManager;

    }



    @Override

    public void startForeground(int id, Notification notification) {

        mForegroundId = id;

        mForegroundNotification = notification;

        mNotificationManager.notify(null, mForegroundId, mForegroundNotification);

    }



    @Override

    public void stopForeground(boolean cancelNotification) {

        mNotificationManager.cancel(null, mForegroundId);

        mForegroundId = -1;

        mForegroundNotification = null;

    }
@@ -45,12 +53,4 @@ class TestForegroundManager implements FileOperationService.ForegroundManager { 
    void assertInBackground() {

        assertNull(mForegroundNotification);

    }



    int getForegroundId() {

        return mForegroundId;

    }



    Notification getForegroundNotification() {

        return mForegroundNotification;

    }

}

tests/common/com/android/documentsui/services/TestNotificationManager.java

+20 −18
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.documentsui.services;



import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertFalse;

import static junit.framework.Assert.assertNotNull;

import static junit.framework.Assert.assertTrue;
@@ -32,21 +33,10 @@ 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.");

        }



    void notify(String tag, int id, Notification notification) {

        if (mNotifications.get(id) == null) {

            mNotifications.put(id, new HashMap<>());

        }
@@ -54,16 +44,12 @@ class TestNotificationManager { 
        mNotifications.get(id).put(tag, notification);

    }



    private void cancel(String tag, int id) {

    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();
@@ -88,6 +74,14 @@ class TestNotificationManager { 
        return null;

    }



    private boolean hasNotification(int id, String jobId) {

        if (mNotifications.get(id) == null) {

            return false;

        }

        Notification notification = mNotifications.get(id).get(jobId);

        return notification != null;

    }



    NotificationManager createNotificationManager() {

        return Mockito.mock(NotificationManager.class, mAnswer);

    }
@@ -100,4 +94,12 @@ class TestNotificationManager { 


        assertEquals(expect, count);

    }



    void assertHasNotification(int id, String jobid) {

        assertTrue(hasNotification(id, jobid));

    }



    void assertNoNotification(int id, String jobid) {

        assertFalse(hasNotification(id, jobid));

    }

}

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

+28 −2
Original line number Diff line number Diff line
@@ -78,8 +78,8 @@ public class FileOperationServiceTest extends ServiceTestCase<FileOperationServi 
        mExecutor = new TestScheduledExecutorService();

        mDeletionExecutor = new TestScheduledExecutorService();

        mHandler = new TestHandler();

        mForegroundManager = new TestForegroundManager();

        mTestNotificationManager = new TestNotificationManager(mForegroundManager);

        mTestNotificationManager = new TestNotificationManager();

        mForegroundManager = new TestForegroundManager(mTestNotificationManager);

        TestFeatures features = new TestFeatures();

        features.notificationChannel = InstrumentationRegistry.getTargetContext()

                .getResources().getBoolean(R.bool.feature_notification_channel);
@@ -294,6 +294,32 @@ public class FileOperationServiceTest extends ServiceTestCase<FileOperationServi 
        mTestNotificationManager.assertNumberOfNotifications(0);

    }



    public void testNotificationUpdateAfterForegroundJobSwitch() throws Exception {

        startService(createCopyIntent(newArrayList(ALPHA_DOC), BETA_DOC));

        startService(createCopyIntent(newArrayList(GAMMA_DOC), DELTA_DOC));

        Job job1 = mCopyJobs.get(0);

        Job job2 = mCopyJobs.get(1);



        mService.onStart(job1);

        mTestNotificationManager.assertHasNotification(

                FileOperationService.NOTIFICATION_ID_PROGRESS, null);



        mService.onStart(job2);

        mTestNotificationManager.assertHasNotification(

                FileOperationService.NOTIFICATION_ID_PROGRESS, job2.id);



        job1.cancel();

        mService.onFinished(job1);

        mTestNotificationManager.assertHasNotification(

                FileOperationService.NOTIFICATION_ID_PROGRESS, null);

        mTestNotificationManager.assertNoNotification(

                FileOperationService.NOTIFICATION_ID_PROGRESS, job2.id);



        job2.cancel();

        mService.onFinished(job2);

        mTestNotificationManager.assertNumberOfNotifications(0);

    }



    private Intent createCopyIntent(ArrayList<DocumentInfo> files, DocumentInfo dest)

            throws Exception {

        DocumentStack stack = new DocumentStack();