Backed out changeset eaa1eb96adbe (bug 1714573) for causing marionette failures in test_refresh_firefox.py.

CLOSED TREE

toolkit/profile/nsProfileLock.cpp

@@ -399,29 +399,18 @@ nsresult nsProfileLock::GetReplacedLockTime(PRTime* aResult) {
   return NS_OK;
 }
 
-#if defined(XP_MACOSX)
-constexpr auto LOCKFILE_NAME = u".parentlock"_ns;
-constexpr auto OLD_LOCKFILE_NAME = u"parent.lock"_ns;
-#elif defined(XP_UNIX)
-constexpr auto OLD_LOCKFILE_NAME = u"lock"_ns;
-constexpr auto LOCKFILE_NAME = u".parentlock"_ns;
-#else
-constexpr auto LOCKFILE_NAME = u"parent.lock"_ns;
-#endif
-
-bool nsProfileLock::IsMaybeLockFile(nsIFile* aFile) {
-  nsAutoString tmp;
-  if (NS_SUCCEEDED(aFile->GetLeafName(tmp))) {
-    if (tmp.Equals(LOCKFILE_NAME)) return true;
-#if (defined(XP_MACOSX) || defined(XP_UNIX))
-    if (tmp.Equals(OLD_LOCKFILE_NAME)) return true;
-#endif
-  }
-  return false;
-}
-
 nsresult nsProfileLock::Lock(nsIFile* aProfileDir,
                              nsIProfileUnlocker** aUnlocker) {
+#if defined(XP_MACOSX)
+  constexpr auto LOCKFILE_NAME = u".parentlock"_ns;
+  constexpr auto OLD_LOCKFILE_NAME = u"parent.lock"_ns;
+#elif defined(XP_UNIX)
+  constexpr auto OLD_LOCKFILE_NAME = u"lock"_ns;
+  constexpr auto LOCKFILE_NAME = u".parentlock"_ns;
+#else
+  constexpr auto LOCKFILE_NAME = u"parent.lock"_ns;
+#endif
+
   nsresult rv;
   if (aUnlocker) *aUnlocker = nullptr;
 

toolkit/profile/nsProfileLock.h

@@ -50,12 +50,6 @@ class nsProfileLock
    */
   nsresult Unlock(bool aFatalSignal = false);
 
-  /**
-   * Checks, if the given file has a name that matches potential lock file
-   * names. It does not check, if it actually is the currently active lock.
-   */
-  static bool IsMaybeLockFile(nsIFile* aFile);
-
   /**
    * Clean up any left over files in the directory.
    */

toolkit/profile/nsToolkitProfileService.cpp

@@ -4,11 +4,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/ArrayUtils.h"
-#include "mozilla/ScopeExit.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/UniquePtrExtensions.h"
 #include "mozilla/WidgetUtils.h"
-#include "nsProfileLock.h"
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -88,98 +86,41 @@ nsTArray<UniquePtr<KeyValue>> GetSectionStrings(nsINIParser* aParser,
   return result;
 }
 
-void RemoveProfileRecursion(const nsCOMPtr<nsIFile>& aDirectoryOrFile,
-                            bool aIsRootDirectory,
-                            nsTArray<nsCOMPtr<nsIFile>>& aOutUndeletedFiles) {
-  auto guardDeletion = MakeScopeExit(
-      [&] { aOutUndeletedFiles.AppendElement(aDirectoryOrFile); });
-
-  // We actually would not expect to see links in our profiles, but still.
-  bool isLink = false;
-  NS_ENSURE_SUCCESS_VOID(aDirectoryOrFile->IsSymlink(&isLink));
-
-  // Only check to see if we have a directory if it isn't a link.
-  bool isDir = false;
-  if (!isLink) {
-    NS_ENSURE_SUCCESS_VOID(aDirectoryOrFile->IsDirectory(&isDir));
-  }
-
-  if (isDir) {
-    nsCOMPtr<nsIDirectoryEnumerator> dirEnum;
-    NS_ENSURE_SUCCESS_VOID(
-        aDirectoryOrFile->GetDirectoryEntries(getter_AddRefs(dirEnum)));
-
-    bool more = false;
-    while (NS_SUCCEEDED(dirEnum->HasMoreElements(&more)) && more) {
-      nsCOMPtr<nsISupports> item;
-      dirEnum->GetNext(getter_AddRefs(item));
-      nsCOMPtr<nsIFile> file = do_QueryInterface(item);
-      if (file) {
-        // Do not delete the profile lock.
-        if (aIsRootDirectory && nsProfileLock::IsMaybeLockFile(file)) continue;
-        // If some children's remove fails, we still continue the loop.
-        RemoveProfileRecursion(file, false, aOutUndeletedFiles);
-      }
-    }
-  }
-  // Do not delete the root directory (yet).
-  if (!aIsRootDirectory) {
-    NS_ENSURE_SUCCESS_VOID(aDirectoryOrFile->Remove(false));
-  }
-  guardDeletion.release();
-}
-
 void RemoveProfileFiles(nsIToolkitProfile* aProfile, bool aInBackground) {
   nsCOMPtr<nsIFile> rootDir;
   aProfile->GetRootDir(getter_AddRefs(rootDir));
   nsCOMPtr<nsIFile> localDir;
   aProfile->GetLocalDir(getter_AddRefs(localDir));
 
-  // XXX If we get here with an active quota manager,
-  // something went very wrong. We want to assert this.
-
   // Just lock the directories, don't mark the profile as locked or the lock
   // will attempt to release its reference to the profile on the background
   // thread which will assert.
   nsCOMPtr<nsIProfileLock> lock;
-  NS_ENSURE_SUCCESS_VOID(
-      NS_LockProfilePath(rootDir, localDir, nullptr, getter_AddRefs(lock)));
+  nsresult rv =
+      NS_LockProfilePath(rootDir, localDir, nullptr, getter_AddRefs(lock));
+  NS_ENSURE_SUCCESS_VOID(rv);
 
   nsCOMPtr<nsIRunnable> runnable = NS_NewRunnableFunction(
       "nsToolkitProfile::RemoveProfileFiles",
       [rootDir, localDir, lock]() mutable {
-        // We try to remove every single file and directory and collect
-        // those whose removal failed.
-        nsTArray<nsCOMPtr<nsIFile>> undeletedFiles;
-        // The root dir might contain the temp dir, so remove the temp dir
-        // first.
         bool equals;
         nsresult rv = rootDir->Equals(localDir, &equals);
+        // The root dir might contain the temp dir, so remove
+        // the temp dir first.
         if (NS_SUCCEEDED(rv) && !equals) {
-          RemoveProfileRecursion(localDir, /* aIsRootDirectory */ false,
-                                 undeletedFiles);
-        }
-        // Now remove the content of the profile dir (except lockfile)
-        RemoveProfileRecursion(rootDir, /* aIsRootDirectory */ true,
-                               undeletedFiles);
-        MOZ_ASSERT(undeletedFiles.Length() == 0);
-        if (undeletedFiles.Length() > 0) {
-          // XXX: We might want to do something with the files we could not
-          // remove, like a retry loop or a report.
-          // There is already an implicit retry below after unlocking.
-          NS_WARNING("Unable to remove all files from the profile directory.");
+          localDir->Remove(true);
         }
 
-        // Now we can unlock the profile safely.
+        // Ideally we'd unlock after deleting but since the lock is a file
+        // in the profile we must unlock before removing.
         lock->Unlock();
         // nsIProfileLock is not threadsafe so release our reference to it on
         // the main thread.
         NS_ReleaseOnMainThread("nsToolkitProfile::RemoveProfileFiles::Unlock",
                                lock.forget());
 
-        // And we can remove the (empty) remaining profile directory.
-        // We do this recursively in case there is some clutter left.
-        NS_ENSURE_SUCCESS_VOID(rootDir->Remove(true));
+        rv = rootDir->Remove(true);
+        NS_ENSURE_SUCCESS_VOID(rv);
       });
 
   if (aInBackground) {
@@ -409,9 +350,6 @@ nsToolkitProfileLock::Unlock() {
     return NS_ERROR_UNEXPECTED;
   }
 
-  // XXX If we get here with an active quota manager,
-  // something went very wrong. We want to assert this.
-
   mLock.Unlock();
 
   if (mProfile) {

toolkit/profile/test/test_create_profile.xhtml

@@ -106,8 +106,6 @@ function createProfile(profileName) {
 
   // Clean up the profile before local profile test.
   profile.remove(true);
-  ok(!jsonFile.exists(), "Times file was removed");
-  ok(!profileDir.exists(), "Profile dir was removed");
 
   // Create with non-null aRootDir
   profile = gProfileService.createProfile(profileDir, profileName);
@@ -118,7 +116,6 @@ function createProfile(profileName) {
 
   // Clean up the profile.
   profile.remove(true);
-  ok(!profileDir.exists(), "Profile dir was removed");
 }
 
   ]]>