Bug 1906041 - Rename Ensure(Persistent|Temporary)OriginIsInitialized to Ensure(Persistent|Temporary)OriginIsInitializedInternal; r=dom-storage-reviewers,jari

One of the goals of the asynchronous temporary storage initialization is to call Ensure(Persistent|Temporary)OriginIsInitialized only from Initialize(Persistent|Temporary)OriginOp. Calling from other places including quota clients will be disallowed by changing the method to a private method. The private nature of the method should be emphasized by adding the Internal suffix. Differential Revision: https://phabricator.services.mozilla.com/D192150
2024-07-16 10:43:40 +00:00 · 2024-07-16 10:43:40 +00:00 · 0dd630db14
--- a/dom/cache/Context.cpp
+++ b/dom/cache/Context.cpp
@ -403,11 +403,11 @@ Context::QuotaInitRunnable::Run() {
        QM_TRY(MOZ_TO_RESULT(
            quotaManager->EnsureTemporaryStorageIsInitializedInternal()));

-        QM_TRY_UNWRAP(
-            mDirectoryMetadata->mDir,
-            quotaManager
-                ->EnsureTemporaryOriginIsInitialized(*mDirectoryMetadata)
-                .map([](const auto& res) { return res.first; }));
+        QM_TRY_UNWRAP(mDirectoryMetadata->mDir,
+                      quotaManager
+                          ->EnsureTemporaryOriginIsInitializedInternal(
+                              *mDirectoryMetadata)
+                          .map([](const auto& res) { return res.first; }));

        auto* cacheQuotaClient = CacheQuotaClient::Get();
        MOZ_DIAGNOSTIC_ASSERT(cacheQuotaClient);
--- a/dom/fs/parent/datamodel/FileSystemFileManager.cpp
+++ b/dom/fs/parent/datamodel/FileSystemFileManager.cpp
@ -175,7 +175,7 @@ nsresult EnsureFileSystemDirectory(

  QM_TRY_INSPECT(
      const auto& fileSystemDirectory,
-      quotaManager->EnsureTemporaryOriginIsInitialized(aOriginMetadata)
+      quotaManager->EnsureTemporaryOriginIsInitializedInternal(aOriginMetadata)
          .map([](const auto& aPair) { return aPair.first; }));

  QM_TRY(QM_TO_RESULT(fileSystemDirectory->AppendRelativePath(
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@ -13240,14 +13240,15 @@ nsresult Maintenance::DirectoryWork() {

                QM_TRY_UNWRAP(
                    const DebugOnly<bool> created,
-                    quotaManager->EnsurePersistentOriginIsInitialized(metadata)
+                    quotaManager
+                        ->EnsurePersistentOriginIsInitializedInternal(metadata)
                        .map([](const auto& res) { return res.second; }),
                    // Not much we can do here...
                    Ok{});

                // We found this origin directory by traversing the repository,
-                // so EnsurePersistentOriginIsInitialized shouldn't report that
-                // a new directory has been created.
+                // so EnsurePersistentOriginIsInitializedInternal shouldn't
+                // report that a new directory has been created.
                MOZ_ASSERT(!created);
              }

@ -15169,14 +15170,15 @@ nsresult OpenDatabaseOp::DoDatabaseWork() {
      ([persistenceType, &quotaManager, this]()
           -> mozilla::Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult> {
        if (persistenceType == PERSISTENCE_TYPE_PERSISTENT) {
-          QM_TRY_RETURN(quotaManager->EnsurePersistentOriginIsInitialized(
-              mOriginMetadata));
+          QM_TRY_RETURN(
+              quotaManager->EnsurePersistentOriginIsInitializedInternal(
+                  mOriginMetadata));
        }

        QM_TRY(MOZ_TO_RESULT(
            quotaManager->EnsureTemporaryStorageIsInitializedInternal()));
-        QM_TRY_RETURN(
-            quotaManager->EnsureTemporaryOriginIsInitialized(mOriginMetadata));
+        QM_TRY_RETURN(quotaManager->EnsureTemporaryOriginIsInitializedInternal(
+            mOriginMetadata));
      }()
                  .map([](const auto& res) { return res.first; })));

@ -16718,12 +16720,12 @@ nsresult GetDatabasesOp::DoDatabaseWork() {
  QM_TRY(([&quotaManager, this]()
              -> mozilla::Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult> {
    if (mPersistenceType == PERSISTENCE_TYPE_PERSISTENT) {
-      QM_TRY_RETURN(
-          quotaManager->EnsurePersistentOriginIsInitialized(mOriginMetadata));
+      QM_TRY_RETURN(quotaManager->EnsurePersistentOriginIsInitializedInternal(
+          mOriginMetadata));
    }

-    QM_TRY_RETURN(
-        quotaManager->EnsureTemporaryOriginIsInitialized(mOriginMetadata));
+    QM_TRY_RETURN(quotaManager->EnsureTemporaryOriginIsInitializedInternal(
+        mOriginMetadata));
  }()
                     .map([](const auto& res) { return Ok{}; })));

--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@ -1355,13 +1355,13 @@ class Connection final : public CachingDatabaseConnection {
 };

 /**
- * Helper to invoke EnsureTemporaryOriginIsInitialized on the QuotaManager IO
- * thread from the LocalStorage connection thread when creating a database
- * connection on demand. This is necessary because we attempt to defer the
- * creation of the origin directory and the database until absolutely needed,
- * but the directory creation and origin initialization must happen on the QM
- * IO thread for invariant reasons. (We can't just use a mutex because there
- * could be logic on the IO thread that also wants to deal with the same
+ * Helper to invoke EnsureTemporaryOriginIsInitializedInternal on the
+ * QuotaManager IO thread from the LocalStorage connection thread when creating
+ * a database connection on demand. This is necessary because we attempt to
+ * defer the creation of the origin directory and the database until absolutely
+ * needed, but the directory creation and origin initialization must happen on
+ * the QM IO thread for invariant reasons. (We can't just use a mutex because
+ * there could be logic on the IO thread that also wants to deal with the same
 * origin, so we need to queue a runnable and wait our turn.)
 */
 class Connection::InitTemporaryOriginHelper final : public Runnable {
@ -4204,7 +4204,7 @@ nsresult Connection::InitTemporaryOriginHelper::RunOnIOThread() {

  QM_TRY_INSPECT(
      const auto& directoryEntry,
-      quotaManager->EnsureTemporaryOriginIsInitialized(mOriginMetadata)
+      quotaManager->EnsureTemporaryOriginIsInitializedInternal(mOriginMetadata)
          .map([](const auto& res) { return res.first; }));

  QM_TRY(MOZ_TO_RESULT(directoryEntry->GetPath(mOriginDirectoryPath)));
@ -6942,10 +6942,10 @@ nsresult PrepareDatastoreOp::DatabaseWork() {
        ([hasDataForMigration, &quotaManager,
          this]() -> mozilla::Result<nsCOMPtr<nsIFile>, nsresult> {
          if (hasDataForMigration) {
-            QM_TRY_RETURN(
-                quotaManager
-                    ->EnsureTemporaryOriginIsInitialized(mOriginMetadata)
-                    .map([](const auto& res) { return res.first; }));
+            QM_TRY_RETURN(quotaManager
+                              ->EnsureTemporaryOriginIsInitializedInternal(
+                                  mOriginMetadata)
+                              .map([](const auto& res) { return res.first; }));
          }

          MOZ_ASSERT(mOriginMetadata.mPersistenceType ==
--- a/dom/quota/ActorsParent.cpp
+++ b/dom/quota/ActorsParent.cpp
@ -2788,12 +2788,13 @@ nsresult QuotaManager::LoadQuota() {
          Unused << lastAccessTimeUpdated;

          // We don't need to update the .metadata-v2 file on disk here,
-          // EnsureTemporaryOriginIsInitialized is responsible for doing that.
-          // We just need to use correct group and last access time before
-          // initializing quota for the given origin. (Note that calling
+          // EnsureTemporaryOriginIsInitializedInternal is responsible for
+          // doing that. We just need to use correct group and last access time
+          // before initializing quota for the given origin. (Note that calling
          // LoadFullOriginMetadataWithRestore below might update the group in
          // the metadata file, but only as a side-effect. The actual place we
-          // ensure consistency is in EnsureTemporaryOriginIsInitialized.)
+          // ensure consistency is in
+          // EnsureTemporaryOriginIsInitializedInternal.)

          if (accessed) {
            QM_TRY_INSPECT(const auto& directory,
@ -2811,7 +2812,8 @@ nsresult QuotaManager::LoadQuota() {

            // Calling LoadFullOriginMetadataWithRestore might update the group
            // in the metadata file, but only as a side-effect. The actual place
-            // we ensure consistency is in EnsureTemporaryOriginIsInitialized.
+            // we ensure consistency is in
+            // EnsureTemporaryOriginIsInitializedInternal.

            QM_TRY_INSPECT(const auto& metadata,
                           LoadFullOriginMetadataWithRestore(directory));
@ -5315,7 +5317,7 @@ RefPtr<UniversalDirectoryLock> QuotaManager::CreateDirectoryLockInternal(
 }

 Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult>
-QuotaManager::EnsurePersistentOriginIsInitialized(
+QuotaManager::EnsurePersistentOriginIsInitializedInternal(
    const OriginMetadata& aOriginMetadata) {
  AssertIsOnIOThread();
  MOZ_ASSERT(aOriginMetadata.mPersistenceType == PERSISTENCE_TYPE_PERSISTENT);
@ -5330,7 +5332,7 @@ QuotaManager::EnsurePersistentOriginIsInitialized(

    QM_TRY_UNWRAP(auto directory, GetOriginDirectory(aOriginMetadata));

-    if (mInitializedOrigins.Contains(aOriginMetadata.mOrigin)) {
+    if (mInitializedOriginsInternal.Contains(aOriginMetadata.mOrigin)) {
      MOZ_ASSERT(firstInitializationAttempt.Recorded());
      return std::pair(std::move(directory), false);
    }
@ -5365,7 +5367,7 @@ QuotaManager::EnsurePersistentOriginIsInitialized(
                                          aOriginMetadata, timestamp,
                                          /* aPersisted */ true, directory)));

-    mInitializedOrigins.AppendElement(aOriginMetadata.mOrigin);
+    mInitializedOriginsInternal.AppendElement(aOriginMetadata.mOrigin);

    return std::pair(std::move(directory), created);
  };
@ -5376,7 +5378,7 @@ QuotaManager::EnsurePersistentOriginIsInitialized(
      innerFunc);
 }

-bool QuotaManager::IsTemporaryOriginInitialized(
+bool QuotaManager::IsTemporaryOriginInitializedInternal(
    const OriginMetadata& aOriginMetadata) const {
  AssertIsOnIOThread();
  MOZ_ASSERT(aOriginMetadata.mPersistenceType != PERSISTENCE_TYPE_PERSISTENT);
@ -5390,7 +5392,7 @@ bool QuotaManager::IsTemporaryOriginInitialized(
 }

 Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult>
-QuotaManager::EnsureTemporaryOriginIsInitialized(
+QuotaManager::EnsureTemporaryOriginIsInitializedInternal(
    const OriginMetadata& aOriginMetadata) {
  AssertIsOnIOThread();
  MOZ_ASSERT(aOriginMetadata.mPersistenceType != PERSISTENCE_TYPE_PERSISTENT);
@ -5453,7 +5455,8 @@ QuotaManager::EnsurePersistentClientIsInitialized(
  MOZ_ASSERT(aClientMetadata.mPersistenceType == PERSISTENCE_TYPE_PERSISTENT);
  MOZ_ASSERT(Client::IsValidType(aClientMetadata.mClientType));
  MOZ_DIAGNOSTIC_ASSERT(IsStorageInitializedInternal());
-  MOZ_DIAGNOSTIC_ASSERT(IsOriginInitialized(aClientMetadata.mOrigin));
+  MOZ_DIAGNOSTIC_ASSERT(
+      IsPersistentOriginInitializedInternal(aClientMetadata.mOrigin));

  QM_TRY_UNWRAP(auto directory, GetOriginDirectory(aClientMetadata));

@ -5489,7 +5492,7 @@ QuotaManager::EnsureTemporaryClientIsInitialized(
  MOZ_ASSERT(Client::IsValidType(aClientMetadata.mClientType));
  MOZ_DIAGNOSTIC_ASSERT(IsStorageInitializedInternal());
  MOZ_DIAGNOSTIC_ASSERT(IsTemporaryStorageInitializedInternal());
-  MOZ_DIAGNOSTIC_ASSERT(IsTemporaryOriginInitialized(aClientMetadata));
+  MOZ_DIAGNOSTIC_ASSERT(IsTemporaryOriginInitializedInternal(aClientMetadata));

  QM_TRY_UNWRAP(auto directory, GetOriginDirectory(aClientMetadata));

@ -5729,7 +5732,7 @@ void QuotaManager::ShutdownStorageInternal() {

  if (mStorageConnection) {
    mInitializationInfo.ResetOriginInitializationInfos();
-    mInitializedOrigins.Clear();
+    mInitializedOriginsInternal.Clear();

    if (mTemporaryStorageInitializedInternal) {
      if (mCacheUsable) {
@ -5802,7 +5805,7 @@ void QuotaManager::OriginClearCompleted(

  if (aClientType.IsNull()) {
    if (aPersistenceType == PERSISTENCE_TYPE_PERSISTENT) {
-      mInitializedOrigins.RemoveElement(aOrigin);
+      mInitializedOriginsInternal.RemoveElement(aOrigin);
    }

    for (Client::Type type : AllClientTypes()) {
@ -5818,7 +5821,7 @@ void QuotaManager::RepositoryClearCompleted(PersistenceType aPersistenceType) {
  AssertIsOnIOThread();

  if (aPersistenceType == PERSISTENCE_TYPE_PERSISTENT) {
-    mInitializedOrigins.Clear();
+    mInitializedOriginsInternal.Clear();
  }

  for (Client::Type type : AllClientTypes()) {
--- a/dom/quota/OriginOperations.cpp
+++ b/dom/quota/OriginOperations.cpp
@ -1584,11 +1584,12 @@ nsresult InitializePersistentOriginOp::DoDirectoryWork(
  QM_TRY(OkIf(aQuotaManager.IsStorageInitializedInternal()),
         NS_ERROR_NOT_INITIALIZED);

-  QM_TRY_UNWRAP(mCreated,
-                (aQuotaManager
-                     .EnsurePersistentOriginIsInitialized(OriginMetadata{
-                         mPrincipalMetadata, PERSISTENCE_TYPE_PERSISTENT})
-                     .map([](const auto& res) { return res.second; })));
+  QM_TRY_UNWRAP(
+      mCreated,
+      (aQuotaManager
+           .EnsurePersistentOriginIsInitializedInternal(
+               OriginMetadata{mPrincipalMetadata, PERSISTENCE_TYPE_PERSISTENT})
+           .map([](const auto& res) { return res.second; })));

  return NS_OK;
 }
@ -1624,7 +1625,7 @@ nsresult InitializeTemporaryOriginOp::DoDirectoryWork(

  QM_TRY_UNWRAP(mCreated,
                (aQuotaManager
-                     .EnsureTemporaryOriginIsInitialized(
+                     .EnsureTemporaryOriginIsInitializedInternal(
                         OriginMetadata{mPrincipalMetadata, mPersistenceType})
                     .map([](const auto& res) { return res.second; })));

@ -1700,9 +1701,9 @@ nsresult InitializePersistentClientOp::DoDirectoryWork(
  QM_TRY(MOZ_TO_RESULT(aQuotaManager.IsStorageInitializedInternal()),
         NS_ERROR_FAILURE);

-  QM_TRY(
-      MOZ_TO_RESULT(aQuotaManager.IsOriginInitialized(mClientMetadata.mOrigin)),
-      NS_ERROR_FAILURE);
+  QM_TRY(MOZ_TO_RESULT(aQuotaManager.IsPersistentOriginInitializedInternal(
+             mClientMetadata.mOrigin)),
+         NS_ERROR_FAILURE);

  QM_TRY_UNWRAP(
      mCreated,
@ -1740,8 +1741,8 @@ nsresult InitializeTemporaryClientOp::DoDirectoryWork(
  QM_TRY(MOZ_TO_RESULT(aQuotaManager.IsTemporaryStorageInitializedInternal()),
         NS_ERROR_FAILURE);

-  QM_TRY(MOZ_TO_RESULT(
-             aQuotaManager.IsTemporaryOriginInitialized(mClientMetadata)),
+  QM_TRY(MOZ_TO_RESULT(aQuotaManager.IsTemporaryOriginInitializedInternal(
+             mClientMetadata)),
         NS_ERROR_FAILURE);

  QM_TRY_UNWRAP(
@ -2055,7 +2056,8 @@ void ClearRequestBase::DeleteFilesInternal(

            const bool initialized =
                aPersistenceType == PERSISTENCE_TYPE_PERSISTENT
-                    ? aQuotaManager.IsOriginInitialized(metadata.mOrigin)
+                    ? aQuotaManager.IsPersistentOriginInitializedInternal(
+                          metadata.mOrigin)
                    : aQuotaManager.IsTemporaryStorageInitializedInternal();

            // If it hasn't been initialized, we don't need to update the
--- a/dom/quota/QuotaManager.h
+++ b/dom/quota/QuotaManager.h
@ -141,10 +141,10 @@ class QuotaManager final : public BackgroundThreadObject {

  void UnregisterNormalOriginOp(NormalOriginOperationBase& aNormalOriginOp);

-  bool IsOriginInitialized(const nsACString& aOrigin) const {
+  bool IsPersistentOriginInitializedInternal(const nsACString& aOrigin) const {
    AssertIsOnIOThread();

-    return mInitializedOrigins.Contains(aOrigin);
+    return mInitializedOriginsInternal.Contains(aOrigin);
  }

  bool IsTemporaryStorageInitializedInternal() const {
@ -362,15 +362,17 @@ class QuotaManager final : public BackgroundThreadObject {
  // Returns a pair of an nsIFile object referring to the directory, and a bool
  // indicating whether the directory was newly created.
  Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult>
-  EnsurePersistentOriginIsInitialized(const OriginMetadata& aOriginMetadata);
+  EnsurePersistentOriginIsInitializedInternal(
+      const OriginMetadata& aOriginMetadata);

-  bool IsTemporaryOriginInitialized(
+  bool IsTemporaryOriginInitializedInternal(
      const OriginMetadata& aOriginMetadata) const;

  // Returns a pair of an nsIFile object referring to the directory, and a bool
  // indicating whether the directory was newly created.
  Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult>
-  EnsureTemporaryOriginIsInitialized(const OriginMetadata& aOriginMetadata);
+  EnsureTemporaryOriginIsInitializedInternal(
+      const OriginMetadata& aOriginMetadata);

  RefPtr<BoolPromise> InitializePersistentClient(
      const PrincipalInfo& aPrincipalInfo, Client::Type aClientType);
@ -770,7 +772,7 @@ class QuotaManager final : public BackgroundThreadObject {

  // A list of all successfully initialized persistent origins. This list isn't
  // protected by any mutex but it is only ever touched on the IO thread.
-  nsTArray<nsCString> mInitializedOrigins;
+  nsTArray<nsCString> mInitializedOriginsInternal;

  // A hash table that is used to cache origin parser results for given
  // sanitized origin strings. This hash table isn't protected by any mutex but
--- a/dom/quota/QuotaUsageRequestBase.cpp
+++ b/dom/quota/QuotaUsageRequestBase.cpp
@ -39,7 +39,8 @@ Result<UsageInfo, nsresult> QuotaUsageRequestBase::GetUsageForOrigin(
  bool initialized;

  if (aPersistenceType == PERSISTENCE_TYPE_PERSISTENT) {
-    initialized = aQuotaManager.IsOriginInitialized(aOriginMetadata.mOrigin);
+    initialized = aQuotaManager.IsPersistentOriginInitializedInternal(
+        aOriginMetadata.mOrigin);
  } else {
    initialized = aQuotaManager.IsTemporaryStorageInitializedInternal();
  }
--- a/dom/quota/test/gtest/TestFileOutputStream.cpp
+++ b/dom/quota/test/gtest/TestFileOutputStream.cpp
@ -61,8 +61,8 @@ TEST_F(TestFileOutputStream, extendFileStreamWithSetEOF) {
        ASSERT_NS_SUCCEEDED(
            quotaManager->EnsureTemporaryStorageIsInitializedInternal());

-        auto res =
-            quotaManager->EnsureTemporaryOriginIsInitialized(originMetadata);
+        auto res = quotaManager->EnsureTemporaryOriginIsInitializedInternal(
+            originMetadata);
        ASSERT_TRUE(res.isOk());
      }

--- a/dom/quota/test/gtest/TestStorageConnection.cpp
+++ b/dom/quota/test/gtest/TestStorageConnection.cpp
@ -38,7 +38,7 @@ void InitializeClientDirectory(const ClientMetadata& aClientMetadata) {

  QM_TRY_INSPECT(
      const auto& directory,
-      quotaManager->EnsureTemporaryOriginIsInitialized(aClientMetadata)
+      quotaManager->EnsureTemporaryOriginIsInitializedInternal(aClientMetadata)
          .map([](const auto& aPair) { return aPair.first; }),
      QM_TEST_FAIL);

--- a/dom/simpledb/ActorsParent.cpp
+++ b/dom/simpledb/ActorsParent.cpp
@ -1185,14 +1185,15 @@ nsresult OpenOp::DatabaseWork() {
        this]()
           -> mozilla::Result<std::pair<nsCOMPtr<nsIFile>, bool>, nsresult> {
        if (persistenceType == PERSISTENCE_TYPE_PERSISTENT) {
-          QM_TRY_RETURN(quotaManager->EnsurePersistentOriginIsInitialized(
-              mOriginMetadata));
+          QM_TRY_RETURN(
+              quotaManager->EnsurePersistentOriginIsInitializedInternal(
+                  mOriginMetadata));
        }

        QM_TRY(MOZ_TO_RESULT(
            quotaManager->EnsureTemporaryStorageIsInitializedInternal()));
-        QM_TRY_RETURN(
-            quotaManager->EnsureTemporaryOriginIsInitialized(mOriginMetadata));
+        QM_TRY_RETURN(quotaManager->EnsureTemporaryOriginIsInitializedInternal(
+            mOriginMetadata));
      }()
                  .map([](const auto& res) { return res.first; })));