From 8246ea0b6cd16f5d3f7faed1bc743cacf6e39762 Mon Sep 17 00:00:00 2001 From: Robin Steuber Date: Thu, 24 Apr 2025 01:43:32 +0000 Subject: [PATCH] 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 | 54 +++++++------------- toolkit/xre/nsUpdateDriver.cpp | 92 ++++++++-------------------------- toolkit/xre/nsUpdateDriver.h | 14 ------ 3 files changed, 40 insertions(+), 120 deletions(-) diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index c6f03084b468..484a048b431e 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -4581,8 +4581,7 @@ enum struct ShouldNotProcessUpdatesReason { DevToolsLaunching, NotAnUpdatingTask, OtherInstanceRunning, - FirstStartup, - MultiSessionInstallLockout + FirstStartup }; 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 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 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 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 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 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 = - ShouldNotProcessUpdates(mDirProvider, updRoot); + ShouldNotProcessUpdates(mDirProvider); if (shouldNotProcessUpdatesReason.isNothing()) { nsCOMPtr exeFile, exeDir; rv = mDirProvider.GetFile(XRE_EXECUTABLE_FILE, &persistent, diff --git a/toolkit/xre/nsUpdateDriver.cpp b/toolkit/xre/nsUpdateDriver.cpp index 6ba3bb4c448d..c2bd7e285201 100644 --- a/toolkit/xre/nsUpdateDriver.cpp +++ b/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 - * access. The reason it is opened for both read and write is to prevent trying - * to update when the user doesn't have write access to the update directory. - * Otherwise we will loop infinitely and try to install it over and over. + * Get the contents of the update.status file when the update.status file can + * be opened with read and write access. The reason it is opened for both read + * and write is to prevent trying to update when the user doesn't have write + * access to the update directory. * - * @param file - * The file object. - * @param buf - * The buffer holding the file contents. + * @param statusFile the status file object. + * @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 -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& 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 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 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(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 outputFile; nsresult rv = updRootDir->Clone(getter_AddRefs(outputFile)); @@ -767,7 +713,11 @@ nsresult ProcessUpdates(nsIFile* greDir, nsIFile* appDir, nsIFile* updRootDir, #endif nsCOMPtr 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 diff --git a/toolkit/xre/nsUpdateDriver.h b/toolkit/xre/nsUpdateDriver.h index a308ed4c5ac5..133de05f17b8 100644 --- a/toolkit/xre/nsUpdateDriver.h +++ b/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,