UpcomingAlarmReceiver and PendingAlarmScheduler load alarm in background

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

@@ -15,22 +15,42 @@ import static com.philliphsu.clock2.util.Preconditions.checkNotNull;
  * your intent at the Alarm instance's normal ring time, so by the time you make a subsequent call
  * to {@link Alarm#ringsAt()}, the value returned refers to the next time the alarm will recur.
  */
+// TODO: Consider registering this locally instead of in the manifest.
 public class PendingAlarmScheduler extends BroadcastReceiver {
     // We include the class name in the string to distinguish this constant from the one defined
     // in UpcomingAlarmReceiver.
     public static final String EXTRA_ALARM_ID = "com.philliphsu.clock2.PendingAlarmScheduler.extra.ALARM_ID";
 
     @Override
-    public void onReceive(Context context, Intent intent) {
+    public void onReceive(final Context context, Intent intent) {
-        long id = intent.getLongExtra(EXTRA_ALARM_ID, -1);
+        final long id = intent.getLongExtra(EXTRA_ALARM_ID, -1);
         if (id < 0) {
             throw new IllegalStateException("No alarm id received");
         }
-        // TODO: Do this in the background. AsyncTask?
+        // Start our own thread to load the alarm instead of:
-        Alarm alarm = checkNotNull(DatabaseManager.getInstance(context).getAlarm(id));
+        //  * using a Loader, because we have no complex lifecycle and thus
+        //  BroadcastReceiver has no built-in LoaderManager, AND getting a Loader
+        //  to work here might be a hassle, let alone it might not even be appropriate to
+        //  use Loaders outside of an Activity/Fragment, since it does depend on LoaderCallbacks.
+        //  * using an AsyncTask, because we don't need to do anything on the UI thread
+        //  after the background work is complete.
+        // TODO: Verify using a Runnable like this won't cause a memory leak.
+        // It *probably* won't because a BroadcastReceiver doesn't hold a Context,
+        // and it also doesn't have a lifecycle, so it likely won't stick around
+        // in memory.
+        new Thread(new Runnable() {
+            @Override
+            public void run() {
+                Alarm alarm = checkNotNull(DatabaseManager
+                        .getInstance(context).getAlarm(id));
                 if (!alarm.isEnabled()) {
                     throw new IllegalStateException("Alarm must be enabled!");
                 }
+                // Because showToast = false, we don't do any UI work.
+                // TODO: Since we're in a worker thread, verify that the
+                // UI related code within will not cause us to crash.
                 AlarmUtils.scheduleAlarm(context, alarm, false);
             }
+        }).start();
+    }
 }

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

@@ -6,6 +6,7 @@ import android.app.PendingIntent;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
+import android.os.AsyncTask;
 import android.support.v4.app.NotificationCompat;
 
 import com.philliphsu.clock2.model.DatabaseManager;
@@ -15,6 +16,7 @@ import static android.app.PendingIntent.FLAG_ONE_SHOT;
 import static com.philliphsu.clock2.util.DateFormatUtils.formatTime;
 import static com.philliphsu.clock2.util.Preconditions.checkNotNull;
 
+// TODO: Consider registering this locally instead of in the manifest.
 public class UpcomingAlarmReceiver extends BroadcastReceiver {
     private static final String TAG = "UpcomingAlarmReceiver";
     /*TOneverDO: not private*/
@@ -25,22 +27,34 @@ public class UpcomingAlarmReceiver extends BroadcastReceiver {
     public static final String EXTRA_ALARM_ID = "com.philliphsu.clock2.extra.ALARM_ID";
 
     @Override
-    public void onReceive(Context context, Intent intent) {
+    public void onReceive(final Context context, final Intent intent) {
-        long id = intent.getLongExtra(EXTRA_ALARM_ID, -1);
+        final long id = intent.getLongExtra(EXTRA_ALARM_ID, -1);
         if (id < 0) {
             throw new IllegalStateException("No alarm id received");
         }
 
-        NotificationManager nm = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
+        final NotificationManager nm = (NotificationManager)
+                context.getSystemService(Context.NOTIFICATION_SERVICE);
         if (ACTION_CANCEL_NOTIFICATION.equals(intent.getAction())) {
-            nm.cancel(getClass().getName(), (int) id);
+            nm.cancel(TAG, (int) id);
         } else {
-            // TODO: AsyncTask/Loader
+            new AsyncTask<Void, Void, Alarm>() {
-            Alarm alarm = checkNotNull(DatabaseManager.getInstance(context).getAlarm(id));
+                @Override
+                protected Alarm doInBackground(Void... params) {
+                    return checkNotNull(DatabaseManager.getInstance(context).getAlarm(id));
+                }
+
+                @Override
+                protected void onPostExecute(Alarm alarm) {
                     if (ACTION_DISMISS_NOW.equals(intent.getAction())) {
+                        // This MUST be done on the UI thread.
                         AlarmUtils.cancelAlarm(context, alarm, true);
                     } else {
                         // Prepare notification
+                        // http://stackoverflow.com/a/15803726/5055032
+                        // Notifications aren't updated on the UI thread, so we could have
+                        // done this in the background. However, no lengthy operations are
+                        // done here, so doing so is a premature optimization.
                         String title;
                         String text;
                         if (ACTION_SHOW_SNOOZING.equals(intent.getAction())) {
@@ -69,8 +83,10 @@ public class UpcomingAlarmReceiver extends BroadcastReceiver {
                                 .setOngoing(true)
                                 .addAction(R.mipmap.ic_launcher, context.getString(R.string.dismiss_now), pi)
                                 .build();
-                nm.notify(getClass().getName(), (int) id, note);
+                        nm.notify(TAG, (int) id, note);
                     }
                 }
+            }.execute();
+        }
     }
 }

app/src/main/java/com/philliphsu/clock2/alarms/AlarmViewHolder.java

@@ -99,7 +99,7 @@ public class AlarmViewHolder extends BaseViewHolder<Alarm> implements AlarmCount
             bindCountdown(false, -1);
             bindDismissButton(false, "");
         }
-        save(); // TODO: Problem! If cancelAlarm() saves the repo, this is a redundant call!
+        save();
     }
     */
 
@@ -164,7 +164,6 @@ public class AlarmViewHolder extends BaseViewHolder<Alarm> implements AlarmCount
         String buttonText = alarm.isSnoozed()
                 ? getContext().getString(R.string.title_snoozing_until, formatTime(getContext(), alarm.snoozingUntil()))
                 : getContext().getString(R.string.dismiss_now);
-        // TODO: Register dynamic broadcast receiver in this class to listen for
         // when this alarm crosses the upcoming threshold, so we can show this button.
         bindDismissButton(visible, buttonText);
     }

app/src/main/java/com/philliphsu/clock2/alarms/AlarmsFragment.java

@@ -85,13 +85,6 @@ public class AlarmsFragment extends Fragment implements LoaderCallbacks<Cursor>,
         return view;
     }
 
-    @Override
-    public void onPause() {
-        super.onPause();
-        // TODO: Do we need to save anything?
-//        AlarmsRepository.getInstance(getActivity()).saveItems();
-    }
-
     @Override
     public void onResume() {
         super.onResume();

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

@@ -11,6 +11,7 @@ import java.util.List;
 /**
  * Created by Phillip Hsu on 7/2/2016.
  */
+@Deprecated
 public class AlarmListLoader extends DataListLoader<Alarm, AlarmCursor> {
 
     public AlarmListLoader(Context context) {

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

@@ -9,6 +9,7 @@ import java.util.List;
 /**
  * Created by Phillip Hsu on 7/2/2016.
  */
+@Deprecated
 // TODO: Consider C extends MyTypeBoundedCursorWrapper<D>
 public abstract class DataListLoader<D, C extends CursorWrapper> extends AsyncTaskLoader<List<D>> {
     

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

@@ -14,7 +14,7 @@ public class DatabaseManager {
 
     private static DatabaseManager sDatabaseManager;
     private final Context mContext;
-    private final AlarmDatabaseHelper mHelper; // TODO: Should we call close() when we're done?
+    private final AlarmDatabaseHelper mHelper; // TODO: Call close() when *the app* is exiting.
 
     private DatabaseManager(Context context) {
         mContext = context.getApplicationContext();

app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java

@@ -90,6 +90,7 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc
             // their lifecycle is not complex like in Activities/Fragments) and our
             // work is simple enough that getting loaders to work here is not
             // worth the effort.
+            // TODO: Will using the Runnable like this cause a memory leak?
             new Thread(new Runnable() {
                 @Override
                 public void run() {

app/src/main/java/com/philliphsu/clock2/util/AlarmUtils.java

@@ -65,6 +65,8 @@ public final class AlarmUtils {
                 notifyUpcomingAlarmIntent(context, alarm, false));
         am.setExact(AlarmManager.RTC_WAKEUP, ringAt, alarmIntent(context, alarm, false));
 
+        // TODO: Consider removing this and letting callers handle Toasts, because
+        // it could be beneficial for callers to schedule the alarm in a worker thread.
         if (showToast) {
             String message;
             if (alarm.isSnoozed()) {
@@ -97,6 +99,10 @@ public final class AlarmUtils {
 
         removeUpcomingAlarmNotification(c, a);
 
+        // We can't remove this and instead let callers handle Toasts
+        // without complication, because callers would then have to
+        // handle cases when the alarm is snoozed and/or has
+        // recurrence themselves.
         // TOneverDO: Place block after making value changes to the alarm.
         if (showToast && (a.ringsWithinHours(hoursBeforeUpcoming(c)) || a.isSnoozed())) {
             String time = formatTime(c, a.isSnoozed() ? a.snoozingUntil() : a.ringsAt());