From 6edcf204fded07a7bae6d700f86e1004bc64fc6d Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Wed, 24 Mar 2021 04:40:10 +0000 Subject: [PATCH] Bug 1686191 - Have a way to specifically report warnings around QM_TRY; r=asuth,sg,dom-storage-reviewers This patch: - adds QM_WARNONLY_TRY/QM_NOTEONLY_TRY macros - adds QM_WARNONLY_TRY_UNWRAP/QM_NOTEONLY_TRY_UNWRAP macros - adds QM_OR_ELSE_WARN/QM_OR_ELSE_NOTE sub macros - replaces non-propagating uses of NS_WARNING with redundant messages by QM_WARNONLY_TRY - replaces uses of QM_TRY with orElse by QM_TRY(QM_OR_ELSE_WARN(...)) - replaces uses of QM_TRY inside an extra lambda with QM_WARNONLY_TRY - replaces uses of QM_TRY with QM_VOID with QM_WARNONLY_TRY. - replaces uses of QM_TRY with unwanted warnings with QM_NOTEONLY_TRY - replaces uses of QM_TRY with additional Maybe wrapping for doing a fallback with QM_TRY(QM_OR_ELSE_WARN(...)) Differential Revision: https://phabricator.services.mozilla.com/D108424 --- dom/cache/CacheParent.cpp | 6 +- dom/cache/CacheStorageParent.cpp | 6 +- dom/cache/CacheStreamControlParent.cpp | 8 +- dom/cache/DBAction.cpp | 15 +- dom/cache/DBSchema.cpp | 24 +- dom/cache/FileUtils.cpp | 43 +-- dom/cache/Manager.cpp | 11 +- dom/cache/PrincipalVerifier.cpp | 8 +- dom/cache/QuotaClientImpl.h | 31 +- dom/indexedDB/ActorsParent.cpp | 396 +++++++++++------------ dom/indexedDB/FileInfoTImpl.h | 12 +- dom/indexedDB/IDBDatabase.cpp | 23 +- dom/indexedDB/IDBObjectStore.cpp | 15 +- dom/localstorage/ActorsParent.cpp | 164 +++++----- dom/quota/ActorsParent.cpp | 207 ++++++------ dom/quota/QuotaCommon.cpp | 74 +++-- dom/quota/QuotaCommon.h | 277 ++++++++++++++-- dom/quota/test/gtest/TestQuotaCommon.cpp | 222 +++++++++++++ 18 files changed, 975 insertions(+), 567 deletions(-) diff --git a/dom/cache/CacheParent.cpp b/dom/cache/CacheParent.cpp index a99b0f1df3d6..811e2c2d7ae4 100644 --- a/dom/cache/CacheParent.cpp +++ b/dom/cache/CacheParent.cpp @@ -57,10 +57,8 @@ mozilla::ipc::IPCResult CacheParent::RecvPCacheOpConstructor( } mozilla::ipc::IPCResult CacheParent::RecvTeardown() { - if (!Send__delete__(this)) { - // child process is gone, warn and allow actor to clean up normally - NS_WARNING("Cache failed to send delete."); - } + // If child process is gone, warn and allow actor to clean up normally + QM_WARNONLY_TRY(OkIf(Send__delete__(this))); return IPC_OK(); } diff --git a/dom/cache/CacheStorageParent.cpp b/dom/cache/CacheStorageParent.cpp index b05e1c26681a..562852ef2ceb 100644 --- a/dom/cache/CacheStorageParent.cpp +++ b/dom/cache/CacheStorageParent.cpp @@ -99,10 +99,8 @@ mozilla::ipc::IPCResult CacheStorageParent::RecvPCacheOpConstructor( } mozilla::ipc::IPCResult CacheStorageParent::RecvTeardown() { - if (!Send__delete__(this)) { - // child process is gone, warn and allow actor to clean up normally - NS_WARNING("CacheStorage failed to delete actor."); - } + // If child process is gone, warn and allow actor to clean up normally + QM_WARNONLY_TRY(OkIf(Send__delete__(this))); return IPC_OK(); } diff --git a/dom/cache/CacheStreamControlParent.cpp b/dom/cache/CacheStreamControlParent.cpp index 238e0988524a..ff25185890a1 100644 --- a/dom/cache/CacheStreamControlParent.cpp +++ b/dom/cache/CacheStreamControlParent.cpp @@ -145,11 +145,9 @@ void CacheStreamControlParent::CloseAll() { void CacheStreamControlParent::Shutdown() { NS_ASSERT_OWNINGTHREAD(CacheStreamControlParent); - if (!Send__delete__(this)) { - // child process is gone, allow actor to be destroyed normally - NS_WARNING("Cache failed to delete stream actor."); - return; - } + + // If child process is gone, warn and allow actor to clean up normally + QM_WARNONLY_TRY(OkIf(Send__delete__(this))); } void CacheStreamControlParent::NotifyCloseAll() { diff --git a/dom/cache/DBAction.cpp b/dom/cache/DBAction.cpp index f2420e0f14f4..a61bd7168c94 100644 --- a/dom/cache/DBAction.cpp +++ b/dom/cache/DBAction.cpp @@ -172,12 +172,13 @@ Result, nsresult> OpenDBConnection( CACHE_TRY_UNWRAP( auto conn, - MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, - storageService, OpenDatabaseWithFileURL, - dbFileUrl, ""_ns) - .orElse([&aQuotaInfo, &aDBFile, &storageService, - &dbFileUrl](const nsresult rv) - -> Result, nsresult> { + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, + storageService, OpenDatabaseWithFileURL, + dbFileUrl, ""_ns), + ([&aQuotaInfo, &aDBFile, &storageService, + &dbFileUrl](const nsresult rv) + -> Result, nsresult> { if (IsDatabaseCorruptionError(rv)) { NS_WARNING( "Cache database corrupted. Recreating empty database."); @@ -191,7 +192,7 @@ Result, nsresult> OpenDBConnection( OpenDatabaseWithFileURL, dbFileUrl, ""_ns)); } return Err(rv); - })); + }))); // Check the schema to make sure it is not too old. CACHE_TRY_INSPECT(const int32_t& schemaVersion, diff --git a/dom/cache/DBSchema.cpp b/dom/cache/DBSchema.cpp index cca6ce25483a..bb7384034da4 100644 --- a/dom/cache/DBSchema.cpp +++ b/dom/cache/DBSchema.cpp @@ -421,16 +421,18 @@ class MOZ_RAII AutoDisableForeignKeyChecking { MOZ_TO_RESULT_INVOKE(*state, GetInt32, 0), QM_VOID); if (mode) { - CACHE_TRY(mConn->ExecuteSimpleSQL("PRAGMA foreign_keys = OFF;"_ns), - QM_VOID); - mForeignKeyCheckingDisabled = true; + QM_WARNONLY_TRY( + ToResult(mConn->ExecuteSimpleSQL("PRAGMA foreign_keys = OFF;"_ns)) + .andThen([this](const auto) -> Result { + mForeignKeyCheckingDisabled = true; + return Ok{}; + })); } } ~AutoDisableForeignKeyChecking() { if (mForeignKeyCheckingDisabled) { - CACHE_TRY(mConn->ExecuteSimpleSQL("PRAGMA foreign_keys = ON;"_ns), - QM_VOID); + QM_WARNONLY_TRY(mConn->ExecuteSimpleSQL("PRAGMA foreign_keys = ON;"_ns)); } } @@ -552,16 +554,8 @@ nsresult InitializeConnection(mozIStorageConnection& aConn) { kPageSize))); // Limit fragmentation by growing the database by many pages at once. - CACHE_TRY( - ToResult(aConn.SetGrowthIncrement(kGrowthSize, ""_ns)) - .orElse([](const nsresult rv) -> Result { - if (rv == NS_ERROR_FILE_TOO_BIG) { - NS_WARNING( - "Not enough disk space to set sqlite growth increment."); - return Ok{}; - } - return Err(rv); - })); + QM_TRY(QM_OR_ELSE_WARN(ToResult(aConn.SetGrowthIncrement(kGrowthSize, ""_ns)), + ErrToDefaultOkOrErr)); // Enable WAL journaling. This must be performed in a separate transaction // after changing the page_size and enabling auto_vacuum. diff --git a/dom/cache/FileUtils.cpp b/dom/cache/FileUtils.cpp index 02f9d1a47069..5321e888c78f 100644 --- a/dom/cache/FileUtils.cpp +++ b/dom/cache/FileUtils.cpp @@ -89,8 +89,9 @@ Result>, nsresult> BodyGetCacheDir(nsIFile& aBaseDir, // the name of the sub-directory. CACHE_TRY(cacheDir->Append(IntToString(aId.m3[7]))); - CACHE_TRY(ToResult(cacheDir->Create(nsIFile::DIRECTORY_TYPE, 0755)) - .orElse(ErrToDefaultOkOrErr)); + QM_TRY( + QM_OR_ELSE_WARN(ToResult(cacheDir->Create(nsIFile::DIRECTORY_TYPE, 0755)), + ErrToDefaultOkOrErr)); return WrapNotNullUnchecked(std::move(cacheDir)); } @@ -101,8 +102,9 @@ nsresult BodyCreateDir(nsIFile& aBaseDir) { CACHE_TRY_INSPECT(const auto& bodyDir, CloneFileAndAppend(aBaseDir, kMorgueDirectory)); - CACHE_TRY(ToResult(bodyDir->Create(nsIFile::DIRECTORY_TYPE, 0755)) - .orElse(ErrToDefaultOkOrErr)); + QM_TRY( + QM_OR_ELSE_WARN(ToResult(bodyDir->Create(nsIFile::DIRECTORY_TYPE, 0755)), + ErrToDefaultOkOrErr)); return NS_OK; } @@ -163,7 +165,7 @@ Result>, nsresult> BodyStartWriteStream( } void BodyCancelWrite(nsISupports& aCopyContext) { - CACHE_TRY(NS_CancelAsyncCopy(&aCopyContext, NS_ERROR_ABORT), QM_VOID); + QM_WARNONLY_TRY(NS_CancelAsyncCopy(&aCopyContext, NS_ERROR_ABORT)); // TODO The partially written file must be cleaned up after the async copy // makes its callback. @@ -432,8 +434,9 @@ Result, nsresult> GetMarkerFileHandle( nsresult CreateMarkerFile(const QuotaInfo& aQuotaInfo) { CACHE_TRY_INSPECT(const auto& marker, GetMarkerFileHandle(aQuotaInfo)); - CACHE_TRY(ToResult(marker->Create(nsIFile::NORMAL_FILE_TYPE, 0644)) - .orElse(MapAlreadyExistsToDefault)); + QM_TRY( + QM_OR_ELSE_WARN(ToResult(marker->Create(nsIFile::NORMAL_FILE_TYPE, 0644)), + MapAlreadyExistsToDefault)); // Note, we don't need to fsync here. We only care about actually // writing the marker if later modifications to the Cache are @@ -503,10 +506,11 @@ nsresult RemoveNsIFile(const QuotaInfo& aQuotaInfo, nsIFile& aFile, const bool aTrackQuota) { int64_t fileSize = 0; if (aTrackQuota) { - CACHE_TRY_INSPECT(const auto& maybeFileSize, - MOZ_TO_RESULT_INVOKE(aFile, GetFileSize) - .map(Some) - .orElse(MapNotFoundToDefault>)); + CACHE_TRY_INSPECT( + const auto& maybeFileSize, + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE(aFile, GetFileSize).map(Some), + MapNotFoundToDefault>)); if (!maybeFileSize) { return NS_OK; @@ -515,8 +519,8 @@ nsresult RemoveNsIFile(const QuotaInfo& aQuotaInfo, nsIFile& aFile, fileSize = *maybeFileSize; } - CACHE_TRY(ToResult(aFile.Remove(/* recursive */ false)) - .orElse(MapNotFoundToDefault<>)); + QM_TRY(QM_OR_ELSE_WARN(ToResult(aFile.Remove(/* recursive */ false)), + MapNotFoundToDefault<>)); if (fileSize > 0) { MOZ_ASSERT(aTrackQuota); @@ -587,10 +591,11 @@ nsresult LockedUpdateDirectoryPaddingFile(nsIFile& aBaseDir, const auto directoryPaddingGetResult = aTemporaryFileExist ? Maybe{} : [&aBaseDir] { - CACHE_TRY_RETURN(LockedDirectoryPaddingGet(aBaseDir) - .map(Some) - .orElse(MapNotFoundToDefault>), - Maybe{}); + CACHE_TRY_RETURN( + QM_OR_ELSE_WARN( + LockedDirectoryPaddingGet(aBaseDir).map(Some), + MapNotFoundToDefault>), + Maybe{}); }(); CACHE_TRY_INSPECT( @@ -708,8 +713,8 @@ nsresult LockedDirectoryPaddingDeleteFile(nsIFile& aBaseDir, ? nsLiteralString(PADDING_TMP_FILE_NAME) : nsLiteralString(PADDING_FILE_NAME))); - CACHE_TRY(ToResult(file->Remove(/* recursive */ false)) - .orElse(MapNotFoundToDefault<>)); + QM_TRY(QM_OR_ELSE_WARN(ToResult(file->Remove(/* recursive */ false)), + MapNotFoundToDefault<>)); return NS_OK; } diff --git a/dom/cache/Manager.cpp b/dom/cache/Manager.cpp index 9717feb93413..455ab8cb981d 100644 --- a/dom/cache/Manager.cpp +++ b/dom/cache/Manager.cpp @@ -140,13 +140,10 @@ class SetupAction final : public SyncDBAction { // failure, but if we entered it and RestorePaddingFile succeeded, we // would have returned NS_OK. Now, we will never propagate a // MaybeUpdatePaddingFile failure. - [&] { - CACHE_TRY(MaybeUpdatePaddingFile( - aDBDir, aConn, /* aIncreaceSize */ 0, - overallDeletedPaddingSize.value(), - [&trans]() mutable { return trans.Commit(); }), - QM_VOID); - }(); + QM_WARNONLY_TRY( + MaybeUpdatePaddingFile(aDBDir, aConn, /* aIncreaceSize */ 0, + overallDeletedPaddingSize.value(), + [&trans]() { return trans.Commit(); })); } if (DirectoryPaddingFileExists(*aDBDir, DirPaddingFile::TMP_FILE) || diff --git a/dom/cache/PrincipalVerifier.cpp b/dom/cache/PrincipalVerifier.cpp index 1df042ab7f8d..cb35405f67ee 100644 --- a/dom/cache/PrincipalVerifier.cpp +++ b/dom/cache/PrincipalVerifier.cpp @@ -177,12 +177,8 @@ void PrincipalVerifier::DispatchToInitiatingThread(nsresult aRv) { // This will result in a new CacheStorage object delaying operations until // shutdown completes and the browser goes away. This is as graceful as // we can get here. - nsresult rv = - mInitiatingEventTarget->Dispatch(this, nsIThread::DISPATCH_NORMAL); - if (NS_FAILED(rv)) { - NS_WARNING( - "Cache unable to complete principal verification due to shutdown."); - } + QM_WARNONLY_TRY( + mInitiatingEventTarget->Dispatch(this, nsIThread::DISPATCH_NORMAL)); } } // namespace mozilla::dom::cache diff --git a/dom/cache/QuotaClientImpl.h b/dom/cache/QuotaClientImpl.h index c1249955ca65..f8f2247af95e 100644 --- a/dom/cache/QuotaClientImpl.h +++ b/dom/cache/QuotaClientImpl.h @@ -94,23 +94,24 @@ class CacheQuotaClient final : public quota::Client { // the next action recalculate the padding size. CACHE_TRY(aCommitHook()); - CACHE_TRY( - ToResult(LockedDirectoryPaddingFinalizeWrite(aBaseDir)) - .orElse([&aBaseDir](const nsresult) -> Result { - // Force restore file next time. - Unused << LockedDirectoryPaddingDeleteFile( - aBaseDir, DirPaddingFile::FILE); + QM_TRY(QM_OR_ELSE_WARN( + ToResult(LockedDirectoryPaddingFinalizeWrite(aBaseDir)), + ([&aBaseDir](const nsresult) -> Result { + // Force restore file next time. + Unused << LockedDirectoryPaddingDeleteFile(aBaseDir, + DirPaddingFile::FILE); - // Ensure that we are able to force the padding file to be - // restored. - MOZ_ASSERT(DirectoryPaddingFileExists( - aBaseDir, DirPaddingFile::TMP_FILE)); + // Ensure that we are able to force the padding file to + // be restored. + MOZ_ASSERT( + DirectoryPaddingFileExists(aBaseDir, DirPaddingFile::TMP_FILE)); - // Since both the body file and header have been stored in the - // file-system, just make the action be resolve and let the - // padding file be restored in the next action. - return Ok{}; - })); + // Since both the body file and header have been stored + // in the file-system, just make the action be resolve + // and let the padding file be restored in the next + // action. + return Ok{}; + }))); } return NS_OK; diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp index 4cd806d2526b..b06808d7d760 100644 --- a/dom/indexedDB/ActorsParent.cpp +++ b/dom/indexedDB/ActorsParent.cpp @@ -679,9 +679,9 @@ nsresult SetDefaultPragmas(mozIStorageConnection& aConnection) { if (kSQLiteGrowthIncrement) { // This is just an optimization so ignore the failure if the disk is // currently too full. - IDB_TRY( - ToResult(aConnection.SetGrowthIncrement(kSQLiteGrowthIncrement, ""_ns)) - .orElse(ErrToDefaultOkOrErr)); + IDB_TRY(QM_OR_ELSE_WARN( + ToResult(aConnection.SetGrowthIncrement(kSQLiteGrowthIncrement, ""_ns)), + (ErrToDefaultOkOrErr))); } #endif // IDB_MOBILE @@ -748,11 +748,12 @@ OpenDatabaseAndHandleBusy(mozIStorageService& aStorageService, IDB_TRY_UNWRAP( auto connection, - OpenDatabase(aStorageService, aFileURL, aTelemetryId) - .map([](auto connection) -> ConnectionType { - return Some(std::move(connection)); - }) - .orElse(ErrToDefaultOkOrErr)); + QM_OR_ELSE_WARN( + OpenDatabase(aStorageService, aFileURL, aTelemetryId) + .map([](auto connection) -> ConnectionType { + return Some(std::move(connection)); + }), + (ErrToDefaultOkOrErr))); if (connection.isNothing()) { #ifdef DEBUG @@ -777,12 +778,12 @@ OpenDatabaseAndHandleBusy(mozIStorageService& aStorageService, IDB_TRY_UNWRAP( connection, - OpenDatabase(aStorageService, aFileURL, aTelemetryId) - .map([](auto connection) -> ConnectionType { - return Some(std::move(connection)); - }) - .orElse([&start]( - nsresult aValue) -> Result { + QM_OR_ELSE_WARN( + OpenDatabase(aStorageService, aFileURL, aTelemetryId) + .map([](auto connection) -> ConnectionType { + return Some(std::move(connection)); + }), + ([&start](nsresult aValue) -> Result { if (aValue != NS_ERROR_STORAGE_BUSY || TimeStamp::NowLoRes() - start > TimeDuration::FromSeconds(10)) { @@ -790,7 +791,7 @@ OpenDatabaseAndHandleBusy(mozIStorageService& aStorageService, } return ConnectionType(); - })); + }))); } while (connection.isNothing()); } @@ -844,12 +845,13 @@ CreateStorageConnection(nsIFile& aDBFile, nsIFile& aFMDirectory, IDB_TRY_UNWRAP( auto connection, - OpenDatabaseAndHandleBusy(*storageService, *dbFileUrl, aTelemetryId) - .map([](auto connection) -> nsCOMPtr { - return std::move(connection).unwrapBasePtr(); - }) - .orElse([&aName](nsresult aValue) - -> Result, nsresult> { + QM_OR_ELSE_WARN( + OpenDatabaseAndHandleBusy(*storageService, *dbFileUrl, aTelemetryId) + .map([](auto connection) -> nsCOMPtr { + return std::move(connection).unwrapBasePtr(); + }), + ([&aName](nsresult aValue) + -> Result, nsresult> { // If we're just opening the database during origin initialization, // then we don't want to erase any files. The failure here will fail // origin initialization too. @@ -858,7 +860,7 @@ CreateStorageConnection(nsIFile& aDBFile, nsIFile& aFMDirectory, } return nsCOMPtr(); - })); + }))); if (!connection) { // XXX Shouldn't we also update quota usage? @@ -5736,9 +5738,10 @@ nsresult DeleteFile(nsIFile& aFile, QuotaManager* const aQuotaManager, if (aQuotaManager) { IDB_TRY_INSPECT( const Maybe& fileSize, - MOZ_TO_RESULT_INVOKE(aFile, GetFileSize) - .map([](const int64_t val) { return Some(val); }) - .orElse(MakeMaybeIdempotentFilter(aIdempotent))); + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE(aFile, GetFileSize) + .map([](const int64_t val) { return Some(val); }), + MakeMaybeIdempotentFilter(aIdempotent))); // XXX Can we really assert that the file size is not 0 if // it existed? This might be violated by external @@ -5756,9 +5759,8 @@ nsresult DeleteFile(nsIFile& aFile, QuotaManager* const aQuotaManager, } IDB_TRY_INSPECT(const auto& didExist, - ToResult(aFile.Remove(false)) - .map(Some) - .orElse(MakeMaybeIdempotentFilter(aIdempotent))); + QM_OR_ELSE_WARN(ToResult(aFile.Remove(false)).map(Some), + MakeMaybeIdempotentFilter(aIdempotent))); if (!didExist) { // XXX If we get here, this means that the file still existed when we @@ -5804,9 +5806,9 @@ nsresult DeleteFilesNoQuota(nsIFile* aDirectory, const nsAString& aFilename) { IDB_TRY_INSPECT(const auto& file, CloneFileAndAppend(*aDirectory, aFilename)); - IDB_TRY_INSPECT( - const auto& didExist, - ToResult(file->Remove(true)).map(Some).orElse(IdempotentFilter)); + IDB_TRY_INSPECT(const auto& didExist, + QM_OR_ELSE_WARN(ToResult(file->Remove(true)).map(Some), + IdempotentFilter)); Unused << didExist; @@ -5826,9 +5828,9 @@ Result, nsresult> CreateMarkerFile( CloneFileAndAppend(aBaseDirectory, kIdbDeletionMarkerFilePrefix + aDatabaseNameBase)); - IDB_TRY( - MOZ_TO_RESULT_INVOKE(markerFile, Create, nsIFile::NORMAL_FILE_TYPE, 0644) - .orElse(ErrToDefaultOkOrErr)); + QM_TRY(QM_OR_ELSE_WARN( + ToResult(markerFile->Create(nsIFile::NORMAL_FILE_TYPE, 0644)), + ErrToDefaultOkOrErr)); return markerFile; } @@ -5860,27 +5862,27 @@ Result DeleteFileManagerDirectory( uint64_t usageValue = fileUsage.GetValue().valueOr(0); - auto res = - MOZ_TO_RESULT_INVOKE(aFileManagerDirectory, Remove, true) - .orElse([&usageValue, &aFileManagerDirectory](nsresult rv) { - // We may have deleted some files, try to update quota - // information before returning the error. + auto res = QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE(aFileManagerDirectory, Remove, true), + ([&usageValue, &aFileManagerDirectory](nsresult rv) { + // We may have deleted some files, try to update quota + // information before returning the error. - // failures of GetUsage are intentionally ignored - Unused << FileManager::GetUsage(&aFileManagerDirectory) - .andThen([&usageValue](const auto& newFileUsage) { - const auto newFileUsageValue = - newFileUsage.GetValue().valueOr(0); - MOZ_ASSERT(newFileUsageValue <= usageValue); - usageValue -= newFileUsageValue; + // failures of GetUsage are intentionally ignored + Unused << FileManager::GetUsage(&aFileManagerDirectory) + .andThen([&usageValue](const auto& newFileUsage) { + const auto newFileUsageValue = + newFileUsage.GetValue().valueOr(0); + MOZ_ASSERT(newFileUsageValue <= usageValue); + usageValue -= newFileUsageValue; - // XXX andThen does not support void return - // values right now, we must return a Result - return Result{Ok{}}; - }); + // XXX andThen does not support void return + // values right now, we must return a Result + return Result{Ok{}}; + }); - return Result{Err(rv)}; - }); + return Result{Err(rv)}; + })); if (usageValue) { aQuotaManager->DecreaseUsageForClient( @@ -6775,11 +6777,8 @@ bool DeallocPBackgroundIndexedDBUtilsParent( bool RecvFlushPendingFileDeletions() { AssertIsOnBackgroundThread(); - QuotaClient* quotaClient = QuotaClient::GetInstance(); - if (quotaClient) { - if (NS_FAILED(quotaClient->FlushPendingFileDeletions())) { - NS_WARNING("Failed to flush pending file deletions!"); - } + if (QuotaClient* quotaClient = QuotaClient::GetInstance()) { + QM_WARNONLY_TRY(quotaClient->FlushPendingFileDeletions()); } return true; @@ -6891,29 +6890,30 @@ nsresult DatabaseConnection::BeginWriteTransaction() { IDB_TRY_INSPECT(const auto& beginStmt, BorrowCachedStatement("BEGIN IMMEDIATE;"_ns)); - IDB_TRY(ToResult(beginStmt->Execute()).orElse([&beginStmt](nsresult rv) { - if (rv == NS_ERROR_STORAGE_BUSY) { - NS_WARNING( - "Received NS_ERROR_STORAGE_BUSY when attempting to start write " - "transaction, retrying for up to 10 seconds"); + QM_TRY(QM_OR_ELSE_WARN( + ToResult(beginStmt->Execute()), ([&beginStmt](nsresult rv) { + if (rv == NS_ERROR_STORAGE_BUSY) { + NS_WARNING( + "Received NS_ERROR_STORAGE_BUSY when attempting to start write " + "transaction, retrying for up to 10 seconds"); - // Another thread must be using the database. Wait up to 10 seconds for - // that to complete. - const TimeStamp start = TimeStamp::NowLoRes(); + // Another thread must be using the database. Wait up to 10 seconds + // for that to complete. + const TimeStamp start = TimeStamp::NowLoRes(); - while (true) { - PR_Sleep(PR_MillisecondsToInterval(100)); + while (true) { + PR_Sleep(PR_MillisecondsToInterval(100)); - rv = beginStmt->Execute(); - if (rv != NS_ERROR_STORAGE_BUSY || - TimeStamp::NowLoRes() - start > TimeDuration::FromSeconds(10)) { - break; + rv = beginStmt->Execute(); + if (rv != NS_ERROR_STORAGE_BUSY || + TimeStamp::NowLoRes() - start > TimeDuration::FromSeconds(10)) { + break; + } + } } - } - } - return Result{rv}; - })); + return Result{rv}; + }))); mInWriteTransaction = true; @@ -6945,14 +6945,21 @@ void DatabaseConnection::RollbackWriteTransaction() { return; } - IDB_TRY_INSPECT(const auto& stmt, BorrowCachedStatement("ROLLBACK;"_ns), - QM_VOID); + QM_WARNONLY_TRY( + BorrowCachedStatement("ROLLBACK;"_ns) + .andThen([&self = *this](const auto& stmt) -> Result { + // This may fail if SQLite already rolled back the transaction + // so ignore any errors. - // This may fail if SQLite already rolled back the transaction so ignore any - // errors. - Unused << stmt->Execute(); + // XXX ROLLBACK can fail quite normmally if a previous statement + // failed to execute successfully so SQLite rolled back the + // transaction already. However, if it failed because of some other + // reason, we could try to close the connection. + Unused << stmt->Execute(); - mInWriteTransaction = false; + self.mInWriteTransaction = false; + return Ok{}; + })); } void DatabaseConnection::FinishWriteTransaction() { @@ -6967,9 +6974,11 @@ void DatabaseConnection::FinishWriteTransaction() { mUpdateRefcountFunction->Reset(); } - IDB_TRY(ExecuteCachedStatement("BEGIN;"_ns), QM_VOID); - - mInReadTransaction = true; + QM_WARNONLY_TRY(ToResult(ExecuteCachedStatement("BEGIN;"_ns)) + .andThen([&](const auto) -> Result { + mInReadTransaction = true; + return Ok{}; + })); } nsresult DatabaseConnection::StartSavepoint() { @@ -7120,18 +7129,17 @@ void DatabaseConnection::DoIdleProcessing(bool aNeedsCheckpoint) { // Truncate the WAL if we were asked to or if we managed to free some space. if (aNeedsCheckpoint || freedSomePages) { - nsresult rv = CheckpointInternal(CheckpointMode::Truncate); - Unused << NS_WARN_IF(NS_FAILED(rv)); + QM_WARNONLY_TRY(CheckpointInternal(CheckpointMode::Truncate)); } // Finally try to restart the read transaction if we rolled it back earlier. if (beginStmt) { - nsresult rv = beginStmt.Borrow()->Execute(); - if (NS_SUCCEEDED(rv)) { - mInReadTransaction = true; - } else { - NS_WARNING("Failed to restart read transaction!"); - } + QM_WARNONLY_TRY( + ToResult(beginStmt.Borrow()->Execute()) + .andThen([&self = *this](const Ok) -> Result { + self.mInReadTransaction = true; + return Ok{}; + })); } } @@ -7373,9 +7381,7 @@ DatabaseConnection::AutoSavepoint::~AutoSavepoint() { mDEBUGTransaction->GetMode() == IDBTransaction::Mode::Cleanup || mDEBUGTransaction->GetMode() == IDBTransaction::Mode::VersionChange); - if (NS_FAILED(mConnection->RollbackSavepoint())) { - NS_WARNING("Failed to rollback savepoint!"); - } + QM_WARNONLY_TRY(mConnection->RollbackSavepoint()); } } @@ -7534,9 +7540,7 @@ void DatabaseConnection::UpdateRefcountFunction::DidCommit() { entry.GetData()->MaybeUpdateDBRefs(); } - if (NS_FAILED(RemoveJournals(mJournalsToRemoveAfterCommit))) { - NS_WARNING("RemoveJournals failed!"); - } + QM_WARNONLY_TRY(RemoveJournals(mJournalsToRemoveAfterCommit)); } void DatabaseConnection::UpdateRefcountFunction::DidAbort() { @@ -7546,9 +7550,7 @@ void DatabaseConnection::UpdateRefcountFunction::DidAbort() { AUTO_PROFILER_LABEL("DatabaseConnection::UpdateRefcountFunction::DidAbort", DOM); - if (NS_FAILED(RemoveJournals(mJournalsToRemoveAfterAbort))) { - NS_WARNING("RemoveJournals failed!"); - } + QM_WARNONLY_TRY(RemoveJournals(mJournalsToRemoveAfterAbort)); } void DatabaseConnection::UpdateRefcountFunction::StartSavepoint() { @@ -7696,7 +7698,7 @@ nsresult DatabaseConnection::UpdateRefcountFunction::RemoveJournals( FileManager::GetFileForId(journalDirectory, journal); IDB_TRY(OkIf(file), NS_ERROR_FAILURE); - [&file] { IDB_TRY(file->Remove(false), QM_VOID); }(); + QM_WARNONLY_TRY(file->Remove(false)); } return NS_OK; @@ -9499,13 +9501,9 @@ void Database::Invalidate() { Unused << SendInvalidate(); } - if (!InvalidateAll(mTransactions)) { - NS_WARNING("Failed to abort all transactions!"); - } + QM_WARNONLY_TRY(OkIf(InvalidateAll(mTransactions))); - if (!InvalidateAll(mMutableFiles)) { - NS_WARNING("Failed to abort all mutable files!"); - } + QM_WARNONLY_TRY(OkIf(InvalidateAll(mMutableFiles))); MOZ_ALWAYS_TRUE(CloseInternal()); } @@ -12030,10 +12028,8 @@ void Cursor::SendResponseInternal( KeyValueBase::ProcessFiles(aResponse, aFiles); // Work around the deleted function by casting to the base class. - auto* const base = static_cast(this); - if (!base->SendResponse(aResponse)) { - NS_WARNING("Failed to send response!"); - } + QM_WARNONLY_TRY(OkIf( + static_cast(this)->SendResponse(aResponse))); mCurrentlyRunningOp = nullptr; } @@ -12446,21 +12442,21 @@ Result FileManager::GetUsage(nsIFile* aDirectory) { if (NS_SUCCEEDED(rv)) { IDB_TRY_INSPECT( const auto& thisUsage, - MOZ_TO_RESULT_INVOKE(file, GetFileSize) - .map([](const int64_t fileSize) { - return FileUsageType(Some(uint64_t(fileSize))); - }) - .orElse( - [](const nsresult rv) -> Result { - if (rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST || - rv == NS_ERROR_FILE_NOT_FOUND) { - // If the file does no longer exist, treat it as - // 0-sized. - return FileUsageType{}; - } + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE(file, GetFileSize) + .map([](const int64_t fileSize) { + return FileUsageType(Some(uint64_t(fileSize))); + }), + ([](const nsresult rv) -> Result { + if (rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST || + rv == NS_ERROR_FILE_NOT_FOUND) { + // If the file does no longer exist, treat it as + // 0-sized. + return FileUsageType{}; + } - return Err(rv); - })); + return Err(rv); + }))); usage += thisUsage; @@ -12790,21 +12786,22 @@ nsresult QuotaClient::GetUsageForOriginInternal( &aOriginMetadata](const nsString& subdirName) -> Result { // The directory must have the correct suffix. nsDependentSubstring subdirNameBase; - IDB_TRY(([&subdirName, &subdirNameBase] { - IDB_TRY_RETURN(OkIf(GetFilenameBase( - subdirName, kFileManagerDirectoryNameSuffix, - subdirNameBase))); - }() - .orElse([&directory, &subdirName]( - const NotOk) -> Result { - // If there is an unexpected directory in the idb - // directory, trying to delete at first instead of - // breaking the whole initialization. - IDB_TRY(DeleteFilesNoQuota(directory, subdirName), - Err(NS_ERROR_UNEXPECTED)); + IDB_TRY(QM_OR_ELSE_WARN( + ([&subdirName, &subdirNameBase] { + IDB_TRY_RETURN(OkIf(GetFilenameBase( + subdirName, kFileManagerDirectoryNameSuffix, + subdirNameBase))); + }()), + ([&directory, + &subdirName](const NotOk) -> Result { + // If there is an unexpected directory in the idb + // directory, trying to delete at first instead of + // breaking the whole initialization. + IDB_TRY(DeleteFilesNoQuota(directory, subdirName), + Err(NS_ERROR_UNEXPECTED)); - return Ok{}; - })), + return Ok{}; + })), Ok{}); if (obsoleteFilenames.Contains(subdirNameBase)) { @@ -12823,21 +12820,22 @@ nsresult QuotaClient::GetUsageForOriginInternal( // If there is an unexpected directory in the idb directory, trying to // delete at first instead of breaking the whole initialization. - IDB_TRY(([&databaseFilenames, &subdirNameBase] { - IDB_TRY_RETURN( - OkIf(databaseFilenames.Contains(subdirNameBase))); - }() - .orElse([&directory, &subdirName]( - const NotOk) -> Result { - // XXX It seems if we really got here, we can fail the - // MOZ_ASSERT(!quotaManager->IsTemporaryStorageInitialized()); - // assertion in DeleteFilesNoQuota. - IDB_TRY(DeleteFilesNoQuota(directory, subdirName), - Err(NS_ERROR_UNEXPECTED)); + // XXX This is still somewhat quirky. It would be nice to make it + // clear that the warning handler is infallible, which would also + // remove the need for the error type conversion. + QM_WARNONLY_TRY(QM_OR_ELSE_WARN( + OkIf(databaseFilenames.Contains(subdirNameBase)) + .mapErr([](const NotOk) { return NS_ERROR_FAILURE; }), + ([&directory, + &subdirName](const nsresult) -> Result { + // XXX It seems if we really got here, we can fail the + // MOZ_ASSERT(!quotaManager->IsTemporaryStorageInitialized()); + // assertion in DeleteFilesNoQuota. + IDB_TRY(DeleteFilesNoQuota(directory, subdirName), + Err(NS_ERROR_UNEXPECTED)); - return Ok{}; - })), - Ok{}); + return Ok{}; + }))); return Ok{}; })); @@ -12878,14 +12876,15 @@ nsresult QuotaClient::GetUsageForOriginInternal( CloneFileAndAppend( *directory, databaseFilename + kSQLiteWALSuffix)); - IDB_TRY_INSPECT(const int64_t& walFileSize, - MOZ_TO_RESULT_INVOKE(walFile, GetFileSize) - .orElse([](const nsresult rv) { + IDB_TRY_INSPECT( + const int64_t& walFileSize, + QM_OR_ELSE_WARN(MOZ_TO_RESULT_INVOKE(walFile, GetFileSize), + ([](const nsresult rv) { return (rv == NS_ERROR_FILE_NOT_FOUND || rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) ? Result{0} : Err(rv); - })); + }))); MOZ_ASSERT(walFileSize >= 0); *aUsageInfo += DatabaseUsageType(Some(uint64_t(walFileSize))); } @@ -14248,31 +14247,33 @@ void DatabaseMaintenance::FullVacuum(mozIStorageConnection& aConnection, return; } - IDB_TRY(aConnection.ExecuteSimpleSQL("VACUUM;"_ns), QM_VOID); + QM_WARNONLY_TRY(([&]() -> Result { + IDB_TRY(aConnection.ExecuteSimpleSQL("VACUUM;"_ns)); - const PRTime vacuumTime = PR_Now(); - MOZ_ASSERT(vacuumTime > 0); + const PRTime vacuumTime = PR_Now(); + MOZ_ASSERT(vacuumTime > 0); - IDB_TRY_INSPECT(const int64_t& fileSize, - MOZ_TO_RESULT_INVOKE(aDatabaseFile, GetFileSize), QM_VOID); + IDB_TRY_INSPECT(const int64_t& fileSize, + MOZ_TO_RESULT_INVOKE(aDatabaseFile, GetFileSize)); - MOZ_ASSERT(fileSize > 0); + MOZ_ASSERT(fileSize > 0); - // The parameter names are not used, parameters are bound by index only - // locally in the same function. - IDB_TRY_INSPECT(const auto& stmt, - MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, - aConnection, CreateStatement, - "UPDATE database " - "SET last_vacuum_time = :time" - ", last_vacuum_size = :size;"_ns), - QM_VOID); + // The parameter names are not used, parameters are bound by index only + // locally in the same function. + IDB_TRY_INSPECT(const auto& stmt, MOZ_TO_RESULT_INVOKE_TYPED( + nsCOMPtr, + aConnection, CreateStatement, + "UPDATE database " + "SET last_vacuum_time = :time" + ", last_vacuum_size = :size;"_ns)); - IDB_TRY(stmt->BindInt64ByIndex(0, vacuumTime), QM_VOID); + IDB_TRY(stmt->BindInt64ByIndex(0, vacuumTime)); - IDB_TRY(stmt->BindInt64ByIndex(1, fileSize), QM_VOID); + IDB_TRY(stmt->BindInt64ByIndex(1, fileSize)); - IDB_TRY(stmt->Execute(), QM_VOID); + IDB_TRY(stmt->Execute()); + return Ok{}; + }())); } void DatabaseMaintenance::RunOnOwningThread() { @@ -14614,29 +14615,28 @@ nsresult DatabaseOperationBase::InsertIndexTableRows( IDB_TRY(aObjectStoreKey.BindToStatement(&*borrowedStmt, kStmtParamNameObjectDataKey)); - IDB_TRY(MOZ_TO_RESULT_INVOKE(&*borrowedStmt, Execute) - .orElse([&info, index, - &aIndexValues](nsresult rv) -> Result { - if (rv == NS_ERROR_STORAGE_CONSTRAINT && info.mUnique) { - // If we're inserting multiple entries for the same unique - // index, then we might have failed to insert due to - // colliding with another entry for the same index in which - // case we should ignore it. - for (int32_t index2 = int32_t(index) - 1; - index2 >= 0 && - aIndexValues[index2].mIndexId == info.mIndexId; - --index2) { - if (info.mPosition == aIndexValues[index2].mPosition) { - // We found a key with the same value for the same - // index. So we must have had a collision with a value - // we just inserted. - return Ok{}; - } - } - } + QM_TRY(QM_OR_ELSE_WARN( + ToResult(borrowedStmt->Execute()), + ([&info, index, &aIndexValues](nsresult rv) -> Result { + if (rv == NS_ERROR_STORAGE_CONSTRAINT && info.mUnique) { + // If we're inserting multiple entries for the same unique + // index, then we might have failed to insert due to + // colliding with another entry for the same index in which + // case we should ignore it. + for (int32_t index2 = int32_t(index) - 1; + index2 >= 0 && aIndexValues[index2].mIndexId == info.mIndexId; + --index2) { + if (info.mPosition == aIndexValues[index2].mPosition) { + // We found a key with the same value for the same + // index. So we must have had a collision with a value + // we just inserted. + return Ok{}; + } + } + } - return Err(rv); - })); + return Err(rv); + }))); } return NS_OK; diff --git a/dom/indexedDB/FileInfoTImpl.h b/dom/indexedDB/FileInfoTImpl.h index bcc1a2d7ac16..9894349b7640 100644 --- a/dom/indexedDB/FileInfoTImpl.h +++ b/dom/indexedDB/FileInfoTImpl.h @@ -9,6 +9,7 @@ #include "FileInfoT.h" +#include "mozilla/dom/quota/QuotaCommon.h" #include "mozilla/Mutex.h" #include "nsIFile.h" @@ -94,10 +95,7 @@ void FileInfoT::UpdateReferences(ThreadSafeAutoRefCnt& aRefCount, if (needsCleanup) { if (aSyncDeleteFile) { - nsresult rv = mFileManager->SyncDeleteFile(Id()); - if (NS_FAILED(rv)) { - NS_WARNING("FileManager cleanup failed!"); - } + QM_WARNONLY_TRY(mFileManager->SyncDeleteFile(Id())); } else { Cleanup(); } @@ -136,11 +134,7 @@ bool FileInfoT::LockedClearDBRefs( template void FileInfoT::Cleanup() { - int64_t id = Id(); - - if (NS_FAILED(mFileManager->AsyncDeleteFile(id))) { - NS_WARNING("Failed to delete file asynchronously!"); - } + QM_WARNONLY_TRY(mFileManager->AsyncDeleteFile(Id())); } template diff --git a/dom/indexedDB/IDBDatabase.cpp b/dom/indexedDB/IDBDatabase.cpp index cda5260baa5c..928940483c4d 100644 --- a/dom/indexedDB/IDBDatabase.cpp +++ b/dom/indexedDB/IDBDatabase.cpp @@ -203,12 +203,10 @@ RefPtr IDBDatabase::Create(IDBOpenDBRequest* aRequest, obsSvc->AddObserver(observer, kWindowObserverTopic, false)); // These topics are not crucial. - if (NS_FAILED(obsSvc->AddObserver(observer, kCycleCollectionObserverTopic, - false)) || - NS_FAILED(obsSvc->AddObserver(observer, kMemoryPressureObserverTopic, - false))) { - NS_WARNING("Failed to add additional memory observers!"); - } + QM_WARNONLY_TRY( + obsSvc->AddObserver(observer, kCycleCollectionObserverTopic, false)); + QM_WARNONLY_TRY( + obsSvc->AddObserver(observer, kMemoryPressureObserverTopic, false)); db->mObserver = std::move(observer); } @@ -369,11 +367,14 @@ RefPtr IDBDatabase::CreateObjectStore( return nullptr; } - // XXX This didn't use to warn before in case of a error. Should we remove the - // warning again? - IDB_TRY_INSPECT(const auto& keyPath, - KeyPath::Parse(aOptionalParameters.mKeyPath), nullptr, - [&aRv](const auto&) { aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); }); + QM_NOTEONLY_TRY_UNWRAP(const auto maybeKeyPath, + KeyPath::Parse(aOptionalParameters.mKeyPath)); + if (!maybeKeyPath) { + aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); + return nullptr; + } + + const auto& keyPath = maybeKeyPath.ref(); auto& objectStores = mSpec->objectStores(); const auto end = objectStores.cend(); diff --git a/dom/indexedDB/IDBObjectStore.cpp b/dom/indexedDB/IDBObjectStore.cpp index 4f2f207e2d75..bd6e1709f3a5 100644 --- a/dom/indexedDB/IDBObjectStore.cpp +++ b/dom/indexedDB/IDBObjectStore.cpp @@ -1378,8 +1378,6 @@ RefPtr IDBObjectStore::CreateIndex( return nullptr; } - // XXX This didn't use to warn before in case of a error. Should we remove the - // warning again? const auto checkValid = [](const auto& keyPath) -> Result { if (!keyPath.IsValid()) { return Err(NS_ERROR_DOM_SYNTAX_ERR); @@ -1388,8 +1386,8 @@ RefPtr IDBObjectStore::CreateIndex( return keyPath; }; - IDB_TRY_INSPECT( - const auto& keyPath, + QM_NOTEONLY_TRY_UNWRAP( + const auto maybeKeyPath, ([&aKeyPath, checkValid]() -> Result { if (aKeyPath.IsString()) { IDB_TRY_RETURN( @@ -1403,8 +1401,13 @@ RefPtr IDBObjectStore::CreateIndex( IDB_TRY_RETURN( KeyPath::Parse(aKeyPath.GetAsStringSequence()).andThen(checkValid)); - })(), - nullptr, [&aRv](const auto&) { aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); }); + })()); + if (!maybeKeyPath) { + aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); + return nullptr; + } + + const auto& keyPath = maybeKeyPath.ref(); if (aOptionalParameters.mMultiEntry && keyPath.IsArray()) { aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR); diff --git a/dom/localstorage/ActorsParent.cpp b/dom/localstorage/ActorsParent.cpp index a1d2edd500fb..4219fea62eab 100644 --- a/dom/localstorage/ActorsParent.cpp +++ b/dom/localstorage/ActorsParent.cpp @@ -438,9 +438,9 @@ nsresult SetDefaultPragmas(mozIStorageConnection* aConnection) { if (kSQLiteGrowthIncrement) { // This is just an optimization so ignore the failure if the disk is // currently too full. - LS_TRY( - ToResult(aConnection->SetGrowthIncrement(kSQLiteGrowthIncrement, ""_ns)) - .orElse(ErrToDefaultOkOrErr)); + QM_TRY(QM_OR_ELSE_WARN(ToResult(aConnection->SetGrowthIncrement( + kSQLiteGrowthIncrement, ""_ns)), + ErrToDefaultOkOrErr)); } #endif // LS_MOBILE @@ -463,23 +463,25 @@ Result, nsresult> CreateStorageConnection( LS_TRY_UNWRAP( auto connection, - MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, - storageService, OpenDatabase, &aDBFile) - .orElse([&aUsageFile, &aDBFile, &aCorruptedFileHandler, - &storageService](const nsresult rv) - -> Result, nsresult> { + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, + storageService, OpenDatabase, &aDBFile), + ([&aUsageFile, &aDBFile, &aCorruptedFileHandler, + &storageService](const nsresult rv) + -> Result, nsresult> { if (IsDatabaseCorruptionError(rv)) { // Remove the usage file first (it might not exist at all due // to corrupted state, which is ignored here). - LS_TRY(ToResult(aUsageFile.Remove(false)) - .orElse([](const nsresult rv) -> Result { - if (rv == NS_ERROR_FILE_NOT_FOUND || - rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { - return Ok{}; - } + QM_TRY(QM_OR_ELSE_WARN( + ToResult(aUsageFile.Remove(false)), + ([](const nsresult rv) -> Result { + if (rv == NS_ERROR_FILE_NOT_FOUND || + rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { + return Ok{}; + } - return Err(rv); - })); + return Err(rv); + }))); // Call the corrupted file handler before trying to remove the // database file, which might fail. @@ -493,7 +495,7 @@ Result, nsresult> CreateStorageConnection( &aDBFile)); } return Err(rv); - })); + }))); LS_TRY(SetDefaultPragmas(connection)); @@ -678,10 +680,11 @@ CreateArchiveStorageConnection(const nsAString& aStoragePath) { LS_TRY_UNWRAP( auto connection, - MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, ss, - OpenUnsharedDatabase, archiveFile) - .orElse([](const nsresult rv) - -> Result, nsresult> { + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, ss, + OpenUnsharedDatabase, archiveFile), + ([](const nsresult rv) + -> Result, nsresult> { if (IsDatabaseCorruptionError(rv)) { // Don't throw an error, leave a corrupted ls-archive database as // it is. @@ -689,7 +692,7 @@ CreateArchiveStorageConnection(const nsAString& aStoragePath) { } return Err(rv); - })); + }))); if (connection) { const nsresult rv = StorageDBUpdater::Update(connection); @@ -814,10 +817,11 @@ Result, nsresult> CreateShadowStorageConnection( LS_TRY_UNWRAP( auto connection, - MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, ss, - OpenUnsharedDatabase, shadowFile) - .orElse([&shadowFile, &ss](const nsresult rv) - -> Result, nsresult> { + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, ss, + OpenUnsharedDatabase, shadowFile), + ([&shadowFile, &ss](const nsresult rv) + -> Result, nsresult> { if (IsDatabaseCorruptionError(rv)) { LS_TRY(shadowFile->Remove(false)); @@ -827,7 +831,7 @@ Result, nsresult> CreateShadowStorageConnection( } return Err(rv); - })); + }))); LS_TRY(SetShadowJournalMode(connection)); @@ -845,23 +849,22 @@ Result, nsresult> CreateShadowStorageConnection( // complicated than it should be. Maybe these two methods can be merged (which // would mean that a parameter must be added that indicates whether it's // handling the shadow file or not). - LS_TRY(ToResult(StorageDBUpdater::Update(connection)) - .orElse([&connection, &shadowFile, - &ss](const nsresult) -> Result { - LS_TRY(connection->Close()); - LS_TRY(shadowFile->Remove(false)); + QM_TRY(QM_OR_ELSE_WARN( + ToResult(StorageDBUpdater::Update(connection)), + ([&connection, &shadowFile, &ss](const nsresult) -> Result { + LS_TRY(connection->Close()); + LS_TRY(shadowFile->Remove(false)); - LS_TRY_UNWRAP(connection, - MOZ_TO_RESULT_INVOKE_TYPED( - nsCOMPtr, ss, - OpenUnsharedDatabase, shadowFile)); + LS_TRY_UNWRAP(connection, MOZ_TO_RESULT_INVOKE_TYPED( + nsCOMPtr, ss, + OpenUnsharedDatabase, shadowFile)); - LS_TRY(SetShadowJournalMode(connection)); + LS_TRY(SetShadowJournalMode(connection)); - LS_TRY(StorageDBUpdater::CreateCurrentSchema(connection)); + LS_TRY(StorageDBUpdater::CreateCurrentSchema(connection)); - return Ok{}; - })); + return Ok{}; + }))); return connection; } @@ -960,19 +963,19 @@ Result ExistsAsFile(nsIFile& aFile) { // the path exists. LS_TRY_INSPECT( const auto& res, - MOZ_TO_RESULT_INVOKE(aFile, IsDirectory) - .map([](const bool isDirectory) { - return isDirectory ? ExistsAsFileResult::IsDirectory - : ExistsAsFileResult::IsFile; - }) - .orElse( - [](const nsresult rv) -> Result { - if (rv != NS_ERROR_FILE_NOT_FOUND && - rv != NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { - return Err(rv); - } - return ExistsAsFileResult::DoesNotExist; - })); + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE(aFile, IsDirectory) + .map([](const bool isDirectory) { + return isDirectory ? ExistsAsFileResult::IsDirectory + : ExistsAsFileResult::IsFile; + }), + ([](const nsresult rv) -> Result { + if (rv != NS_ERROR_FILE_NOT_FOUND && + rv != NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { + return Err(rv); + } + return ExistsAsFileResult::DoesNotExist; + }))); LS_TRY(OkIf(res != ExistsAsFileResult::IsDirectory), Err(NS_ERROR_FAILURE)); @@ -3079,17 +3082,16 @@ void InitializeLocalStorage() { MOZ_ASSERT(NS_IsMainThread()); MOZ_ASSERT(!gLocalStorageInitialized); + // XXX Isn't this redundant? It's already done in InitializeQuotaManager. if (!QuotaManager::IsRunningGTests()) { // This service has to be started on the main thread currently. - nsCOMPtr ss; - if (NS_WARN_IF(!(ss = do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID)))) { - NS_WARNING("Failed to get storage service!"); - } + const nsCOMPtr ss = + do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID); + + QM_WARNONLY_TRY(OkIf(ss)); } - if (NS_FAILED(QuotaClient::Initialize())) { - NS_WARNING("Failed to initialize quota client!"); - } + QM_WARNONLY_TRY(QuotaClient::Initialize()); Preferences::RegisterCallbackAndCall(ShadowWritesPrefChangedCallback, kShadowWritesPref); @@ -8102,31 +8104,29 @@ Result QuotaClient::InitOrigin( ([fileExists, usageFileExists, &file, &usageFile, &usageJournalFile, &aOriginMetadata]() -> Result { if (fileExists) { - LS_TRY_RETURN( + LS_TRY_RETURN(QM_OR_ELSE_WARN( // To simplify control flow, we call LoadUsageFile unconditionally // here, even though it will necessarily fail if usageFileExists // is false. - LoadUsageFile(*usageFile) - .orElse([&file, &usageFile, &usageJournalFile, - &aOriginMetadata]( - const nsresult) -> Result { - LS_TRY_INSPECT( - const auto& connection, - CreateStorageConnection( - *file, *usageFile, aOriginMetadata.mOrigin, [] {})); + LoadUsageFile(*usageFile), + ([&file, &usageFile, &usageJournalFile, &aOriginMetadata]( + const nsresult) -> Result { + LS_TRY_INSPECT( + const auto& connection, + CreateStorageConnection(*file, *usageFile, + aOriginMetadata.mOrigin, [] {})); - LS_TRY_INSPECT( - const int64_t& usage, - GetUsage(*connection, - /* aArchivedOriginScope */ nullptr)); + LS_TRY_INSPECT(const int64_t& usage, + GetUsage(*connection, + /* aArchivedOriginScope */ nullptr)); - LS_TRY(UpdateUsageFile(usageFile, usageJournalFile, usage)); + LS_TRY(UpdateUsageFile(usageFile, usageJournalFile, usage)); - LS_TRY(usageJournalFile->Remove(false)); + LS_TRY(usageJournalFile->Remove(false)); - MOZ_ASSERT(usage >= 0); - return UsageInfo{DatabaseUsageType(Some(uint64_t(usage)))}; - })); + MOZ_ASSERT(usage >= 0); + return UsageInfo{DatabaseUsageType(Some(uint64_t(usage)))}; + }))); } if (usageFileExists) { @@ -8741,12 +8741,10 @@ AutoWriteTransaction::~AutoWriteTransaction() { MOZ_COUNT_DTOR(mozilla::dom::AutoWriteTransaction); if (mConnection) { - if (NS_FAILED(mConnection->RollbackWriteTransaction())) { - NS_WARNING("Failed to rollback write transaction!"); - } + QM_WARNONLY_TRY(mConnection->RollbackWriteTransaction()); - if (mShadowWrites && NS_FAILED(DetachShadowDatabaseAndUnlock())) { - NS_WARNING("Failed to detach shadow database!"); + if (mShadowWrites) { + QM_WARNONLY_TRY(DetachShadowDatabaseAndUnlock()); } } } diff --git a/dom/quota/ActorsParent.cpp b/dom/quota/ActorsParent.cpp index 7ab0553fade0..a3edbc37abef 100644 --- a/dom/quota/ActorsParent.cpp +++ b/dom/quota/ActorsParent.cpp @@ -429,24 +429,22 @@ nsresult InvalidateCache(mozIStorageConnection& aConnection) { static constexpr auto kDeleteCacheQuery = "DELETE FROM origin;"_ns; static constexpr auto kSetInvalidFlagQuery = "UPDATE cache SET valid = 0"_ns; - // XXX Use QM_TRY_OR_WARN here in/after Bug 1686191. - QM_TRY(([&]() -> Result { - mozStorageTransaction transaction(&aConnection, false); + QM_TRY(QM_OR_ELSE_WARN( + ([&]() -> Result { + mozStorageTransaction transaction(&aConnection, false); - QM_TRY(transaction.Start()); - QM_TRY(aConnection.ExecuteSimpleSQL(kDeleteCacheQuery)); - QM_TRY(aConnection.ExecuteSimpleSQL(kSetInvalidFlagQuery)); - QM_TRY(transaction.Commit()); + QM_TRY(transaction.Start()); + QM_TRY(aConnection.ExecuteSimpleSQL(kDeleteCacheQuery)); + QM_TRY(aConnection.ExecuteSimpleSQL(kSetInvalidFlagQuery)); + QM_TRY(transaction.Commit()); - return Ok{}; - }() - .orElse([&](const nsresult rv) -> Result { - QM_TRY(aConnection.ExecuteSimpleSQL( - kSetInvalidFlagQuery)); - - return Ok{}; - }))); + return Ok{}; + }()), + ([&](const nsresult rv) -> Result { + QM_TRY(aConnection.ExecuteSimpleSQL(kSetInvalidFlagQuery)); + return Ok{}; + }))); return NS_OK; } @@ -571,21 +569,21 @@ Result, nsresult> CreateWebAppsStoreConnection( return nsCOMPtr{}; } - QM_TRY_INSPECT( - const auto& connection, - MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, - aStorageService, OpenUnsharedDatabase, - &aWebAppsStoreFile) - .orElse([](const nsresult rv) - -> Result, nsresult> { - if (IsDatabaseCorruptionError(rv)) { - // Don't throw an error, leave a corrupted webappsstore database - // as it is. - return nsCOMPtr{}; - } + QM_TRY_INSPECT(const auto& connection, + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE_TYPED( + nsCOMPtr, aStorageService, + OpenUnsharedDatabase, &aWebAppsStoreFile), + ([](const nsresult rv) + -> Result, nsresult> { + if (IsDatabaseCorruptionError(rv)) { + // Don't throw an error, leave a corrupted webappsstore + // database as it is. + return nsCOMPtr{}; + } - return Err(rv); - })); + return Err(rv); + }))); if (connection) { // Don't propagate an error, leave a non-updateable webappsstore database as @@ -2391,12 +2389,12 @@ int64_t GetLastModifiedTime(PersistenceType aPersistenceType, nsIFile& aFile) { Result EnsureDirectory(nsIFile& aDirectory) { AssertIsOnIOThread(); - // TODO: Convert to mapOrElse once mozilla::Result supports it. QM_TRY_INSPECT( const auto& exists, - MOZ_TO_RESULT_INVOKE(aDirectory, Create, nsIFile::DIRECTORY_TYPE, 0755) - .andThen(OkToOk) - .orElse(ErrToOkOrErr)); + QM_OR_ELSE_WARN(MOZ_TO_RESULT_INVOKE(aDirectory, Create, + nsIFile::DIRECTORY_TYPE, 0755) + .map([](Ok) { return false; }), + (ErrToOkOrErr))); if (exists) { QM_TRY_INSPECT(const bool& isDirectory, @@ -2631,17 +2629,19 @@ void InitializeQuotaManager() { MOZ_ASSERT(NS_IsMainThread()); MOZ_ASSERT(!gQuotaManagerInitialized); +#ifdef QM_ENABLE_SCOPED_LOG_EXTRA_INFO + ScopedLogExtraInfo::Initialize(); +#endif + if (!QuotaManager::IsRunningGTests()) { // This service has to be started on the main thread currently. - nsCOMPtr ss; - if (NS_WARN_IF(!(ss = do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID)))) { - NS_WARNING("Failed to get storage service!"); - } + const nsCOMPtr ss = + do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID); + + QM_WARNONLY_TRY(OkIf(ss)); } - if (NS_FAILED(QuotaManager::Initialize())) { - NS_WARNING("Failed to initialize quota manager!"); - } + QM_WARNONLY_TRY(QuotaManager::Initialize()); #ifdef DEBUG gQuotaManagerInitialized = true; @@ -3225,10 +3225,6 @@ QuotaManager::~QuotaManager() { nsresult QuotaManager::Initialize() { MOZ_ASSERT(NS_IsMainThread()); -#ifdef QM_ENABLE_SCOPED_LOG_EXTRA_INFO - ScopedLogExtraInfo::Initialize(); -#endif - nsresult rv = Observer::Initialize(); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; @@ -3798,9 +3794,7 @@ void QuotaManager::Shutdown() { } // Cancel the timer regardless of whether it actually fired. - if (NS_FAILED((*mShutdownTimer)->Cancel())) { - NS_WARNING("Failed to cancel shutdown timer!"); - } + QM_WARNONLY_TRY((*mShutdownTimer)->Cancel()); // NB: It's very important that runnable is destroyed on this thread // (i.e. after we join the IO thread) because we can't release the @@ -3812,14 +3806,10 @@ void QuotaManager::Shutdown() { MOZ_ASSERT(runnable); // Give clients a chance to cleanup IO thread only objects. - if (NS_FAILED((*mIOThread)->Dispatch(runnable, NS_DISPATCH_NORMAL))) { - NS_WARNING("Failed to dispatch runnable!"); - } + QM_WARNONLY_TRY((*mIOThread)->Dispatch(runnable, NS_DISPATCH_NORMAL)); // Make sure to join with our IO thread. - if (NS_FAILED((*mIOThread)->Shutdown())) { - NS_WARNING("Failed to shutdown IO thread!"); - } + QM_WARNONLY_TRY((*mIOThread)->Shutdown()); for (RefPtr& lock : mPendingDirectoryLocks) { lock->Invalidate(); @@ -4620,21 +4610,14 @@ QuotaManager::LoadFullOriginMetadataWithRestore(nsIFile* aDirectory) { const auto& persistenceType = maybePersistenceType.value(); - QM_TRY_UNWRAP(auto maybeFirstAttemptResult, - ([&]() -> Result, nsresult> { - QM_TRY_RETURN( - LoadFullOriginMetadata(aDirectory, persistenceType) - .map(Some), - Maybe{}); - }())); + QM_TRY_RETURN(QM_OR_ELSE_WARN( + LoadFullOriginMetadata(aDirectory, persistenceType), + ([&aDirectory, &persistenceType, + this](const nsresult rv) -> Result { + QM_TRY(RestoreDirectoryMetadata2(aDirectory)); - if (!maybeFirstAttemptResult) { - QM_TRY(RestoreDirectoryMetadata2(aDirectory)); - - QM_TRY_RETURN(LoadFullOriginMetadata(aDirectory, persistenceType)); - } - - return maybeFirstAttemptResult.extract(); + QM_TRY_RETURN(LoadFullOriginMetadata(aDirectory, persistenceType)); + }))); } nsresult QuotaManager::InitializeRepository(PersistenceType aPersistenceType) { @@ -4728,26 +4711,26 @@ nsresult QuotaManager::InitializeRepository(PersistenceType aPersistenceType) { // they won't be accessed after initialization. } - QM_TRY( + QM_TRY(QM_OR_ELSE_WARN( ToResult(InitializeOrigin( - aPersistenceType, metadata, - metadata.mLastAccessTime, - metadata.mPersisted, childDirectory)) - .orElse([&childDirectory](const nsresult rv) - -> Result { - if (IsDatabaseCorruptionError(rv)) { - // If the origin can't be initialized due - // to corruption, this is a permanent - // condition, and we need to remove all - // data for the origin on disk. + aPersistenceType, metadata, + metadata.mLastAccessTime, metadata.mPersisted, + childDirectory)), + ([&childDirectory]( + const nsresult rv) -> Result { + if (IsDatabaseCorruptionError(rv)) { + // If the origin can't be initialized due to + // corruption, this is a permanent + // condition, and we need to remove all data + // for the origin on disk. - QM_TRY(childDirectory->Remove(true)); + QM_TRY(childDirectory->Remove(true)); - return Ok{}; - } + return Ok{}; + } - return Err(rv); - })); + return Err(rv); + }))); break; } @@ -6004,11 +5987,13 @@ nsresult QuotaManager::EnsureStorageIsInitialized() { MOZ_SELECT_OVERLOAD(do_GetService), MOZ_STORAGE_SERVICE_CONTRACTID)); - QM_TRY_UNWRAP(auto connection, - MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, ss, - OpenUnsharedDatabase, storageFile) - .orElse(FilterDatabaseCorruptionError< - nullptr, nsCOMPtr>)); + QM_TRY_UNWRAP( + auto connection, + QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr, ss, + OpenUnsharedDatabase, storageFile), + (FilterDatabaseCorruptionError>))); if (!connection) { // Nuke the database file. @@ -6031,14 +6016,14 @@ nsresult QuotaManager::EnsureStorageIsInitialized() { GetLocalStorageArchiveFile(*mStoragePath)); if (CachedNextGenLocalStorageEnabled()) { - QM_TRY(MaybeCreateOrUpgradeLocalStorageArchive(*lsArchiveFile) - .orElse([&](const nsresult rv) -> Result { - if (IsDatabaseCorruptionError(rv)) { - QM_TRY_RETURN( - CreateEmptyLocalStorageArchive(*lsArchiveFile)); - } - return Err(rv); - })); + QM_TRY(QM_OR_ELSE_WARN( + MaybeCreateOrUpgradeLocalStorageArchive(*lsArchiveFile), + ([&](const nsresult rv) -> Result { + if (IsDatabaseCorruptionError(rv)) { + QM_TRY_RETURN(CreateEmptyLocalStorageArchive(*lsArchiveFile)); + } + return Err(rv); + }))); } else { QM_TRY(MaybeRemoveLocalStorageDataAndArchive(*lsArchiveFile)); } @@ -9009,11 +8994,9 @@ void ClearRequestBase::DeleteFiles(QuotaManager& aQuotaManager, // We can't guarantee that this will always succeed on // Windows... - if (NS_FAILED((file->Remove(true)))) { - NS_WARNING("Failed to remove directory, retrying later."); - + QM_WARNONLY_TRY(file->Remove(true), [&](const auto&) { directoriesForRemovalRetry.AppendElement(std::move(file)); - } + }); const bool initialized = aPersistenceType == PERSISTENCE_TYPE_PERSISTENT @@ -10563,22 +10546,20 @@ nsresult CreateOrUpgradeDirectoryMetadataHelper::MaybeUpgradeOriginDirectory( QM_TRY_INSPECT(const auto& idbDirectory, CloneFileAndAppend(*aDirectory, idbDirectoryName)); - QM_TRY( - ToResult(idbDirectory->Create(nsIFile::DIRECTORY_TYPE, 0755)) - .orElse([&idbDirectory](const nsresult rv) -> Result { - if (rv == NS_ERROR_FILE_ALREADY_EXISTS) { - NS_WARNING("IDB directory already exists!"); + QM_TRY(QM_OR_ELSE_WARN( + ToResult(idbDirectory->Create(nsIFile::DIRECTORY_TYPE, 0755)), + ([&idbDirectory](const nsresult rv) -> Result { + if (rv == NS_ERROR_FILE_ALREADY_EXISTS) { + QM_TRY_INSPECT(const bool& isDirectory, + MOZ_TO_RESULT_INVOKE(idbDirectory, IsDirectory)); - QM_TRY_INSPECT(const bool& isDirectory, - MOZ_TO_RESULT_INVOKE(idbDirectory, IsDirectory)); + QM_TRY(OkIf(isDirectory), Err(NS_ERROR_UNEXPECTED)); - QM_TRY(OkIf(isDirectory), Err(NS_ERROR_UNEXPECTED)); + return Ok{}; + } - return Ok{}; - } - - return Err(rv); - })); + return Err(rv); + }))); QM_TRY(CollectEachFile( *aDirectory, diff --git a/dom/quota/QuotaCommon.cpp b/dom/quota/QuotaCommon.cpp index 832985ca138d..6890bed2ba76 100644 --- a/dom/quota/QuotaCommon.cpp +++ b/dom/quota/QuotaCommon.cpp @@ -164,27 +164,26 @@ Result, nsresult> CloneFileAndAppend( } Result GetDirEntryKind(nsIFile& aFile) { - QM_TRY_RETURN( - MOZ_TO_RESULT_INVOKE(aFile, IsDirectory) - .map([](const bool isDirectory) { - return isDirectory ? nsIFileKind::ExistsAsDirectory - : nsIFileKind::ExistsAsFile; - }) - .orElse([](const nsresult rv) -> Result { - if (rv == NS_ERROR_FILE_NOT_FOUND || - rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST + QM_TRY_RETURN(QM_OR_ELSE_WARN( + MOZ_TO_RESULT_INVOKE(aFile, IsDirectory).map([](const bool isDirectory) { + return isDirectory ? nsIFileKind::ExistsAsDirectory + : nsIFileKind::ExistsAsFile; + }), + ([](const nsresult rv) -> Result { + if (rv == NS_ERROR_FILE_NOT_FOUND || + rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST #ifdef WIN32 - // We treat ERROR_FILE_CORRUPT as if the file did not exist at - // all. - || (NS_ERROR_GET_MODULE(rv) == NS_ERROR_MODULE_WIN32 && - NS_ERROR_GET_CODE(rv) == ERROR_FILE_CORRUPT) + // We treat ERROR_FILE_CORRUPT as if the file did not exist at + // all. + || (NS_ERROR_GET_MODULE(rv) == NS_ERROR_MODULE_WIN32 && + NS_ERROR_GET_CODE(rv) == ERROR_FILE_CORRUPT) #endif - ) { - return nsIFileKind::DoesNotExist; - } + ) { + return nsIFileKind::DoesNotExist; + } - return Err(rv); - })); + return Err(rv); + }))); } Result, nsresult> CreateStatement( @@ -348,9 +347,9 @@ nsDependentCSubstring MakeRelativeSourceFileName( } // namespace detail -void LogError(const nsACString& aExpr, const nsACString& aSourceFile, - const int32_t aSourceLine, const Maybe aRv, - const bool aIsWarning) { +void LogError(const nsACString& aExpr, const Maybe aRv, + const nsACString& aSourceFile, const int32_t aSourceLine, + const Severity aSeverity) { #if defined(EARLY_BETA_OR_EARLIER) || defined(DEBUG) nsAutoCString extraInfosString; @@ -371,6 +370,18 @@ void LogError(const nsACString& aExpr, const nsACString& aSourceFile, const auto relativeSourceFile = detail::MakeRelativeSourceFileName(aSourceFile); + + const auto severityString = [&aSeverity]() -> nsLiteralCString { + switch (aSeverity) { + case Severity::Error: + return "ERROR"_ns; + case Severity::Warning: + return "WARNING"_ns; + case Severity::Note: + return "NOTE"_ns; + } + MOZ_MAKE_COMPILER_ASSUME_IS_UNREACHABLE("Bad severity value!"); + }(); #endif #ifdef QM_ENABLE_SCOPED_LOG_EXTRA_INFO @@ -382,20 +393,22 @@ void LogError(const nsACString& aExpr, const nsACString& aSourceFile, #endif #ifdef DEBUG - NS_DebugBreak(NS_DEBUG_WARNING, nsAutoCString("QM_TRY failure"_ns).get(), - (extraInfosString.IsEmpty() - ? nsPromiseFlatCString(aExpr) - : static_cast( - nsAutoCString(aExpr + extraInfosString))) - .get(), - nsPromiseFlatCString(relativeSourceFile).get(), aSourceLine); + NS_DebugBreak( + NS_DEBUG_WARNING, + nsAutoCString("QM_TRY failure ("_ns + severityString + ")"_ns).get(), + (extraInfosString.IsEmpty() ? nsPromiseFlatCString(aExpr) + : static_cast(nsAutoCString( + aExpr + extraInfosString))) + .get(), + nsPromiseFlatCString(relativeSourceFile).get(), aSourceLine); #endif #if defined(EARLY_BETA_OR_EARLIER) || defined(DEBUG) nsCOMPtr console = do_GetService(NS_CONSOLESERVICE_CONTRACTID); if (console) { - NS_ConvertUTF8toUTF16 message("QM_TRY failure: '"_ns + aExpr + "' at "_ns + + NS_ConvertUTF8toUTF16 message("QM_TRY failure ("_ns + severityString + + ")"_ns + ": '"_ns + aExpr + "' at "_ns + relativeSourceFile + ":"_ns + IntToCString(aSourceLine) + extraInfosString); @@ -423,8 +436,7 @@ void LogError(const nsACString& aExpr, const nsACString& aSourceFile, EventExtraEntry{"source_line"_ns, IntToCString(aSourceLine)}); res.AppendElement(EventExtraEntry{ "context"_ns, nsPromiseFlatCString{*contextIt->second}}); - res.AppendElement(EventExtraEntry{ - "severity"_ns, aIsWarning ? "WARNING"_ns : "ERROR"_ns}); + res.AppendElement(EventExtraEntry{"severity"_ns, severityString}); if (!rvName.IsEmpty()) { res.AppendElement(EventExtraEntry{"result"_ns, nsCString{rvName}}); diff --git a/dom/quota/QuotaCommon.h b/dom/quota/QuotaCommon.h index b3aa1c67b97a..c580938e741b 100644 --- a/dom/quota/QuotaCommon.h +++ b/dom/quota/QuotaCommon.h @@ -447,11 +447,31 @@ class NotNull; } while (0) #ifdef DEBUG -# define QM_HANDLE_ERROR(expr, error) \ - HandleError(#expr, error, __FILE__, __LINE__) +# define QM_HANDLE_ERROR(expr, error, severity) \ + HandleError(#expr, error, __FILE__, __LINE__, severity) #else -# define QM_HANDLE_ERROR(expr, error) \ - HandleError("Unavailable", error, __FILE__, __LINE__) +# define QM_HANDLE_ERROR(expr, error, severity) \ + HandleError("Unavailable", error, __FILE__, __LINE__, severity) +#endif + +#ifdef DEBUG +# define QM_HANDLE_ERROR_RETURN_NOTHING(expr, error, severity) \ + HandleErrorReturnNothing(#expr, error, __FILE__, __LINE__, severity) +#else +# define QM_HANDLE_ERROR_RETURN_NOTHING(expr, error, severity) \ + HandleErrorReturnNothing("Unavailable", error, __FILE__, __LINE__, severity) +#endif + +#ifdef DEBUG +# define QM_HANDLE_ERROR_WITH_CLEANUP_RETURN_NOTHING(expr, error, severity, \ + cleanup) \ + HandleErrorWithCleanupReturnNothing(#expr, error, __FILE__, __LINE__, \ + severity, cleanup) +#else +# define QM_HANDLE_ERROR_WITH_CLEANUP_RETURN_NOTHING(expr, error, severity, \ + cleanup) \ + HandleErrorWithCleanupReturnNothing("Unavailable", error, __FILE__, \ + __LINE__, severity, cleanup) #endif // QM_TRY_PROPAGATE_ERR, QM_TRY_CUSTOM_RET_VAL, @@ -463,7 +483,8 @@ class NotNull; auto tryResult = ::mozilla::ToResult(expr); \ static_assert(std::is_empty_v); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ - ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr()); \ + ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr(), \ + mozilla::dom::quota::Severity::Error); \ return tryResult.propagateErr(); \ } @@ -474,7 +495,8 @@ class NotNull; static_assert(std::is_empty_v); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ auto tryTempError MOZ_MAYBE_UNUSED = tryResult.unwrapErr(); \ - ns::QM_HANDLE_ERROR(expr, tryTempError); \ + ns::QM_HANDLE_ERROR(expr, tryTempError, \ + mozilla::dom::quota::Severity::Error); \ return customRetVal; \ } @@ -486,7 +508,8 @@ class NotNull; static_assert(std::is_empty_v); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ auto tryTempError = tryResult.unwrapErr(); \ - ns::QM_HANDLE_ERROR(expr, tryTempError); \ + ns::QM_HANDLE_ERROR(expr, tryTempError, \ + mozilla::dom::quota::Severity::Error); \ cleanup(tryTempError); \ return customRetVal; \ } @@ -513,8 +536,8 @@ class NotNull; /** * QM_TRY(expr[, customRetVal, cleanup]) is the C++ equivalent of Rust's - * `try!(expr);`. First, it evaluates expr, which must produce a Result value. - * On success, it discards the result altogether. On error, it calls + * `try!(expr);`. First, it evaluates expr, which must produce a Result value + * with empty ok_type. On Success, it does nothing else. On error, it calls * HandleError and an additional cleanup function (if the third argument was * passed) and finally returns an error Result from the enclosing function or a * custom return value (if the second argument was passed). @@ -531,7 +554,8 @@ class NotNull; expr) \ auto tryResult = (expr); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ - ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr()); \ + ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr(), \ + mozilla::dom::quota::Severity::Error); \ return tryResult.propagateErr(); \ } \ MOZ_REMOVE_PAREN(target) = tryResult.accessFunction(); @@ -543,7 +567,8 @@ class NotNull; auto tryResult = (expr); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ auto tryTempError MOZ_MAYBE_UNUSED = tryResult.unwrapErr(); \ - ns::QM_HANDLE_ERROR(expr, tryTempError); \ + ns::QM_HANDLE_ERROR(expr, tryTempError, \ + mozilla::dom::quota::Severity::Error); \ return customRetVal; \ } \ MOZ_REMOVE_PAREN(target) = tryResult.accessFunction(); @@ -555,7 +580,8 @@ class NotNull; auto tryResult = (expr); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ auto tryTempError = tryResult.unwrapErr(); \ - ns::QM_HANDLE_ERROR(expr, tryTempError); \ + ns::QM_HANDLE_ERROR(expr, tryTempError, \ + mozilla::dom::quota::Severity::Error); \ cleanup(tryTempError); \ return customRetVal; \ } \ @@ -612,11 +638,12 @@ class NotNull; // Note that this deliberately uses a single return statement without going // through unwrap/unwrapErr/propagateErr, so that this does not prevent NRVO or // tail call optimizations when possible. -#define QM_TRY_RETURN_PROPAGATE_ERR(ns, tryResult, expr) \ - auto tryResult = ::mozilla::ToResult(expr); \ - if (MOZ_UNLIKELY(tryResult.isErr())) { \ - ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr()); \ - } \ +#define QM_TRY_RETURN_PROPAGATE_ERR(ns, tryResult, expr) \ + auto tryResult = ::mozilla::ToResult(expr); \ + if (MOZ_UNLIKELY(tryResult.isErr())) { \ + ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr(), \ + mozilla::dom::quota::Severity::Error); \ + } \ return tryResult; // Handles the four arguments case when a custom return value needs to be @@ -625,7 +652,8 @@ class NotNull; auto tryResult = ::mozilla::ToResult(expr); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ auto tryTempError MOZ_MAYBE_UNUSED = tryResult.unwrapErr(); \ - ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr()); \ + ns::QM_HANDLE_ERROR(expr, tryResult.inspectErr(), \ + mozilla::dom::quota::Severity::Error); \ return customRetVal; \ } \ return tryResult.unwrap(); @@ -637,7 +665,8 @@ class NotNull; auto tryResult = ::mozilla::ToResult(expr); \ if (MOZ_UNLIKELY(tryResult.isErr())) { \ auto tryTempError = tryResult.unwrapErr(); \ - ns::QM_HANDLE_ERROR(expr, tryTempError); \ + ns::QM_HANDLE_ERROR(expr, tryTempError, \ + mozilla::dom::quota::Severity::Error); \ cleanup(tryTempError); \ return customRetVal; \ } \ @@ -675,15 +704,15 @@ class NotNull; // details of QM_FAIL and shouldn't be used directly. // Handles the two arguments case when just an error is returned -#define QM_FAIL_RET_VAL(ns, retVal) \ - ns::QM_HANDLE_ERROR(Failure, 0); \ +#define QM_FAIL_RET_VAL(ns, retVal) \ + ns::QM_HANDLE_ERROR(Failure, 0, mozilla::dom::quota::Severity::Error); \ return retVal; // Handles the three arguments case when a cleanup function needs to be called // before a return value is returned -#define QM_FAIL_RET_VAL_WITH_CLEANUP(ns, retVal, cleanup) \ - ns::QM_HANDLE_ERROR(Failure, 0); \ - cleanup(); \ +#define QM_FAIL_RET_VAL_WITH_CLEANUP(ns, retVal, cleanup) \ + ns::QM_HANDLE_ERROR(Failure, 0, mozilla::dom::quota::Severity::Error); \ + cleanup(); \ return retVal; // Chooses the final implementation macro for given argument count. @@ -703,6 +732,159 @@ class NotNull; */ #define QM_FAIL(...) QM_FAIL_GLUE(__VA_ARGS__) +// QM_REPORTONLY_TRY, QM_REPORTONLY_TRY_WITH_CLEANUP, QM_REPORTONLY_TRY_GLUE +// macros are implementation details of QM_WARNONLY_TRY/QM_NOTEONLY_TRY and +// shouldn't be used directly. + +// Handles the four arguments case when only a warning/note is reported. +#define QM_REPORTONLY_TRY(ns, tryResult, severity, expr) \ + auto tryResult = ::mozilla::ToResult(expr); \ + static_assert(std::is_empty_v); \ + if (MOZ_UNLIKELY(tryResult.isErr())) { \ + ns::QM_HANDLE_ERROR(expr, tryResult.unwrapErr(), \ + mozilla::dom::quota::Severity::severity); \ + } + +// Handles the five arguments case when a cleanup function needs to be called +#define QM_REPORTONLY_TRY_WITH_CLEANUP(ns, tryResult, severity, expr, cleanup) \ + auto tryResult = ::mozilla::ToResult(expr); \ + static_assert(std::is_empty_v); \ + if (MOZ_UNLIKELY(tryResult.isErr())) { \ + auto tryTempError = tryResult.unwrapErr(); \ + ns::QM_HANDLE_ERROR(expr, tryTempError, \ + mozilla::dom::quota::Severity::severity); \ + cleanup(tryTempError); \ + } + +// Chooses the final implementation macro for given argument count. +// It can be used by other modules to define module specific error handling. +// See also the comment for QM_TRY_META. +#define QM_REPORTONLY_TRY_META(...) \ + { \ + MOZ_ARG_7(, ##__VA_ARGS__, QM_REPORTONLY_TRY_WITH_CLEANUP(__VA_ARGS__), \ + QM_REPORTONLY_TRY(__VA_ARGS__), QM_MISSING_ARGS(__VA_ARGS__), \ + QM_MISSING_ARGS(__VA_ARGS__), QM_MISSING_ARGS(__VA_ARGS__), \ + QM_MISSING_ARGS(__VA_ARGS__)) \ + } + +// Specifies the namespace and generates unique variable name. This extra +// internal macro (along with __COUNTER__) allows nesting of the final macro. +#define QM_REPORTONLY_TRY_GLUE(severity, ...) \ + QM_REPORTONLY_TRY_META(mozilla::dom::quota, MOZ_UNIQUE_VAR(tryResult), \ + severity, ##__VA_ARGS__) + +/** + * QM_WARNONLY_TRY(expr[, cleanup]) evaluates expr, which must produce a + * Result value with empty ok_type. On Success, it does nothing else. On error, + * it calls HandleError and an additional cleanup function (if the second + * argument was passed). This macro never returns and failures are always + * reported using a lower level of severity relative to failures reported by + * QM_TRY. The macro is intended for failures that should not be propagated. + */ +#define QM_WARNONLY_TRY(...) QM_REPORTONLY_TRY_GLUE(Warning, __VA_ARGS__) + +/** + * QM_NOTEONLY_TRY is like QM_WARNONLY_TRY. The only difference is that + * failures are reported using a lower level of severity relative to failures + * reported by QM_WARNONLY_TRY. + */ +#define QM_NOTEONLY_TRY(...) QM_REPORTONLY_TRY_GLUE(Note, __VA_ARGS__) + +// QM_REPORTONLY_TRY_ASSIGN, QM_REPORTONLY_TRY_ASSIGN_WITH_CLEANUP, +// QM_REPORTONLY_TRY_ASSIGN_GLUE macros are implementation details of +// QM_WARNONLY_TRY_UNWRAP/QM_NOTEONLY_TRY_UNWRAP and shouldn't be used +// directly. + +// Handles the five arguments case when only a warning/note is reported. +#define QM_REPORTONLY_TRY_ASSIGN(ns, tryResult, severity, target, expr) \ + auto tryResult = (expr); \ + MOZ_REMOVE_PAREN(target) = \ + MOZ_LIKELY(tryResult.isOk()) \ + ? Some(tryResult.unwrap()) \ + : ns::QM_HANDLE_ERROR_RETURN_NOTHING( \ + expr, tryResult.unwrapErr(), \ + mozilla::dom::quota::Severity::severity); + +// Handles the six arguments case when a cleanup function needs to be called +#define QM_REPORTONLY_TRY_ASSIGN_WITH_CLEANUP(ns, tryResult, severity, target, \ + expr, cleanup) \ + auto tryResult = (expr); \ + MOZ_REMOVE_PAREN(target) = \ + MOZ_LIKELY(tryResult.isOk()) \ + ? Some(tryResult.unwrap()) \ + : ns::QM_HANDLE_ERROR_WITH_CLEANUP_RETURN_NOTHING( \ + expr, tryResult.unwrapErr(), \ + mozilla::dom::quota::Severity::severity, cleanup); + +// Chooses the final implementation macro for given argument count. +// It can be used by other modules to define module specific error handling. +// See also the comment for QM_TRY_META. +#define QM_REPORTONLY_TRY_ASSIGN_META(...) \ + MOZ_ARG_8( \ + , ##__VA_ARGS__, QM_REPORTONLY_TRY_ASSIGN_WITH_CLEANUP(__VA_ARGS__), \ + QM_REPORTONLY_TRY_ASSIGN(__VA_ARGS__), QM_MISSING_ARGS(__VA_ARGS__), \ + QM_MISSING_ARGS(__VA_ARGS__), QM_MISSING_ARGS(__VA_ARGS__), \ + QM_MISSING_ARGS(__VA_ARGS__), QM_MISSING_ARGS(__VA_ARGS__)) + +// Specifies the namespace and generates unique variable name. This extra +// internal macro (along with __COUNTER__) allows nesting of the final macro. +#define QM_REPORTONLY_TRY_ASSIGN_GLUE(severity, ...) \ + QM_REPORTONLY_TRY_ASSIGN_META( \ + mozilla::dom::quota, MOZ_UNIQUE_VAR(tryResult), severity, ##__VA_ARGS__) + +/** + * QM_WARNONLY_TRY_UNWRAP(target, expr[, cleanup]) evaluates expr, which must + * produce a Result value. On success, the result's success value is first + * unwrapped from the Result, then wrapped in a Maybe and finally assigned to + * target. On error, it calls HandleError and an additional cleanup + * function (if the third argument was passed) and finally assigns Nothing to + * target. |target| must be an lvalue. This macro never returns and failures + * are always reported using a lower level of severity relative to failures + * reported by QM_TRY_UNWRAP/QM_TRY_INSPECT. The macro is intended for failures + * that should not be propagated. + */ +#define QM_WARNONLY_TRY_UNWRAP(...) \ + QM_REPORTONLY_TRY_ASSIGN_GLUE(Warning, __VA_ARGS__) + +// QM_WARNONLY_TRY_INSPECT doesn't make sense. + +/** + * QM_NOTEONLY_TRY_UNWRAP is like QM_WARN_CHECK_UNWRAP. The only difference is + * that failures are reported using a lower level of severity relative to + * failures reported by QM_WARN_CHECK_UNWRAP. + */ +#define QM_NOTEONLY_TRY_UNWRAP(...) \ + QM_REPORTONLY_TRY_ASSIGN_GLUE(Note, __VA_ARGS__) + +// QM_NOTEONLY_TRY_INSPECT doesn't make sense. + +/* + * QM_OR_ELSE_WARN(expr, orElse) evaluates expr, which must produce a Result + * value. On Success, it just moves the success over. On error, it calls + * HandleError and a custom orElse function (passed as the second argument). + * Failures are always reported as warnings. The macro essentially wraps the + * orElse function with a warning. The macro is a sub macro and is intended to + * be used along with one of the main macros such as QM_TRY. + */ +#define QM_OR_ELSE_WARN(expr, orElseFunc) \ + (expr).orElse([&](const auto& firstRes) { \ + mozilla::dom::quota::QM_HANDLE_ERROR( \ + #expr, firstRes, mozilla::dom::quota::Severity::Warning); \ + return orElseFunc(firstRes); \ + }) + +/** + * QM_OR_ELSE_NOTE is like QM_OR_ELSE_WARN. The only difference is that + * failures are reported using a lower level of severity relative to failures + * reported by QM_OR_ELSE_WARN. + */ +#define QM_OR_ELSE_NOTE(expr, orElseFunc) \ + (expr).orElse([&](const auto& firstRes) { \ + mozilla::dom::quota::QM_HANDLE_ERROR(#expr, firstRes, \ + mozilla::dom::quota::Severity::Note); \ + return orElseFunc(firstRes); \ + }) + // Telemetry probes to collect number of failure during the initialization. #ifdef NIGHTLY_BUILD # define RECORD_IN_NIGHTLY(_recorder, _status) \ @@ -760,7 +942,7 @@ auto ErrToOkOrErr(nsresult aValue) -> Result { return Err(aValue); } -template +template auto ErrToDefaultOkOrErr(nsresult aValue) -> Result { if (aValue == ErrorValue) { return V{}; @@ -1016,9 +1198,15 @@ nsDependentCSubstring MakeRelativeSourceFileName(const nsACString& aSourceFile); } // namespace detail -void LogError(const nsACString& aExpr, const nsACString& aSourceFile, - int32_t aSourceLine, Maybe aRv, - bool aIsWarning = false); +enum class Severity { + Error, + Warning, + Note, +}; + +void LogError(const nsACString& aExpr, Maybe aRv, + const nsACString& aSourceFile, int32_t aSourceLine, + Severity aSeverity); #ifdef DEBUG Result WarnIfFileIsUnknown(nsIFile& aFile, @@ -1096,24 +1284,45 @@ struct MOZ_STACK_CLASS ScopedLogExtraInfo { #if defined(EARLY_BETA_OR_EARLIER) || defined(DEBUG) template MOZ_COLD void HandleError(const char* aExpr, const T& aRv, - const char* aSourceFile, int32_t aSourceLine) { + const char* aSourceFile, int32_t aSourceLine, + const Severity aSeverity) { if constexpr (std::is_same_v) { - mozilla::dom::quota::LogError(nsDependentCString(aExpr), + mozilla::dom::quota::LogError(nsDependentCString(aExpr), Some(aRv), nsDependentCString(aSourceFile), aSourceLine, - Some(aRv)); + aSeverity); } else { - mozilla::dom::quota::LogError(nsDependentCString(aExpr), + mozilla::dom::quota::LogError(nsDependentCString(aExpr), Nothing{}, nsDependentCString(aSourceFile), aSourceLine, - Nothing{}); + aSeverity); } } #else template MOZ_ALWAYS_INLINE constexpr void HandleError(const char* aExpr, const T& aRv, const char* aSourceFile, - int32_t aSourceLine) {} + int32_t aSourceLine, + const Severity aSeverity) {} #endif +template +Nothing HandleErrorReturnNothing(const char* aExpr, const T& aRv, + const char* aSourceFile, int32_t aSourceLine, + const Severity aSeverity) { + HandleError(aExpr, aRv, aSourceFile, aSourceLine, aSeverity); + return Nothing(); +} + +template +Nothing HandleErrorWithCleanupReturnNothing(const char* aExpr, const T& aRv, + const char* aSourceFile, + int32_t aSourceLine, + const Severity aSeverity, + CleanupFunc&& aCleanupFunc) { + HandleError(aExpr, aRv, aSourceFile, aSourceLine, aSeverity); + std::forward(aCleanupFunc)(aRv); + return Nothing(); +} + template Result, nsresult> diff --git a/dom/quota/test/gtest/TestQuotaCommon.cpp b/dom/quota/test/gtest/TestQuotaCommon.cpp index b3106f8d6058..157f712fdf85 100644 --- a/dom/quota/test/gtest/TestQuotaCommon.cpp +++ b/dom/quota/test/gtest/TestQuotaCommon.cpp @@ -1022,6 +1022,228 @@ TEST(QuotaCommon_Fail, ReturnValue_WithCleanup) EXPECT_EQ(rv, NS_ERROR_FAILURE); } +TEST(QuotaCommon_WarnOnlyTry, Success) +{ + bool warnOnlyTryDidNotReturn = false; + + const auto res = + [&warnOnlyTryDidNotReturn]() -> mozilla::Result { + QM_WARNONLY_TRY(OkIf(true)); + + warnOnlyTryDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_TRUE(warnOnlyTryDidNotReturn); +} + +TEST(QuotaCommon_WarnOnlyTry, Success_WithCleanup) +{ + bool warnOnlyTryCleanupRan = false; + bool warnOnlyTryDidNotReturn = false; + + const auto res = + [&warnOnlyTryCleanupRan, + &warnOnlyTryDidNotReturn]() -> mozilla::Result { + QM_WARNONLY_TRY(OkIf(true), [&warnOnlyTryCleanupRan](const auto&) { + warnOnlyTryCleanupRan = true; + }); + + warnOnlyTryDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_FALSE(warnOnlyTryCleanupRan); + EXPECT_TRUE(warnOnlyTryDidNotReturn); +} + +TEST(QuotaCommon_WarnOnlyTry, Failure) +{ + bool warnOnlyTryDidNotReturn = false; + + const auto res = + [&warnOnlyTryDidNotReturn]() -> mozilla::Result { + QM_WARNONLY_TRY(OkIf(false)); + + warnOnlyTryDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_TRUE(warnOnlyTryDidNotReturn); +} + +TEST(QuotaCommon_WarnOnlyTry, Failure_WithCleanup) +{ + bool warnOnlyTryCleanupRan = false; + bool warnOnlyTryDidNotReturn = false; + + const auto res = + [&warnOnlyTryCleanupRan, + &warnOnlyTryDidNotReturn]() -> mozilla::Result { + QM_WARNONLY_TRY(OkIf(false), ([&warnOnlyTryCleanupRan](const auto&) { + warnOnlyTryCleanupRan = true; + })); + + warnOnlyTryDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_TRUE(warnOnlyTryCleanupRan); + EXPECT_TRUE(warnOnlyTryDidNotReturn); +} + +TEST(QuotaCommon_WarnOnlyTryUnwrap, Success) +{ + bool warnOnlyTryUnwrapDidNotReturn = false; + + const auto res = [&warnOnlyTryUnwrapDidNotReturn]() + -> mozilla::Result { + QM_WARNONLY_TRY_UNWRAP(const auto x, (Result{42})); + EXPECT_TRUE(x); + EXPECT_EQ(*x, 42); + + warnOnlyTryUnwrapDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_TRUE(warnOnlyTryUnwrapDidNotReturn); +} + +TEST(QuotaCommon_WarnOnlyTryUnwrap, Success_WithCleanup) +{ + bool warnOnlyTryUnwrapCleanupRan = false; + bool warnOnlyTryUnwrapDidNotReturn = false; + + const auto res = [&warnOnlyTryUnwrapCleanupRan, + &warnOnlyTryUnwrapDidNotReturn]() + -> mozilla::Result { + QM_WARNONLY_TRY_UNWRAP(const auto x, (Result{42}), + [&warnOnlyTryUnwrapCleanupRan](const auto&) { + warnOnlyTryUnwrapCleanupRan = true; + }); + EXPECT_TRUE(x); + EXPECT_EQ(*x, 42); + + warnOnlyTryUnwrapDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_FALSE(warnOnlyTryUnwrapCleanupRan); + EXPECT_TRUE(warnOnlyTryUnwrapDidNotReturn); +} + +TEST(QuotaCommon_WarnOnlyTryUnwrap, Failure) +{ + bool warnOnlyTryUnwrapDidNotReturn = false; + + const auto res = [&warnOnlyTryUnwrapDidNotReturn]() + -> mozilla::Result { + QM_WARNONLY_TRY_UNWRAP(const auto x, + (Result{Err(NotOk{})})); + EXPECT_FALSE(x); + + warnOnlyTryUnwrapDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_TRUE(warnOnlyTryUnwrapDidNotReturn); +} + +TEST(QuotaCommon_WarnOnlyTryUnwrap, Failure_WithCleanup) +{ + bool warnOnlyTryUnwrapCleanupRan = false; + bool warnOnlyTryUnwrapDidNotReturn = false; + + const auto res = [&warnOnlyTryUnwrapCleanupRan, + &warnOnlyTryUnwrapDidNotReturn]() + -> mozilla::Result { + QM_WARNONLY_TRY_UNWRAP(const auto x, (Result{Err(NotOk{})}), + [&warnOnlyTryUnwrapCleanupRan](const auto&) { + warnOnlyTryUnwrapCleanupRan = true; + }); + EXPECT_FALSE(x); + + warnOnlyTryUnwrapDidNotReturn = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_TRUE(warnOnlyTryUnwrapCleanupRan); + EXPECT_TRUE(warnOnlyTryUnwrapDidNotReturn); +} + +TEST(QuotaCommon_OrElseWarn, Success) +{ + bool orElseWarnRun = false; + bool tryContinued = false; + + const auto res = [&]() -> mozilla::Result { + QM_TRY(QM_OR_ELSE_WARN(OkIf(true), ([&orElseWarnRun](const NotOk) { + orElseWarnRun = true; + return mozilla::Result{ + mozilla::Ok{}}; + }))); + + tryContinued = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_FALSE(orElseWarnRun); + EXPECT_TRUE(tryContinued); +} + +TEST(QuotaCommon_OrElseWarn, Failure_MappedToSuccess) +{ + bool orElseWarnRun = false; + bool tryContinued = false; + + // XXX Consider allowing to set a custom error handler, so that we can + // actually assert that a warning was emitted. + const auto res = [&]() -> mozilla::Result { + QM_TRY(QM_OR_ELSE_WARN(OkIf(false), ([&orElseWarnRun](const NotOk) { + orElseWarnRun = true; + return mozilla::Result{ + mozilla::Ok{}}; + }))); + tryContinued = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isOk()); + EXPECT_TRUE(orElseWarnRun); + EXPECT_TRUE(tryContinued); +} + +TEST(QuotaCommon_OrElseWarn, Failure_MappedToError) +{ + bool orElseWarnRun = false; + bool tryContinued = false; + + // XXX Consider allowing to set a custom error handler, so that we can + // actually assert that a warning was emitted. + const auto res = [&]() -> mozilla::Result { + QM_TRY(QM_OR_ELSE_WARN(OkIf(false), ([&orElseWarnRun](const NotOk) { + orElseWarnRun = true; + return mozilla::Result{ + NotOk{}}; + }))); + tryContinued = true; + return mozilla::Ok{}; + }(); + + EXPECT_TRUE(res.isErr()); + EXPECT_TRUE(orElseWarnRun); + EXPECT_FALSE(tryContinued); +} + TEST(QuotaCommon_OkIf, True) { auto res = OkIf(true);