1643313 - Consider having separate subclasses for DirectoryLockImpl to differentiate whether mOpenListener is used

Status: NEW

(Core :: Storage: Quota Manager, enhancement, P3)

Description

[Tom Tung [:tt, :ttung]]

See discussion in: https://phabricator.services.mozilla.com/D78066?id=289977#inline-450803

mOpenListener is always set when aInternal is false in CTOR. It can only be set in CTOR so I make it as InitializedOnce in Bug 1641231.

However, the consumers for InitializedOnce are often set (thus, it often used in these two patterns: InitializedOnce<const NotNull<T>> or LazyInitializedOnceEarlyDestructible<const NotNull<T>>)

Here, it can be a nullptr for the whole life of DirectoryLockImpl. My understanding is that it can only be a nullptr for eviction (for QM) or database maintenance (for IDB).

Meanwhile, there is a member function DirectoryLockImpl::NotifyOpenListener() which should only be called when it's not in these two cases. There is a debug assertion to avoid misuse, but still it's not 100% obvious from the code.

Simon suggested at least we should move the assertion to the call sites and add comments around there to assert/explain why we are sure mOpenListener is valid or have seperate subclasses or add a template argument for this distinction to make it safer and exclude the possibility of nullptr dereferences.