1691885 - Calendar notifications are not emitted sequentially

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, defect)

Description

[Geoff Lankow (:darktrojan)]

The CalDAV provider emits onStartBatch, onEndBatch, and onLoad when it's performing network operations. However, the onLoad notification should happen after the others when everything is complete. Currently for calendars without offline support it is happening first, and for calendars with offline support it is not happening at all.

I think this might be what's causing the problems with alarms (bug 1666156 etc.) but I'm not sure because I can't reproduce them at the moment. It certainly is causing some problems with alarms such as requiring dismissal a second time in some circumstances.

In this bug I'm going to make the notifications happen in sequence, and always with onLoad at the end (except in one case where that seems to be wrong and it causes a problem anyway). I'm also going to have alarms for network calendars only fire in response to the onLoad notification to stop onAddItem/onModifyItem causing a double firing.

Hopefully that will make things less prone to problems.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1691885 - Make CalDAV calendar notifications fire in the right sequence. r?mkmelin

This patch makes the onLoad notification happen at the end of a load (as opposed to the start or not at all)
and has the alarm service respond only to that notification for network calendars.

Comment 2

[Geoff Lankow (:darktrojan)]

Even with this patch the onAddItem/onModifyItem notifications usually happen outside of the onStartBatch/onEndBatch notifications for calendars with offline storage. This is because they come from a bunch of spaghetti code that tells the storage back end what to do, and I've not found a way to wait for that to happen before continuing.

Comment 3

[Richard Leger]

(In reply to Geoff Lankow (:darktrojan) from comment #0)

I think this might be what's causing the problems with alarms (bug 1666156 etc.) but I'm not sure because I can't reproduce them at the moment.

Which version of TB are you testing with?
Have you tried with the reproducible steps described in Bug 1666156 Comment 22 ?

Comment 4

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1691885 - Test notifications from CalDAV and ICS calendars. r?mkmelin

This tests each combination of calendar provider and offline storage setting to ensure they behave consistently.
To do this I've added mock CalDAV and ICS servers (forked from CardDAVServer.jsm).

Depends on D104651

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/88c4fe2fed10
Make CalDAV calendar notifications fire in the right sequence. r=mkmelin
https://hg.mozilla.org/comm-central/rev/6ac8a49c6e5d
Test notifications from CalDAV and ICS calendars. r=mkmelin

Comment 6

[Geoff Lankow (:darktrojan)]

I'm about to file a follow-up bug for the cached ICS calendar.