Bug 1685677 - Extract functions from CheckTemporaryStorageLimits and rename it to CleanupTemporaryStorage. r=dom-workers-and-storage-reviewers,janv
☠☠ backed out by 49fff125656d ☠ ☠
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 02 Feb 2021 12:53:44 +0000
changeset 565610 bcbba85964cb5177fb754449fd83324f080b8154
parent 565609 8037fdb56c2f2d787d76872989786cc677918b60
child 565611 cc4f20677a7e28acc58e49bd212e4e09260018c3
push id135529
push usersgiesecke@mozilla.com
push dateTue, 02 Feb 2021 14:30:08 +0000
treeherderautoland@129b45a961fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-workers-and-storage-reviewers, janv
bugs1685677
milestone87.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 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
@@ -947,23 +947,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;
@@ -3778,18 +3778,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);
@@ -3827,17 +3827,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();
 
@@ -6544,17 +6544,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) {
@@ -7086,160 +7086,205 @@ 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,
+      MOZ_ASSERT(!doomedOriginInfo->LockedPersisted());
+    }
+#endif
+
+    DeleteFilesForOrigin(doomedOriginInfo->mGroupInfo->mPersistenceType,
+                         doomedOriginInfo->mOrigin);
+  }
+
+  struct OriginParams {
+    nsCString mOrigin;
+    PersistenceType mPersistenceType;
+  };
+
+  nsTArray<OriginParams> clearedOrigins;
+
+  {
+    MutexAutoLock lock(mQuotaMutex);
+
+    for (const auto& doomedOriginInfo : aDoomedOriginInfos) {
+      // LockedRemoveQuotaForOrigin might remove the group info;
+      // OriginInfo::mGroupInfo is only a raw pointer, so we need to store the
+      // information for calling OriginClearCompleted below in a separate array.
+      clearedOrigins.AppendElement(
+          OriginParams{doomedOriginInfo->mOrigin,
+                       doomedOriginInfo->mGroupInfo->mPersistenceType});
+
+      LockedRemoveQuotaForOrigin(
+          doomedOriginInfo->mGroupInfo->mPersistenceType,
+          {doomedOriginInfo->mGroupInfo->mGroup, doomedOriginInfo->mOrigin});
+    }
+  }
+
+  for (const auto& clearedOrigin : clearedOrigins) {
+    OriginClearCompleted(clearedOrigin.mPersistenceType, clearedOrigin.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
@@ -109,24 +109,16 @@ class NS_NO_VTABLE OpenDirectoryListener
   virtual void DirectoryLockAcquired(DirectoryLock* aLock) = 0;
 
   virtual void DirectoryLockFailed() = 0;
 
  protected:
   virtual ~OpenDirectoryListener() = default;
 };
 
-struct OriginParams {
-  OriginParams(PersistenceType aPersistenceType, const nsACString& aOrigin)
-      : mOrigin(aOrigin), mPersistenceType(aPersistenceType) {}
-
-  nsCString mOrigin;
-  PersistenceType mPersistenceType;
-};
-
 class QuotaManager final : public BackgroundThreadObject {
   friend class DirectoryLockImpl;
   friend class GroupInfo;
   friend class OriginInfo;
   friend class QuotaObject;
 
   typedef mozilla::ipc::PrincipalInfo PrincipalInfo;
   typedef nsClassHashtable<nsCStringHashKey,
@@ -561,17 +553,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();
@@ -600,17 +605,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: