Bug 1695906 - Use NotNull for StorageOperationsBase::OriginProps::mDirectory; r=dom-storage-reviewers,sg
☠☠ backed out by 7adfa8f8a78c ☠ ☠
authorJan Varga <jvarga@mozilla.com>
Tue, 09 Mar 2021 19:01:59 +0000
changeset 570356 f483f50d292a1da61bc1294bb832522b1aa65b85
parent 570355 470d617d76739c4d09327b034e582fa7ec445671
child 570357 9be2fd0b5b6a4ece2514a70d1c8b3bf3ca8b9ded
push id138061
push userjvarga@mozilla.com
push dateTue, 09 Mar 2021 19:04:34 +0000
treeherderautoland@43a39c491548 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-storage-reviewers, sg
bugs1695906
milestone88.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 1695906 - Use NotNull for StorageOperationsBase::OriginProps::mDirectory; r=dom-storage-reviewers,sg Differential Revision: https://phabricator.services.mozilla.com/D106908
dom/quota/ActorsParent.cpp
--- a/dom/quota/ActorsParent.cpp
+++ b/dom/quota/ActorsParent.cpp
@@ -1968,43 +1968,42 @@ void UnregisterNormalOriginOp(NormalOrig
   }
 }
 
 class StorageOperationBase {
  protected:
   struct OriginProps {
     enum Type { eChrome, eContent, eObsolete, eInvalid };
 
-    // XXX Change to NotNull<nsCOMPtr<nsIFile>> mDirectory;
-    nsCOMPtr<nsIFile> mDirectory;
+    NotNull<nsCOMPtr<nsIFile>> mDirectory;
     nsString mLeafName;
     nsCString mSpec;
     OriginAttributes mAttrs;
     int64_t mTimestamp;
     OriginMetadata mOriginMetadata;
     nsCString mOriginalSuffix;
 
     LazyInitializedOnceEarlyDestructible<const PersistenceType>
         mPersistenceType;
     Type mType;
     bool mNeedsRestore;
     bool mNeedsRestore2;
     bool mIgnore;
 
    public:
-    explicit OriginProps()
-        : mTimestamp(0),
+    explicit OriginProps(MovingNotNull<nsCOMPtr<nsIFile>> aDirectory)
+        : mDirectory(std::move(aDirectory)),
+          mTimestamp(0),
           mType(eContent),
           mNeedsRestore(false),
           mNeedsRestore2(false),
           mIgnore(false) {}
 
     template <typename PersistenceTypeFunc>
-    nsresult Init(nsIFile* aDirectory,
-                  PersistenceTypeFunc&& aPersistenceTypeFunc);
+    nsresult Init(PersistenceTypeFunc&& aPersistenceTypeFunc);
   };
 
   nsTArray<OriginProps> mOriginProps;
 
   nsCOMPtr<nsIFile> mDirectory;
 
  public:
   explicit StorageOperationBase(nsIFile* aDirectory) : mDirectory(aDirectory) {
@@ -9717,32 +9716,30 @@ int64_t StorageOperationBase::GetOriginL
     const OriginProps& aOriginProps) {
   return GetLastModifiedTime(*aOriginProps.mPersistenceType,
                              *aOriginProps.mDirectory);
 }
 
 nsresult StorageOperationBase::RemoveObsoleteOrigin(
     const OriginProps& aOriginProps) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
 
   QM_WARNING(
       "Deleting obsolete %s directory that is no longer a legal "
       "origin!",
       NS_ConvertUTF16toUTF8(aOriginProps.mLeafName).get());
 
   QM_TRY(aOriginProps.mDirectory->Remove(/* recursive */ true));
 
   return NS_OK;
 }
 
 Result<bool, nsresult> StorageOperationBase::MaybeRenameOrigin(
     const OriginProps& aOriginProps) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
 
   const nsAString& oldLeafName = aOriginProps.mLeafName;
 
   const auto newLeafName =
       MakeSanitizedOriginString(aOriginProps.mOriginMetadata.mOrigin);
 
   if (oldLeafName == newLeafName) {
     return false;
@@ -9753,17 +9750,17 @@ Result<bool, nsresult> StorageOperationB
                                  aOriginProps.mOriginMetadata));
 
   QM_TRY(CreateDirectoryMetadata2(
       *aOriginProps.mDirectory, aOriginProps.mTimestamp,
       /* aPersisted */ false, aOriginProps.mOriginMetadata));
 
   QM_TRY_INSPECT(const auto& newFile,
                  MOZ_TO_RESULT_INVOKE_TYPED(
-                     nsCOMPtr<nsIFile>, aOriginProps.mDirectory, GetParent));
+                     nsCOMPtr<nsIFile>, *aOriginProps.mDirectory, GetParent));
 
   QM_TRY(newFile->Append(newLeafName));
 
   QM_TRY_INSPECT(const bool& exists, MOZ_TO_RESULT_INVOKE(newFile, Exists));
 
   if (exists) {
     QM_WARNING(
         "Can't rename %s directory to %s, the target already exists, removing "
@@ -9884,23 +9881,22 @@ nsresult StorageOperationBase::ProcessOr
   return NS_OK;
 }
 
 // XXX Do the fallible initialization in a separate non-static member function
 // of StorageOperationBase and eventually get rid of this method and use a
 // normal constructor instead.
 template <typename PersistenceTypeFunc>
 nsresult StorageOperationBase::OriginProps::Init(
-    nsIFile* aDirectory, PersistenceTypeFunc&& aPersistenceTypeFunc) {
-  AssertIsOnIOThread();
-  MOZ_ASSERT(aDirectory);
+    PersistenceTypeFunc&& aPersistenceTypeFunc) {
+  AssertIsOnIOThread();
 
   QM_TRY_INSPECT(
       const auto& leafName,
-      MOZ_TO_RESULT_INVOKE_TYPED(nsAutoString, aDirectory, GetLeafName));
+      MOZ_TO_RESULT_INVOKE_TYPED(nsAutoString, *mDirectory, GetLeafName));
 
   nsCString spec;
   OriginAttributes attrs;
   nsCString originalSuffix;
   OriginParser::ResultType result = OriginParser::ParseOrigin(
       NS_ConvertUTF16toUTF8(leafName), spec, &attrs, originalSuffix);
   if (NS_WARN_IF(result == OriginParser::InvalidOrigin)) {
     mType = OriginProps::eInvalid;
@@ -9914,17 +9910,16 @@ nsresult StorageOperationBase::OriginPro
     // the errors. Until it's fixed, we have to treat obsolete origins as
     // origins with unknown/invalid persistence type.
     if (result != OriginParser::ValidOrigin) {
       return PERSISTENCE_TYPE_INVALID;
     }
     return std::forward<PersistenceTypeFunc>(aPersistenceTypeFunc)(spec);
   }();
 
-  mDirectory = aDirectory;
   mLeafName = leafName;
   mSpec = spec;
   mAttrs = attrs;
   mOriginalSuffix = originalSuffix;
   mPersistenceType.init(persistenceType);
   if (result == OriginParser::ObsoleteOrigin) {
     mType = eObsolete;
   } else if (mSpec.EqualsLiteral(kChromeOrigin)) {
@@ -10419,18 +10414,18 @@ nsresult RepositoryOperationBase::Proces
         // them.
         if (!IsOSMetadata(leafName)) {
           UNKNOWN_FILE_WARNING(leafName);
         }
 
         return mozilla::Ok{};
       },
       [&self = *this](const auto& originDir) -> Result<mozilla::Ok, nsresult> {
-        OriginProps originProps;
-        QM_TRY(originProps.Init(originDir, [&self](const auto& aSpec) {
+        OriginProps originProps(WrapMovingNotNullUnchecked(originDir));
+        QM_TRY(originProps.Init([&self](const auto& aSpec) {
           return self.PersistenceTypeFromSpec(aSpec);
         }));
         // Bypass invalid origins while upgrading
         QM_TRY(OkIf(originProps.mType != OriginProps::eInvalid), mozilla::Ok{});
 
         if (originProps.mType != OriginProps::eObsolete) {
           QM_TRY_INSPECT(
               const bool& removed,
@@ -10453,17 +10448,16 @@ nsresult RepositoryOperationBase::Proces
 
   return NS_OK;
 }
 
 template <typename UpgradeMethod>
 nsresult RepositoryOperationBase::MaybeUpgradeClients(
     const OriginProps& aOriginProps, UpgradeMethod aMethod) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
   MOZ_ASSERT(aMethod);
 
   QuotaManager* quotaManager = QuotaManager::Get();
   MOZ_ASSERT(quotaManager);
 
   QM_TRY(CollectEachFileEntry(
       *aOriginProps.mDirectory,
       [](const auto& file) -> Result<mozilla::Ok, nsresult> {
@@ -10625,29 +10619,28 @@ nsresult CreateOrUpgradeDirectoryMetadat
   }
 
   return NS_OK;
 }
 
 nsresult CreateOrUpgradeDirectoryMetadataHelper::PrepareOriginDirectory(
     OriginProps& aOriginProps, bool* aRemoved) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
   MOZ_ASSERT(aRemoved);
 
   if (*mLegacyPersistenceType == LegacyPersistenceType::Persistent) {
-    QM_TRY(MaybeUpgradeOriginDirectory(aOriginProps.mDirectory));
+    QM_TRY(MaybeUpgradeOriginDirectory(aOriginProps.mDirectory.get()));
 
     aOriginProps.mTimestamp = GetOriginLastModifiedTime(aOriginProps);
   } else {
     int64_t timestamp;
     nsCString group;
     nsCString origin;
     Nullable<bool> isApp;
-    nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory, timestamp,
+    nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory.get(), timestamp,
                                        group, origin, isApp);
     if (NS_FAILED(rv)) {
       aOriginProps.mTimestamp = GetOriginLastModifiedTime(aOriginProps);
       aOriginProps.mNeedsRestore = true;
     } else if (!isApp.IsNull()) {
       aOriginProps.mIgnore = true;
     }
   }
@@ -10735,25 +10728,24 @@ PersistenceType UpgradeStorageHelperBase
   // There's no moving of origin directories between repositories like in the
   // CreateOrUpgradeDirectoryMetadataHelper
   return *mPersistenceType;
 }
 
 nsresult UpgradeStorageFrom0_0To1_0Helper::PrepareOriginDirectory(
     OriginProps& aOriginProps, bool* aRemoved) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
   MOZ_ASSERT(aRemoved);
 
   int64_t timestamp;
   nsCString group;
   nsCString origin;
   Nullable<bool> isApp;
-  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory, timestamp, group,
-                                     origin, isApp);
+  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory.get(), timestamp,
+                                     group, origin, isApp);
   if (NS_FAILED(rv) || isApp.IsNull()) {
     aOriginProps.mTimestamp = GetOriginLastModifiedTime(aOriginProps);
     aOriginProps.mNeedsRestore = true;
   } else {
     aOriginProps.mTimestamp = timestamp;
   }
 
   *aRemoved = false;
@@ -10783,26 +10775,25 @@ nsresult UpgradeStorageFrom0_0To1_0Helpe
       /* aPersisted */ false, aOriginProps.mOriginMetadata));
 
   return NS_OK;
 }
 
 nsresult UpgradeStorageFrom1_0To2_0Helper::MaybeRemoveMorgueDirectory(
     const OriginProps& aOriginProps) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
 
   // The Cache API was creating top level morgue directories by accident for
   // a short time in nightly.  This unfortunately prevents all storage from
   // working.  So recover these profiles permanently by removing these corrupt
   // directories as part of this upgrade.
 
   QM_TRY_INSPECT(const auto& morgueDir,
                  MOZ_TO_RESULT_INVOKE_TYPED(nsCOMPtr<nsIFile>,
-                                            aOriginProps.mDirectory, Clone));
+                                            *aOriginProps.mDirectory, Clone));
 
   QM_TRY(morgueDir->Append(u"morgue"_ns));
 
   QM_TRY_INSPECT(const bool& exists, MOZ_TO_RESULT_INVOKE(morgueDir, Exists));
 
   if (exists) {
     QM_WARNING("Deleting accidental morgue directory!");
 
@@ -10843,17 +10834,16 @@ Result<bool, nsresult> UpgradeStorageFro
   }
 
   return false;
 }
 
 nsresult UpgradeStorageFrom1_0To2_0Helper::PrepareOriginDirectory(
     OriginProps& aOriginProps, bool* aRemoved) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
   MOZ_ASSERT(aRemoved);
 
   QM_TRY(MaybeRemoveMorgueDirectory(aOriginProps));
 
   QM_TRY(
       MaybeUpgradeClients(aOriginProps, &Client::UpgradeStorageFrom1_0To2_0));
 
   QM_TRY_INSPECT(const bool& removed, MaybeRemoveAppsData(aOriginProps));
@@ -10861,25 +10851,25 @@ nsresult UpgradeStorageFrom1_0To2_0Helpe
     *aRemoved = true;
     return NS_OK;
   }
 
   int64_t timestamp;
   nsCString group;
   nsCString origin;
   Nullable<bool> isApp;
-  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory, timestamp, group,
-                                     origin, isApp);
+  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory.get(), timestamp,
+                                     group, origin, isApp);
   if (NS_FAILED(rv) || isApp.IsNull()) {
     aOriginProps.mNeedsRestore = true;
   }
 
   nsCString suffix;
-  rv = GetDirectoryMetadata2(aOriginProps.mDirectory, timestamp, suffix, group,
-                             origin, isApp.SetValue());
+  rv = GetDirectoryMetadata2(aOriginProps.mDirectory.get(), timestamp, suffix,
+                             group, origin, isApp.SetValue());
   if (NS_FAILED(rv)) {
     aOriginProps.mTimestamp = GetOriginLastModifiedTime(aOriginProps);
     aOriginProps.mNeedsRestore2 = true;
   } else {
     aOriginProps.mTimestamp = timestamp;
   }
 
   *aRemoved = false;
@@ -10910,35 +10900,34 @@ nsresult UpgradeStorageFrom1_0To2_0Helpe
   }
 
   return NS_OK;
 }
 
 nsresult UpgradeStorageFrom2_0To2_1Helper::PrepareOriginDirectory(
     OriginProps& aOriginProps, bool* aRemoved) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
   MOZ_ASSERT(aRemoved);
 
   QM_TRY(
       MaybeUpgradeClients(aOriginProps, &Client::UpgradeStorageFrom2_0To2_1));
 
   int64_t timestamp;
   nsCString group;
   nsCString origin;
   Nullable<bool> isApp;
-  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory, timestamp, group,
-                                     origin, isApp);
+  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory.get(), timestamp,
+                                     group, origin, isApp);
   if (NS_FAILED(rv) || isApp.IsNull()) {
     aOriginProps.mNeedsRestore = true;
   }
 
   nsCString suffix;
-  rv = GetDirectoryMetadata2(aOriginProps.mDirectory, timestamp, suffix, group,
-                             origin, isApp.SetValue());
+  rv = GetDirectoryMetadata2(aOriginProps.mDirectory.get(), timestamp, suffix,
+                             group, origin, isApp.SetValue());
   if (NS_FAILED(rv)) {
     aOriginProps.mTimestamp = GetOriginLastModifiedTime(aOriginProps);
     aOriginProps.mNeedsRestore2 = true;
   } else {
     aOriginProps.mTimestamp = timestamp;
   }
 
   *aRemoved = false;
@@ -10962,35 +10951,34 @@ nsresult UpgradeStorageFrom2_0To2_1Helpe
   }
 
   return NS_OK;
 }
 
 nsresult UpgradeStorageFrom2_1To2_2Helper::PrepareOriginDirectory(
     OriginProps& aOriginProps, bool* aRemoved) {
   AssertIsOnIOThread();
-  MOZ_ASSERT(aOriginProps.mDirectory);
   MOZ_ASSERT(aRemoved);
 
   QM_TRY(
       MaybeUpgradeClients(aOriginProps, &Client::UpgradeStorageFrom2_1To2_2));
 
   int64_t timestamp;
   nsCString group;
   nsCString origin;
   Nullable<bool> isApp;
-  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory, timestamp, group,
-                                     origin, isApp);
+  nsresult rv = GetDirectoryMetadata(aOriginProps.mDirectory.get(), timestamp,
+                                     group, origin, isApp);
   if (NS_FAILED(rv) || isApp.IsNull()) {
     aOriginProps.mNeedsRestore = true;
   }
 
   nsCString suffix;
-  rv = GetDirectoryMetadata2(aOriginProps.mDirectory, timestamp, suffix, group,
-                             origin, isApp.SetValue());
+  rv = GetDirectoryMetadata2(aOriginProps.mDirectory.get(), timestamp, suffix,
+                             group, origin, isApp.SetValue());
   if (NS_FAILED(rv)) {
     aOriginProps.mTimestamp = GetOriginLastModifiedTime(aOriginProps);
     aOriginProps.mNeedsRestore2 = true;
   } else {
     aOriginProps.mTimestamp = timestamp;
   }
 
   *aRemoved = false;
@@ -11046,20 +11034,19 @@ nsresult RestoreDirectoryMetadata2Helper
   QM_TRY(OkIf(maybePersistenceType.isSome()), Err(NS_ERROR_FAILURE));
 
   mPersistenceType.init(maybePersistenceType.value());
 
   return NS_OK;
 }
 
 nsresult RestoreDirectoryMetadata2Helper::RestoreMetadata2File() {
-  OriginProps originProps;
-  QM_TRY(originProps.Init(mDirectory, [&self = *this](const auto& aSpec) {
-    return *self.mPersistenceType;
-  }));
+  OriginProps originProps(WrapMovingNotNull(mDirectory));
+  QM_TRY(originProps.Init(
+      [&self = *this](const auto& aSpec) { return *self.mPersistenceType; }));
 
   QM_TRY(OkIf(originProps.mType != OriginProps::eInvalid), NS_ERROR_FAILURE);
 
   originProps.mTimestamp = GetOriginLastModifiedTime(originProps);
 
   mOriginProps.AppendElement(std::move(originProps));
 
   QM_TRY(ProcessOriginDirectories());