Bug 1658032 Comment 6 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on 
This patch may not fully solve the whole problem, but it at least fixes part of it.  From discussions this with Philipp over chat, the idea was that calls to `getItems` in `CalStorageCalendar.jsm` (https://searchfox.org/comm-central/source/calendar/providers/storage/CalStorageCalendar.jsm#603) shouldn't be happening for disabled calendars, due to various checks, but we could just effectively return zero items if `getItems` was called for a disabled calendar.  That should solve the problem.

However, I was seeing a lot of calls to `getItems` and trying to check whether the calendar was disabled, but getting `null` instead of `true` for the disabled property, even when all calendars were disabled.  Other properties like "name" and "readOnly" were also null.  Apparently these calls were being made before the calendar properties were fully restored?  

So I started looking into how the calendars were initialized, and how the calendar component was loaded... long story short, I found that many of the `getItems` calls were originating from two window load event handlers.  When calendar was an add-on, it was loaded via a window load handler too, but we had moved that loading step later due to some UI issues. (See bug 1615453.)  So things were apparently happening out of order, and indeed, after converting those window load handlers into function calls made when the calendar component was loaded, those calls to getItems went away.

That left only a single call to `getItems`, originating with the `loadCalendarComponent` call, for each of the caldav calendars I had set up.  A big improvement.  However, the calendars were disabled, but the disabled property was still `null` in the `getItems` call.  Here's the trace.

```
console.trace() CalStorageCalendar.jsm:617:13
    getItems resource:///modules/CalStorageCalendar.jsm:617
    ensureMetaData resource:///modules/CalDavCalendar.jsm:292
    fetchCachedMetaData resource:///modules/CalDavCalendar.jsm:341
    set offlineStorage resource:///modules/CalDavCalendar.jsm:166
    calCachedCalendar resource:///components/calCachedCalendar.js:126
    maybeWrapCachedCalendar resource:///modules/CalCalendarManager.jsm:912
    initializeCalendar resource:///modules/CalCalendarManager.jsm:218
    assureCache resource:///modules/CalCalendarManager.jsm:536
    getCalendars resource:///modules/CalCalendarManager.jsm:496
    set prefPrefix resource:///modules/CalCompositeCalendar.jsm:134
    getCompositeCalendar resource:///modules/calendar/utils/calViewUtils.jsm:176
    loadCalendarManager chrome://calendar/content/calendar-management.js:106
    loadCalendarComponent chrome://calendar/content/calendar-chrome-startup.js:55
```

So this may not fully fix the problem, but it is a step in the right direction.
Revision 1 by
on 
This patch may not fully solve the whole problem, but it at least fixes part of it.  From discussions this with Philipp over chat, the idea was that calls to `getItems` in `CalStorageCalendar.jsm` (https://searchfox.org/comm-central/source/calendar/providers/storage/CalStorageCalendar.jsm#603) shouldn't be happening for disabled calendars, due to various checks, but we could just effectively return zero items if `getItems` was called for a disabled calendar.  That should solve the problem.

However, I was seeing a lot of calls to `getItems` and trying to check whether the calendar was disabled, but getting `null` instead of `true` for the disabled property, even when all calendars were disabled.  Other properties like "name" and "readOnly" were also null.  Apparently these calls were being made before the calendar properties were fully restored?  

So I started looking into how the calendars were initialized, and how the calendar component was loaded... long story short, I found that many of the `getItems` calls were originating from two window load event handlers.  When calendar was an add-on, it was loaded via a window load handler too, but we had moved that loading step later due to some UI issues. (See bug 1615453.)  So things were apparently happening out of order, and indeed, after converting those window load handlers into function calls made when the calendar component is loaded, those calls to getItems went away.

That left only a single call to `getItems`, originating with the `loadCalendarComponent` call, for each of the caldav calendars I had set up.  A big improvement.  However, the calendars were disabled, but the disabled property was still `null` in the `getItems` call.  Here's the trace.

```
console.trace() CalStorageCalendar.jsm:617:13
    getItems resource:///modules/CalStorageCalendar.jsm:617
    ensureMetaData resource:///modules/CalDavCalendar.jsm:292
    fetchCachedMetaData resource:///modules/CalDavCalendar.jsm:341
    set offlineStorage resource:///modules/CalDavCalendar.jsm:166
    calCachedCalendar resource:///components/calCachedCalendar.js:126
    maybeWrapCachedCalendar resource:///modules/CalCalendarManager.jsm:912
    initializeCalendar resource:///modules/CalCalendarManager.jsm:218
    assureCache resource:///modules/CalCalendarManager.jsm:536
    getCalendars resource:///modules/CalCalendarManager.jsm:496
    set prefPrefix resource:///modules/CalCompositeCalendar.jsm:134
    getCompositeCalendar resource:///modules/calendar/utils/calViewUtils.jsm:176
    loadCalendarManager chrome://calendar/content/calendar-management.js:106
    loadCalendarComponent chrome://calendar/content/calendar-chrome-startup.js:55
```

So this may not fully fix the problem, but it is a step in the right direction.
Revision 2 by
on 
This patch may not fully solve the whole problem, but it at least fixes part of it.  From discussions this with Philipp over chat, the idea was that calls to `getItems` in `CalStorageCalendar.jsm` (https://searchfox.org/comm-central/source/calendar/providers/storage/CalStorageCalendar.jsm#603) shouldn't be happening for disabled calendars, due to various checks, but we could just effectively return zero items if `getItems` was called for a disabled calendar.  That should solve the problem.

However, I was seeing a lot of calls to `getItems` and trying to check whether the calendar was disabled, but getting `null` instead of `true` for the disabled property, even when all calendars were disabled.  Other properties like "name" and "readOnly" were also null.  Apparently these calls were being made before the calendar properties were fully restored?  

So I started looking into how the calendars were initialized, and how the calendar component was loaded... long story short, I found that many of the `getItems` calls were originating from two window load event handlers.  When calendar was an add-on, it was loaded via a window load handler too, but we had moved that loading step later due to some UI issues. (See bug 1615453.)  So things were apparently happening out of order, and indeed, after converting those window load handlers into function calls made when the calendar component is loaded, those calls to getItems went away.

That left only a single call to `getItems`, originating with the `loadCalendarComponent` call, for each of the caldav calendars I had set up.  A big improvement.  However, the calendars were disabled, but the disabled property was still `null` in the `getItems` call.  Here's the trace.

```
console.trace() CalStorageCalendar.jsm:617:13
    getItems resource:///modules/CalStorageCalendar.jsm:617
    ensureMetaData resource:///modules/CalDavCalendar.jsm:292
    fetchCachedMetaData resource:///modules/CalDavCalendar.jsm:341
    set offlineStorage resource:///modules/CalDavCalendar.jsm:166
    calCachedCalendar resource:///components/calCachedCalendar.js:126
    maybeWrapCachedCalendar resource:///modules/CalCalendarManager.jsm:912
    initializeCalendar resource:///modules/CalCalendarManager.jsm:218
    assureCache resource:///modules/CalCalendarManager.jsm:536
    getCalendars resource:///modules/CalCalendarManager.jsm:496
    set prefPrefix resource:///modules/CalCompositeCalendar.jsm:134
    getCompositeCalendar resource:///modules/calendar/utils/calViewUtils.jsm:176
    loadCalendarManager chrome://calendar/content/calendar-management.js:106
    loadCalendarComponent chrome://calendar/content/calendar-chrome-startup.js:55
```

So this may not fully fix the problem, but it is a step in the right direction.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ec36e08c8aa99d618d6b038daade292ea6d6c72b

Back to Bug 1658032 Comment 6