Backed out changeset fe441360b591 (bug 1915216) for causing leaks at ThreadSafeWeakReference. CLOSED TREE

browser/components/backup/BackupService.sys.mjs

@@ -2698,7 +2698,7 @@ export class BackupService extends EventTarget {
       );
       await IOUtils.writeJSON(postRecoveryPath, postRecovery);
 
-      await profileSvc.asyncFlush();
+      profileSvc.flush();
 
       if (shouldLaunch) {
         Services.startup.createInstanceWithProfile(profile);

browser/components/backup/tests/marionette/test_backup.py

@@ -279,15 +279,15 @@ class BackupTest(MarionetteTestCase):
         self.marionette.instance.profile = originalProfile
         self.marionette.start_session()
         self.marionette.set_context("chrome")
-        self.marionette.execute_async_script(
+        self.marionette.execute_script(
             """
-          let [newProfileName, outerResolve] = arguments;
+          let newProfileName = arguments[0];
           let profileSvc = Cc["@mozilla.org/toolkit/profile-service;1"].getService(
             Ci.nsIToolkitProfileService
           );
           let profile = profileSvc.getProfileByName(newProfileName);
           profile.remove(true);
-          profileSvc.asyncFlush().then(outerResolve);
+          profileSvc.flush();
         """,
             script_args=[newProfileName],
         )

browser/components/profiles/SelectableProfileService.sys.mjs

@@ -32,14 +32,6 @@ XPCOMUtils.defineLazyServiceGetter(
 
 const PROFILES_CRYPTO_SALT_LENGTH_BYTES = 16;
 
-async function attemptFlush() {
-  try {
-    await lazy.ProfileService.asyncFlush();
-  } catch (e) {
-    await lazy.ProfileService.asyncFlushCurrentProfile();
-  }
-}
-
 /**
  * The service that manages selectable profiles
  */
@@ -108,7 +100,7 @@ class SelectableProfileServiceClass {
       .replace("{", "")
       .split("-")[0];
     this.#groupToolkitProfile.storeID = storageID;
-    await attemptFlush();
+    lazy.ProfileService.flush();
   }
 
   async getProfilesStorePath() {
@@ -325,7 +317,7 @@ class SelectableProfileServiceClass {
     }
 
     this.#groupToolkitProfile.storeID = null;
-    await attemptFlush();
+    lazy.ProfileService.flush();
     await this.vacuumAndCloseGroupDB();
   }
 
@@ -394,7 +386,7 @@ class SelectableProfileServiceClass {
       return;
     }
     this.#groupToolkitProfile.rootDir = await this.currentProfile.rootDir;
-    await attemptFlush();
+    lazy.ProfileService.flush();
   }
 
   /**
@@ -403,13 +395,13 @@ class SelectableProfileServiceClass {
    *
    * @param {boolean} shouldShow Whether or not we should show the profile selector
    */
-  async showProfileSelectorWindow(shouldShow) {
+  showProfileSelectorWindow(shouldShow) {
     if (shouldShow === this.groupToolkitProfile.showProfileSelector) {
       return;
     }
 
     this.groupToolkitProfile.showProfileSelector = shouldShow;
-    await attemptFlush();
+    lazy.ProfileService.flush();
   }
 
   /**

toolkit/components/remote/nsRemoteService.cpp

@@ -5,7 +5,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "nsRemoteClient.h"
 #ifdef MOZ_WIDGET_GTK
 #  ifdef MOZ_ENABLE_DBUS
 #    include "nsDBusRemoteServer.h"
@@ -27,44 +26,26 @@
 #include "nsString.h"
 #include "nsServiceManagerUtils.h"
 #include "SpecialSystemDirectory.h"
-#include "mozilla/StaticPtr.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/UniquePtr.h"
 
-// Time to wait for the startup lock
+// Time to wait for the remoting service to start
-#define START_TIMEOUT_MSEC 5000
+#define START_TIMEOUT_SEC 5
 #define START_SLEEP_MSEC 100
 
+using namespace mozilla;
+
 extern int gArgc;
 extern char** gArgv;
 
 using namespace mozilla;
 
-nsStartupLock::nsStartupLock(nsIFile* aDir, nsProfileLock& aLock) : mDir(aDir) {
-  mLock = aLock;
-}
-
-nsStartupLock::~nsStartupLock() {
-  mLock.Unlock();
-  mLock.Cleanup();
-
-  mDir->Remove(false);
-}
-
-ThreadSafeWeakPtr<nsStartupLock> nsRemoteService::gStartupLock;
-StaticRefPtr<nsRemoteService::StartupLockPromise>
-    nsRemoteService::gStartupLockPromise;
-
 NS_IMPL_ISUPPORTS(nsRemoteService, nsIObserver, nsIRemoteService)
 
-nsRemoteService::nsRemoteService() : mProgram("mozilla") {
+nsRemoteService::nsRemoteService(const char* aProgram) : mProgram(aProgram) {
   ToLowerCase(mProgram);
 }
 
-void nsRemoteService::SetProgram(const char* aProgram) {
-  mProgram = aProgram;
-  ToLowerCase(mProgram);
-}
 void nsRemoteService::SetProfile(nsACString& aProfile) { mProfile = aProfile; }
 
 #ifdef MOZ_WIDGET_GTK
@@ -73,120 +54,48 @@ void nsRemoteService::SetStartupToken(nsACString& aStartupToken) {
 }
 #endif
 
-// Attempts to lock the given directory by polling until the timeout is reached.
+void nsRemoteService::LockStartup() {
-static nsresult AcquireLock(nsIFile* aMutexDir, double aTimeout,
+  nsCOMPtr<nsIFile> mutexDir;
-                            nsProfileLock& aProfileLock) {
+  nsresult rv = GetSpecialSystemDirectory(OS_TemporaryDirectory,
+                                          getter_AddRefs(mutexDir));
+  NS_ENSURE_SUCCESS_VOID(rv);
+  rv = mutexDir->AppendNative(mProgram);
+  NS_ENSURE_SUCCESS_VOID(rv);
+
   const mozilla::TimeStamp epoch = mozilla::TimeStamp::Now();
   do {
     // If we have been waiting for another instance to release the lock it will
-    // have deleted the lock directory when doing so so we have to make sure it
+    // have deleted the lock directory when doing so we have to make sure it
     // exists every time we poll for the lock.
-    nsresult rv = aMutexDir->Create(nsIFile::DIRECTORY_TYPE, 0700);
+    rv = mutexDir->Create(nsIFile::DIRECTORY_TYPE, 0700);
-    if (NS_FAILED(rv) && rv != NS_ERROR_FILE_ALREADY_EXISTS) {
+    if (NS_SUCCEEDED(rv) || rv == NS_ERROR_FILE_ALREADY_EXISTS) {
+      mRemoteLockDir = mutexDir;
+    } else {
       NS_WARNING("Unable to create startup lock directory.");
-      return rv;
+      return;
     }
 
-    rv = aProfileLock.Lock(aMutexDir, nullptr);
+    rv = mRemoteLock.Lock(mRemoteLockDir, nullptr);
     if (NS_SUCCEEDED(rv)) {
-      return NS_OK;
+      return;
     }
 
+    mRemoteLockDir = nullptr;
     PR_Sleep(START_SLEEP_MSEC);
   } while ((mozilla::TimeStamp::Now() - epoch) <
-           mozilla::TimeDuration::FromMilliseconds(aTimeout));
+           mozilla::TimeDuration::FromSeconds(START_TIMEOUT_SEC));
 
-  return NS_ERROR_FAILURE;
-}
-
-RefPtr<nsRemoteService::StartupLockPromise> nsRemoteService::AsyncLockStartup(
-    double aTimeout) {
-  // If startup is already locked we can just resolve immediately.
-  RefPtr<nsStartupLock> lock(gStartupLock);
-  if (lock) {
-    return StartupLockPromise::CreateAndResolve(lock, __func__);
-  }
-
-  // If there is already an executing promise we can just return that otherwise
-  // we have to start a new one.
-  if (gStartupLockPromise) {
-    return gStartupLockPromise;
-  }
-
-  nsCOMPtr<nsIFile> mutexDir;
-  nsresult rv = GetSpecialSystemDirectory(OS_TemporaryDirectory,
-                                          getter_AddRefs(mutexDir));
-  if (NS_FAILED(rv)) {
-    return StartupLockPromise::CreateAndReject(rv, __func__);
-  }
-
-  rv = mutexDir->AppendNative(mProgram);
-  if (NS_FAILED(rv)) {
-    return StartupLockPromise::CreateAndReject(rv, __func__);
-  }
-
-  nsCOMPtr<nsISerialEventTarget> queue;
-  MOZ_ALWAYS_SUCCEEDS(NS_CreateBackgroundTaskQueue("StartupLockTaskQueue",
-                                                   getter_AddRefs(queue)));
-
-  gStartupLockPromise = InvokeAsync(
-      queue, __func__, [mutexDir = std::move(mutexDir), aTimeout]() {
-        nsProfileLock lock;
-        nsresult rv = AcquireLock(mutexDir, aTimeout, lock);
-        if (NS_SUCCEEDED(rv)) {
-          return StartupLockPromise::CreateAndResolve(
-              new nsStartupLock(mutexDir, lock), __func__);
-        }
-
-        return StartupLockPromise::CreateAndReject(rv, __func__);
-      });
-
-  // Note this is the guaranteed first `Then` and will run before any other
-  // `Then`s.
-  gStartupLockPromise->Then(
-      GetCurrentSerialEventTarget(), __func__,
-      [](const StartupLockPromise::ResolveOrRejectValue& aResult) {
-        if (aResult.IsResolve()) {
-          gStartupLock = aResult.ResolveValue();
-        }
-        gStartupLockPromise = nullptr;
-      });
-
-  return gStartupLockPromise;
-}
-
-already_AddRefed<nsStartupLock> nsRemoteService::LockStartup() {
-  MOZ_RELEASE_ASSERT(!gStartupLockPromise,
-                     "Should not have started an asynchronous lock attempt");
-
-  // If we're already locked just return the current lock.
-  RefPtr<nsStartupLock> lock(gStartupLock);
-  if (lock) {
-    return lock.forget();
-  }
-
-  nsCOMPtr<nsIFile> mutexDir;
-  nsresult rv = GetSpecialSystemDirectory(OS_TemporaryDirectory,
-                                          getter_AddRefs(mutexDir));
-  if (NS_FAILED(rv)) {
-    return nullptr;
-  }
-
-  rv = mutexDir->AppendNative(mProgram);
-  if (NS_FAILED(rv)) {
-    return nullptr;
-  }
-
-  nsProfileLock profileLock;
-  rv = AcquireLock(mutexDir, START_TIMEOUT_MSEC, profileLock);
-
-  if (NS_FAILED(rv)) {
   NS_WARNING("Failed to lock for startup, continuing anyway.");
-    return nullptr;
+}
-  }
 
-  gStartupLock = new nsStartupLock(mutexDir, profileLock);
+void nsRemoteService::UnlockStartup() {
-  return do_AddRef(gStartupLock);
+  if (mRemoteLockDir) {
+    mRemoteLock.Unlock();
+    mRemoteLock.Cleanup();
+
+    mRemoteLockDir->Remove(false);
+    mRemoteLockDir = nullptr;
+  }
 }
 
 nsresult nsRemoteService::SendCommandLine(const nsACString& aProfile,
@@ -294,7 +203,10 @@ void nsRemoteService::StartupServer() {
 
 void nsRemoteService::ShutdownServer() { mRemoteServer = nullptr; }
 
-nsRemoteService::~nsRemoteService() { ShutdownServer(); }
+nsRemoteService::~nsRemoteService() {
+  UnlockStartup();
+  ShutdownServer();
+}
 
 NS_IMETHODIMP
 nsRemoteService::Observe(nsISupports* aSubject, const char* aTopic,

toolkit/components/remote/nsRemoteService.h

@@ -11,26 +11,9 @@
 #include "nsRemoteServer.h"
 #include "nsIObserver.h"
 #include "nsIRemoteService.h"
-#include "mozilla/ThreadSafeWeakPtr.h"
 #include "mozilla/UniquePtr.h"
 #include "nsIFile.h"
 #include "nsProfileLock.h"
-#include "mozilla/MozPromise.h"
-
-class nsStartupLock final
-    : public mozilla::SupportsThreadSafeWeakPtr<nsStartupLock> {
- public:
-  MOZ_DECLARE_REFCOUNTED_TYPENAME(nsStartupLock)
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsStartupLock)
-
-  nsStartupLock(nsIFile* aDir, nsProfileLock& aLock);
-
- private:
-  ~nsStartupLock();
-
-  nsCOMPtr<nsIFile> mDir;
-  nsProfileLock mLock;
-};
 
 class nsRemoteService final : public nsIObserver, public nsIRemoteService {
  public:
@@ -39,62 +22,27 @@ class nsRemoteService final : public nsIObserver, public nsIRemoteService {
   NS_DECL_NSIOBSERVER
   NS_DECL_NSIREMOTESERVICE
 
-  nsRemoteService();
+  explicit nsRemoteService(const char* aProgram);
-  void SetProgram(const char* aProgram);
   void SetProfile(nsACString& aProfile);
 #ifdef MOZ_WIDGET_GTK
   void SetStartupToken(nsACString& aStartupToken);
 #endif
 
-  using StartupLockPromise =
+  void LockStartup();
-      mozilla::MozPromise<RefPtr<nsStartupLock>, nsresult, false>;
+  void UnlockStartup();
-
-  /**
-   * Attempts to asynchronously lock Firefox startup files. Resolves when the
-   * lock is acquired or the timeout is reached
-   *
-   * Locking is attempted by polling so if multiple instances are attempting to
-   * lock it is undefined which one will acquire it when it becomes available.
-   * If this instance already has the lock then this returns the same lock.
-   * The lock will be released once all instances of `nsStartupLock` have been
-   * released.
-   *
-   * Since this blocks the main thread it should only be called during startup.
-   */
-  RefPtr<StartupLockPromise> AsyncLockStartup(double aTimeout);
-
-  /**
-   * Attempts to synchronously lock startup files. Returns then the lock is
-   * acquired or a timeout is reached. In the event of a timeout or other
-   * failure a nullptr is returned. Since this blocks the main thread it should
-   * only be called during startup.
-   *
-   * Locking is attempted by polling so if multiple instances are attempting to
-   * lock it is undefined which one will acquire it when it becomes available.
-   * If this instance already has the lock then this returns the same lock.
-   * The lock will be released once all instances of `nsStartupLock` have been
-   * released.
-   */
-  already_AddRefed<nsStartupLock> LockStartup();
 
   nsresult StartClient();
   void StartupServer();
   void ShutdownServer();
 
  private:
-  friend nsStartupLock;
-
-  // There can only ever be one startup lock so to simplify things hold these
-  // statically.
-  static mozilla::ThreadSafeWeakPtr<nsStartupLock> gStartupLock;
-  static mozilla::StaticRefPtr<nsRemoteService::StartupLockPromise>
-      gStartupLockPromise;
-
   ~nsRemoteService();
   nsresult SendCommandLine(const nsACString& aProfile, size_t aArgc,
                            const char** aArgv, bool aRaise);
 
   mozilla::UniquePtr<nsRemoteServer> mRemoteServer;
+  nsProfileLock mRemoteLock;
+  nsCOMPtr<nsIFile> mRemoteLockDir;
   nsCString mProgram;
   nsCString mProfile;
 #ifdef MOZ_WIDGET_GTK

toolkit/content/aboutProfiles.js

@@ -17,7 +17,7 @@ XPCOMUtils.defineLazyServiceGetter(
 
 async function flush() {
   try {
-    await ProfileService.asyncFlush();
+    ProfileService.flush();
     rebuildProfileList();
   } catch (e) {
     let [title, msg, button] = await document.l10n.formatValues([

toolkit/profile/content/profileSelection.js

@@ -89,7 +89,7 @@ async function flush(cancelled) {
 
   if (gNeedsFlush) {
     try {
-      await gProfileService.asyncFlush();
+      gProfileService.flush();
     } catch (e) {
       let appName = gBrandBundle.getString("brandShortName");
 

toolkit/profile/moz.build

@@ -34,9 +34,6 @@ LOCAL_INCLUDES += [
     "../xre",
 ]
 
-if CONFIG["MOZ_HAS_REMOTE"]:
-    LOCAL_INCLUDES += ["../components/remote"]
-
 FINAL_LIBRARY = "xul"
 
 for var in ("MOZ_APP_NAME", "MOZ_APP_BASENAME"):

toolkit/profile/nsIToolkitProfileService.idl

@@ -147,30 +147,9 @@ interface nsIToolkitProfileService : nsISupports
     /**
      * Flush the profiles list file. This will fail with
      * NS_ERROR_DATABASE_CHANGED if the files on disk have changed since the
-     * profiles were loaded. Should not be called outside of startup.
+     * profiles were loaded.
      */
     void flush();
-
-    /**
-     * Flushes the profiles list file on a background thread after acquiring the
-     * startup lock. Like `flush` this will fail with NS_ERROR_DATABASE_CHANGED
-     * if the files on disk have changed since the profiles were loaded.
-     * It is safe to call this while a previous flush is still in progress. The
-     * promise returned will resolve when the last call to flush completes.
-     */
-    [implicit_jscontext]
-    Promise asyncFlush();
-
-    /**
-     * Flushes the mutable data about the current profile to disk on a
-     * background thread after acquiring the startup lock. Unlike other flushing
-     * methods this can usually succeed even if the files on disk have changed
-     * since the profiles were loaded.
-     * It is safe to call this while a previous flush is still in progress. The
-     * promise returned will resolve when the last call to flush completes.
-     */
-    [implicit_jscontext]
-    Promise asyncFlushCurrentProfile();
 };
 
 %{C++

toolkit/profile/nsProfileLock.cpp

@@ -68,11 +68,8 @@ nsProfileLock::nsProfileLock(nsProfileLock& src) { *this = src; }
 nsProfileLock& nsProfileLock::operator=(nsProfileLock& rhs) {
   Unlock();
 
-  mLockFile = rhs.mLockFile;
-  rhs.mLockFile = nullptr;
   mHaveLock = rhs.mHaveLock;
   rhs.mHaveLock = false;
-  mReplacedLockTime = rhs.mReplacedLockTime;
 
 #if defined(XP_WIN)
   mLockFileHandle = rhs.mLockFileHandle;

toolkit/profile/nsToolkitProfileService.cpp

@@ -5,7 +5,6 @@
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Preferences.h"
-#include "mozilla/ErrorResult.h"
 #include "mozilla/ScopeExit.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/UniquePtrExtensions.h"
@@ -54,19 +53,12 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/Sprintf.h"
 #include "nsPrintfCString.h"
-#include "mozilla/dom/DOMMozPromiseRequestHolder.h"
-#include "mozilla/dom/Promise.h"
 #include "mozilla/UniquePtr.h"
 #include "nsIToolkitShellService.h"
 #include "mozilla/Telemetry.h"
 #include "nsProxyRelease.h"
-#ifdef MOZ_HAS_REMOTE
-#  include "nsRemoteService.h"
-#endif
 #include "prinrval.h"
 #include "prthread.h"
-#include "xpcpublic.h"
-#include "nsProxyRelease.h"
 #ifdef MOZ_BACKGROUNDTASKS
 #  include "mozilla/BackgroundTasks.h"
 #  include "SpecialSystemDirectory.h"
@@ -2303,277 +2295,8 @@ nsToolkitProfileService::GetProfileCount(uint32_t* aResult) {
   return NS_OK;
 }
 
-// Attempts to merge the given profile data into the on-disk versions which may
-// have changed since rthey were loaded.
-nsresult WriteProfileInfo(nsIFile* profilesDBFile, nsIFile* installDBFile,
-                          const nsCString& installSection,
-                          const CurrentProfileData* profileInfo) {
-  nsINIParser profilesIni;
-  nsresult rv = profilesIni.Init(profilesDBFile);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // The INI data may have changed on disk so we cannot guarantee the section
-  // mapping remains the same. So we attempt to find the current profile's info
-  // by path or store ID.
-  nsCString iniSection;
-  profilesIni.GetSections(
-      [&profileInfo, &profilesIni, &iniSection](const char* section) {
-        nsCString value;
-        nsresult rv = profilesIni.GetString(section, "StoreID", value);
-
-        if (NS_SUCCEEDED(rv)) {
-          if (profileInfo->mStoreID.Equals(value)) {
-            iniSection = section;
-            // This is definitely the right one so no need to continue.
-            return false;
-          }
-        }
-
-        if (iniSection.IsEmpty()) {
-          rv = profilesIni.GetString(section, "Path", value);
-          if (NS_SUCCEEDED(rv) && profileInfo->mPath.Equals(value)) {
-            // This might be right but we would prefer to find by store ID.
-            iniSection = section;
-          }
-        }
-
-        return true;
-      });
-
-  if (iniSection.IsEmpty()) {
-    // No section found. Should we write a new one?
-    return NS_ERROR_UNEXPECTED;
-  }
-
-  bool changed = false;
-  nsCString oldValue;
-  rv = profilesIni.GetString(iniSection.get(), "StoreID", oldValue);
-  if (NS_FAILED(rv) || !oldValue.Equals(profileInfo->mStoreID)) {
-    rv = profilesIni.SetString(iniSection.get(), "StoreID",
-                               profileInfo->mStoreID.get());
-    NS_ENSURE_SUCCESS(rv, rv);
-    changed = true;
-  }
-
-  rv = profilesIni.GetString(iniSection.get(), "ShowSelector", oldValue);
-  if (NS_FAILED(rv) ||
-      !oldValue.Equals(profileInfo->mShowSelector ? "1" : "0")) {
-    rv = profilesIni.SetString(iniSection.get(), "ShowSelector",
-                               profileInfo->mShowSelector ? "1" : "0");
-    NS_ENSURE_SUCCESS(rv, rv);
-    changed = true;
-  }
-
-  profilesIni.GetString(iniSection.get(), "Path", oldValue);
-  if (NS_FAILED(rv) || !oldValue.Equals(profileInfo->mPath)) {
-    rv = profilesIni.SetString(iniSection.get(), "Path",
-                               profileInfo->mPath.get());
-    NS_ENSURE_SUCCESS(rv, rv);
-    changed = true;
-
-    // We must update the install default profile if it matches the old profile.
-
-    nsCString oldDefault;
-    rv = profilesIni.GetString(installSection.get(), "Default", oldDefault);
-    if (NS_SUCCEEDED(rv) && oldDefault.Equals(oldValue)) {
-      rv = profilesIni.SetString(installSection.get(), "Default",
-                                 profileInfo->mPath.get());
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      // We don't care so much if we fail to update the backup DB.
-      const nsDependentCSubstring& installHash =
-          Substring(installSection, INSTALL_PREFIX_LENGTH);
-
-      nsINIParser installsIni;
-      rv = installsIni.Init(installDBFile);
-      if (NS_SUCCEEDED(rv)) {
-        rv = installsIni.SetString(PromiseFlatCString(installHash).get(),
-                                   "Default", profileInfo->mPath.get());
-        if (NS_SUCCEEDED(rv)) {
-          installsIni.WriteToFile(installDBFile);
-        }
-      }
-    }
-  }
-
-  if (changed) {
-    rv = profilesIni.WriteToFile(profilesDBFile);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
-  return NS_OK;
-}
-
 NS_IMETHODIMP
-nsToolkitProfileService::AsyncFlushCurrentProfile(JSContext* aCx,
+nsToolkitProfileService::Flush() {
-                                                  dom::Promise** aPromise) {
-#ifndef MOZ_HAS_REMOTE
-  return NS_ERROR_FAILURE;
-#else
-  if (!mCurrent) {
-    return NS_ERROR_ILLEGAL_VALUE;
-  }
-
-  nsIGlobalObject* global = xpc::CurrentNativeGlobal(aCx);
-
-  if (!global) {
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-  }
-
-  ErrorResult result;
-  RefPtr<dom::Promise> promise = dom::Promise::Create(global, result);
-
-  if (MOZ_UNLIKELY(result.Failed())) {
-    return result.StealNSResult();
-  }
-
-  UniquePtr<CurrentProfileData> profileData = MakeUnique<CurrentProfileData>();
-  profileData->mStoreID = mCurrent->mStoreID;
-  profileData->mShowSelector = mCurrent->mShowProfileSelector;
-
-  bool isRelative;
-  GetProfileDescriptor(mCurrent, profileData->mPath, &isRelative);
-
-  if (!mAsyncQueue) {
-    MOZ_ALWAYS_SUCCEEDS(NS_CreateBackgroundTaskQueue(
-        "nsToolkitProfileService", getter_AddRefs(mAsyncQueue)));
-  }
-
-  nsCOMPtr<nsIRemoteService> rs = GetRemoteService();
-  RefPtr<nsRemoteService> remoteService =
-      static_cast<nsRemoteService*>(rs.get());
-
-  RefPtr<AsyncFlushPromise> p = remoteService->AsyncLockStartup(5000)->Then(
-      mAsyncQueue, __func__,
-      [self = RefPtr{this}, this, profileData = std::move(profileData)](
-          const nsRemoteService::StartupLockPromise::ResolveOrRejectValue&
-              aValue) {
-        if (aValue.IsReject()) {
-          // Locking failed.
-          return AsyncFlushPromise::CreateAndReject(aValue.RejectValue(),
-                                                    __func__);
-        }
-
-        nsresult rv = WriteProfileInfo(mProfileDBFile, mInstallDBFile,
-                                       mInstallSection, profileData.get());
-
-        if (NS_FAILED(rv)) {
-          return AsyncFlushPromise::CreateAndReject(rv, __func__);
-        }
-
-        return AsyncFlushPromise::CreateAndResolve(true, __func__);
-      });
-
-  // This is responsible for cancelling the MozPromise if the global goes
-  // away.
-  auto requestHolder =
-      MakeRefPtr<dom::DOMMozPromiseRequestHolder<AsyncFlushPromise>>(global);
-
-  // This keeps the promise alive after this method returns.
-  nsMainThreadPtrHandle<dom::Promise> promiseHolder(
-      new nsMainThreadPtrHolder<dom::Promise>(
-          "nsToolkitProfileService::AsyncFlushCurrentProfile", promise));
-
-  p->Then(GetCurrentSerialEventTarget(), __func__,
-          [requestHolder, promiseHolder](
-              const AsyncFlushPromise::ResolveOrRejectValue& result) {
-            requestHolder->Complete();
-
-            if (result.IsReject()) {
-              promiseHolder->MaybeReject(result.RejectValue());
-            } else {
-              promiseHolder->MaybeResolveWithUndefined();
-            }
-          })
-      ->Track(*requestHolder);
-
-  promise.forget(aPromise);
-
-  return NS_OK;
-#endif
-}
-
-NS_IMETHODIMP
-nsToolkitProfileService::AsyncFlush(JSContext* aCx, dom::Promise** aPromise) {
-#ifndef MOZ_HAS_REMOTE
-  return NS_ERROR_FAILURE;
-#else
-  nsIGlobalObject* global = xpc::CurrentNativeGlobal(aCx);
-
-  if (!global) {
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-  }
-
-  ErrorResult result;
-  RefPtr<dom::Promise> promise = dom::Promise::Create(global, result);
-
-  if (MOZ_UNLIKELY(result.Failed())) {
-    return result.StealNSResult();
-  }
-
-  UniquePtr<IniData> iniData = MakeUnique<IniData>();
-  BuildIniData(iniData->mProfiles, iniData->mInstalls);
-
-  if (!mAsyncQueue) {
-    MOZ_ALWAYS_SUCCEEDS(NS_CreateBackgroundTaskQueue(
-        "nsToolkitProfileService", getter_AddRefs(mAsyncQueue)));
-  }
-
-  nsCOMPtr<nsIRemoteService> rs = GetRemoteService();
-  RefPtr<nsRemoteService> remoteService =
-      static_cast<nsRemoteService*>(rs.get());
-
-  RefPtr<AsyncFlushPromise> p = remoteService->AsyncLockStartup(5000)->Then(
-      mAsyncQueue, __func__,
-      [self = RefPtr{this}, this, iniData = std::move(iniData)](
-          const nsRemoteService::StartupLockPromise::ResolveOrRejectValue&
-              aValue) {
-        if (aValue.IsReject()) {
-          // Locking failed.
-          return AsyncFlushPromise::CreateAndReject(aValue.RejectValue(),
-                                                    __func__);
-        }
-
-        nsresult rv = FlushData(iniData->mProfiles, iniData->mInstalls);
-
-        if (NS_FAILED(rv)) {
-          return AsyncFlushPromise::CreateAndReject(rv, __func__);
-        }
-
-        return AsyncFlushPromise::CreateAndResolve(true, __func__);
-      });
-
-  // This is responsible for cancelling the MozPromise if the global goes
-  // away.
-  auto requestHolder =
-      MakeRefPtr<dom::DOMMozPromiseRequestHolder<AsyncFlushPromise>>(global);
-
-  // This keeps the promise alive after this method returns.
-  nsMainThreadPtrHandle<dom::Promise> promiseHolder(
-      new nsMainThreadPtrHolder<dom::Promise>(
-          "nsToolkitProfileService::AsyncFlushCurrentProfile", promise));
-
-  p->Then(GetCurrentSerialEventTarget(), __func__,
-          [requestHolder, promiseHolder](
-              const AsyncFlushPromise::ResolveOrRejectValue& result) {
-            requestHolder->Complete();
-
-            if (result.IsReject()) {
-              promiseHolder->MaybeReject(result.RejectValue());
-            } else {
-              promiseHolder->MaybeResolveWithUndefined();
-            }
-          })
-      ->Track(*requestHolder);
-
-  promise.forget(aPromise);
-
-  return NS_OK;
-#endif
-}
-
-nsresult nsToolkitProfileService::FlushData(const nsCString& aProfilesIniData,
-                                            const nsCString& aInstallsIniData) {
   if (GetIsListOutdated()) {
     return NS_ERROR_DATABASE_CHANGED;
   }
@@ -2583,14 +2306,39 @@ nsresult nsToolkitProfileService::FlushData(const nsCString& aProfilesIniData,
   // If we aren't using dedicated profiles then nothing about the list of
   // installs can have changed, so no need to update the backup.
   if (mUseDedicatedProfile) {
-    if (!aInstallsIniData.IsEmpty()) {
+    // Export the installs to the backup.
+    nsTArray<nsCString> installs = GetKnownInstalls();
+
+    if (!installs.IsEmpty()) {
+      nsCString data;
+      nsCString buffer;
+
+      for (uint32_t i = 0; i < installs.Length(); i++) {
+        nsTArray<UniquePtr<KeyValue>> strings =
+            GetSectionStrings(&mProfileDB, installs[i].get());
+        if (strings.IsEmpty()) {
+          continue;
+        }
+
+        // Strip "Install" from the start.
+        const nsDependentCSubstring& install =
+            Substring(installs[i], INSTALL_PREFIX_LENGTH);
+        data.AppendPrintf("[%s]\n", PromiseFlatCString(install).get());
+
+        for (uint32_t j = 0; j < strings.Length(); j++) {
+          data.AppendPrintf("%s=%s\n", strings[j]->key.get(),
+                            strings[j]->value.get());
+        }
+
+        data.Append("\n");
+      }
+
       FILE* writeFile;
       rv = mInstallDBFile->OpenANSIFileDesc("w", &writeFile);
       NS_ENSURE_SUCCESS(rv, rv);
 
-      uint32_t length = aInstallsIniData.Length();
+      uint32_t length = data.Length();
-      if (fwrite(aInstallsIniData.get(), sizeof(char), length, writeFile) !=
+      if (fwrite(data.get(), sizeof(char), length, writeFile) != length) {
-          length) {
         fclose(writeFile);
         return NS_ERROR_UNEXPECTED;
       }
@@ -2604,19 +2352,9 @@ nsresult nsToolkitProfileService::FlushData(const nsCString& aProfilesIniData,
     }
   }
 
-  FILE* writeFile;
+  rv = mProfileDB.WriteToFile(mProfileDBFile);
-  rv = mProfileDBFile->OpenANSIFileDesc("w", &writeFile);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  uint32_t length = aProfilesIniData.Length();
-  if (fwrite(aProfilesIniData.get(), sizeof(char), length, writeFile) !=
-      length) {
-    fclose(writeFile);
-    return NS_ERROR_UNEXPECTED;
-  }
-
-  fclose(writeFile);
-
   rv = UpdateFileStats(mProfileDBFile, &mProfileDBExists,
                        &mProfileDBModifiedTime, &mProfileDBFileSize);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -2624,52 +2362,6 @@ nsresult nsToolkitProfileService::FlushData(const nsCString& aProfilesIniData,
   return NS_OK;
 }
 
-void nsToolkitProfileService::BuildIniData(nsCString& aProfilesIniData,
-                                           nsCString& aInstallsIniData) {
-  // If we aren't using dedicated profiles then nothing about the list of
-  // installs can have changed, so no need to update the backup.
-  if (mUseDedicatedProfile) {
-    // Export the installs to the backup.
-    nsTArray<nsCString> installs = GetKnownInstalls();
-
-    if (!installs.IsEmpty()) {
-      nsCString buffer;
-
-      for (uint32_t i = 0; i < installs.Length(); i++) {
-        nsTArray<UniquePtr<KeyValue>> strings =
-            GetSectionStrings(&mProfileDB, installs[i].get());
-        if (strings.IsEmpty()) {
-          continue;
-        }
-
-        // Strip "Install" from the start.
-        const nsDependentCSubstring& install =
-            Substring(installs[i], INSTALL_PREFIX_LENGTH);
-        aInstallsIniData.AppendPrintf("[%s]\n",
-                                      PromiseFlatCString(install).get());
-
-        for (uint32_t j = 0; j < strings.Length(); j++) {
-          aInstallsIniData.AppendPrintf("%s=%s\n", strings[j]->key.get(),
-                                        strings[j]->value.get());
-        }
-
-        aInstallsIniData.Append("\n");
-      }
-    }
-  }
-
-  mProfileDB.WriteToString(aProfilesIniData);
-}
-
-NS_IMETHODIMP
-nsToolkitProfileService::Flush() {
-  nsCString profilesIniData;
-  nsCString installsIniData;
-
-  BuildIniData(profilesIniData, installsIniData);
-  return FlushData(profilesIniData, installsIniData);
-}
-
 nsresult nsToolkitProfileService::GetLocalDirFromRootDir(nsIFile* aRootDir,
                                                          nsIFile** aResult) {
   NS_ASSERTION(nsToolkitProfileService::gService, "Where did my service go?");

toolkit/profile/nsToolkitProfileService.h

@@ -16,21 +16,6 @@
 #include "nsSimpleEnumerator.h"
 #include "nsProfileLock.h"
 #include "nsINIParser.h"
-#include "mozilla/MozPromise.h"
-#include "nsProxyRelease.h"
-
-class nsStartupLock;
-
-struct CurrentProfileData {
-  nsCString mPath;
-  nsCString mStoreID;
-  bool mShowSelector;
-};
-
-struct IniData {
-  nsCString mProfiles;
-  nsCString mInstalls;
-};
 
 class nsToolkitProfile final
     : public nsIToolkitProfile,
@@ -85,7 +70,7 @@ class nsToolkitProfileLock final : public nsIProfileLock {
 
 class nsToolkitProfileService final : public nsIToolkitProfileService {
  public:
-  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_ISUPPORTS
   NS_DECL_NSITOOLKITPROFILESERVICE
 
   nsresult SelectStartupProfile(int* aArgc, char* aArgv[], bool aIsResetting,
@@ -96,9 +81,6 @@ class nsToolkitProfileService final : public nsIToolkitProfileService {
   nsresult ApplyResetProfile(nsIToolkitProfile* aOldProfile);
   void CompleteStartup();
 
-  using AsyncFlushPromise =
-      mozilla::MozPromise<bool /* ignored */, nsresult, false>;
-
  private:
   friend class nsToolkitProfile;
   friend already_AddRefed<nsToolkitProfileService>
@@ -134,12 +116,6 @@ class nsToolkitProfileService final : public nsIToolkitProfileService {
   void SetNormalDefault(nsToolkitProfile* aProfile);
   already_AddRefed<nsToolkitProfile> GetDefaultProfile();
   nsresult GetLocalDirFromRootDir(nsIFile* aRootDir, nsIFile** aResult);
-  void FlushProfileData(
-      const nsMainThreadPtrHandle<nsStartupLock>& aStartupLock,
-      const CurrentProfileData* aProfileInfo);
-  void BuildIniData(nsCString& aProfilesIniData, nsCString& aInstallsIniData);
-  nsresult FlushData(const nsCString& aProfilesIniData,
-                     const nsCString& aInstallsIniData);
 
   // Returns the known install hashes from the installs database. Modifying the
   // installs database is safe while iterating the returned array.
@@ -198,9 +174,6 @@ class nsToolkitProfileService final : public nsIToolkitProfileService {
   int64_t mProfileDBFileSize;
   PRTime mProfileDBModifiedTime;
 
-  // A background task queue for the async flushing operations.
-  nsCOMPtr<nsISerialEventTarget> mAsyncQueue;
-
   static nsToolkitProfileService* gService;
 
   class ProfileEnumerator final : public nsSimpleEnumerator {

toolkit/profile/test/xpcshell/test_async_flush.js

@@ -1,163 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-/*
- * Verifies the async flushing methods.
- */
-
-add_task(async () => {
-  let defaultProfile = makeRandomProfileDir("default");
-
-  let hash = xreDirProvider.getInstallHash();
-
-  writeCompatibilityIni(defaultProfile);
-
-  writeProfilesIni({
-    profiles: [
-      {
-        name: "default",
-        path: defaultProfile.leafName,
-        default: false,
-      },
-    ],
-    installs: {
-      [hash]: { default: defaultProfile.leafName },
-    },
-  });
-
-  selectStartupProfile();
-  checkStartupReason("default");
-
-  let profileData = readProfilesIni();
-  checkProfileService(profileData);
-
-  let service = getProfileService();
-
-  let newProfileDir = makeRandomProfileDir("newProfile");
-  service.createProfile(newProfileDir, "new");
-
-  await service.asyncFlush();
-  profileData = readProfilesIni();
-
-  Assert.equal(profileData.profiles.length, 2, "Should now have two profiles.");
-
-  checkProfileService(profileData);
-
-  let other1 = makeRandomProfileDir("other1");
-  let other2 = makeRandomProfileDir("other2");
-
-  // Write out a different ini file.
-  writeProfilesIni({
-    profiles: [
-      {
-        name: "changedname",
-        path: defaultProfile.leafName,
-      },
-      {
-        name: "other1",
-        path: other1.leafName,
-      },
-      {
-        name: "other2",
-        path: other2.leafName,
-      },
-    ],
-    installs: {
-      [hash]: { default: defaultProfile.leafName },
-    },
-  });
-
-  // Change the modified time.
-  let profilesini = gDataHome.clone();
-  profilesini.append("profiles.ini");
-  let oldTime = profilesini.lastModifiedTime;
-  profilesini.lastModifiedTime = oldTime - 10000;
-
-  try {
-    await service.asyncFlush();
-    Assert.ok(false, "Flushing should have failed");
-  } catch (e) {
-    Assert.ok(true, "Flushing should have failed");
-  }
-
-  // Flushing the current profile should always work.
-  await service.asyncFlushCurrentProfile();
-
-  profileData = readProfilesIni();
-  Assert.equal(profileData.profiles.length, 3, "Should have three profiles.");
-
-  let found = profileData.profiles.find(p => p.name == "changedname");
-  Assert.ok(found, "Should have found the current profile.");
-  Assert.equal(found.path, defaultProfile.leafName);
-  Assert.equal(found.storeID, null);
-
-  found = profileData.profiles.find(p => p.name == "other1");
-  Assert.ok(found, "Should have found the other1 profile.");
-  Assert.equal(found.path, other1.leafName);
-  Assert.equal(found.storeID, null);
-
-  found = profileData.profiles.find(p => p.name == "other2");
-  Assert.ok(found, "Should have found the other2 profile.");
-  Assert.equal(found.path, other2.leafName);
-  Assert.equal(found.storeID, null);
-
-  let installData = readInstallsIni();
-  Assert.equal(profileData.installs[hash].default, defaultProfile.leafName);
-  Assert.equal(installData.installs[hash].default, defaultProfile.leafName);
-
-  if (AppConstants.MOZ_SELECTABLE_PROFILES) {
-    // Set a store ID on the profile. Flushing will succeed because the profile path hasn't changed.
-    service.currentProfile.storeID = "7126354jdf";
-
-    await service.asyncFlushCurrentProfile();
-
-    profileData = readProfilesIni();
-    Assert.equal(profileData.profiles.length, 3, "Should have three profiles.");
-
-    found = profileData.profiles.find(p => p.name == "changedname");
-    Assert.ok(found, "Should have found the current profile.");
-    Assert.equal(found.path, defaultProfile.leafName);
-    Assert.equal(found.storeID, "7126354jdf");
-
-    found = profileData.profiles.find(p => p.name == "other1");
-    Assert.ok(found, "Should have found the other1 profile.");
-    Assert.equal(found.path, other1.leafName);
-    Assert.equal(found.storeID, null);
-
-    found = profileData.profiles.find(p => p.name == "other2");
-    Assert.ok(found, "Should have found the other2 profile.");
-    Assert.equal(found.path, other2.leafName);
-    Assert.equal(found.storeID, null);
-
-    installData = readInstallsIni();
-    Assert.equal(profileData.installs[hash].default, defaultProfile.leafName);
-    Assert.equal(installData.installs[hash].default, defaultProfile.leafName);
-
-    // Change the profile path. Flushing will succeed because the store ID now matches.
-    service.currentProfile.rootDir = newProfileDir;
-
-    await service.asyncFlushCurrentProfile();
-
-    profileData = readProfilesIni();
-    Assert.equal(profileData.profiles.length, 3, "Should have three profiles.");
-
-    found = profileData.profiles.find(p => p.name == "changedname");
-    Assert.ok(found, "Should have found the current profile.");
-    Assert.equal(found.path, newProfileDir.leafName);
-    Assert.equal(found.storeID, "7126354jdf");
-
-    found = profileData.profiles.find(p => p.name == "other1");
-    Assert.ok(found, "Should have found the other1 profile.");
-    Assert.equal(found.path, other1.leafName);
-    Assert.equal(found.storeID, null);
-
-    found = profileData.profiles.find(p => p.name == "other2");
-    Assert.ok(found, "Should have found the other2 profile.");
-    Assert.equal(found.path, other2.leafName);
-    Assert.equal(found.storeID, null);
-
-    installData = readInstallsIni();
-    Assert.equal(profileData.installs[hash].default, newProfileDir.leafName);
-    Assert.equal(installData.installs[hash].default, newProfileDir.leafName);
-  }
-});

toolkit/profile/test/xpcshell/xpcshell.toml

@@ -2,8 +2,6 @@
 head = "head.js"
 skip-if = ["os == 'android'"]
 
-["test_async_flush.js"]
-
 ["test_check_backup.js"]
 
 ["test_claim_locked.js"]

toolkit/xre/nsAppRunner.cpp

@@ -312,7 +312,6 @@ extern const char gToolkitBuildID[];
 static nsIProfileLock* gProfileLock;
 #if defined(MOZ_HAS_REMOTE)
 static RefPtr<nsRemoteService> gRemoteService;
-static RefPtr<nsStartupLock> gStartupLock;
 #endif
 
 int gRestartArgc;
@@ -2035,11 +2034,6 @@ nsresult ScopedXPCOMStartup::SetWindowCreator(nsINativeAppSupport* native) {
 
 #ifdef MOZ_HAS_REMOTE
 /* static */ already_AddRefed<nsIRemoteService> GetRemoteService() {
-  AssertIsOnMainThread();
-
-  if (!gRemoteService) {
-    gRemoteService = new nsRemoteService();
-  }
   nsCOMPtr<nsIRemoteService> remoteService = gRemoteService.get();
   return remoteService.forget();
 }
@@ -2673,12 +2667,6 @@ static ReturnAbortOnError ProfileLockedDialog(nsIFile* aProfileDir,
                                               nsIProfileLock** aResult) {
   nsresult rv;
 
-  // We aren't going to start this instance so we can unblock other instances
-  // from starting up.
-#if defined(MOZ_HAS_REMOTE)
-  gStartupLock = nullptr;
-#endif
-
   bool exists;
   aProfileDir->Exists(&exists);
   if (!exists) {
@@ -2776,6 +2764,11 @@ static ReturnAbortOnError ProfileLockedDialog(nsIFile* aProfileDir,
         SaveFileToEnv("XRE_PROFILE_PATH", aProfileDir);
         SaveFileToEnv("XRE_PROFILE_LOCAL_PATH", aProfileLocalDir);
 
+#if defined(MOZ_HAS_REMOTE)
+        if (gRemoteService) {
+          gRemoteService->UnlockStartup();
+        }
+#endif
         return LaunchChild(false, true);
       }
     } else {
@@ -2796,12 +2789,6 @@ static ReturnAbortOnError ShowProfileDialog(
   bool offline = false;
   int32_t dialogReturn;
 
-  // We aren't going to start this instance so we can unblock other instances
-  // from starting up.
-#if defined(MOZ_HAS_REMOTE)
-  gStartupLock = nullptr;
-#endif
-
   {
     ScopedXPCOMStartup xpcom;
     rv = xpcom.Initialize();
@@ -2888,7 +2875,11 @@ static ReturnAbortOnError ShowProfileDialog(
     gRestartArgv[gRestartArgc++] = const_cast<char*>("-os-restarted");
     gRestartArgv[gRestartArgc] = nullptr;
   }
-
+#if defined(MOZ_HAS_REMOTE)
+  if (gRemoteService) {
+    gRemoteService->UnlockStartup();
+  }
+#endif
   return LaunchChild(false, true);
 }
 
@@ -4343,7 +4334,8 @@ int XREMain::XRE_mainInit(bool* aExitFlag) {
   CheckArg("no-remote");
 
 #if defined(MOZ_HAS_REMOTE)
-  // Handle the --new-instance command line arguments.
+  // Handle the --new-instance command line arguments. Setup the environment to
+  // better accommodate other components and various restart scenarios.
   ar = CheckArg("new-instance");
   if (ar == ARG_FOUND || EnvHasValue("MOZ_NEW_INSTANCE")) {
     mDisableRemoteClient = true;
@@ -4853,13 +4845,9 @@ int XREMain::XRE_mainStartup(bool* aExitFlag) {
 #endif
 #if defined(MOZ_HAS_REMOTE)
   // handle --remote now that xpcom is fired up
-  gRemoteService = new nsRemoteService();
+  gRemoteService = new nsRemoteService(gAppData->remotingName);
   if (gRemoteService) {
-    gRemoteService->SetProgram(gAppData->remotingName);
+    gRemoteService->LockStartup();
-    gStartupLock = gRemoteService->LockStartup();
-    if (!gStartupLock) {
-      NS_WARNING("Failed to lock for startup, continuing anyway.");
-    }
   }
 #endif
 #if defined(MOZ_WIDGET_GTK)
@@ -4968,18 +4956,18 @@ int XREMain::XRE_mainStartup(bool* aExitFlag) {
 
         if (NS_SUCCEEDED(rv)) {
           *aExitFlag = true;
-          gStartupLock = nullptr;
+          gRemoteService->UnlockStartup();
           return 0;
         }
 
         if (rv == NS_ERROR_INVALID_ARG) {
-          gStartupLock = nullptr;
+          gRemoteService->UnlockStartup();
           return 1;
         }
       }
     }
   }
-#endif /* MOZ_HAS_REMOTE */
+#endif
 
 #if defined(MOZ_UPDATER) && !defined(MOZ_WIDGET_ANDROID)
 #  ifdef XP_WIN
@@ -5115,8 +5103,7 @@ int XREMain::XRE_mainStartup(bool* aExitFlag) {
   if (rv == NS_ERROR_LAUNCHED_CHILD_PROCESS || rv == NS_ERROR_ABORT) {
     *aExitFlag = true;
     return 0;
-  }
+  } else if (NS_FAILED(rv)) {
-  if (NS_FAILED(rv)) {
     return 1;
   }
 
@@ -5161,8 +5148,7 @@ int XREMain::XRE_mainStartup(bool* aExitFlag) {
       if (rv == NS_ERROR_LAUNCHED_CHILD_PROCESS || rv == NS_ERROR_ABORT) {
         *aExitFlag = true;
         return 0;
-      }
+      } else if (NS_FAILED(rv)) {
-      if (NS_FAILED(rv)) {
         return 1;
       }
     } else {
@@ -5730,9 +5716,9 @@ nsresult XREMain::XRE_mainRun() {
       // proxy window.
       if (gRemoteService) {
         gRemoteService->StartupServer();
-        gStartupLock = nullptr;
+        gRemoteService->UnlockStartup();
       }
-#endif
+#endif /* MOZ_WIDGET_GTK */
 
       mNativeApp->Enable();
     }
@@ -6061,10 +6047,9 @@ int XREMain::XRE_main(int argc, char* argv[], const BootstrapConfig& aConfig) {
   // remote to this instance.
   if (gRemoteService) {
     gRemoteService->ShutdownServer();
-    gStartupLock = nullptr;
     gRemoteService = nullptr;
   }
-#endif
+#endif /* MOZ_WIDGET_GTK */
 
   mScopedXPCOM = nullptr;