1571336 - Call startBatch and endBatch on calDAV calendar even if it isn't cached

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, enhancement)

Description

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

For some reason I don't understand, we don't call startBatch or endBatch on a calDAV calendar if it's uncached. We should.

Comment 1

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

Attachment: 1571336-caldav-batch-1.diff

Comment 2

[Paul Morris [:pmorris]]

Comment on attachment 9082921 [details] [diff] [review]
1571336-caldav-batch-1.diff

Review of attachment 9082921 [details] [diff] [review]:
-----------------------------------------------------------------

I don't know the reason why we weren't calling startBatch/endBatch for cached calDAV calendars, so these changes seem good to me.  r+ with my one comment addressed.  I'm also curious why you are preferring 'calendar.offlineStorage' rather than 'calendar.superCalendar' for getting at these startBatch/endBatch functions, since that wasn't clear to me.

::: calendar/providers/caldav/calDavRequestHandlers.js
@@ +93,5 @@
>              this._reader = null;
>          }
>  
>          // Now that we are done, check which items need fetching.
> +        this.calendar.mOfflineStorage.startBatch();

Since we are using these "m" prefixes to signal a "private" property, only to be accessed internally, I think we should use the "public" `this.calendar.offlineStorage` instead.

Comment 3

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

In this case the "m" really seems to mean "not exposed through XPCOM". It's far from the only usage in this file, so I think it's okay to use.

Comment 4

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

Attachment: 1571336-caldav-batch-2.diff

I've changed my mind about whether to use this.calendar.superCalendar or this.calendar.mOfflineStorage so many times. this.calendar itself is also an option. I'm pretty sure they all work fine. But on the whole I guess superCalendar makes more sense.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1933f7704fb2
Call startBatch and endBatch on calDAV calendar even if it isn't cached. r=pmorris