From 55e3bdd7f881cfd0f1950daf6dab471a517d5dfb Mon Sep 17 00:00:00 2001 From: Simon Giesecke Date: Thu, 25 Jun 2020 09:31:39 +0000 Subject: [PATCH] Bug 1641512 - Use Result as return type of InitOrigin and add separate InitOriginWithoutTracking. r=dom-workers-and-storage-reviewers,janv Differential Revision: https://phabricator.services.mozilla.com/D77453 --- dom/cache/QuotaClient.cpp | 35 ++++++++---- dom/cache/QuotaClientImpl.h | 13 +++-- dom/indexedDB/ActorsParent.cpp | 40 ++++++++++---- dom/localstorage/ActorsParent.cpp | 90 ++++++++++++++++++------------- dom/quota/ActorsParent.cpp | 51 +++++++++--------- dom/quota/Client.h | 13 +++-- dom/simpledb/ActorsParent.cpp | 35 ++++++------ 7 files changed, 169 insertions(+), 108 deletions(-) diff --git a/dom/cache/QuotaClient.cpp b/dom/cache/QuotaClient.cpp index 3eec399c0c25..c733263ea943 100644 --- a/dom/cache/QuotaClient.cpp +++ b/dom/cache/QuotaClient.cpp @@ -203,21 +203,34 @@ CacheQuotaClient* CacheQuotaClient::Get() { CacheQuotaClient::Type CacheQuotaClient::GetType() { return DOMCACHE; } -nsresult CacheQuotaClient::InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, - const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) { +Result CacheQuotaClient::InitOrigin( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { AssertIsOnIOThread(); - // The QuotaManager passes a nullptr UsageInfo if there is no quota being - // enforced against the origin. - if (!aUsageInfo) { - return NS_OK; + UsageInfo res; + + nsresult rv = + GetUsageForOriginInternal(aPersistenceType, aGroup, aOrigin, aCanceled, + /* aInitializing*/ true, &res); + if (NS_WARN_IF(NS_FAILED(rv))) { + return Err(rv); } - return GetUsageForOriginInternal(aPersistenceType, aGroup, aOrigin, aCanceled, - /* aInitializing*/ true, aUsageInfo); + return res; +} + +nsresult CacheQuotaClient::InitOriginWithoutTracking( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { + AssertIsOnIOThread(); + + // This is called when a storage/permanent/chrome/cache directory exists. Even + // though this shouldn't happen with a "good" profile, we shouldn't return an + // error here, since that would cause origin initialization to fail. We just + // warn and otherwise ignore that. + UNKNOWN_FILE_WARNING(NS_LITERAL_STRING(DOMCACHE_DIRECTORY_NAME)); + return NS_OK; } Result CacheQuotaClient::GetUsageForOrigin( diff --git a/dom/cache/QuotaClientImpl.h b/dom/cache/QuotaClientImpl.h index c502a1aa7416..9572e86d0d98 100644 --- a/dom/cache/QuotaClientImpl.h +++ b/dom/cache/QuotaClientImpl.h @@ -26,11 +26,14 @@ class CacheQuotaClient final : public quota::Client { static CacheQuotaClient* Get(); virtual Type GetType() override; - virtual nsresult InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, - const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) override; + + virtual Result InitOrigin( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) override; + + virtual nsresult InitOriginWithoutTracking( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) override; virtual Result GetUsageForOrigin( PersistenceType aPersistenceType, const nsACString& aGroup, diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp index 7a72dfb852d8..0968db9510f0 100644 --- a/dom/indexedDB/ActorsParent.cpp +++ b/dom/indexedDB/ActorsParent.cpp @@ -8563,10 +8563,15 @@ class QuotaClient final : public mozilla::dom::quota::Client { nsresult UpgradeStorageFrom2_1To2_2(nsIFile* aDirectory) override; - nsresult InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) override; + Result InitOrigin(PersistenceType aPersistenceType, + const nsACString& aGroup, + const nsACString& aOrigin, + const AtomicBool& aCanceled) override; + + nsresult InitOriginWithoutTracking(PersistenceType aPersistenceType, + const nsACString& aGroup, + const nsACString& aOrigin, + const AtomicBool& aCanceled) override; Result GetUsageForOrigin( PersistenceType aPersistenceType, const nsACString& aGroup, @@ -17180,15 +17185,30 @@ nsresult QuotaClient::UpgradeStorageFrom2_1To2_2(nsIFile* aDirectory) { return NS_OK; } -nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, - const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) { +Result QuotaClient::InitOrigin( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { + AssertIsOnIOThread(); + + UsageInfo res; + + nsresult rv = + GetUsageForOriginInternal(aPersistenceType, aGroup, aOrigin, aCanceled, + /* aInitializing*/ true, &res); + if (NS_WARN_IF(NS_FAILED(rv))) { + return Err(rv); + } + + return res; +} + +nsresult QuotaClient::InitOriginWithoutTracking( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { AssertIsOnIOThread(); return GetUsageForOriginInternal(aPersistenceType, aGroup, aOrigin, aCanceled, - /* aInitializing*/ true, aUsageInfo); + /* aInitializing*/ true, nullptr); } Result QuotaClient::GetUsageForOrigin( diff --git a/dom/localstorage/ActorsParent.cpp b/dom/localstorage/ActorsParent.cpp index 85554af30171..ac87733c023d 100644 --- a/dom/localstorage/ActorsParent.cpp +++ b/dom/localstorage/ActorsParent.cpp @@ -2819,10 +2819,15 @@ class QuotaClient final : public mozilla::dom::quota::Client { Type GetType() override; - nsresult InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) override; + Result InitOrigin(PersistenceType aPersistenceType, + const nsACString& aGroup, + const nsACString& aOrigin, + const AtomicBool& aCanceled) override; + + nsresult InitOriginWithoutTracking(PersistenceType aPersistenceType, + const nsACString& aGroup, + const nsACString& aOrigin, + const AtomicBool& aCanceled) override; Result GetUsageForOrigin( PersistenceType aPersistenceType, const nsACString& aGroup, @@ -8817,11 +8822,9 @@ mozilla::dom::quota::Client::Type QuotaClient::GetType() { return QuotaClient::LS; } -nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, - const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) { +Result QuotaClient::InitOrigin( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { AssertIsOnIOThread(); MOZ_ASSERT(aPersistenceType == PERSISTENCE_TYPE_DEFAULT); @@ -8833,7 +8836,7 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, getter_AddRefs(directory)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetDirForOrigin); - return rv; + return Err(rv); } MOZ_ASSERT(directory); @@ -8841,7 +8844,7 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = directory->Append(NS_LITERAL_STRING(LS_DIRECTORY_NAME)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Append); - return rv; + return Err(rv); } #ifdef DEBUG @@ -8849,7 +8852,7 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = directory->Exists(&exists); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Exists); - return rv; + return Err(rv); } MOZ_ASSERT(exists); @@ -8859,14 +8862,14 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = directory->GetPath(directoryPath); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetPath); - return rv; + return Err(rv); } nsCOMPtr usageFile; rv = GetUsageFile(directoryPath, getter_AddRefs(usageFile)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetUsageFile); - return rv; + return Err(rv); } bool usageFileExists; @@ -8877,12 +8880,12 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv != NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_IsDirectory); - return rv; + return Err(rv); } if (NS_WARN_IF(isDirectory)) { REPORT_TELEMETRY_INIT_ERR(kQuotaInternalError, LS_UnexpectedDir); - return NS_ERROR_FAILURE; + return Err(NS_ERROR_FAILURE); } usageFileExists = true; @@ -8890,11 +8893,13 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, usageFileExists = false; } + UsageInfo res; + nsCOMPtr usageJournalFile; rv = GetUsageJournalFile(directoryPath, getter_AddRefs(usageJournalFile)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetUsageForJFile); - return rv; + return Err(rv); } rv = usageJournalFile->IsDirectory(&isDirectory); @@ -8902,19 +8907,19 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv != NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_IsDirectory2); - return rv; + return Err(rv); } if (NS_WARN_IF(isDirectory)) { REPORT_TELEMETRY_INIT_ERR(kQuotaInternalError, LS_UnexpectedDir2); - return NS_ERROR_FAILURE; + return Err(NS_ERROR_FAILURE); } if (usageFileExists) { rv = usageFile->Remove(false); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Remove); - return rv; + return Err(rv); } usageFileExists = false; @@ -8923,7 +8928,7 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = usageJournalFile->Remove(false); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Remove2); - return rv; + return Err(rv); } } @@ -8931,13 +8936,13 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = directory->Clone(getter_AddRefs(file)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Clone); - return rv; + return Err(rv); } rv = file->Append(NS_LITERAL_STRING(DATA_FILE_NAME)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Append2); - return rv; + return Err(rv); } rv = file->IsDirectory(&isDirectory); @@ -8945,12 +8950,12 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv != NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_IsDirectory3); - return rv; + return Err(rv); } if (NS_WARN_IF(isDirectory)) { REPORT_TELEMETRY_INIT_ERR(kQuotaInternalError, LS_UnexpectedDir3); - return NS_ERROR_FAILURE; + return Err(NS_ERROR_FAILURE); } int64_t usage; @@ -8962,36 +8967,36 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, getter_AddRefs(connection), &dummy); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_CreateConnection); - return rv; + return Err(rv); } rv = GetUsage(connection, /* aArchivedOriginScope */ nullptr, &usage); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetUsage); - return rv; + return Err(rv); } rv = UpdateUsageFile(usageFile, usageJournalFile, usage); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_UpdateUsageFile); - return rv; + return Err(rv); } rv = usageJournalFile->Remove(false); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Remove3); - return rv; + return Err(rv); } } MOZ_ASSERT(usage >= 0); - aUsageInfo->IncrementDatabaseUsage(Some(uint64_t(usage))); + res.IncrementDatabaseUsage(Some(uint64_t(usage))); } else if (usageFileExists) { rv = usageFile->Remove(false); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_Remove4); - return rv; + return Err(rv); } } @@ -9002,11 +9007,11 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = directory->GetDirectoryEntries(getter_AddRefs(directoryEntries)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetDirEntries); - return rv; + return Err(rv); } if (!directoryEntries) { - return NS_OK; + return res; } while (true) { @@ -9018,7 +9023,7 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = directoryEntries->GetNextFile(getter_AddRefs(file)); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetNextFile); - return rv; + return Err(rv); } if (!file) { @@ -9029,7 +9034,7 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = file->GetLeafName(leafName); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_GetLeafName); - return rv; + return Err(rv); } // Don't need to check for USAGE_JOURNAL_FILE_NAME. We removed it above @@ -9045,7 +9050,7 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, rv = file->IsDirectory(&isDirectory); if (NS_WARN_IF(NS_FAILED(rv))) { REPORT_TELEMETRY_INIT_ERR(kQuotaExternalError, LS_IsDirectory4); - return rv; + return Err(rv); } if (!isDirectory) { @@ -9057,6 +9062,19 @@ nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, } #endif + return res; +} + +nsresult QuotaClient::InitOriginWithoutTracking( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { + AssertIsOnIOThread(); + + // This is called when a storage/permanent/chrome/ls directory exists. Even + // though this shouldn't happen with a "good" profile, we shouldn't return an + // error here, since that would cause origin initialization to fail. We just + // warn and otherwise ignore that. + UNKNOWN_FILE_WARNING(NS_LITERAL_STRING(LS_DIRECTORY_NAME)); return NS_OK; } diff --git a/dom/quota/ActorsParent.cpp b/dom/quota/ActorsParent.cpp index 1e9cf730efb2..16ec407a54e1 100644 --- a/dom/quota/ActorsParent.cpp +++ b/dom/quota/ActorsParent.cpp @@ -5307,23 +5307,31 @@ nsresult QuotaManager::InitializeOrigin(PersistenceType aPersistenceType, continue; } - UsageInfo usageInfo; - rv = mClients[clientType]->InitOrigin(aPersistenceType, aGroup, aOrigin, - /* aCanceled */ Atomic(false), - trackQuota ? &usageInfo : nullptr); - if (NS_WARN_IF(NS_FAILED(rv))) { - // error should have reported in InitOrigin - RECORD_IN_NIGHTLY(statusKeeper, rv); - CONTINUE_IN_NIGHTLY_RETURN_IN_OTHERS(rv); - } - if (trackQuota) { - Maybe clientUsage = usageInfo.TotalUsage(); + auto usageInfoOrErr = + mClients[clientType]->InitOrigin(aPersistenceType, aGroup, aOrigin, + /* aCanceled */ Atomic(false)); + if (NS_WARN_IF(usageInfoOrErr.isErr())) { + // error should have reported in InitOrigin + RECORD_IN_NIGHTLY(statusKeeper, usageInfoOrErr.inspectErr()); + CONTINUE_IN_NIGHTLY_RETURN_IN_OTHERS(usageInfoOrErr.inspectErr()); + } + + const Maybe clientUsage = usageInfoOrErr.inspect().TotalUsage(); clientUsages[clientType] = clientUsage; AssertNoOverflow(usage, clientUsage.valueOr(0)); usage += clientUsage.valueOr(0); + } else { + rv = mClients[clientType]->InitOriginWithoutTracking( + aPersistenceType, aGroup, aOrigin, + /* aCanceled */ Atomic(false)); + if (NS_WARN_IF(NS_FAILED(rv))) { + // error should have reported in InitOrigin + RECORD_IN_NIGHTLY(statusKeeper, rv); + CONTINUE_IN_NIGHTLY_RETURN_IN_OTHERS(rv); + } } } if (NS_WARN_IF(NS_FAILED(rv))) { @@ -9146,20 +9154,15 @@ Result QuotaUsageRequestBase::GetUsageForOriginEntries( Client* client = aQuotaManager.GetClient(clientType); MOZ_ASSERT(client); - if (aInitialized) { - auto usageInfoOrErr = client->GetUsageForOrigin(aPersistenceType, aGroup, - aOrigin, mCanceled); - if (NS_WARN_IF(usageInfoOrErr.isErr())) { - return usageInfoOrErr.propagateErr(); - } - usageInfo += usageInfoOrErr.inspect(); - } else { - rv = client->InitOrigin(aPersistenceType, aGroup, aOrigin, mCanceled, - &usageInfo); - if (NS_WARN_IF(NS_FAILED(rv))) { - return Err(rv); - } + auto usageInfoOrErr = + aInitialized + ? client->GetUsageForOrigin(aPersistenceType, aGroup, aOrigin, + mCanceled) + : client->InitOrigin(aPersistenceType, aGroup, aOrigin, mCanceled); + if (NS_WARN_IF(usageInfoOrErr.isErr())) { + return usageInfoOrErr.propagateErr(); } + usageInfo += usageInfoOrErr.inspect(); } if (NS_WARN_IF(NS_FAILED(rv))) { return Err(rv); diff --git a/dom/quota/Client.h b/dom/quota/Client.h index 75e445b77fe6..a7cb39153d3e 100644 --- a/dom/quota/Client.h +++ b/dom/quota/Client.h @@ -92,11 +92,14 @@ class Client { return NS_OK; } - virtual nsresult InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, - const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) = 0; + virtual Result InitOrigin( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) = 0; + + virtual nsresult InitOriginWithoutTracking(PersistenceType aPersistenceType, + const nsACString& aGroup, + const nsACString& aOrigin, + const AtomicBool& aCanceled) = 0; virtual Result GetUsageForOrigin( PersistenceType aPersistenceType, const nsACString& aGroup, diff --git a/dom/simpledb/ActorsParent.cpp b/dom/simpledb/ActorsParent.cpp index 643e8abd1c54..4dd4ac1bfea4 100644 --- a/dom/simpledb/ActorsParent.cpp +++ b/dom/simpledb/ActorsParent.cpp @@ -447,10 +447,15 @@ class QuotaClient final : public mozilla::dom::quota::Client { Type GetType() override; - nsresult InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) override; + Result InitOrigin(PersistenceType aPersistenceType, + const nsACString& aGroup, + const nsACString& aOrigin, + const AtomicBool& aCanceled) override; + + nsresult InitOriginWithoutTracking(PersistenceType aPersistenceType, + const nsACString& aGroup, + const nsACString& aOrigin, + const AtomicBool& aCanceled) override; Result GetUsageForOrigin( PersistenceType aPersistenceType, const nsACString& aGroup, @@ -1623,22 +1628,18 @@ mozilla::dom::quota::Client::Type QuotaClient::GetType() { return QuotaClient::SDB; } -nsresult QuotaClient::InitOrigin(PersistenceType aPersistenceType, - const nsACString& aGroup, - const nsACString& aOrigin, - const AtomicBool& aCanceled, - UsageInfo* aUsageInfo) { +Result QuotaClient::InitOrigin( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { AssertIsOnIOThread(); - if (aUsageInfo) { - auto usageInfoOrErr = - GetUsageForOrigin(aPersistenceType, aGroup, aOrigin, aCanceled); - if (NS_WARN_IF(usageInfoOrErr.isErr())) { - return usageInfoOrErr.unwrapErr(); - } + return GetUsageForOrigin(aPersistenceType, aGroup, aOrigin, aCanceled); +} - *aUsageInfo = usageInfoOrErr.unwrap(); - } +nsresult QuotaClient::InitOriginWithoutTracking( + PersistenceType aPersistenceType, const nsACString& aGroup, + const nsACString& aOrigin, const AtomicBool& aCanceled) { + AssertIsOnIOThread(); return NS_OK; }