1736931 - Tidy up database use in folder code (prevent null mDatabase issues).

Status: NEW

(MailNews Core :: General, task)

Description

[Ben Campbell]

(OK, so this is a bit of an annoyingly open-ended bug, but I think we need to clarify how folders use the nsIMsgDatabase (i.e. summary/.msf file). Maybe this becomes a [meta] bug and we file bugs with concrete actions underneath...)

Following on from Bug 1699968, there's a lot of brittleness in how folder code uses the msg database. In particular, everywhere seems a little fuzzy on when nsMsgDBFolder::mDatabase is null and when it's not, and what that means.

Here's a off-the-top-of-my-head set of questions I wrote in https://bugzilla.mozilla.org/show_bug.cgi?id=1699968#c11

• what does a null mDatabase mean? (no database at all? an database unloaded, to save memory? something else?)
• why do we do lazy database initialization? (via GetDatabase()).
• Is there any specific reason we shouldn't initialise the database upon folder creation, so folder functions can rely on having a valid mDatabase?
• what simplification can we do to GetDatabase()/GetDatabasewithoutReparse()

From a quick look through, it seems like mDatabase is only ever nulled when the folder is being shut down/deleted.
(well, also for folder compaction and renaming and maybe a couple of other special operations).
My gut feeling is that the life cycle of the database object (mDatabase) should be managed by the folder, and almost always be non-null (except for special operations which are managed by the folder anyway). I think this would simplify a lot of code.

A good place to start might be to turn nsIMsgFolder.msgDatabase into a read-only attribute, and prevent code outside the folder from setting it (covered by Bug 792915, albeit with a misleading title). I think the folder should be fully responsible for the msgdb life cycle. Outside code should keep it's hands off.

Comment 1

[Ben Campbell]

More notes and some questions about OpenDatabase():
Bug 1535993 comment 14

Comment 2

[Ben Campbell]

Notes on the nsIMsgFolder.msgDatabase attribute:

Reading .msgDatabase on a folder with a null mDatabase will cause the database to be opened/created.
This may fail, if the database is being reparsed (NS_MSG_ERROR_FOLDER_SUMMARY_OUT_OF_DATE - reparsing is an async operation).
If there's no .msf file, I think it'll create a new blank database (without reparsing local mail or asking IMAP server or whatever).

There is also the nsIMsgFolder.databaseOpen bool attribute which corresponds directly to the state of .mDatabase and doesn't trigger any side effects.