1957633 - QM: Transition origin access time updates from directory lock to handle-based tracking

Status: ASSIGNED

(Core :: Storage: Quota Manager, task, P2)

Description

[Jan Varga [:janv]]

This bug tracks a transition in how origin access time updates are triggered in QuotaManager.

Currently, access time is updated when the first ClientDirectoryLock is registered and again when the last one is unregistered. However, this mechanism is not tied to actual usage by quota clients. Instead, we want to associate access time updates more directly with ClientDirectoryLockHandle, which represents actual access to origin directories by clients.

By updating access time when the first handle is registered and the last is unregistered, we gain better control and accuracy. This approach will also eventually support more reliable tracking of other metadata flags, especially the "accessed" flag, which determines whether an origin should be initialized from cache or by a full scan.

This particular bug only handles the transition from lock-based registration to handle-based registration. Actual changes to the flags or metadata updates will be addressed in follow-up bugs.

Comment 1

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Add MutableManagerRef() to DirectoryLockImpl; r=#dom-storage

Adds a new method, MutableManagerRef(), to DirectoryLockImpl, which returns a
reference to the owning QuotaManager instance.

Returning mQuotaManager is safe and doesn't require assertions since it is
stored as a NotNull<RefPtr<QuotaManager>>.

Comment 2

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Add GetCompositeKey() to OriginMetadata for flat map usage; r=#dom-storage

Adds a GetCompositeKey() method to OriginMetadata, which returns a composite
string key combining persistence type and origin.

This is intended for use in data structures where a flat hash map is preferred
over a tree-based representation (e.g., for simple lookup or registration
tracking keyed by both persistence type and origin).

Comment 3

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Introduce OpenClientDirectoryInfo to support handle-based origin access tracking; r=#dom-storage

Introduces the OpenClientDirectoryInfo class, which tracks active
ClientDirectoryLockHandle instances for a given origin.

This class will be used in a follow-up patch to support origin access time
updates based on the lifetime of handles rather than raw directory locks.
For now, this class is not yet connected to OpenClientDirectory directly.

OpenClientDirectoryInfo keeps track of the origin metadata and a count of
active handles. It is intended to live only while at least one handle is
active for a given origin, and provides a foundation for metadata updates
tied to actual client activity.

Comment 4

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Transition origin access time updates from directory lock to handle-based tracking; r=#dom-storage

Before patch:

• Access time was updated on first client directory lock registration, which
  could happen before the lock was actually acquired, and even before storage
  and origin initialization were complete. As a result, the update could run
  while the origin directory didn't yet exist, so the access time was not
  updated at all.
• Access time was also updated on the last client directory lock
  unregistration, but by that point, a clear or shutdown operation could have
  been scheduled, meaning the update would run too late and potentially after
  the origin directory had been deleted.
• Tracking was done only for best-effort persistence types.

After patch:

• Access time is now updated on the first client directory lock handle
  registration, which happens after all necessary initialization. While it can
  still encounter an empty origin directory (due to LSNG optimizations), it's a
  cleaner and more predictable behavior.
• On last client directory lock handle unregistration, the access time is
  updated before the lock is dropped. This is slightly different, and even if
  it were updated after the lock is dropped, that wouldn't help much because
  dropping is asynchronous, and the given lock is not immediately unregistered.
  This difference will become irrelevant once we pre-acquire a directory lock
  for saving origin access time in OpenClientDirectory.
  The problem where the save operation might run after a clear or shutdown
  operation is still pending and will be addressed by pre-acquiring a lock for
  saving origin access time in a follow-up bug.
• Tracking is now done for all persistence types. (Access time updates are
  still performed only for best-effort persistence types, but this lays the
  groundwork for future improvements like updating the last maintenance date.)

Additional changes:

• Behavior hasn't changed much, but correctness has improved slightly and
  the logic is now simpler.
• The DirectoryLockImpl::ShouldUpdateLockTable() method has been removed. It
  was not very clean and is no longer necessary with the new structure.
• Registration has been simplified: instead of maintaining separate hash maps
  per persistence type or a map with array values keyed by origin, a single
  hash map using a composite key (persistence type + origin) is now used.
• Also addresses an old XXX comment about avoiding raw pointers and
  replacing them with a simple counter.

Comment 5

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Move access time updates out of Register/UnregisterClientDirectoryLockHandle; r=#dom-storage

QuotaManager::OpenClientDirectory will eventually coordinate more closely
with OpenClientDirectoryInfo, including tracking the initial saving of origin
access time and managing pre-acquired directory locks. To avoid cluttering
the registration methods with high-level logic, access time updates are now
handled by the caller rather than from within RegisterClientDirectoryLockHandle
and UnregisterClientDirectoryLockHandle.

This patch refactors RegisterClientDirectoryLockHandle to accept a templated
callback, which is invoked when the first handle is registered for an origin.

UnregisterClientDirectoryLockHandle is also updated to accept a templated
callback, which is invoked when the last handle is unregistered.

Comment 6

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Rationalize QuotaManager::UpdateOriginAccessTime arguments; r=#dom-storage

Refactors UpdateOriginAccessTime to take OriginMetadata as a single argument
instead of passing the persistence type and metadata separately. This reduces
redundancy, simplifies usage, and helps prevent mismatches between parameters.

This is a continuation of bug 1697115, which also focused on simplifying
some QuotaManager methods by consolidating arguments.

Comment 7

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Add SaveOriginAccessTime helper to QuotaManagerDependencyFixture; r=#dom-storage

Adds a static helper method to QuotaManagerDependencyFixture that runs
QuotaManager::SaveOriginAccessTime on the PBackground thread.

This simplifies calling SaveOriginAccessTime from gtests and ensures
consistency across test cases.

For more complex testing, use of this helper is optional.

Comment 8

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Add support for tracking SaveOriginAccessTime metadata updates; r=#dom-storage

Introduce mSaveOriginAccessTimeCount and methods SaveOriginAccessTimeCount and
IncreaseSaveOriginAccessTimeCount to track SaveOriginAccessTime metadata
updates.

This is mainly preparation for more complex testing related to the planned
transition from directory lock based to handle based access time tracking,
which will be handled by QuotaManager::OpenClientDirectory.

Currently, access time updates may be skipped in some edge cases. The
transition and follow-up patches will address those cases, and this counter
will help test that updates are applied consistently.

Comment 9

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Add helper to process pending normal origin operations; r=#dom-storage

Adds QuotaManager::ProcessPendingNormalOriginOperations, which ensures that
any pending normal origin operations and their follow-up events are processed
and completed.

This helper is intended for use in gtests where asynchronous operations are
triggered without a way to explicitly await their completion.

Comment 10

[Phabricator Automation]

Comment on attachment 9477744 [details]
Bug 1957633 - QM: Add SaveOriginAccessTime helper to QuotaManagerDependencyFixture; r=#dom-storage

Revision D244752 was moved to bug 1959622. Setting attachment 9477744 [details] to obsolete.

Comment 11

[Phabricator Automation]

Comment on attachment 9477783 [details]
Bug 1957633 - QM: Add support for tracking SaveOriginAccessTime metadata updates; r=#dom-storage

Revision D244776 was moved to bug 1959622. Setting attachment 9477783 [details] to obsolete.

Comment 12

[Phabricator Automation]

Comment on attachment 9478030 [details]
Bug 1957633 - QM: Add helper to process pending normal origin operations; r=#dom-storage

Revision D244929 was moved to bug 1959622. Setting attachment 9478030 [details] to obsolete.

Comment 13

[Jan Varga [:janv]]

Attachment: Bug 1957633 - QM: Reduce indentation in QuotaManager::OpenClientDirectory promise chain; r=#dom-storage

Refactors the OpenClientDirectory implementation to reduce indentation in the
promise chain, improving readability and maintainability. This change is
motivated by upcoming additions to this method, and prepares the structure for
easier integration of follow-up changes without excessive nesting.

The functional behavior remains unchanged.