Bug 1960981 - Backed out changeset 118a3fe99c1e (Bug 1907127 - Read update.timestamp at startup and use it to delay updates) r=nrishel

Differential Revision: https://phabricator.services.mozilla.com/D246304

toolkit/xre/nsAppRunner.cpp

@@ -4581,8 +4581,7 @@ enum struct ShouldNotProcessUpdatesReason {
   DevToolsLaunching,
   NotAnUpdatingTask,
   OtherInstanceRunning,
-  FirstStartup,
+  FirstStartup
-  MultiSessionInstallLockout
 };
 
 const char* ShouldNotProcessUpdatesReasonAsString(
@@ -4594,17 +4593,13 @@ const char* ShouldNotProcessUpdatesReasonAsString(
       return "NotAnUpdatingTask";
     case ShouldNotProcessUpdatesReason::OtherInstanceRunning:
       return "OtherInstanceRunning";
-    case ShouldNotProcessUpdatesReason::FirstStartup:
-      return "FirstStartup";
-    case ShouldNotProcessUpdatesReason::MultiSessionInstallLockout:
-      return "MultiSessionInstallLockout";
     default:
       MOZ_CRASH("impossible value for ShouldNotProcessUpdatesReason");
   }
 }
 
 Maybe<ShouldNotProcessUpdatesReason> ShouldNotProcessUpdates(
-    nsXREDirProvider& aDirProvider, nsIFile* aUpdateRoot) {
+    nsXREDirProvider& aDirProvider) {
   // Don't process updates when launched from the installer.
   // It's possible for a stale update to be present in the case of a paveover;
   // ignore it and leave the update service to discard it.
@@ -4632,33 +4627,6 @@ Maybe<ShouldNotProcessUpdatesReason> ShouldNotProcessUpdates(
     }
   }
 
-  bool otherInstance = false;
-  // At this point we have a dir provider but no XPCOM directory service.  We
-  // launch the update sync manager using that information so that it doesn't
-  // need to ask for (and fail to find) the directory service.
-  nsCOMPtr<nsIFile> anAppFile;
-  bool persistent;
-  nsresult rv = aDirProvider.GetFile(XRE_EXECUTABLE_FILE, &persistent,
-                                     getter_AddRefs(anAppFile));
-  if (NS_SUCCEEDED(rv) && anAppFile) {
-    auto updateSyncManager = new nsUpdateSyncManager(anAppFile);
-    rv = updateSyncManager->IsOtherInstanceRunning(&otherInstance);
-    if (NS_FAILED(rv)) {
-      // Unless we know that there is another instance, we default to assuming
-      // there is not one.
-      otherInstance = false;
-    }
-  }
-
-  if (otherInstance) {
-    bool msilActive = false;
-    rv = IsMultiSessionInstallLockoutActive(aUpdateRoot, msilActive);
-    if (NS_SUCCEEDED(rv) && msilActive) {
-      NS_WARNING("ShouldNotProcessUpdates(): MultiSessionInstallLockout");
-      return Some(ShouldNotProcessUpdatesReason::MultiSessionInstallLockout);
-    }
-  }
-
 #  ifdef MOZ_BACKGROUNDTASKS
   // Do not process updates if we're running a background task mode and another
   // instance is already running.  This avoids periodic maintenance updating
@@ -4682,6 +4650,22 @@ Maybe<ShouldNotProcessUpdatesReason> ShouldNotProcessUpdates(
       return Some(ShouldNotProcessUpdatesReason::NotAnUpdatingTask);
     }
 
+    // At this point we have a dir provider but no XPCOM directory service.  We
+    // launch the update sync manager using that information so that it doesn't
+    // need to ask for (and fail to find) the directory service.
+    nsCOMPtr<nsIFile> anAppFile;
+    bool persistent;
+    nsresult rv = aDirProvider.GetFile(XRE_EXECUTABLE_FILE, &persistent,
+                                       getter_AddRefs(anAppFile));
+    if (NS_FAILED(rv) || !anAppFile) {
+      // Strange, but not a reason to skip processing updates.
+      return Nothing();
+    }
+
+    auto updateSyncManager = new nsUpdateSyncManager(anAppFile);
+
+    bool otherInstance = false;
+    updateSyncManager->IsOtherInstanceRunning(&otherInstance);
     if (otherInstance) {
       NS_WARNING("ShouldNotProcessUpdates(): OtherInstanceRunning");
       return Some(ShouldNotProcessUpdatesReason::OtherInstanceRunning);
@@ -5137,7 +5121,7 @@ int XREMain::XRE_mainStartup(bool* aExitFlag) {
   }
 
   Maybe<ShouldNotProcessUpdatesReason> shouldNotProcessUpdatesReason =
-      ShouldNotProcessUpdates(mDirProvider, updRoot);
+      ShouldNotProcessUpdates(mDirProvider);
   if (shouldNotProcessUpdatesReason.isNothing()) {
     nsCOMPtr<nsIFile> exeFile, exeDir;
     rv = mDirProvider.GetFile(XRE_EXECUTABLE_FILE, &persistent,

toolkit/xre/nsUpdateDriver.cpp

@@ -152,30 +152,32 @@ static void GetPidString(nsACString& output) {
 }
 
 /**
- * Get the contents of the file when it can be opened with read and write
+ * Get the contents of the update.status file when the update.status file can
- * access. The reason it is opened for both read and write is to prevent trying
+ * be opened with read and write access. The reason it is opened for both read
- * to update when the user doesn't have write access to the update directory.
+ * and write is to prevent trying to update when the user doesn't have write
- * Otherwise we will loop infinitely and try to install it over and over.
+ * access to the update directory.
  *
- * @param  file
+ * @param statusFile the status file object.
- *         The file object.
+ * @param buf        the buffer holding the file contents
- * @param  buf
- *         The buffer holding the file contents.
  *
- * @return The result of `PR_Read`: number of characters read or -1 on error.
+ * @return true if successful, false otherwise.
  */
 template <size_t Size>
-static int32_t ReadWritableFile(nsIFile* file, char (&buf)[Size]) {
+static bool GetStatusFileContents(nsIFile* statusFile, char (&buf)[Size]) {
+  static_assert(
+      Size > 16,
+      "Buffer needs to be large enough to hold the known status codes");
+
   PRFileDesc* fd = nullptr;
-  nsresult rv = file->OpenNSPRFileDesc(PR_RDWR, 0660, &fd);
+  nsresult rv = statusFile->OpenNSPRFileDesc(PR_RDWR, 0660, &fd);
   if (NS_FAILED(rv)) {
-    return 0;
+    return false;
   }
 
   const int32_t n = PR_Read(fd, buf, Size);
   PR_Close(fd);
 
-  return n;
+  return (n >= 0);
 }
 
 static nsresult WriteFile(nsIFile* file, nsACString& toWrite) {
@@ -212,9 +214,8 @@ enum UpdateStatus {
 static UpdateStatus GetUpdateStatus(nsIFile* dir,
                                     nsCOMPtr<nsIFile>& statusFile) {
   if (GetStatusFile(dir, statusFile)) {
-    // This buffer must be big enough to hold all valid status codes
     char buf[32];
-    if (ReadWritableFile(statusFile, buf) >= 0) {
+    if (GetStatusFileContents(statusFile, buf)) {
       const char kPending[] = "pending";
       const char kPendingService[] = "pending-service";
       const char kPendingElevate[] = "pending-elevate";
@@ -276,61 +277,6 @@ static bool IsOlderVersion(nsIFile* versionFile, const char* appVersion) {
   return mozilla::Version(appVersion) > buf;
 }
 
-nsresult GetUpdatePatchDir(nsIFile* updRootDir, nsIFile** updatesDirOut) {
-  nsresult rv;
-  nsCOMPtr<nsIFile> updatesDir;
-  rv = updRootDir->Clone(getter_AddRefs(updatesDir));
-  rv = updatesDir->AppendNative("updates"_ns);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = updatesDir->AppendNative("0"_ns);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  updatesDir.forget(updatesDirOut);
-  return NS_OK;
-}
-
-nsresult IsMultiSessionInstallLockoutActive(nsIFile* updRootDir,
-                                            bool& isActive) {
-  nsresult rv;
-
-  nsCOMPtr<nsIFile> timestampFile;
-  rv = GetUpdatePatchDir(updRootDir, getter_AddRefs(timestampFile));
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = timestampFile->AppendNative("update.timestamp"_ns);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // Let's make sure we can hold any valid, unsigned 64 bit integer plus a null.
-  // Maximum 64 bit integer: 18446744073709551615 (20 characters)
-  const size_t bufferSize = 21;
-  char buffer[bufferSize];
-  int32_t readLen = ReadWritableFile(timestampFile, buffer);
-  NS_ENSURE_TRUE(readLen >= 0 && readLen < static_cast<int32_t>(bufferSize),
-                 NS_ERROR_FAILURE);
-  buffer[readLen] = '\0';
-
-  // If we couldn't read anything from the file, the lockout is not active.
-  if (readLen == 0) {
-    isActive = false;
-    return NS_OK;
-  }
-
-  nsDependentCString timestampString(buffer);
-  // This timestamp represents the end of the Multi Session Install Lockout.
-  uint64_t msilEnd = timestampString.ToInteger64(&rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  uint64_t now = PR_Now() / PR_USEC_PER_MSEC;
-
-  isActive = now < msilEnd;
-
-#ifdef DEBUG
-  printf_stderr("Multi Session Install Lockout %s active\n",
-                isActive ? "is" : "is not");
-#endif
-
-  return NS_OK;
-}
-
 nsresult WriteUpdateCompleteTestFile(nsIFile* updRootDir) {
   nsCOMPtr<nsIFile> outputFile;
   nsresult rv = updRootDir->Clone(getter_AddRefs(outputFile));
@@ -767,7 +713,11 @@ nsresult ProcessUpdates(nsIFile* greDir, nsIFile* appDir, nsIFile* updRootDir,
 #endif
 
   nsCOMPtr<nsIFile> updatesDir;
-  rv = GetUpdatePatchDir(updRootDir, getter_AddRefs(updatesDir));
+  rv = updRootDir->Clone(getter_AddRefs(updatesDir));
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = updatesDir->AppendNative("updates"_ns);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = updatesDir->AppendNative("0"_ns);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Return early since there isn't a valid update when the update application

toolkit/xre/nsUpdateDriver.h

@@ -62,20 +62,6 @@ nsresult ProcessUpdates(nsIFile* greDir, nsIFile* appDir, nsIFile* updRootDir,
                         int argc, char** argv, const char* appVersion,
                         bool restart = true, ProcessType* pid = nullptr);
 
-/**
- * Checks if the Multi Session Install Lockout is active. This is a window after
- * an update is downloaded where we won't install updates at startup if another
- * application instance is running.
- *
- * @param  updRootDir
- *         The root update directory for this installation.
- * @param  isActive
- *         Outparam. On success, it is set to `true` if the MSIL lockout is
- *         active or `false` if it is not.
- */
-nsresult IsMultiSessionInstallLockoutActive(nsIFile* updRootDir,
-                                            bool& isActive);
-
 /**
  * This function is only needed for testing. When Firefox is started up with
  * `--test-process-updates`, we go through all the "update at startup" logic,