Bug 1110183 - Displayed reminders can be improperly snoozed or duplicated when the calendar is refreshed. r=philipp

calendar/base/content/dialogs/calendar-alarm-dialog.js

@@ -279,18 +279,13 @@ function removeWidgetFor(aItem, aAlarm) {
  */
 function closeIfEmpty() {
     let alarmRichlist = document.getElementById("alarm-richlist");
-    if (!alarmRichlist.hasChildNodes()) {
-        // check again in a short while since this removeWidgetFor call may be
-        // followed by an addWidgetFor call (e.g. when refreshing), and
-        // we don't want to close and open the window in that case.
-        function closer() {
-            if (!alarmRichlist.hasChildNodes()) {
+
+    // we don't want to close if the alarm service is still loading, as the
+    // removed alarms may be immediately added again.
+    if (!alarmRichlist.hasChildNodes() && !getAlarmService().isLoading) {
         window.close();
     }
 }
-        setTimeout(closer, 500);
-    }
-}
 
 /**
  * Handler function called when an alarm entry in the richlistbox is selected

calendar/base/public/calIAlarmService.idl

@@ -41,7 +41,7 @@ interface calIAlarmServiceObserver : nsISupports
   void onAlarmsLoaded(in calICalendar calendar);
 };
 
-[scriptable,uuid(03669cf3-bf4f-4692-97a1-cca891964a1d)]
+[scriptable,uuid(42cfa9ce-49d6-11e5-b88c-5b90eedc1c47)]
 interface calIAlarmService : nsISupports
 {
   /**
@@ -50,6 +50,11 @@ interface calIAlarmService : nsISupports
    */
   attribute calITimezone timezone;
 
+  /**
+   * Will return true while the alarm service is in the process of loading alarms
+   */
+  attribute boolean isLoading;
+
   /**
    * Cause the alarm service to start up, create a list of upcoming
    * alarms in all registered calendars, add observers to watch for

calendar/base/src/calAlarmMonitor.js

@@ -170,5 +170,11 @@ calAlarmMonitor.prototype = {
         });
     },
 
-    onAlarmsLoaded: function cAM_onAlarmsLoaded(aCalendar) {}
+    onAlarmsLoaded: function cAM_onAlarmsLoaded(aCalendar) {
+        // the alarm dialog won't close while alarms are loading, check again now
+        let calAlarmWindow = peekAlarmWindow();
+        if (calAlarmWindow) {
+            calAlarmWindow.closeIfEmpty();
+        }
+    }
 };

calendar/base/src/calAlarmService.js

@@ -484,6 +484,8 @@ calAlarmService.prototype = {
             QueryInterface: XPCOMUtils.generateQI([Components.interfaces.calIOperationListener]),
             alarmService: this,
             addRemovePromise: PromiseUtils.defer(),
+            batchCount: 0,
+            results: false,
             onOperationComplete: function cAS_fA_onOperationComplete(aCalendar,
                                                                      aStatus,
                                                                      aOperationType,
@@ -495,9 +497,16 @@ calAlarmService.prototype = {
 
                     // notify observers that the alarms for the calendar have been loaded
                     this.alarmService.mObservers.notify("onAlarmsLoaded", [aCalendar]);
-                }, function onReject(aReason) {
+                }, (aReason) => {
                     Components.utils.reportError("Promise was rejected: " + aReason);
+                    this.alarmService.mLoadedCalendars[aCalendar.id] = true;
+                    this.alarmService.mObservers.notify("onAlarmsLoaded", [aCalendar]);
                 });
+
+                // if no results were returned we still need to resolve the promise
+                if (!this.results) {
+                    this.addRemovePromise.resolve();
+                }
             },
             onGetResult: function cAS_fA_onGetResult(aCalendar,
                                                      aStatus,
@@ -506,6 +515,9 @@ calAlarmService.prototype = {
                                                      aCount,
                                                      aItems) {
                 let promise = this.addRemovePromise;
+                this.batchCount++;
+                this.results = true;
+
                 cal.forEach(aItems, (item) => {
                     try {
                         this.alarmService.removeAlarmsForItem(item);
@@ -513,8 +525,10 @@ calAlarmService.prototype = {
                     } catch (ex) {
                         promise.reject(ex);
                     }
-                }, function completed() {
+                }, () => {
+                    if (--this.batchCount <= 0) {
                         promise.resolve();
+                    }
                 });
             }
         };
@@ -528,17 +542,24 @@ calAlarmService.prototype = {
             // assuming that suppressAlarms does not change anymore until refresh:
             if (!calendar.getProperty("suppressAlarms") &&
                 !calendar.getProperty("disabled")) {
+                this.mLoadedCalendars[calendar.id] = false;
                 calendar.getItems(filter, 0, aStart, aUntil, getListener);
+            } else {
+                this.mLoadedCalendars[calendar.id] = true;
+                this.mObservers.notify("onAlarmsLoaded", [calendar]);
             }
         }
     },
 
     initAlarms: function cAS_initAlarms(aCalendars) {
-        // Purge out all alarm timers belonging to the refreshed/loaded calendar:
+        // Purge out all alarm timers belonging to the refreshed/loaded calendars
         this.disposeCalendarTimers(aCalendars);
 
-        // Purge out all alarms from dialog belonging to the refreshed/loaded calendar:
-        this.mObservers.notify("onRemoveAlarmsByCalendar", aCalendars);
+        // Purge out all alarms from dialog belonging to the refreshed/loaded calendars
+        for (let calendar of aCalendars) {
+            this.mLoadedCalendars[calendar.id] = false;
+            this.mObservers.notify("onRemoveAlarmsByCalendar", [calendar]);
+        }
 
         // Total refresh similar to startup.  We're going to look for
         // alarms +/- 1 month from now.  If someone sets an alarm more than
@@ -557,5 +578,12 @@ calAlarmService.prototype = {
             aItem.getProperty("STATUS") != "CANCELLED") {
             this.mObservers.notify("onAlarm", [aItem, aAlarm]);
         }
+    },
+
+    get isLoading() {
+        for (let calId in this.mLoadedCalendars) {
+            if (!this.mLoadedCalendars[calId]) return true;
+        }
+        return false;
     }
 };

calendar/test/unit/test_alarmservice.js

@@ -28,14 +28,19 @@ let alarmObserver = {
         }
     },
 
+    onRemoveAlarmsByCalendar: function() {},
+
     onAlarmsLoaded: function obs_onAlarmsLoaded(aCalendar) {
+        this.checkLoadStatus();
         if (aCalendar.id in this.pendingOps) {
             this.pendingOps[aCalendar.id].call();
         }
     },
 
     doOnAlarmsLoaded: function obs_doOnAlarmsLoaded(aCalendar, aOperation) {
-        if (aCalendar.id in this.service.mLoadedCalendars) {
+        this.checkLoadStatus();
+        if (aCalendar.id in this.service.mLoadedCalendars &&
+            this.service.mLoadedCalendars[aCalendar.id]) {
             // the calendar's alarms have already been loaded, do the callback now
             aOperation.call();
         } else {
@@ -85,6 +90,17 @@ let alarmObserver = {
         }
     },
 
+    checkLoadStatus: function obs_checkLoadStatus() {
+        for (let calId in this.service.mLoadedCalendars) {
+            if (!this.service.mLoadedCalendars[calId]) {
+                // at least one calendar hasn't finished loading alarms
+                ok(this.service.isLoading);
+                return;
+            }
+        }
+        ok(!this.service.isLoading);
+    },
+
     clear: function obs_clear() {
         this.firedMap = {};
         this.pendingOps = {};
@@ -95,16 +111,21 @@ let alarmObserver = {
 function run_test() {
     do_get_profile();
 
-    add_test(test_addItems);
-    add_test(test_loadCalendar);
-    add_test(test_modifyItems);
-
+    add_test(() => {
+        // initialization needs to be done within the first test in order for
+        // the subsequent tests to run properly
         initializeAlarmService();
         cal.getCalendarManager().startup({onResult: function() {
             cal.getTimezoneService().startup({onResult: function() {
                 run_next_test();
             }});
         }});
+    });
+    add_test(test_addItems);
+    add_test(test_loadCalendar);
+    add_test(test_modifyItems);
+
+    run_next_test();
 }
 
 function initializeAlarmService() {