Bug 1671369 - Change GetBodyUsage to return a Result. r=dom-workers-and-storage-reviewers,ttung
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 17 Nov 2020 09:04:23 +0000
changeset 557501 41c717fa78a6dc6e72bb9a492dfd4f432753e298
parent 557500 455a0080b317403241b510b3a08784fde4d7f181
child 557502 d1d8b7135769f54732918452816bb0e2446998d4
push id37959
push userbtara@mozilla.com
push dateTue, 17 Nov 2020 21:55:29 +0000
treeherdermozilla-central@9dd0b13d77b9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-workers-and-storage-reviewers, ttung
bugs1671369
milestone85.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1671369 - Change GetBodyUsage to return a Result. r=dom-workers-and-storage-reviewers,ttung Differential Revision: https://phabricator.services.mozilla.com/D93769
dom/cache/QuotaClient.cpp
--- a/dom/cache/QuotaClient.cpp
+++ b/dom/cache/QuotaClient.cpp
@@ -29,95 +29,92 @@ using mozilla::dom::quota::GroupAndOrigi
 using mozilla::dom::quota::PERSISTENCE_TYPE_DEFAULT;
 using mozilla::dom::quota::PersistenceType;
 using mozilla::dom::quota::QuotaManager;
 using mozilla::dom::quota::UsageInfo;
 using mozilla::ipc::AssertIsOnBackgroundThread;
 
 namespace {
 
-nsresult GetBodyUsage(nsIFile* aMorgueDir, const Atomic<bool>& aCanceled,
-                      UsageInfo* aUsageInfo, const bool aInitializing) {
+Result<UsageInfo, nsresult> GetBodyUsage(nsIFile& aMorgueDir,
+                                         const Atomic<bool>& aCanceled,
+                                         const bool aInitializing) {
   AssertIsOnIOThread();
 
-  nsCOMPtr<nsIDirectoryEnumerator> entries;
-  nsresult rv = aMorgueDir->GetDirectoryEntries(getter_AddRefs(entries));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
+  UsageInfo usageInfo;
+
+  // XXX The following loop (including the cancellation check) is very similar
+  // to QuotaClient::GetDatabaseFilenames in dom/indexedDB/ActorsParent.cpp
+  // (Also, it is a fallible variant of std::reduce)
+  CACHE_TRY_INSPECT(const auto& entries, MOZ_TO_RESULT_INVOKE_TYPED(
+                                             nsCOMPtr<nsIDirectoryEnumerator>,
+                                             aMorgueDir, GetDirectoryEntries));
+
+  CACHE_TRY(CollectEach(
+      [&entries, &aCanceled]() -> Result<nsCOMPtr<nsIFile>, nsresult> {
+        if (aCanceled) {
+          return nsCOMPtr<nsIFile>{};
+        }
 
-  nsCOMPtr<nsIFile> bodyDir;
-  while (NS_SUCCEEDED(rv = entries->GetNextFile(getter_AddRefs(bodyDir))) &&
-         bodyDir && !aCanceled) {
-    if (NS_WARN_IF(QuotaManager::IsShuttingDown())) {
-      return NS_ERROR_ABORT;
-    }
-    bool isDir;
-    rv = bodyDir->IsDirectory(&isDir);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
+        CACHE_TRY_RETURN(MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr<nsIFile>, entries,
+                                                    GetNextFile));
+      },
+      [&usageInfo, aInitializing](
+          const nsCOMPtr<nsIFile>& bodyDir) -> Result<Ok, nsresult> {
+        CACHE_TRY(OkIf(!QuotaManager::IsShuttingDown()), Err(NS_ERROR_ABORT));
 
-    if (!isDir) {
-      QuotaInfo dummy;
-      DebugOnly<nsresult> result =
-          RemoveNsIFile(dummy, bodyDir, /* aTrackQuota */ false);
-      // Try to remove the unexpected files, and keep moving on even if it fails
-      // because it might be created by virus or the operation system
-      MOZ_ASSERT(NS_SUCCEEDED(result));
-      continue;
-    }
+        CACHE_TRY_INSPECT(const bool& isDir,
+                          MOZ_TO_RESULT_INVOKE(bodyDir, IsDirectory));
 
-    const QuotaInfo dummy;
-    const auto getUsage = [&aUsageInfo](nsIFile* bodyFile,
-                                        const nsACString& leafName,
-                                        bool& fileDeleted) {
-      MOZ_DIAGNOSTIC_ASSERT(bodyFile);
-      Unused << leafName;
+        if (!isDir) {
+          const DebugOnly<nsresult> result =
+              RemoveNsIFile(QuotaInfo{}, bodyDir, /* aTrackQuota */ false);
+          // Try to remove the unexpected files, and keep moving on even if it
+          // fails because it might be created by virus or the operation system
+          MOZ_ASSERT(NS_SUCCEEDED(result));
+          return Ok{};
+        }
 
-      int64_t fileSize;
-      nsresult rv = bodyFile->GetFileSize(&fileSize);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
-      MOZ_DIAGNOSTIC_ASSERT(fileSize >= 0);
-      // FIXME: Separate file usage and database usage in OriginInfo so that the
-      // workaround for treating body file size as database usage can be
-      // removed.
-      //
-      // This is needed because we want to remove the mutex lock for padding
-      // files. The lock is needed because the padding file is accessed on the
-      // QM IO thread while getting origin usage and is accessed on the Cache IO
-      // thread in normal Cache operations.
-      // Using the cached usage in QM while getting origin usage can remove the
-      // access on the QM IO thread and thus we can remove the mutex lock.
-      // However, QM only separates usage types in initialization, and the
-      // separation is gone after that. So, before extending the separation of
-      // usage types in QM, this is a workaround to avoid the file usage
-      // mismatching in our tests. Note that file usage hasn't been exposed to
-      // users yet.
-      *aUsageInfo += DatabaseUsageType(Some(fileSize));
+        const auto getUsage = [&usageInfo](nsIFile* bodyFile,
+                                           const nsACString& leafName,
+                                           bool& fileDeleted) -> nsresult {
+          MOZ_DIAGNOSTIC_ASSERT(bodyFile);
+          Unused << leafName;
 
-      fileDeleted = false;
+          CACHE_TRY_INSPECT(const int64_t& fileSize,
+                            MOZ_TO_RESULT_INVOKE(bodyFile, GetFileSize));
+          MOZ_DIAGNOSTIC_ASSERT(fileSize >= 0);
+          // FIXME: Separate file usage and database usage in OriginInfo so that
+          // the workaround for treating body file size as database usage can be
+          // removed.
+          //
+          // This is needed because we want to remove the mutex lock for padding
+          // files. The lock is needed because the padding file is accessed on
+          // the QM IO thread while getting origin usage and is accessed on the
+          // Cache IO thread in normal Cache operations. Using the cached usage
+          // in QM while getting origin usage can remove the access on the QM IO
+          // thread and thus we can remove the mutex lock. However, QM only
+          // separates usage types in initialization, and the separation is gone
+          // after that. So, before extending the separation of usage types in
+          // QM, this is a workaround to avoid the file usage mismatching in our
+          // tests. Note that file usage hasn't been exposed to users yet.
+          usageInfo += DatabaseUsageType(Some(fileSize));
 
-      return NS_OK;
-    };
-    rv = BodyTraverseFiles(dummy, bodyDir, getUsage,
-                           /* aCanRemoveFiles */
-                           aInitializing,
-                           /* aTrackQuota */ false);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-  }
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
+          fileDeleted = false;
 
-  return NS_OK;
+          return NS_OK;
+        };
+        CACHE_TRY(BodyTraverseFiles(QuotaInfo{}, bodyDir, getUsage,
+                                    /* aCanRemoveFiles */
+                                    aInitializing,
+                                    /* aTrackQuota */ false));
+        return Ok{};
+      }));
+
+  return usageInfo;
 }
 
 Result<int64_t, nsresult> LockedGetPaddingSizeFromDB(
     nsIFile& aDir, const GroupAndOrigin& aGroupAndOrigin) {
   QuotaInfo quotaInfo;
   static_cast<GroupAndOrigin&>(quotaInfo) = aGroupAndOrigin;
   // quotaInfo.mDirectoryLockId must be -1 (which is default for new QuotaInfo)
   // because this method should only be called from QuotaClient::InitOrigin
@@ -394,16 +391,17 @@ Result<UsageInfo, nsresult> CacheQuotaCl
   }
 
   // FIXME: Separate file usage and database usage in OriginInfo so that the
   // workaround for treating padding file size as database usage can be removed.
   usageInfo += DatabaseUsageType(maybePaddingSize);
 
   // XXX The following loop (including the cancellation check) is very similar
   // to QuotaClient::GetDatabaseFilenames in dom/indexedDB/ActorsParent.cpp
+  // (Also, it is a fallible variant of std::reduce)
   CACHE_TRY_INSPECT(const auto& entries,
                     MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr<nsIDirectoryEnumerator>,
                                                dir, GetDirectoryEntries));
 
   CACHE_TRY(CollectEach(
       [&entries, &aCanceled]() -> Result<nsCOMPtr<nsIFile>, nsresult> {
         if (aCanceled) {
           return nsCOMPtr<nsIFile>{};
@@ -422,17 +420,19 @@ Result<UsageInfo, nsresult> CacheQuotaCl
 
         CACHE_TRY_INSPECT(const bool& isDir,
                           MOZ_TO_RESULT_INVOKE(file, IsDirectory));
 
         if (isDir) {
           if (leafName.EqualsLiteral("morgue")) {
             // XXX This didn't use to warn for NS_ERROR_ABORT, should we keep
             // that? (but it was and is propagated)
-            CACHE_TRY(GetBodyUsage(file, aCanceled, &usageInfo, aInitializing));
+            CACHE_TRY_INSPECT(const auto& bodyUsageInfo,
+                              GetBodyUsage(*file, aCanceled, aInitializing));
+            usageInfo += bodyUsageInfo;
           } else {
             NS_WARNING("Unknown Cache directory found!");
           }
 
           return Ok{};
         }
 
         // Ignore transient sqlite files and marker files