Calling AlarmUtils.cancelAlarm() in RingtoneActivity and RingtoneService was actually necessary

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

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
+            if (ACTION_UNBIND.equals(intent.getAction())) {
-            /*
+                // Someone else wants us to unbind
-            // Cannot rely on finish() to call onDestroy() on time before the activity is restarted.
+                sIsAlive = false; // don't wait until onDestroy() to reset
-            unbindService();
+                //dismiss();
-            // Calling recreate() would recreate this with its current intent, not the new intent passed in here.
+                unbindAndFinish();
-            finish();
+            } else {
-            */
+                mBoundService.interrupt(); // prepare to notify the alarm was missed
-            dismiss(); // unbinds and finishes for you
+                /*
-            startActivity(intent);
+                // 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()
+        unbindAndFinish();
-        //finish();
     }
 
     private void dismiss() {
-        // TODO: Do we really need to cancel the PendingIntent and the alarm in AlarmManager? They've
+        AlarmUtils.cancelAlarm(this, mAlarm, false);
-        // already fired, so what point is there to cancelling them?
+        unbindAndFinish();
-        // ===================================== 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();
     }
 
     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() {

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();
+        long id = intent.getLongExtra(EXTRA_ITEM_ID, -1);
-        if (!action.equals(ACTION_SNOOZE) && !action.equals(ACTION_DISMISS))
+        if (id < 0)
-            throw new UnsupportedOperationException();
+            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!
         }
     }