Bug 1749504 - Make sure storage is initialized before saving origin access time; r=dom-storage-reviewers,jstutte

SaveOriginAccessTimeOp::DoDirectoryWork currently doesn't call
QuotaManager::EnsureStorageIsInitializedInternal and just expects that
something else initialized storage previously. This seems to work, but it would
be cleaner to always make sure that storage is initialized. However, adding
QuotaManager::EnsureStorageIsInitializedInternal revealed another problem.
Storage shudown or storage clearing acquires an exlusive lock over entire
storage area which essentially forces that all existing directory locks are
released first. When the last directory lock for an origin is released, saving
of origin access time is scheduled. The problem is that it's scheduled after
the exclusive lock for storage shutdown or storage clearing, so storage would
be initialized again in the end or access time wouldn't be saved at all due
to quota manager shutdown being already in progress.

Changes done in this patch:
- added QuotaManager::EnsureStorageIsInitializedInternal call to
  SaveOriginAccessTimeOp::DoDirectoryWork
- changed QuotaManager::UnregisterDirectoryLock to work with already cleared
  directory lock tables
- added a new QuotaManager::ClearDirectoryLockTables method
- added QuotaManager::ClearDirectoryLockTables call to
  ShutdownStorageOp::OpenDirectory and ClearStorageOp::OpenDirectory
- made ClearStorageOp and ShutdownStorageOp friend classes of QuotaManager

Differential Revision: https://phabricator.services.mozilla.com/D187877
This commit is contained in:
Jan Varga 2023-09-20 10:37:44 +00:00
Родитель 4dc6bccc99
Коммит d2162e1c48
3 изменённых файлов: 46 добавлений и 8 удалений

Просмотреть файл

@ -1861,11 +1861,11 @@ void QuotaManager::UnregisterDirectoryLock(DirectoryLockImpl& aLock) {
DirectoryLockTable& directoryLockTable =
GetDirectoryLockTable(aLock.GetPersistenceType());
// ClearDirectoryLockTables may have been called, so the element or entire
// array may not exist anymre.
nsTArray<NotNull<DirectoryLockImpl*>>* array;
MOZ_ALWAYS_TRUE(directoryLockTable.Get(aLock.Origin(), &array));
MOZ_ALWAYS_TRUE(array->RemoveElement(&aLock));
if (array->IsEmpty()) {
if (directoryLockTable.Get(aLock.Origin(), &array) &&
array->RemoveElement(&aLock) && array->IsEmpty()) {
directoryLockTable.Remove(aLock.Origin());
if (!IsShuttingDown()) {
@ -6346,6 +6346,30 @@ auto QuotaManager::GetDirectoryLockTable(PersistenceType aPersistenceType)
}
}
void QuotaManager::ClearDirectoryLockTables() {
AssertIsOnOwningThread();
for (const PersistenceType type : kBestEffortPersistenceTypes) {
DirectoryLockTable& directoryLockTable = GetDirectoryLockTable(type);
if (!IsShuttingDown()) {
for (const auto& entry : directoryLockTable) {
const auto& array = entry.GetData();
// It doesn't matter which lock is used, they all have the same
// persistence type and origin metadata.
MOZ_ASSERT(!array->IsEmpty());
const auto& lock = array->ElementAt(0);
UpdateOriginAccessTime(lock->GetPersistenceType(),
lock->OriginMetadata());
}
}
directoryLockTable.Clear();
}
}
bool QuotaManager::IsSanitizedOriginValid(const nsACString& aSanitizedOrigin) {
AssertIsOnIOThread();

Просмотреть файл

@ -68,8 +68,6 @@ namespace mozilla::dom::quota {
using namespace mozilla::ipc;
namespace {
class FinalizeOriginEvictionOp : public OriginOperationBase {
nsTArray<RefPtr<OriginDirectoryLock>> mLocks;
@ -617,8 +615,6 @@ class ListOriginsOp final : public QuotaRequestBase,
void CloseDirectory() override;
};
} // namespace
RefPtr<OriginOperationBase> CreateFinalizeOriginEvictionOp(
MovingNotNull<RefPtr<QuotaManager>> aQuotaManager,
nsTArray<RefPtr<OriginDirectoryLock>>&& aLocks) {
@ -795,6 +791,8 @@ nsresult SaveOriginAccessTimeOp::DoDirectoryWork(QuotaManager& aQuotaManager) {
AUTO_PROFILER_LABEL("SaveOriginAccessTimeOp::DoDirectoryWork", OTHER);
QM_TRY(MOZ_TO_RESULT(aQuotaManager.EnsureStorageIsInitializedInternal()));
QM_TRY_INSPECT(const auto& file,
aQuotaManager.GetOriginDirectory(mOriginMetadata));
@ -873,6 +871,12 @@ void ClearPrivateRepositoryOp::CloseDirectory() {
RefPtr<BoolPromise> ShutdownStorageOp::OpenDirectory() {
AssertIsOnOwningThread();
// Clear directory lock tables (which also saves origin access time) before
// acquiring the exclusive lock below. Otherwise, saving of origin access
// time would be scheduled after storage shutdown and that would initialize
// storage again in the end.
mQuotaManager->ClearDirectoryLockTables();
mDirectoryLock = mQuotaManager->CreateDirectoryLockInternal(
Nullable<PersistenceType>(), OriginScope::FromNull(),
Nullable<Client::Type>(),
@ -1606,6 +1610,12 @@ void ClearStorageOp::DeleteStorageFile(QuotaManager& aQuotaManager) {
RefPtr<BoolPromise> ClearStorageOp::OpenDirectory() {
AssertIsOnOwningThread();
// Clear directory lock tables (which also saves origin access time) before
// acquiring the exclusive lock below. Otherwise, saving of origin access
// time would be scheduled after storage clearing and that would initialize
// storage again in the end.
mQuotaManager->ClearDirectoryLockTables();
mDirectoryLock = mQuotaManager->CreateDirectoryLockInternal(
Nullable<PersistenceType>(), OriginScope::FromNull(),
Nullable<Client::Type>(),

Просмотреть файл

@ -78,9 +78,11 @@ class UniversalDirectoryLock;
class QuotaManager final : public BackgroundThreadObject {
friend class CanonicalQuotaObject;
friend class ClearStorageOp;
friend class DirectoryLockImpl;
friend class GroupInfo;
friend class OriginInfo;
friend class ShutdownStorageOp;
using PrincipalInfo = mozilla::ipc::PrincipalInfo;
using DirectoryLockTable =
@ -615,6 +617,8 @@ class QuotaManager final : public BackgroundThreadObject {
DirectoryLockTable& GetDirectoryLockTable(PersistenceType aPersistenceType);
void ClearDirectoryLockTables();
bool IsSanitizedOriginValid(const nsACString& aSanitizedOrigin);
Result<nsCString, nsresult> EnsureStorageOriginFromOrigin(