1658032 - Calendar should not try to load events/alarms for disabled calendars

Status: RESOLVED FIXED

(Calendar :: General, defect, P1)

Description

[Mark Banner (:standard8)]

STR:

1. Set up a profile with at least one calendar that has a reasonable amount of events.
2. Disable the calendar.
3. Restart Thunderbird

Expected Results

Thunderbird starts up promptly, and is usable as early as possible.

Actual Results

Thunderbird opens, the folder pane is displayed, then about 3-4 seconds later, the thread pane and message view is displayed.

Extract from startup profile: https://share.firefox.dev/3a3940O

This profile shows that during that time there is lots of activity loading events, alarms etc via CalStorageCalendar.

As these Calendars are disabled, the calendar shouldn't be attempting to load them at all. They should be filtered out at SQL level, or at least before all the event/task objects are created.

Comment 1

[Stefan Sitter [:ssitter]]

Did you completely switch off the calendar from the calendar properties dialog? Or did you just temporarily toggle the visibility in the left pane? In the later case it is expected that the calendar i loaded and that alarms will be displayed.

Comment 2

[Mark Banner (:standard8)]

(In reply to Stefan Sitter [:ssitter] from comment #1)

Did you completely switch off the calendar from the calendar properties dialog? Or did you just temporarily toggle the visibility in the left pane? In the later case it is expected that the calendar i loaded and that alarms will be displayed.

I right-clicked the properties dialog and de-selected "Enable this Calendar" for all the calendars. I got the "All calendars are currently disabled" warning message.

Comment 3

[Richard Leger]

Same behaviour in TB 81.0b3 (64-bit) when TB UI appears the left column with folders showing but the right pane with message list is missing... it takes more time for it to appear... then in my IMAP mailbox I am affected by Bug 1660672 which delay even more ability to read new email (can select email but loading message is delayed till after looking for folders process completes).

Here is a perf profile published in Bug 1660672 comment 17

(In reply to Richard Leger from Bug 1660672 comment 17)

New perf profile for TB 81.0b3 (64-bit)
https://share.firefox.dev/2ZtbYYF

It happens even if all network calendars are disabled...

You would see a lot of repeated calls to chromeutils.import related to the calendar modules that keep being imported multiple times in row... ideally it should happen only once... and not at all if Calendars are disabled... at least not till Calendar view is becoming active perhaps... or TB idle...

Hope that help!

Comment 4

[Mark Banner (:standard8)]

(In reply to Richard Leger from comment #3)

Same behaviour in TB 81.0b3 (64-bit) when TB UI appears the left column with folders showing but the right pane with message list is missing... it takes more time for it to appear... then in my IMAP mailbox I am affected by Bug 1660672 which delay even more ability to read new email (can select email but loading message is delayed till after looking for folders process completes).

Yes, nothing has landed for this, so it is unlikely to be magically fixed.

You would see a lot of repeated calls to chromeutils.import related to the calendar modules that keep being imported multiple times in row... ideally it should happen only once... and not at all if Calendars are disabled... at least not till Calendar view is becoming active perhaps... or TB idle...

The ChromeUtils.import is bug 1659558 that has been fixed in the latest daily builds.

The issue here is still as I described in comment 0:

As these Calendars are disabled, the calendar shouldn't be attempting to load them at all. They should be filtered out at SQL level, or at least before all the event/task objects are created.

Of course, it'd be even better if calendar didn't load everything at startup (only essentials, and delayed other things to other times), but that's for bug 1658026 and others.

Comment 5

[Paul Morris [:pmorris]]

Attachment: Bug 1658032 - Reduce calls to `getItems` for disabled calendars. r=darktrojan

Comment 6

[Paul Morris [:pmorris]]

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/eb083986f94a
Reduce calls to getItems for disabled calendars. r=darktrojan