1866217 - Introduce PersistenceScope class à la OriginScope

Status: RESOLVED FIXED

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

Description

[Jan Varga [:janv]]

Directory locks currently only support specifying a concrete persistence type or all persistence types. There are situations like temporary storage
initialization when multiple persistence types need to be specified. It would be possible to create multiple directory locks for that, but that would also introduce non-trivial additional complexity. A new PersistenceScope class in the style of OriginScope will make it easy to specify multiple persistence types.

Comment 1

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Add a dedicated method for getting cached origin usage; r=#dom-storage

Cached origin usage can be currently obtained by passing fromMemory=true to
nsIQuotaManagerService::GetUsageForPrincipal. However, supporting both ways of
getting origin on one operation is not ideal for sevaral reasons. The return
value is a plain integer in the case of cached origin usage and getting cached
origin usage in theory doesn't have to go the QM IO thread. So it would be
better to have a dedicated method for that.

Depends on D194542

Comment 2

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Replace some nsIQuotaManagerService::GetUsageForPrincipal calls with nsIQuotaManagerService::GetCachedUsageForPrincipal; r=#dom-storage

There's now a dedicated nsIQuotaManagerService::GetCachedUsageForPrincipal
method for getting cached origin usage. All callers of
nsIQuotaManagerService::GetUsageForPrincipal which want to get cached origin
usage for given principal can be now converted to call the new method.

Depends on D195360

Comment 3

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Change nsIQuotaManagerService::GetUsageForPrincipal to support getting origin usage only by inspecting disk; r=#dom-storage

Depends on D195363

Comment 4

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Introduce PersistenceScope class à la OriginScope; r=#dom-storage

Directory locks currently only support specifying a concrete persistence type
or all persistence types. There are situations like temporary storage
initialization when multiple persistence types need to be specified. It would
be possible to create multiple directory locks for that, but that would also
introduce non-trivial additional complexity. A new PersistenceScope class in
the style of OriginScope will makes it easy to specify multiple persistence
types. Support for actual persistence type sets will be added in a separate
patch, this patch just adds basic functionality.

Depends on D195365

Comment 5

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Replace uses of Nullable<PersistenceType> type with PersistenceScope; r=#dom-storage

Nullable<PersistenceType> only allows to specify a concrete persistence type or
all persistence types while PersistenceScope is intended to allow specification
of multiple persistence types as well. Support for that will be added in a
separate patch. This patch is about using the new type especially in directory
locks.

Depends on D195366

Comment 6

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Add support for persistence type sets to PersistenceScope; r=#dom-storage

Temporary storage initialization can finally lock only specific repositories
instead of locking all repositories (actual temporary storage initialization
locking chagnes will be done in a separate patch).

Depends on D195373

Comment 7

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Scope temporary storage initialization to temporary and default repository only; r=#dom-storage

Temporary storage initialization currently uses a directory lock over all
repositories which blocks any exclusive directory locks for the persistent
repository for now reason.

Depends on D195374

Comment 8

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Add support for initializing temporary storage to QuotaManager::OpenClientDirectory; r=#dom-storage

QuotaManager::OpenClientDirectory can now detect the requested persistence type
and initializa temporary storage if it's needed.

Depends on D195389

Comment 9

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Remove QuotaManager::EnsureTemporaryStorageIsInitializedInternal calls from places where a directory lock is obtained via QuotaManager::OpenClientDirectory; r=#dom-storage

QuotaManager::OpenClientDirectory already makes sure that temporary storage
is initialized so all these calls are now redundant.

Depends on D195390

Comment 10

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Add support for initializing temporary storage to QuotaManager::OpenStorageDirectory; r=#dom-storage

QuotaManager::OpenStorageDirectory can now detect the requested persistence
scope and initialize temporary storage if it's needed.

Depends on D195391

Comment 11

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Remove QuotaManager::EnsureTemporaryStorageIsInitializedInternal calls from places where a directory lock is obtained via QuotaManager::OpenStorageDirectory; r=#dom-storage

QuotaManager::OpenStorageDirectory already makes sure that temporary storage
is initialized so all these calls are now redundant.

Depends on D195393

Comment 12

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Change QuotaManager::EnsureTemporaryStorageIsInitializedInternal to be a private method; r=#dom-storage

The goal to call QuotaManager::EnsureTemporaryStorageIsInitializedInternal only
from InitTemporaryStorageOp has been achieved. It's now easy to change the
method to be a private method.

Depends on D195395

Comment 13

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Update QuotaManager gtests to use Await instead of manual spinning of the event loop; r=#dom-storage

TestQuotaManager.cpp is getting big and the use of the Await helper reduces the
size of the file significantly and also make the tests more readable.

Depends on D195396

Comment 14

[Jan Varga [:janv]]

Attachment: Bug 1866217 - Add more tests for QuotaManager::InitializeTemporaryStorage method; r=#dom-storage

These additional tests are mainly to improve code coverage of
QuotaManager::InitializeTemporaryStorage method.

Depends on D195399

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d70ebc8c1c45
Add a dedicated method for getting cached origin usage; r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/31f3c500ec27
Replace some nsIQuotaManagerService::GetUsageForPrincipal calls with nsIQuotaManagerService::GetCachedUsageForPrincipal; r=dom-storage-reviewers,jari

Comment 16

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/d70ebc8c1c45
https://hg.mozilla.org/mozilla-central/rev/31f3c500ec27

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2ccb09583e3
Change nsIQuotaManagerService::GetUsageForPrincipal to support getting origin usage only by inspecting disk; r=dom-storage-reviewers,jari

Comment 18

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/b2ccb09583e3

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d20d53aeb69a
Introduce PersistenceScope class à la OriginScope; r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/a18fea81cdd6
Replace uses of Nullable<PersistenceType> type with PersistenceScope; r=dom-storage-reviewers,jari

Comment 20

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d20d53aeb69a
https://hg.mozilla.org/mozilla-central/rev/a18fea81cdd6

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/441d2c47e27f
Add support for persistence type sets to PersistenceScope; r=dom-storage-reviewers,jari

Comment 22

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/441d2c47e27f

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0bd45addadc
Scope temporary storage initialization to temporary and default repository only; r=dom-storage-reviewers,jari

Comment 24

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/f0bd45addadc

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97190135be72
Add support for initializing temporary storage to QuotaManager::OpenClientDirectory; r=dom-storage-reviewers,jari

Comment 26

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures @ test_cache_orphaned_body.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/2507f6a9f003677d3d1802e1994f1dba8ffff3a0

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | dom/cache/test/mochitest/test_cache_orphaned_body.html | Test timed out

Comment 27

[Phabricator Automation]

Comment on attachment 9366768 [details]
Bug 1866217 - Add support for initializing temporary storage to QuotaManager::OpenClientDirectory; r=#dom-storage

Revision D195390 was moved to bug 1913561. Setting attachment 9366768 [details] to obsolete.

Comment 28

[Phabricator Automation]

Comment on attachment 9366769 [details]
Bug 1866217 - Remove QuotaManager::EnsureTemporaryStorageIsInitializedInternal calls from places where a directory lock is obtained via QuotaManager::OpenClientDirectory; r=#dom-storage

Revision D195391 was moved to bug 1913561. Setting attachment 9366769 [details] to obsolete.

Comment 29

[Phabricator Automation]

Comment on attachment 9366771 [details]
Bug 1866217 - Add support for initializing temporary storage to QuotaManager::OpenStorageDirectory; r=#dom-storage

Revision D195393 was moved to bug 1913561. Setting attachment 9366771 [details] to obsolete.

Comment 30

[Phabricator Automation]

Comment on attachment 9366777 [details]
Bug 1866217 - Remove QuotaManager::EnsureTemporaryStorageIsInitializedInternal calls from places where a directory lock is obtained via QuotaManager::OpenStorageDirectory; r=#dom-storage

Revision D195395 was moved to bug 1913561. Setting attachment 9366777 [details] to obsolete.

Comment 31

[Phabricator Automation]

Comment on attachment 9366782 [details]
Bug 1866217 - Change QuotaManager::EnsureTemporaryStorageIsInitializedInternal to be a private method; r=#dom-storage

Revision D195396 was moved to bug 1913561. Setting attachment 9366782 [details] to obsolete.

Comment 32

[Phabricator Automation]

Comment on attachment 9366785 [details]
Bug 1866217 - Update QuotaManager gtests to use Await instead of manual spinning of the event loop; r=#dom-storage

Revision D195399 was moved to bug 1913561. Setting attachment 9366785 [details] to obsolete.

Comment 33

[Phabricator Automation]

Comment on attachment 9366787 [details]
Bug 1866217 - Add more tests for QuotaManager::InitializeTemporaryStorage method; r=#dom-storage

Revision D195400 was moved to bug 1913561. Setting attachment 9366787 [details] to obsolete.