From 514bf414753ddcc4a4e3ddaf43081f8a6bd17839 Mon Sep 17 00:00:00 2001 From: Phillip Hsu Date: Wed, 8 Jun 2016 14:01:54 -0700 Subject: [PATCH] Calling AlarmUtils.cancelAlarm() in RingtoneActivity and RingtoneService was actually necessary --- .../clock2/editalarm/EditAlarmActivity.java | 6 ++ .../clock2/ringtone/RingtoneActivity.java | 61 +++++++++++-------- .../clock2/ringtone/RingtoneService.java | 25 +++----- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java b/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java index b537e6b..a5fe682 100644 --- a/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java +++ b/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java @@ -28,6 +28,7 @@ import com.philliphsu.clock2.DaysOfWeek; import com.philliphsu.clock2.R; import com.philliphsu.clock2.SharedPreferencesHelper; import com.philliphsu.clock2.model.AlarmsRepository; +import com.philliphsu.clock2.ringtone.RingtoneActivity; import com.philliphsu.clock2.util.AlarmUtils; import java.util.Date; @@ -433,6 +434,11 @@ public class EditAlarmActivity extends BaseActivity implements AlarmNumpad.KeyLi @Override public void cancelAlarm(Alarm alarm, boolean showToast) { AlarmUtils.cancelAlarm(this, alarm, showToast); + if (RingtoneActivity.isAlive()) { + Intent intent = new Intent(this, RingtoneActivity.class) + .setAction(RingtoneActivity.ACTION_UNBIND); + startActivity(intent); + } } @Override diff --git a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java index 18e4e10..a26dde1 100644 --- a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java +++ b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java @@ -31,14 +31,18 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi // Shared with RingtoneService public static final String EXTRA_ITEM_ID = "com.philliphsu.clock2.ringtone.extra.ITEM_ID"; + public static final String ACTION_UNBIND = "com.philliphsu.clock2.ringtone.action.UNBIND"; private Alarm mAlarm; private boolean mBound = false; + private static boolean sIsAlive = false; + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); setContentView(R.layout.activity_ringtone); + sIsAlive = true; getWindow().addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON); getWindow().addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON); @@ -80,15 +84,22 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi protected void onNewIntent(Intent intent) { //super.onNewIntent(intent); // Not needed since no fragments hosted? if (mBound) { - mBoundService.interrupt(); // prepare to notify the alarm was missed - /* - // Cannot rely on finish() to call onDestroy() on time before the activity is restarted. - unbindService(); - // Calling recreate() would recreate this with its current intent, not the new intent passed in here. - finish(); - */ - dismiss(); // unbinds and finishes for you - startActivity(intent); + if (ACTION_UNBIND.equals(intent.getAction())) { + // Someone else wants us to unbind + sIsAlive = false; // don't wait until onDestroy() to reset + //dismiss(); + unbindAndFinish(); + } else { + mBoundService.interrupt(); // prepare to notify the alarm was missed + /* + // Cannot rely on finish() to call onDestroy() on time before the activity is restarted. + unbindService(); + // Calling recreate() would recreate this with its current intent, not the new intent passed in here. + finish(); + */ + unbindAndFinish(); + startActivity(intent); + } } } @@ -126,37 +137,28 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi protected void onDestroy() { super.onDestroy(); unbindService(); + sIsAlive = false; } @Override public void onServiceFinish() { - dismiss(); + unbindAndFinish(); + } + + public static boolean isAlive() { + return sIsAlive; } private void snooze() { AlarmUtils.snoozeAlarm(this, mAlarm); - // TODO: If dismiss() calls AlarmUtils.cancelAlarm(), don't call dismiss(). - dismiss(); // Can't call dismiss() because we don't want to also call cancelAlarm()! Why? For example, // we don't want the alarm, if it has no recurrence, to be turned off right now. - //unbindService(); // don't wait for finish() to call onDestroy() - //finish(); + unbindAndFinish(); } private void dismiss() { - // TODO: Do we really need to cancel the PendingIntent and the alarm in AlarmManager? They've - // already fired, so what point is there to cancelling them? - // ===================================== WARNING ========================================== - // If you call cancelAlarm(), then you MUST make sure you are not interfering with a recent - // scheduleAlarm() or snoozeAlarm() call. This can actually be the case, so I recommend you - // do NOT call it! A PendingIntent and alarm that have already been fired won't bother - // you, so just let it sit until the next time the same Alarm is scheduled and they subsequently - // get cancelled! - // ======================================================================================== - //AlarmUtils.cancelAlarm(this, mAlarm); // not necessary? - - unbindService(); // don't wait for finish() to call onDestroy() - finish(); + AlarmUtils.cancelAlarm(this, mAlarm, false); + unbindAndFinish(); } private void unbindService() { @@ -166,6 +168,11 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi } } + private void unbindAndFinish() { + unbindService(); // don't wait for finish() to call onDestroy() + finish(); + } + private RingtoneService mBoundService; private ServiceConnection mConnection = new ServiceConnection() { 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 08143f9..3ebffcb 100644 --- a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java +++ b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java @@ -77,25 +77,18 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc // stopService(Intent) [or stopSelf()] is called, regardless of whether any clients are connected to it." // I have found the regardless part does not apply here. You MUST also unbind any clients from this service // at the same time you stop this service! - String action = intent.getAction(); - if (!action.equals(ACTION_SNOOZE) && !action.equals(ACTION_DISMISS)) - throw new UnsupportedOperationException(); + long id = intent.getLongExtra(EXTRA_ITEM_ID, -1); + if (id < 0) + throw new IllegalStateException("No item id set"); + Alarm alarm = checkNotNull(AlarmsRepository.getInstance(this).getItem(id)); if (ACTION_SNOOZE.equals(intent.getAction())) { - long id = intent.getLongExtra(EXTRA_ITEM_ID, -1); - if (id < 0) - throw new IllegalStateException("No item id set"); - Alarm alarm = checkNotNull(AlarmsRepository.getInstance(this).getItem(id)); AlarmUtils.snoozeAlarm(this, alarm); + } else if (ACTION_DISMISS.equals(intent.getAction())) { + AlarmUtils.cancelAlarm(this, alarm, false); + } else { + throw new UnsupportedOperationException(); } - // ============================== WARNING =================================== - // I AM RECOMMENDING MYSELF TO NOT DO ANYTHING FOR ACTION_DISMISS. - // We don't really need to cancel the PendingIntent and the alarm in AlarmManager if - // they've already been fired. We can just let it sit! See the similar warning - // in RingtoneActivity#dismiss(). - // /*else if (ACTION_DISMISS.equals(intent.getAction())) { - // AlarmUtils.cancelAlarm(this, alarm); - // }*/ // ========================================================================== stopSelf(startId); if (mRingtoneCallback != null) { @@ -193,7 +186,7 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc // Needed so clients can get the Service instance and e.g. call setRingtoneCallback(). public class RingtoneBinder extends Binder { RingtoneService getService() { - return RingtoneService.this; + return RingtoneService.this; // Precludes the class from being static! } }