From 9380f8f57932429b0606858826a977bf59b6a864 Mon Sep 17 00:00:00 2001 From: Phillip Hsu Date: Thu, 7 Jul 2016 03:18:03 -0700 Subject: [PATCH] UpcomingAlarmReceiver and PendingAlarmScheduler load alarm in background --- .../clock2/PendingAlarmScheduler.java | 36 ++++++-- .../clock2/UpcomingAlarmReceiver.java | 92 +++++++++++-------- .../clock2/alarms/AlarmViewHolder.java | 3 +- .../clock2/alarms/AlarmsFragment.java | 7 -- .../clock2/model/AlarmListLoader.java | 1 + .../clock2/model/DataListLoader.java | 1 + .../clock2/model/DatabaseManager.java | 2 +- .../clock2/ringtone/RingtoneService.java | 1 + .../philliphsu/clock2/util/AlarmUtils.java | 6 ++ 9 files changed, 93 insertions(+), 56 deletions(-) diff --git a/app/src/main/java/com/philliphsu/clock2/PendingAlarmScheduler.java b/app/src/main/java/com/philliphsu/clock2/PendingAlarmScheduler.java index e896f91..bd2ccbc 100644 --- a/app/src/main/java/com/philliphsu/clock2/PendingAlarmScheduler.java +++ b/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) { - long id = intent.getLongExtra(EXTRA_ALARM_ID, -1); + public void onReceive(final Context context, Intent intent) { + 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? - Alarm alarm = checkNotNull(DatabaseManager.getInstance(context).getAlarm(id)); - if (!alarm.isEnabled()) { - throw new IllegalStateException("Alarm must be enabled!"); - } - AlarmUtils.scheduleAlarm(context, alarm, false); + // Start our own thread to load the alarm instead of: + // * 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(); } } diff --git a/app/src/main/java/com/philliphsu/clock2/UpcomingAlarmReceiver.java b/app/src/main/java/com/philliphsu/clock2/UpcomingAlarmReceiver.java index 6d1384a..97394c6 100644 --- a/app/src/main/java/com/philliphsu/clock2/UpcomingAlarmReceiver.java +++ b/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,52 +27,66 @@ 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) { - long id = intent.getLongExtra(EXTRA_ALARM_ID, -1); + public void onReceive(final Context context, final Intent intent) { + 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 - Alarm alarm = checkNotNull(DatabaseManager.getInstance(context).getAlarm(id)); - if (ACTION_DISMISS_NOW.equals(intent.getAction())) { - AlarmUtils.cancelAlarm(context, alarm, true); - } else { - // Prepare notification - String title; - String text; - if (ACTION_SHOW_SNOOZING.equals(intent.getAction())) { - if (!alarm.isSnoozed()) - throw new IllegalStateException("Can't show snoozing notif. if alarm not snoozed!"); - title = alarm.label().isEmpty() ? context.getString(R.string.alarm) : alarm.label(); - text = context.getString(R.string.title_snoozing_until, - formatTime(context, alarm.snoozingUntil())); - } else { - // No intent action required for default behavior - title = context.getString(R.string.upcoming_alarm); - text = formatTime(context, alarm.ringsAt()); - if (!alarm.label().isEmpty()) { - text = alarm.label() + ", " + text; - } + new AsyncTask() { + @Override + protected Alarm doInBackground(Void... params) { + return checkNotNull(DatabaseManager.getInstance(context).getAlarm(id)); } - Intent in = new Intent(context, getClass()) - .putExtra(EXTRA_ALARM_ID, id) // TOneverDO: cast to int - .setAction(ACTION_DISMISS_NOW); - PendingIntent pi = PendingIntent.getBroadcast(context, (int) id, in, FLAG_ONE_SHOT); - Notification note = new NotificationCompat.Builder(context) - .setSmallIcon(R.mipmap.ic_launcher) // TODO: alarm icon - .setContentTitle(title) - .setContentText(text) - .setOngoing(true) - .addAction(R.mipmap.ic_launcher, context.getString(R.string.dismiss_now), pi) - .build(); - nm.notify(getClass().getName(), (int) id, note); - } + @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())) { + if (!alarm.isSnoozed()) + throw new IllegalStateException("Can't show snoozing notif. if alarm not snoozed!"); + title = alarm.label().isEmpty() ? context.getString(R.string.alarm) : alarm.label(); + text = context.getString(R.string.title_snoozing_until, + formatTime(context, alarm.snoozingUntil())); + } else { + // No intent action required for default behavior + title = context.getString(R.string.upcoming_alarm); + text = formatTime(context, alarm.ringsAt()); + if (!alarm.label().isEmpty()) { + text = alarm.label() + ", " + text; + } + } + + Intent in = new Intent(context, getClass()) + .putExtra(EXTRA_ALARM_ID, id) // TOneverDO: cast to int + .setAction(ACTION_DISMISS_NOW); + PendingIntent pi = PendingIntent.getBroadcast(context, (int) id, in, FLAG_ONE_SHOT); + Notification note = new NotificationCompat.Builder(context) + .setSmallIcon(R.mipmap.ic_launcher) // TODO: alarm icon + .setContentTitle(title) + .setContentText(text) + .setOngoing(true) + .addAction(R.mipmap.ic_launcher, context.getString(R.string.dismiss_now), pi) + .build(); + nm.notify(TAG, (int) id, note); + } + } + }.execute(); } } } diff --git a/app/src/main/java/com/philliphsu/clock2/alarms/AlarmViewHolder.java b/app/src/main/java/com/philliphsu/clock2/alarms/AlarmViewHolder.java index ae1c915..604c81f 100644 --- a/app/src/main/java/com/philliphsu/clock2/alarms/AlarmViewHolder.java +++ b/app/src/main/java/com/philliphsu/clock2/alarms/AlarmViewHolder.java @@ -99,7 +99,7 @@ public class AlarmViewHolder extends BaseViewHolder 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 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); } diff --git a/app/src/main/java/com/philliphsu/clock2/alarms/AlarmsFragment.java b/app/src/main/java/com/philliphsu/clock2/alarms/AlarmsFragment.java index fcb5bdd..917cbcd 100644 --- a/app/src/main/java/com/philliphsu/clock2/alarms/AlarmsFragment.java +++ b/app/src/main/java/com/philliphsu/clock2/alarms/AlarmsFragment.java @@ -85,13 +85,6 @@ public class AlarmsFragment extends Fragment implements LoaderCallbacks, 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(); diff --git a/app/src/main/java/com/philliphsu/clock2/model/AlarmListLoader.java b/app/src/main/java/com/philliphsu/clock2/model/AlarmListLoader.java index f9657cf..7ed0b51 100644 --- a/app/src/main/java/com/philliphsu/clock2/model/AlarmListLoader.java +++ b/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 { public AlarmListLoader(Context context) { diff --git a/app/src/main/java/com/philliphsu/clock2/model/DataListLoader.java b/app/src/main/java/com/philliphsu/clock2/model/DataListLoader.java index 0b05c7d..c7a71f9 100644 --- a/app/src/main/java/com/philliphsu/clock2/model/DataListLoader.java +++ b/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 public abstract class DataListLoader extends AsyncTaskLoader> { diff --git a/app/src/main/java/com/philliphsu/clock2/model/DatabaseManager.java b/app/src/main/java/com/philliphsu/clock2/model/DatabaseManager.java index e5f7d9d..d3c7c9d 100644 --- a/app/src/main/java/com/philliphsu/clock2/model/DatabaseManager.java +++ b/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(); diff --git a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java index a5389a8..b269d69 100644 --- a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java +++ b/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() { diff --git a/app/src/main/java/com/philliphsu/clock2/util/AlarmUtils.java b/app/src/main/java/com/philliphsu/clock2/util/AlarmUtils.java index 6ca0641..8553c3a 100644 --- a/app/src/main/java/com/philliphsu/clock2/util/AlarmUtils.java +++ b/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());