Bug 1685677 - Extract functions from CheckTemporaryStorageLimits and rename it to CleanupTemporaryStorage. r=dom-workers-and-storage-reviewers,janv
☠☠ backed out by 082a0d699c97 ☠ ☠
authorSimon Giesecke <sgiesecke@mozilla.com>
Mon, 01 Feb 2021 13:37:30 +0000
changeset 3480198 121e93b3787f9498461cd9b51888bdd44ad1cdf2
parent 3480197 8e8261bb094f44136f53df00f35dddb770b42f03
child 3480199 ea958694bd1772aa13a7e6c030a9429d89101e77
push id648815
push userwptsync@mozilla.com
push dateMon, 01 Feb 2021 17:48:27 +0000
treeherdertry@abfb9840bad0 [default view] [failures only]
reviewersdom-workers-and-storage-reviewers, janv
bugs1685677
milestone87.0a1
Bug 1685677 - Extract functions from CheckTemporaryStorageLimits and rename it to CleanupTemporaryStorage. r=dom-workers-and-storage-reviewers,janv Extracts parts of CheckTemporaryStorageLimits into the following new private member functions of QuotaManager: * LockedGetOriginInfosExceedingGroupLimit * LockedGetOriginInfosExceedingGlobalLimit * GetOriginInfosExceedingLimits * ClearOrigins Differential Revision: https://phabricator.services.mozilla.com/D101145
dom/quota/ActorsParent.cpp
dom/quota/QuotaManager.h
mfbt/NotNull.h
--- a/dom/quota/ActorsParent.cpp
+++ b/dom/quota/ActorsParent.cpp
@@ -939,23 +939,23 @@ class OriginInfo final {
    * where a client like LocalStorage has reserved quota for disk writes, but
    * has not yet flushed the data to disk.
    */
   bool mDirectoryExists;
 };
 
 class OriginInfoLRUComparator {
  public:
-  bool Equals(const NotNull<RefPtr<OriginInfo>>& a,
-              const NotNull<RefPtr<OriginInfo>>& b) const {
+  bool Equals(const NotNull<RefPtr<const OriginInfo>>& a,
+              const NotNull<RefPtr<const OriginInfo>>& b) const {
     return a->LockedAccessTime() == b->LockedAccessTime();
   }
 
-  bool LessThan(const NotNull<RefPtr<OriginInfo>>& a,
-                const NotNull<RefPtr<OriginInfo>>& b) const {
+  bool LessThan(const NotNull<RefPtr<const OriginInfo>>& a,
+                const NotNull<RefPtr<const OriginInfo>>& b) const {
     return a->LockedAccessTime() < b->LockedAccessTime();
   }
 };
 
 class GroupInfo final {
   friend class GroupInfoPair;
   friend class OriginInfo;
   friend class QuotaManager;
@@ -3734,18 +3734,18 @@ uint64_t QuotaManager::CollectOriginsFor
     uint64_t aMinSizeToBeFreed, nsTArray<RefPtr<DirectoryLockImpl>>& aLocks) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(aLocks.IsEmpty());
 
   struct MOZ_STACK_CLASS Helper final {
     static void GetInactiveOriginInfos(
         const nsTArray<NotNull<RefPtr<OriginInfo>>>& aOriginInfos,
         const nsTArray<NotNull<DirectoryLockImpl*>>& aLocks,
-        nsTArray<NotNull<RefPtr<OriginInfo>>>& aInactiveOriginInfos) {
-      for (const NotNull<RefPtr<OriginInfo>>& originInfo : aOriginInfos) {
+        OriginInfosFlatTraversable& aInactiveOriginInfos) {
+      for (const auto& originInfo : aOriginInfos) {
         MOZ_ASSERT(originInfo->mGroupInfo->mPersistenceType !=
                    PERSISTENCE_TYPE_PERSISTENT);
 
         if (originInfo->LockedPersisted()) {
           continue;
         }
 
         const auto originScope = OriginScope::FromOrigin(originInfo->mOrigin);
@@ -3783,17 +3783,17 @@ uint64_t QuotaManager::CollectOriginsFor
       defaultStorageLocks.AppendElement(lock);
     } else {
       MOZ_ASSERT(persistenceType.Value() == PERSISTENCE_TYPE_PERSISTENT);
 
       // Do nothing here, persistent origins don't need to be collected ever.
     }
   }
 
-  nsTArray<NotNull<RefPtr<OriginInfo>>> inactiveOrigins;
+  OriginInfosFlatTraversable inactiveOrigins;
 
   // Enumerate and process inactive origins. This must be protected by the
   // mutex.
   MutexAutoLock lock(mQuotaMutex);
 
   for (const auto& entry : mGroupInfoPairs) {
     const auto& pair = entry.GetData();
 
@@ -6531,17 +6531,17 @@ nsresult QuotaManager::EnsureTemporarySt
   // Available disk space shouldn't be used directly for temporary storage
   // limit calculation since available disk space is affected by existing data
   // stored in temporary storage. So we need to increase it by the temporary
   // storage size (that has been calculated in LoadQuota) before passing to
   // GetTemporaryStorageLimit..
   mTemporaryStorageLimit = GetTemporaryStorageLimit(
       /* aAvailableSpaceBytes */ diskSpaceAvailable + mTemporaryStorageUsage);
 
-  CheckTemporaryStorageLimits();
+  CleanupTemporaryStorage();
 
   return NS_OK;
 }
 
 void QuotaManager::ShutdownStorage() {
   AssertIsOnIOThread();
 
   if (mStorageConnection) {
@@ -7073,160 +7073,191 @@ already_AddRefed<OriginInfo> QuotaManage
     if (groupInfo) {
       return groupInfo->LockedGetOriginInfo(aGroupAndOrigin.mOrigin);
     }
   }
 
   return nullptr;
 }
 
-void QuotaManager::CheckTemporaryStorageLimits() {
-  AssertIsOnIOThread();
-
-  const auto doomedOrigins = [this] {
-    const auto doomedOriginInfos = [this] {
-      nsTArray<NotNull<RefPtr<OriginInfo>>> doomedOriginInfos;
-      MutexAutoLock lock(mQuotaMutex);
-
-      for (const auto& entry : mGroupInfoPairs) {
-        const auto& pair = entry.GetData();
-
-        MOZ_ASSERT(!entry.GetKey().IsEmpty());
-        MOZ_ASSERT(pair);
-
-        uint64_t groupUsage = 0;
-
-        RefPtr<GroupInfo> temporaryGroupInfo =
-            pair->LockedGetGroupInfo(PERSISTENCE_TYPE_TEMPORARY);
-        if (temporaryGroupInfo) {
-          groupUsage += temporaryGroupInfo->mUsage;
-        }
-
-        RefPtr<GroupInfo> defaultGroupInfo =
-            pair->LockedGetGroupInfo(PERSISTENCE_TYPE_DEFAULT);
-        if (defaultGroupInfo) {
-          groupUsage += defaultGroupInfo->mUsage;
-        }
-
-        if (groupUsage > 0) {
-          QuotaManager* quotaManager = QuotaManager::Get();
-          MOZ_ASSERT(quotaManager, "Shouldn't be null!");
-
-          if (groupUsage > quotaManager->GetGroupLimit()) {
-            nsTArray<NotNull<RefPtr<OriginInfo>>> originInfos;
-            if (temporaryGroupInfo) {
-              originInfos.AppendElements(temporaryGroupInfo->mOriginInfos);
-            }
-            if (defaultGroupInfo) {
-              originInfos.AppendElements(defaultGroupInfo->mOriginInfos);
-            }
-            originInfos.Sort(OriginInfoLRUComparator());
-
-            for (uint32_t i = 0; i < originInfos.Length(); i++) {
-              const NotNull<RefPtr<OriginInfo>>& originInfo = originInfos[i];
-              if (originInfo->LockedPersisted()) {
-                continue;
-              }
-
-              doomedOriginInfos.AppendElement(originInfo);
-              groupUsage -= originInfo->LockedUsage();
-
-              if (groupUsage <= quotaManager->GetGroupLimit()) {
-                break;
-              }
-            }
+QuotaManager::OriginInfosFlatTraversable
+QuotaManager::LockedGetOriginInfosExceedingGroupLimit() const {
+  mQuotaMutex.AssertCurrentThreadOwns();
+
+  OriginInfosFlatTraversable originInfos;
+
+  for (const auto& entry : mGroupInfoPairs) {
+    const auto& pair = entry.GetData();
+
+    MOZ_ASSERT(!entry.GetKey().IsEmpty());
+    MOZ_ASSERT(pair);
+
+    uint64_t groupUsage = 0;
+
+    const RefPtr<GroupInfo> temporaryGroupInfo =
+        pair->LockedGetGroupInfo(PERSISTENCE_TYPE_TEMPORARY);
+    if (temporaryGroupInfo) {
+      groupUsage += temporaryGroupInfo->mUsage;
+    }
+
+    const RefPtr<GroupInfo> defaultGroupInfo =
+        pair->LockedGetGroupInfo(PERSISTENCE_TYPE_DEFAULT);
+    if (defaultGroupInfo) {
+      groupUsage += defaultGroupInfo->mUsage;
+    }
+
+    if (groupUsage > 0) {
+      QuotaManager* quotaManager = QuotaManager::Get();
+      MOZ_ASSERT(quotaManager, "Shouldn't be null!");
+
+      if (groupUsage > quotaManager->GetGroupLimit()) {
+        const auto allOriginInfosLRUSorted = [&temporaryGroupInfo,
+                                              &defaultGroupInfo] {
+          OriginInfosFlatTraversable originInfos;
+          if (temporaryGroupInfo) {
+            originInfos.AppendElements(temporaryGroupInfo->mOriginInfos);
+          }
+          if (defaultGroupInfo) {
+            originInfos.AppendElements(defaultGroupInfo->mOriginInfos);
+          }
+          originInfos.Sort(OriginInfoLRUComparator());
+          return originInfos;
+        }();
+
+        for (const auto& originInfo : allOriginInfosLRUSorted) {
+          // XXX We can remove these before sorting and then just truncate the
+          // existing array at the right point.
+          if (originInfo->LockedPersisted()) {
+            continue;
+          }
+
+          originInfos.AppendElement(originInfo);
+          groupUsage -= originInfo->LockedUsage();
+
+          if (groupUsage <= quotaManager->GetGroupLimit()) {
+            break;
           }
         }
       }
-
-      uint64_t usage = std::accumulate(
-          doomedOriginInfos.cbegin(), doomedOriginInfos.cend(), uint64_t(0),
-          [](uint64_t oldValue, const auto& originInfo) {
-            return oldValue + originInfo->LockedUsage();
-          });
-
-      if (mTemporaryStorageUsage - usage > mTemporaryStorageLimit) {
-        nsTArray<NotNull<RefPtr<OriginInfo>>> originInfos;
-
-        for (const auto& entry : mGroupInfoPairs) {
-          const auto& pair = entry.GetData();
-
-          MOZ_ASSERT(!entry.GetKey().IsEmpty());
-          MOZ_ASSERT(pair);
-
-          RefPtr<GroupInfo> groupInfo =
-              pair->LockedGetGroupInfo(PERSISTENCE_TYPE_TEMPORARY);
-          if (groupInfo) {
-            originInfos.AppendElements(groupInfo->mOriginInfos);
-          }
-
-          groupInfo = pair->LockedGetGroupInfo(PERSISTENCE_TYPE_DEFAULT);
-          if (groupInfo) {
-            originInfos.AppendElements(groupInfo->mOriginInfos);
-          }
-        }
-
-        originInfos.RemoveElementsBy(
-            [&doomedOriginInfos](const auto& originInfo) {
-              return doomedOriginInfos.Contains(originInfo) ||
-                     originInfo->LockedPersisted();
-            });
-
-        originInfos.Sort(OriginInfoLRUComparator());
-
-        for (uint32_t i = 0; i < originInfos.Length(); i++) {
-          if (mTemporaryStorageUsage - usage <= mTemporaryStorageLimit) {
-            originInfos.TruncateLength(i);
-            break;
-          }
-
-          usage += originInfos[i]->LockedUsage();
-        }
-
-        doomedOriginInfos.AppendElements(originInfos);
-      }
-
-      return doomedOriginInfos;
-    }();
-
-    for (const auto& doomedOriginInfo : doomedOriginInfos) {
+    }
+  }
+
+  return originInfos;
+}
+
+QuotaManager::OriginInfosFlatTraversable
+QuotaManager::LockedGetOriginInfosExceedingGlobalLimit(
+    const OriginInfosFlatTraversable& aAlreadyDoomedOriginInfos,
+    const uint64_t aAlreadyDoomedUsage) const {
+  mQuotaMutex.AssertCurrentThreadOwns();
+
+  OriginInfosFlatTraversable originInfos;
+  for (const auto& entry : mGroupInfoPairs) {
+    const auto& pair = entry.GetData();
+
+    MOZ_ASSERT(!entry.GetKey().IsEmpty());
+    MOZ_ASSERT(pair);
+
+    RefPtr<GroupInfo> groupInfo =
+        pair->LockedGetGroupInfo(PERSISTENCE_TYPE_TEMPORARY);
+    if (groupInfo) {
+      originInfos.AppendElements(groupInfo->mOriginInfos);
+    }
+
+    groupInfo = pair->LockedGetGroupInfo(PERSISTENCE_TYPE_DEFAULT);
+    if (groupInfo) {
+      originInfos.AppendElements(groupInfo->mOriginInfos);
+    }
+  }
+
+  originInfos.RemoveElementsBy(
+      [&aAlreadyDoomedOriginInfos](const auto& originInfo) {
+        return aAlreadyDoomedOriginInfos.Contains(originInfo) ||
+               originInfo->LockedPersisted();
+      });
+
+  originInfos.Sort(OriginInfoLRUComparator());
+
+  uint64_t doomedUsage = 0;
+  for (uint32_t i = 0; i < originInfos.Length(); i++) {
+    if (mTemporaryStorageUsage - aAlreadyDoomedUsage - doomedUsage <=
+        mTemporaryStorageLimit) {
+      originInfos.TruncateLength(i);
+      break;
+    }
+
+    doomedUsage += originInfos[i]->LockedUsage();
+  }
+
+  return originInfos;
+}
+
+QuotaManager::OriginInfosFlatTraversable
+QuotaManager::GetOriginInfosExceedingLimits() const {
+  MutexAutoLock lock(mQuotaMutex);
+
+  auto originInfos = LockedGetOriginInfosExceedingGroupLimit();
+
+  const uint64_t doomedUsage =
+      std::accumulate(originInfos.cbegin(), originInfos.cend(), uint64_t(0),
+                      [](uint64_t oldValue, const auto& originInfo) {
+                        return oldValue + originInfo->LockedUsage();
+                      });
+
+  // Evicting origins that exceed their group limit also affects the global
+  // temporary storage usage. If the global temporary storage limit would still
+  // be exceeded after evicting the origins that were already selected, we need
+  // to specifically evict origins to get below the global limit.
+  if (mTemporaryStorageUsage - doomedUsage > mTemporaryStorageLimit) {
+    originInfos.AppendElements(
+        LockedGetOriginInfosExceedingGlobalLimit(originInfos, doomedUsage));
+  }
+
+  return originInfos;
+}
+
+void QuotaManager::ClearOrigins(
+    const OriginInfosFlatTraversable& aDoomedOriginInfos) {
+  AssertIsOnIOThread();
+
+  // XXX Does this need to be done a) in order and/or b) sequentially?
+  // XXX We don't need to concatenate the results of the two steps. It would be
+  // sufficient to chain the ranges for iteration.
+  for (const auto& doomedOriginInfo : aDoomedOriginInfos) {
 #ifdef DEBUG
-      {
-        MutexAutoLock lock(mQuotaMutex);
-        MOZ_ASSERT(!doomedOriginInfo->LockedPersisted());
-      }
-#endif
-
-      DeleteFilesForOrigin(doomedOriginInfo->mGroupInfo->mPersistenceType,
-                           doomedOriginInfo->mOrigin);
-    }
-
-    nsTArray<OriginParams> doomedOrigins;
     {
       MutexAutoLock lock(mQuotaMutex);
-
-      for (const auto& doomedOriginInfo : doomedOriginInfos) {
-        PersistenceType persistenceType =
-            doomedOriginInfo->mGroupInfo->mPersistenceType;
-        const GroupAndOrigin groupAndOrigin = {
-            doomedOriginInfo->mGroupInfo->mGroup, doomedOriginInfo->mOrigin};
-        LockedRemoveQuotaForOrigin(persistenceType, groupAndOrigin);
-
-        doomedOrigins.EmplaceBack(
-            OriginParams(persistenceType, groupAndOrigin.mOrigin));
-      }
-    }
-
-    return doomedOrigins;
-  }();
-
-  for (const OriginParams& doomedOrigin : doomedOrigins) {
-    OriginClearCompleted(doomedOrigin.mPersistenceType, doomedOrigin.mOrigin,
-                         Nullable<Client::Type>());
-  }
+      MOZ_ASSERT(!doomedOriginInfo->LockedPersisted());
+    }
+#endif
+
+    DeleteFilesForOrigin(doomedOriginInfo->mGroupInfo->mPersistenceType,
+                         doomedOriginInfo->mOrigin);
+  }
+
+  {
+    MutexAutoLock lock(mQuotaMutex);
+
+    for (const auto& doomedOriginInfo : aDoomedOriginInfos) {
+      LockedRemoveQuotaForOrigin(
+          doomedOriginInfo->mGroupInfo->mPersistenceType,
+          {doomedOriginInfo->mGroupInfo->mGroup, doomedOriginInfo->mOrigin});
+    }
+  }
+
+  for (const auto& doomedOriginInfo : aDoomedOriginInfos) {
+    OriginClearCompleted(doomedOriginInfo->mGroupInfo->mPersistenceType,
+                         doomedOriginInfo->mOrigin, Nullable<Client::Type>());
+  }
+}
+
+void QuotaManager::CleanupTemporaryStorage() {
+  AssertIsOnIOThread();
+
+  ClearOrigins(GetOriginInfosExceedingLimits());
 
   if (mTemporaryStorageUsage > mTemporaryStorageLimit) {
     // If disk space is still low after origin clear, notify storage pressure.
     NotifyStoragePressure(mTemporaryStorageUsage);
   }
 }
 
 void QuotaManager::DeleteFilesForOrigin(PersistenceType aPersistenceType,
--- a/dom/quota/QuotaManager.h
+++ b/dom/quota/QuotaManager.h
@@ -555,17 +555,30 @@ class QuotaManager final : public Backgr
 
   nsresult InitializeRepository(PersistenceType aPersistenceType);
 
   nsresult InitializeOrigin(PersistenceType aPersistenceType,
                             const GroupAndOrigin& aGroupAndOrigin,
                             int64_t aAccessTime, bool aPersisted,
                             nsIFile* aDirectory);
 
-  void CheckTemporaryStorageLimits();
+  using OriginInfosFlatTraversable =
+      nsTArray<NotNull<RefPtr<const OriginInfo>>>;
+
+  OriginInfosFlatTraversable LockedGetOriginInfosExceedingGroupLimit() const;
+
+  OriginInfosFlatTraversable LockedGetOriginInfosExceedingGlobalLimit(
+      const OriginInfosFlatTraversable& aAlreadyDoomedOriginInfos,
+      uint64_t aAlreadyDoomedUsage) const;
+
+  OriginInfosFlatTraversable GetOriginInfosExceedingLimits() const;
+
+  void ClearOrigins(const OriginInfosFlatTraversable& aDoomedOriginInfos);
+
+  void CleanupTemporaryStorage();
 
   void DeleteFilesForOrigin(PersistenceType aPersistenceType,
                             const nsACString& aOrigin);
 
   void FinalizeOriginEviction(nsTArray<RefPtr<DirectoryLockImpl>>&& aLocks);
 
   void ReleaseIOThreadObjects() {
     AssertIsOnIOThread();
@@ -594,17 +607,17 @@ class QuotaManager final : public Backgr
 
   EnumeratedArray<Client::Type, Client::TYPE_MAX, nsCString> mShutdownSteps;
   LazyInitializedOnce<const TimeStamp> mShutdownStartedAt;
   Atomic<bool> mShutdownStarted;
 
   // Accesses to mQuotaManagerShutdownSteps must be protected by mQuotaMutex.
   nsCString mQuotaManagerShutdownSteps;
 
-  mozilla::Mutex mQuotaMutex;
+  mutable mozilla::Mutex mQuotaMutex;
 
   nsClassHashtable<nsCStringHashKey, GroupInfoPair> mGroupInfoPairs;
 
   // Maintains a list of directory locks that are queued.
   nsTArray<RefPtr<DirectoryLockImpl>> mPendingDirectoryLocks;
 
   // Maintains a list of directory locks that are acquired or queued. It can be
   // accessed on the owning (PBackground) thread only.
--- a/mfbt/NotNull.h
+++ b/mfbt/NotNull.h
@@ -72,17 +72,20 @@
 namespace mozilla {
 
 namespace detail {
 template <typename T>
 struct CopyablePtr {
   T mPtr;
 
   template <typename U>
-  explicit CopyablePtr(U aPtr) : mPtr{std::move(aPtr)} {}
+  explicit CopyablePtr(U&& aPtr) : mPtr{std::forward<U>(aPtr)} {}
+
+  template <typename U>
+  explicit CopyablePtr(CopyablePtr<U> aPtr) : mPtr{std::move(aPtr.mPtr)} {}
 };
 }  // namespace detail
 
 template <typename T>
 class MovingNotNull;
 
 // NotNull can be used to wrap a "base" pointer (raw or smart) to indicate it
 // is not null. Some examples: