1890049 - Message list is loaded twice at startup

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect, P1)

Description

[Alessandro Castellani [:aleca]]

When launching Thunderbird, a stack trace for each of the message rows can be observed.

chrome://messenger/content/thread-card.mjs 81 set index
chrome://messenger/content/tree-view.mjs 1258 _addRowAtIndex
chrome://messenger/content/tree-view.mjs 1086 _ensureVisibleRowsAreDisplayed
chrome://messenger/content/tree-view.mjs 710 reset
chrome://messenger/content/tree-view.mjs 584 set view
chrome://messenger/content/about3Pane.js 4552 setTreeView
chrome://messenger/content/mailCommon.js 988 onCreatedView
resource:///modules/DBViewWrapper.sys.mjs 1544 _applyViewChanges
resource:///modules/DBViewWrapper.sys.mjs 1272 _enterFolder
resource:///modules/DBViewWrapper.sys.mjs 864 open
chrome://messenger/content/about3Pane.js 2511 _onSelect
chrome://messenger/content/about3Pane.js 1540 handleEvent
chrome://messenger/content/about3Pane.js 144

After a few seconds (inconsistent, sometimes it's 1 second, sometimes it's 2), another stack trace is triggered forcing all messages to be set again.

chrome://messenger/content/thread-card.mjs 81 set index
chrome://messenger/content/tree-view.mjs 1258 _addRowAtIndex
chrome://messenger/content/tree-view.mjs 1086 _ensureVisibleRowsAreDisplayed
chrome://messenger/content/tree-view.mjs 710 reset
chrome://messenger/content/mailCommon.js 1049 onMessagesLoaded
resource:///modules/DBViewWrapper.sys.mjs 1263 _enterFolder
resource:///modules/DBViewWrapper.sys.mjs 1250 _folderLoaded
resource:///modules/DBViewWrapper.sys.mjs 237 onFolderEvent

I noticed this because some data properties needed on the tree view weren't being applied right away.
The missing data attributes are due to the lack of window.threadPane.restoreThreadState(); in onCreatedView(), which is called in the first stack. The following stack uses onMessagesLoaded(), which applies all the correct styles.

Comment 1

[Alessandro Castellani [:aleca]]

I identified that this method is called twice on startup.

It seems to come from this other method that first runs on launch, and then waits for the IMAP connection to return a response and stop running the URL.

This is not completely wrong per se, since we need to notify the folder after whatever server ping we do for the currently visible folder to check if there's anything to sync.

The problem is that we then call NotifyFolderEvent() regardless if there's a delta between what we have and what the server tells us, and we have no safeguard that prevents the discard of the message list view and regeneration.

Adding NI to a bunch of people so we can keep the conversation and investigation aspect going.

Comment 2

[Ben Campbell]

(In reply to Alessandro Castellani [:aleca] from comment #1)

The problem is that we then call NotifyFolderEvent() regardless if there's a delta between what we have and what the server tells us, and we have no safeguard that prevents the discard of the message list view and regeneration.

I haven't dug into this in any depth yet, but I have noticed (while doing folder compaction stuff) that the notification meanings are pretty loosely defined, and can be emitted and handled quite differently by the different folder types.
I'm planing to write up a specific bug for the compaction notifications, but I suspect something similar likely applies here.

Since the notifications are so predominantly something emitted by the back end and handled by the front end, I think it makes sense to first try and precisely define what the front end wants: What the event signifies, when it is triggered, and what data (if any) it carries with it.
Then fix the back end to provide that.

General architectural approach: If the back end isn't providing what the front end requires/expects, then it's the back end which is most at fault. Working around it on the front end might work in the short term, but it'll cause pain down the line.

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: Bug 1890049 - Avoid loading message list twice for select of local folders. r=darktrojan

For IMAP, it seems the double load is inevitable, see bug 520272 for history...

For local folders there was a counfusing double load which I've fixed, along
with some added documentation to clarify a few code paths.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/86e4e439b8dc
Avoid loading message list twice for select of local folders. r=darktrojan