Implement multiple timer notifications, restore TimerNotificationService on process restart, release timer resources as they are cancelled

app/src/main/java/com/philliphsu/clock2/ChronometerNotificationService.java

@@ -22,7 +22,7 @@ public abstract class ChronometerNotificationService extends Service {
     public static final String ACTION_START_PAUSE = "com.philliphsu.clock2.timers.action.START_PAUSE";
     public static final String ACTION_STOP = "com.philliphsu.clock2.timers.action.STOP";
 
-    public static final String EXTRA_ID = "com.philliphsu.clock2.extra.ID";
+    public static final String EXTRA_ACTION_ID = "com.philliphsu.clock2.extra.ID";
 
     // TODO: I think we'll need a collection of builders too. However, we can have a common immutable
     // builder instance with attributes that all timer notifications will have.
@@ -83,18 +83,19 @@ public abstract class ChronometerNotificationService extends Service {
      * @param flags
      * @param startId
      */
-    protected abstract void handleDefaultAction(Intent intent, int flags, long startId);
+    protected abstract void handleDefaultAction(Intent intent, int flags, int startId);
 
-    protected abstract void handleStartPauseAction(Intent intent, int flags, long startId);
+    protected abstract void handleStartPauseAction(Intent intent, int flags, int startId);
 
-    protected abstract void handleStopAction(Intent intent, int flags, long startId);
+    protected abstract void handleStopAction(Intent intent, int flags, int startId);
 
     /**
      * This will be called if the command in {@link #onStartCommand(Intent, int, int)}
      * has an action that your subclass defines.
      * @param action Your custom action.
+     * @param startId
      */
-    protected abstract void handleAction(@NonNull String action, Intent intent, int flags, long startId);
+    protected abstract void handleAction(@NonNull String action, Intent intent, int flags, int startId);
 
     @Override
     public void onCreate() {
@@ -102,7 +103,6 @@ public abstract class ChronometerNotificationService extends Service {
         mNotificationManager = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE);
         if (isForeground()) {
             registerNewNoteBuilder(getNoteId());
-            registerNewChronometer(getNoteId());
             // IGNORE THE LINT WARNING ABOUT UNNECESSARY BOXING. Because getNoteId() returns an int,
             // it gets boxed to an Integer. A Long and an Integer are never interchangeable, even
             // if they wrap the same integer value.
@@ -110,13 +110,17 @@ public abstract class ChronometerNotificationService extends Service {
         }
     }
 
-    protected final void registerNewChronometer(long id) {
+    private void registerNewChronometer(long id) {
         ChronometerDelegate delegate = new ChronometerDelegate();
         delegate.init();
         delegate.setCountDown(isCountDown());
         mDelegates.put(id, delegate);
     }
 
+    /**
+     * Registers a new Notification builder with the provided ID. Each new builder
+     * is associated with a new chronometer.
+     */
     protected final void registerNewNoteBuilder(long id) {
         NotificationCompat.Builder builder = new NotificationCompat.Builder(this)
                 .setSmallIcon(getSmallIcon())
@@ -124,6 +128,7 @@ public abstract class ChronometerNotificationService extends Service {
                 .setOngoing(true)
                 .setContentIntent(getContentIntent());
         mNoteBuilders.put(id, builder);
+        registerNewChronometer(id);
     }
 
     // Didn't work!
@@ -194,7 +199,7 @@ public abstract class ChronometerNotificationService extends Service {
             // Display any notification updates associated with the current state
             // of the chronometer. If we relied on the HandlerThread to do this for us,
             // the message delivery would be delayed.
-            thread.updateNotification(/*TODO:pass in id*/false/*updateText*/);
+            thread.updateNotification(false/*updateText*/);
             // If the chronometer has been set to not run, the effect is obvious.
             // Otherwise, we're preparing for the start of a new thread.
             quitThread(id);
@@ -237,6 +242,20 @@ public abstract class ChronometerNotificationService extends Service {
         // -------------------------------------------------------------------------------
     }
 
+    /**
+     * Releases all resources associated with this id. This is only
+     * necessary for subclasses that support multiple notifications,
+     * because they don't have the convenience of stopping the service
+     * altogether to GC all resources.
+     */
+    @CallSuper
+    protected void releaseResources(long id) {
+        mNoteBuilders.remove(id);
+        quitThread(id);
+        mThreads.remove(id);
+        mDelegates.remove(id);
+    }
+
     /**
      * Helper method to add the start/pause action to the notification's builder.
      * @param running whether the chronometer is running
@@ -309,7 +328,7 @@ public abstract class ChronometerNotificationService extends Service {
     protected final void addAction(String action, @DrawableRes int icon, String actionTitle, long id) {
         Intent intent = new Intent(this, getClass())
                 .setAction(action)
-                .putExtra(EXTRA_ID, id);
+                .putExtra(EXTRA_ACTION_ID, id);
         PendingIntent pi = PendingIntent.getService(
                 this, (int) id, intent, 0/*no flags*/);
         mNoteBuilders.get(id).addAction(icon, actionTitle, pi);

app/src/main/java/com/philliphsu/clock2/model/TimersTableManager.java

@@ -3,6 +3,7 @@ package com.philliphsu.clock2.model;
 import android.content.ContentValues;
 import android.content.Context;
 import android.database.Cursor;
+import android.os.SystemClock;
 import android.util.Log;
 
 import com.philliphsu.clock2.Timer;
@@ -32,6 +33,12 @@ public class TimersTableManager extends DatabaseTableManager<Timer> {
         return wrapInTimerCursor(super.queryItems());
     }
 
+    public TimerCursor queryStartedTimers() {
+        String where = TimersTable.COLUMN_END_TIME + " > " + SystemClock.elapsedRealtime()
+                + " OR " + TimersTable.COLUMN_PAUSE_TIME + " > 0";
+        return queryItems(where, null);
+    }
+
     @Override
     protected TimerCursor queryItems(String where, String limit) {
         return wrapInTimerCursor(super.queryItems(where, limit));

app/src/main/java/com/philliphsu/clock2/stopwatch/StopwatchNotificationService.java

@@ -100,7 +100,7 @@ public class StopwatchNotificationService extends ChronometerNotificationService
     }
 
     @Override
-    protected void handleDefaultAction(Intent intent, int flags, long startId) {
+    protected void handleDefaultAction(Intent intent, int flags, int startId) {
         // TODO: Why do we need this check? Won't KEY_START_TIME always have a value of 0 here?
         if (mPrefs.getLong(StopwatchFragment.KEY_START_TIME, 0) == 0) {
             mCurrentLap = new Lap();
@@ -112,7 +112,7 @@ public class StopwatchNotificationService extends ChronometerNotificationService
     }
 
     @Override
-    protected void handleStartPauseAction(Intent intent, int flags, long startId) {
+    protected void handleStartPauseAction(Intent intent, int flags, int startId) {
         boolean running = mPrefs.getBoolean(StopwatchFragment.KEY_CHRONOMETER_RUNNING, false);
         SharedPreferences.Editor editor = mPrefs.edit();
         editor.putBoolean(StopwatchFragment.KEY_CHRONOMETER_RUNNING, !running);
@@ -137,7 +137,7 @@ public class StopwatchNotificationService extends ChronometerNotificationService
     }
 
     @Override
-    protected void handleStopAction(Intent intent, int flags, long startId) {
+    protected void handleStopAction(Intent intent, int flags, int startId) {
         mPrefs.edit()
                 .putLong(StopwatchFragment.KEY_START_TIME, 0)
                 .putLong(StopwatchFragment.KEY_PAUSE_TIME, 0)
@@ -157,7 +157,7 @@ public class StopwatchNotificationService extends ChronometerNotificationService
     }
 
     @Override
-    protected void handleAction(@NonNull String action, Intent intent, int flags, long startId) {
+    protected void handleAction(@NonNull String action, Intent intent, int flags, int startId) {
         if (ACTION_ADD_LAP.equals(action)) {
             if (mPrefs.getBoolean(StopwatchFragment.KEY_CHRONOMETER_RUNNING, false)) {
                 mDelegate.setBase(mPrefs.getLong(StopwatchFragment.KEY_START_TIME, SystemClock.elapsedRealtime()));

app/src/main/java/com/philliphsu/clock2/timers/TimerNotificationService.java

@@ -6,12 +6,15 @@ import android.content.Intent;
 import android.os.SystemClock;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.support.v4.util.SimpleArrayMap;
+import android.util.Log;
 
 import com.philliphsu.clock2.AsyncTimersTableUpdateHandler;
 import com.philliphsu.clock2.ChronometerNotificationService;
 import com.philliphsu.clock2.MainActivity;
 import com.philliphsu.clock2.R;
 import com.philliphsu.clock2.Timer;
+import com.philliphsu.clock2.model.TimerCursor;
 
 /**
  * Handles the notification for an active Timer.
@@ -23,14 +26,17 @@ import com.philliphsu.clock2.Timer;
 public class TimerNotificationService extends ChronometerNotificationService {
     private static final String TAG = "TimerNotifService";
 
+    private static final String ACTION_CANCEL_NOTIFICATION = "com.philliphsu.clock2.timers.action.CANCEL_NOTIFICATION";
     public static final String ACTION_ADD_ONE_MINUTE = "com.philliphsu.clock2.timers.action.ADD_ONE_MINUTE";
 
     public static final String EXTRA_TIMER = "com.philliphsu.clock2.timers.extra.TIMER";
+    private static final String EXTRA_CANCEL_TIMER_ID = "com.philliphsu.clock2.timers.extra.CANCEL_TIMER_ID";
 
-    // TODO: I think we may need a list of timers.
+    private AsyncTimersTableUpdateHandler mUpdateHandler;
-    private Timer mTimer;
+    private final SimpleArrayMap<Long, Timer> mTimers = new SimpleArrayMap<>();
-    private TimerController mController;
+    private final SimpleArrayMap<Long, TimerController> mControllers = new SimpleArrayMap<>();
-    private Intent mIntent;
+
+    private long mMostRecentTimerId;
 
     /**
      * Helper method to start this Service for its default action: to show
@@ -50,14 +56,11 @@ public class TimerNotificationService extends ChronometerNotificationService {
      * @param timerId the id of the Timer associated with the notification
      *                you want to cancel
      */
-    public static void cancelNotification(Context context, long timerId) { // TODO: remove long param
+    public static void cancelNotification(Context context, long timerId) {
-        // TODO: We do this in onDestroy() for a single notification.
+        Intent intent = new Intent(context, TimerNotificationService.class)
-        // Multiples will probably need something like this.
+                .setAction(ACTION_CANCEL_NOTIFICATION)
-//        NotificationManager nm = (NotificationManager)
+                .putExtra(EXTRA_CANCEL_TIMER_ID, timerId);
-//                context.getSystemService(Context.NOTIFICATION_SERVICE);
+        context.startService(intent);
-//        nm.cancel(getNoteTag(), (int) timerId);
-        // TODO: We only do this for a single notification. Remove this for multiples.
-        context.stopService(new Intent(context, TimerNotificationService.class));
     }
 
     @Override
@@ -68,7 +71,10 @@ public class TimerNotificationService extends ChronometerNotificationService {
     @Nullable
     @Override
     protected PendingIntent getContentIntent() {
-        mIntent = new Intent(this, MainActivity.class);
+        // The base class won't call this for us because this is not a foreground service,
+        // as we require multiple notifications created as needed. Instead, this is called after
+        // we call registerNewNoteBuilder() in handleDefaultAction().
+        Intent intent = new Intent(this, MainActivity.class);
         // http://stackoverflow.com/a/3128418/5055032
         // "For some unspecified reason, extras will be delivered only if you've set some action"
         // This ONLY applies to PendingIntents...
@@ -76,10 +82,11 @@ public class TimerNotificationService extends ChronometerNotificationService {
         // as another PendingIntent's dummy action. For example, StopwatchNotificationService
         // uses the dummy action "foo"; we previously used "foo" here as well, and firing this
         // intent scrolled us to MainActivity.PAGE_STOPWATCH...
-        mIntent.setAction("bar");
+        intent.setAction("bar");
-        mIntent.putExtra(MainActivity.EXTRA_SHOW_PAGE, MainActivity.PAGE_TIMERS);
+        intent.putExtra(MainActivity.EXTRA_SHOW_PAGE, MainActivity.PAGE_TIMERS);
-        // Request code not needed because we're only going to have one foreground notification.
+        // Before we called registerNewNoteBuilder(), we saved a reference to the most recent timer id.
-        return PendingIntent.getActivity(this, 0, mIntent, 0);
+        intent.putExtra(TimersFragment.EXTRA_SCROLL_TO_TIMER_ID, mMostRecentTimerId);
+        return PendingIntent.getActivity(this, (int) mMostRecentTimerId, intent, 0);
     }
 
     @Override
@@ -93,6 +100,13 @@ public class TimerNotificationService extends ChronometerNotificationService {
         return 0;
     }
 
+    @Override
+    protected String getNoteTag() {
+        // This is so we can cancel notifications in our static helper method
+        // cancelNotification(Context, long) with the static TAG constant
+        return TAG;
+    }
+
     @Override
     protected boolean isForeground() {
         // We're going to post a separate notification for each Timer.
@@ -100,6 +114,12 @@ public class TimerNotificationService extends ChronometerNotificationService {
         return false;
     }
 
+    @Override
+    public void onCreate() {
+        super.onCreate();
+        mUpdateHandler = new AsyncTimersTableUpdateHandler(this, null);
+    }
+
     @Override
     public void onDestroy() {
         super.onDestroy();
@@ -108,73 +128,132 @@ public class TimerNotificationService extends ChronometerNotificationService {
         // our thread has enough leeway to sneak in a final call to post the notification before it
         // is actually quit().
         // As such, try cancelling the notification with this (tag, id) pair again.
-        cancelNotification(getNoteId());
+        for (int i = 0; i < mTimers.size(); i++) {
+            cancelNotification(mTimers.keyAt(i));
+        }
     }
 
     @Override
-    protected void handleDefaultAction(Intent intent, int flags, long startId) {
+    public int onStartCommand(Intent intent, int flags, int startId) {
-        if ((mTimer = intent.getParcelableExtra(EXTRA_TIMER)) == null) {
+        if (intent == null) {
+            Log.d(TAG, "Recreated service, starting chronometer again.");
+            // Restore all running timers. This restores all of the base
+            // class's member state as well, due to the various API
+            // calls required.
+            new Thread(new Runnable() {
+                @Override
+                public void run() {
+                    TimerCursor cursor = mUpdateHandler.getTableManager().queryStartedTimers();
+                    while (cursor.moveToNext()) {
+                        // We actually don't need any args since this will be
+                        // passed directly to our handler method. If we were going
+                        // to startService() with this, then we would need to
+                        // specify them.
+                        Intent intent = new Intent(
+                                /*TimerNotificationService.this,
+                                TimerNotificationService.class*/);
+                        intent.putExtra(EXTRA_TIMER, cursor.getItem());
+                        // TODO: Should we startService() instead?
+                        handleDefaultAction(intent, 0, 0);
+                    }
+                }
+            }).start();
+        }
+        return super.onStartCommand(intent, flags, startId);
+    }
+
+    @Override
+    protected void handleDefaultAction(Intent intent, int flags, int startId) {
+        Timer timer = intent.getParcelableExtra(EXTRA_TIMER);
+        if (timer == null) {
             throw new IllegalStateException("Cannot start TimerNotificationService without a Timer");
         }
-        // TODO: Wrap this around an `if (only one timer running)` statement.
+        final long id = timer.getId();
-        // TODO: We have to update the PendingIntent.. so write an API in the base class to do so.
+        mTimers.put(id, timer);
-        // TODO: Not implemented for simplicity. Future release??
+        mControllers.put(id, new TimerController(timer, mUpdateHandler));
-//        mIntent.putExtra(TimersFragment.EXTRA_SCROLL_TO_TIMER_ID, mTimer.getId());
+
-        mController = new TimerController(mTimer, new AsyncTimersTableUpdateHandler(this, null));
+        mMostRecentTimerId = timer.getId();
+        registerNewNoteBuilder(id);
+        
         // The note's title should change here every time, especially if the Timer's label was updated.
-        String title = mTimer.label();
+        String title = timer.label();
         if (title.isEmpty()) {
             title = getString(R.string.timer);
         }
-        setContentTitle(mTimer.getId(), title);
+        setContentTitle(id, title);
-        syncNotificationWithTimerState(mTimer.isRunning());
+        syncNotificationWithTimerState(id, timer.isRunning());
     }
 
     @Override
-    protected void handleStartPauseAction(Intent intent, int flags, long startId) {
+    protected void handleStartPauseAction(Intent intent, int flags, int startId) {
-        mController.startPause();
+        long id = getActionId(intent);
-        syncNotificationWithTimerState(mTimer.isRunning());
+        mControllers.get(id).startPause();
+        syncNotificationWithTimerState(id, mTimers.get(id).isRunning());
     }
 
     @Override
-    protected void handleStopAction(Intent intent, int flags, long startId) {
+    protected void handleStopAction(Intent intent, int flags, int startId) {
-        mController.stop();
+        long id = getActionId(intent);
-        stopSelf();
+        mControllers.get(id).stop();
         // We leave removing the notification up to AsyncTimersTableUpdateHandler
         // when it calls cancelAlarm() from onPostAsyncUpdate().
+        // This calls the static helper cancelNotification(), which
+        // starts this service to handle ACTION_CANCEL_NOTIFICATION.
     }
 
     @Override
-    protected void handleAction(@NonNull String action, Intent intent, int flags, long startId) {
+    protected void handleAction(@NonNull String action, Intent intent, int flags, int startId) {
         if (ACTION_ADD_ONE_MINUTE.equals(action)) {
             // While the notification's countdown would automatically be extended by one minute,
             // there is a noticeable delay before the minute gets added on.
             // Update the text immediately, because there's no harm in doing so.
-            long id = intent.getLongExtra(EXTRA_ID, -1);
+            long id = getActionId(intent);
             setBase(id, getBase(id) + 60000);
             updateNotification(id, true);
-            mController.addOneMinute();
+            mControllers.get(id).addOneMinute();
+        } else if (ACTION_CANCEL_NOTIFICATION.equals(action)) {
+            long id = intent.getLongExtra(EXTRA_CANCEL_TIMER_ID, -1);
+            cancelNotification(id);
+            // TODO: SHould this be before cancelNotification()? I'm worried
+            // that the thread's handler will have enough leeway to sneak
+            // in a notification update before it is quit. If it did,
+            // then at least cancelNotification should theoretically
+            // remove it...
+            releaseResources(id);
         } else {
             throw new IllegalArgumentException("TimerNotificationService cannot handle action " + action);
         }
     }
 
-    private void syncNotificationWithTimerState(boolean running) {
+    @Override
+    protected void releaseResources(long id) {
+        super.releaseResources(id);
+        mTimers.remove(id);
+        mControllers.remove(id);
+        // TODO: Should we make a private method?
+        // This private method would first call releaseResources(),
+        // and then this block.
+        if (mTimers.isEmpty()) { // We could check any map, since they should all have the same sizes
+            stopSelf();
+        }
+    }
+
+    private void syncNotificationWithTimerState(long id, boolean running) {
         // The actions from the last time we configured the Builder are still here.
         // We have to retain the relative ordering of the actions while updating
         // just the start/pause action, so clear them and set them again.
-        final long timerId = mTimer.getId();
+        clearActions(id);
-        clearActions(timerId);
+        addAction(ACTION_ADD_ONE_MINUTE, R.drawable.ic_add_24dp, getString(R.string.minute), id);
-        addAction(ACTION_ADD_ONE_MINUTE,
+        addStartPauseAction(running, id);
-                R.drawable.ic_add_24dp,
+        addStopAction(id);
-                getString(R.string.minute),
-                timerId);
-        addStartPauseAction(running, timerId);
-        addStopAction(timerId);
 
-        quitCurrentThread(getNoteId());
+        quitCurrentThread(id);
         if (running) {
-            startNewThread(getNoteId(), SystemClock.elapsedRealtime() + mTimer.timeRemaining());
+            startNewThread(id, SystemClock.elapsedRealtime() + mTimers.get(id).timeRemaining());
         }
     }
+    
+    private long getActionId(Intent intent) {
+        return intent.getLongExtra(EXTRA_ACTION_ID, -1);
+    }
 }