commit a5505b77d4a9355b2009e40039d8e7817dd08c3f parent 1f42324e5adca11b2e47b5afd23a9d926c51689f Author: Narcis Beleuzu <nbeleuzu@mozilla.com> Date: Fri, 24 Oct 2025 05:00:10 +0300 Revert "Bug 1990634: Release circular reference when done searching zip archives r=kershaw" for causing bc failures on browser_asrouter_targeting.js This reverts commit 685b1d62bc22ada1cc7cc06a514217f387038b79. This reverts commit e4cdb388d44d6438687cea366201f16510659ff2. This reverts commit 9e491f2a6fefcdeff634c3c9b6cdda127d836e3d. This reverts commit ee231a59a0abee2a974a5280b4e75031951a1e48. Diffstat:
22 files changed, 63 insertions(+), 608 deletions(-)
diff --git a/browser/components/backup/Archive.worker.mjs b/browser/components/backup/Archive.worker.mjs @@ -410,8 +410,6 @@ ${ArchiveUtils.INLINE_MIME_END_MARKER} oldBuffer = buffer; } - syncReadFile.close(); - if (!contentType) { throw new BackupError( "Failed to find embedded data in archive", diff --git a/browser/components/backup/BackupService.sys.mjs b/browser/components/backup/BackupService.sys.mjs @@ -19,7 +19,6 @@ import { } from "chrome://browser/content/backup/backup-constants.mjs"; import { BackupError } from "resource:///modules/backup/BackupError.mjs"; -const BACKUP_PREFS_UI_PREF_NAME = "browser.backup.preferences.ui.enabled"; const BACKUP_DIR_PREF_NAME = "browser.backup.location"; const BACKUP_ERROR_CODE_PREF_NAME = "browser.backup.errorCode"; const SCHEDULED_BACKUPS_ENABLED_PREF_NAME = "browser.backup.scheduled.enabled"; @@ -299,14 +298,6 @@ class BinaryReadableStream { * The number of bytes available in the stream */ onDataAvailable(request, stream, offset, count) { - if (this._done) { - // No need to load anything else - abort reading in more - // attachments. - throw Components.Exception( - "Got binary block - cancelling loading the multipart stream.", - Cr.NS_BINDING_ABORTED - ); - } if (!this._enabled) { // We don't care about this data, just move on. return; @@ -328,6 +319,13 @@ class BinaryReadableStream { this._done = true; controller.close(); + + // No need to load anything else - abort reading in more + // attachments. + throw Components.Exception( + "Got binary block - cancelling loading the multipart stream.", + Cr.NS_BINDING_ABORTED + ); } }, @@ -1026,15 +1024,6 @@ export class BackupService extends EventTarget { return null; } - static get #observedPrefs() { - return [ - BACKUP_PREFS_UI_PREF_NAME, - BACKUP_ARCHIVE_ENABLED_PREF_NAME, - BACKUP_RESTORE_ENABLED_PREF_NAME, - SCHEDULED_BACKUPS_ENABLED_PREF_NAME, - ]; - } - /** * Returns a reference to a BackupService singleton. If this is the first time * that this getter is accessed, this causes the BackupService singleton to be @@ -2298,14 +2287,6 @@ export class BackupService extends EventTarget { * The number of bytes available in the stream */ onDataAvailable(request, stream, offset, count) { - if (this._done) { - // No need to load anything else - abort reading in more - // attachments. - throw Components.Exception( - "Got JSON block. Aborting further reads.", - Cr.NS_BINDING_ABORTED - ); - } if (!this._enabled) { // We don't care about this data, just move on. return; @@ -2337,6 +2318,12 @@ export class BackupService extends EventTarget { ) ); } + // No need to load anything else - abort reading in more + // attachments. + throw Components.Exception( + "Got JSON block. Aborting further reads.", + Cr.NS_BINDING_ABORTED + ); } }, @@ -3697,8 +3684,10 @@ export class BackupService extends EventTarget { * * The scheduler will automatically uninitialize itself on the * quit-application-granted observer notification. + * + * @returns {Promise<undefined>} */ - initBackupScheduler() { + async initBackupScheduler() { if (this.#backupSchedulerInitted) { lazy.logConsole.warn( "BackupService scheduler already initting or initted." @@ -3766,21 +3755,14 @@ export class BackupService extends EventTarget { Services.obs.addObserver(this.#observer, "session-cookie-changed"); Services.obs.addObserver(this.#observer, "newtab-linkBlocked"); Services.obs.addObserver(this.#observer, "quit-application-granted"); - Services.obs.addObserver(this.#observer, "sps-profile-created"); - - for (const pref of BackupService.#observedPrefs) { - Services.prefs.addObserver(pref, this.#observer); - } - if (lazy.SelectableProfileService.hasCreatedSelectableProfiles()) { - // Trigger our observer, to make sure backups are disabled. - this.onObserve(null, "sps-profile-created", "true"); - } } /** * Uninitializes the backup scheduling system. + * + * @returns {Promise<undefined>} */ - uninitBackupScheduler() { + async uninitBackupScheduler() { if (!this.#backupSchedulerInitted) { lazy.logConsole.warn( "Tried to uninitBackupScheduler when it wasn't yet enabled." @@ -3811,26 +3793,12 @@ export class BackupService extends EventTarget { Services.obs.removeObserver(this.#observer, "session-cookie-changed"); Services.obs.removeObserver(this.#observer, "newtab-linkBlocked"); Services.obs.removeObserver(this.#observer, "quit-application-granted"); - - for (const pref of BackupService.#observedPrefs) { - Services.prefs.removeObserver(pref, this.#observer); - } - this.#observer = null; this.#regenerationDebouncer.disarm(); this.#backupWriteAbortController.abort(); } - #disableBackupAndRestore() { - if (lazy.SelectableProfileService.hasCreatedSelectableProfiles()) { - // This will end up clearing SCHEDULED_BACKUPS_ENABLED_PREF_NAME. - this.setScheduledBackups(false); - Services.prefs.setBoolPref(BACKUP_ARCHIVE_ENABLED_PREF_NAME, false); - Services.prefs.setBoolPref(BACKUP_RESTORE_ENABLED_PREF_NAME, false); - } - } - /** * Called by this.#observer on idle from the nsIUserIdleService or * quit-application-granted from the nsIObserverService. Exposed as a public @@ -3900,29 +3868,6 @@ export class BackupService extends EventTarget { } break; } - case "sps-profile-created": { - // Disallow (scheduled and manual) backups of selectable profiles and - // restores from selectable profiles. See bug 1990980. - if (data == "true") { - this.#disableBackupAndRestore(); - } else { - lazy.logConsole.error("Received sps-profile-created = false"); - } - break; - } - case "nsPref:changed": { - switch (data) { - case BACKUP_ARCHIVE_ENABLED_PREF_NAME: - case BACKUP_RESTORE_ENABLED_PREF_NAME: - case SCHEDULED_BACKUPS_ENABLED_PREF_NAME: - case BACKUP_PREFS_UI_PREF_NAME: - // Disallow (scheduled and manual) backups of selectable profiles and - // restores from selectable profiles. See bug 1990980. - this.#disableBackupAndRestore(); - break; - } - break; - } } } @@ -4434,10 +4379,7 @@ export class BackupService extends EventTarget { "Attempting to delete last backup file at ", backupFilePath ); - await IOUtils.remove(backupFilePath, { - ignoreAbsent: true, - retryReadonly: true, - }); + await IOUtils.remove(backupFilePath, { ignoreAbsent: true }); } this.#_state.lastBackupDate = null; diff --git a/browser/components/backup/tests/xpcshell/head.js b/browser/components/backup/tests/xpcshell/head.js @@ -15,7 +15,6 @@ ChromeUtils.defineESModuleGetters(this, { MockRegistrar: "resource://testing-common/MockRegistrar.sys.mjs", PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.sys.mjs", AppConstants: "resource://gre/modules/AppConstants.sys.mjs", - setTimeout: "resource://gre/modules/Timer.sys.mjs", }); const HISTORY_ENABLED_PREF = "places.history.enabled"; @@ -48,10 +47,6 @@ add_setup(async () => { await OSKeyStoreTestUtils.cleanup(); MockRegistrar.unregister(osKeyStoreCID); }); - - // This will set gDirServiceProvider, which doesn't happen automatically - // in xpcshell tests, but is needed by BackupService. - Cc["@mozilla.org/xre/directory-provider;1"].getService(Ci.nsIXREDirProvider); }); const BYTES_IN_KB = 1000; @@ -203,62 +198,21 @@ async function assertFilesExist(parentPath, testFilesArray) { } /** - * Perform removalFn, potentially a few times, with special consideration for - * Windows, which has issues with file removal. Sometimes remove() throws - * ERROR_LOCK_VIOLATION when the file is not unlocked soon enough. This can - * happen because kernel operations still hold a handle to it, or because e.g. - * Windows Defender started a scan, or whatever. Some applications (for - * example SQLite) retry deletes with a delay on Windows. We emulate that - * here, since this happens very frequently in tests. - * - * This registers a test failure if it does not succeed. - * - * @param {Function} removalFn Async function to (potentially repeatedly) - * call. - * @throws The resultant file system exception if removal fails, despite the - * special considerations. - */ -async function doFileRemovalOperation(removalFn) { - const kMaxRetries = 5; - let nRetries = 0; - let lastException; - while (nRetries < kMaxRetries) { - nRetries++; - try { - await removalFn(); - // Success! - return; - } catch (error) { - if ( - AppConstants.platform !== "win" || - !/NS_ERROR_FILE_IS_LOCKED/.test(error.message) - ) { - throw error; - } - lastException = error; - } - await new Promise(res => setTimeout(res, 100)); - } - - // All retries failed. Re-throw last exception. - Assert.ok(false, `doRemovalOperation failed after ${nRetries} attempts`); - throw lastException; -} - -/** * Remove a file or directory at a path if it exists and files are unlocked. - * Prefer this to IOUtils.remove in tests to avoid intermittent failures - * because of the Windows-ism mentioned in doFileRemovalOperation. * * @param {string} path path to remove. */ async function maybeRemovePath(path) { - let nAttempts = 0; - await doFileRemovalOperation(async () => { - nAttempts++; + try { await IOUtils.remove(path, { ignoreAbsent: true, recursive: true }); - Assert.ok(true, `Removed ${path} on attempt #${nAttempts}`); - }); + } catch (error) { + // Sometimes remove() throws when the file is not unlocked soon + // enough. + if (error.name != "NS_ERROR_FILE_IS_LOCKED") { + // Ignoring any errors, as the temp folder will be cleaned up. + console.error(error); + } + } } /** diff --git a/browser/components/backup/tests/xpcshell/test_BackupService.js b/browser/components/backup/tests/xpcshell/test_BackupService.js @@ -3,6 +3,9 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; +const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" +); const { BasePromiseWorker } = ChromeUtils.importESModule( "resource://gre/modules/PromiseWorker.sys.mjs" ); @@ -337,7 +340,7 @@ async function testCreateBackupHelper(sandbox, taskFn) { "initiated recovery" ); - await taskFn(bs, manifest); + taskFn(bs, manifest); await maybeRemovePath(backupFilePath); await maybeRemovePath(fakeProfilePath); @@ -406,11 +409,7 @@ async function testDeleteLastBackupHelper(taskFn) { await taskFn(LAST_BACKUP_FILE_PATH); } - // NB: On Windows, deletes of backups in tests run into an issue where - // the file is locked briefly by the system and deletes fail with - // NS_ERROR_FILE_IS_LOCKED. See doFileRemovalOperation for details. - // We therefore retry this delete a few times before accepting failure. - await doFileRemovalOperation(async () => await bs.deleteLastBackup()); + await bs.deleteLastBackup(); Assert.equal( bs.state.lastBackupDate, @@ -808,126 +807,6 @@ add_task( ); /** - * Tests that the existence of selectable profiles prevent backups (see bug - * 1990980). - * - * @param {boolean} aSetCreatedSelectableProfilesBeforeSchedulingBackups - * If true (respectively, false), set browser.profiles.created before - * (respectively, after) attempting to setScheduledBackups. - */ -async function testSelectableProfilesPreventBackup( - aSetCreatedSelectableProfilesBeforeSchedulingBackups -) { - let sandbox = sinon.createSandbox(); - Services.fog.testResetFOG(); - const TEST_UID = "ThisIsMyTestUID"; - const TEST_EMAIL = "foxy@mozilla.org"; - sandbox.stub(UIState, "get").returns({ - status: UIState.STATUS_SIGNED_IN, - uid: TEST_UID, - email: TEST_EMAIL, - }); - - const SELECTABLE_PROFILES_CREATED_PREF = "browser.profiles.created"; - - // Make sure backup and restore are enabled. - const BACKUP_ARCHIVE_ENABLED_PREF_NAME = "browser.backup.archive.enabled"; - const BACKUP_RESTORE_ENABLED_PREF_NAME = "browser.backup.restore.enabled"; - Services.prefs.setBoolPref(BACKUP_ARCHIVE_ENABLED_PREF_NAME, true); - Assert.ok( - Services.prefs.getBoolPref(BACKUP_ARCHIVE_ENABLED_PREF_NAME), - `${BACKUP_ARCHIVE_ENABLED_PREF_NAME} = true` - ); - Services.prefs.setBoolPref(BACKUP_RESTORE_ENABLED_PREF_NAME, true); - Assert.ok( - Services.prefs.getBoolPref(BACKUP_RESTORE_ENABLED_PREF_NAME), - `${BACKUP_RESTORE_ENABLED_PREF_NAME} = true` - ); - - // Make sure created profiles pref is not set until we want it to be. - Services.prefs.setBoolPref(SELECTABLE_PROFILES_CREATED_PREF, false); - Assert.ok( - !Services.prefs.getBoolPref(SELECTABLE_PROFILES_CREATED_PREF), - `${SELECTABLE_PROFILES_CREATED_PREF} = false` - ); - - const setHasSelectableProfiles = () => { - // "Enable" selectable profiles by pref. - Services.prefs.setBoolPref(SELECTABLE_PROFILES_CREATED_PREF, true); - Assert.ok( - Services.prefs.getBoolPref(SELECTABLE_PROFILES_CREATED_PREF), - "set has selectable profiles | browser.profiles.created = true" - ); - }; - - if (aSetCreatedSelectableProfilesBeforeSchedulingBackups) { - setHasSelectableProfiles(); - } - - let bs = new BackupService({}); - bs.initBackupScheduler(); - bs.setScheduledBackups(true); - - const SCHEDULED_BACKUP_ENABLED_PREF = "browser.backup.scheduled.enabled"; - if (!aSetCreatedSelectableProfilesBeforeSchedulingBackups) { - Assert.ok( - Services.prefs.getBoolPref(SCHEDULED_BACKUP_ENABLED_PREF, true), - "enabled scheduled backups | browser.backup.scheduled.enabled = true" - ); - registerCleanupFunction(() => { - // Just in case the test fails. - bs.setScheduledBackups(false); - info("cleared scheduled backups"); - }); - - setHasSelectableProfiles(); - } - - // browser.backup.archive.enabled, browser.backup.restore.enabled and - // browser.backup.scheduled.enabled should ignore earlier setting and be - // false. - Assert.ok( - !Services.prefs.getBoolPref(BACKUP_ARCHIVE_ENABLED_PREF_NAME, true), - `backups are disabled | ${BACKUP_ARCHIVE_ENABLED_PREF_NAME} = false` - ); - Assert.ok( - !Services.prefs.getBoolPref(BACKUP_RESTORE_ENABLED_PREF_NAME, true), - `restore is disabled | ${BACKUP_RESTORE_ENABLED_PREF_NAME} = false` - ); - Assert.ok( - !Services.prefs.getBoolPref(SCHEDULED_BACKUP_ENABLED_PREF, true), - `scheduled backups are not enabled | ${SCHEDULED_BACKUP_ENABLED_PREF} = false` - ); - - // Test cleanup - if (!aSetCreatedSelectableProfilesBeforeSchedulingBackups) { - bs.uninitBackupScheduler(); - } - - Services.prefs.clearUserPref(SELECTABLE_PROFILES_CREATED_PREF); - // These tests assume that backups and restores have been enabled. - Services.prefs.setBoolPref(BACKUP_ARCHIVE_ENABLED_PREF_NAME, true); - Services.prefs.setBoolPref(BACKUP_RESTORE_ENABLED_PREF_NAME, true); - sandbox.restore(); -} - -add_task( - async function test_managing_profiles_before_scheduling_prevents_backup() { - await testSelectableProfilesPreventBackup( - true /* aSetCreatedSelectableProfilesBeforeSchedulingBackups */ - ); - } -); - -add_task( - async function test_managing_profiles_after_scheduling_prevents_backup() { - await testSelectableProfilesPreventBackup( - false /* aSetCreatedSelectableProfilesBeforeSchedulingBackups */ - ); - } -); - -/** * Tests that if there's a post-recovery.json file in the profile directory * when checkForPostRecovery() is called, that it is processed, and the * postRecovery methods on the associated BackupResources are called with the diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_archive.js b/browser/components/backup/tests/xpcshell/test_BackupService_archive.js @@ -69,7 +69,7 @@ add_setup(async () => { registerCleanupFunction(async () => { await OSKeyStoreTestUtils.cleanup(); - await maybeRemovePath(testProfilePath); + await IOUtils.remove(testProfilePath, { recursive: true }); }); }); @@ -112,8 +112,8 @@ add_task(async function test_createArchive_unencrypted() { assertExtractionsMatch(EXTRACTION_PATH); - await maybeRemovePath(FAKE_ARCHIVE_PATH); - await maybeRemovePath(EXTRACTION_PATH); + await IOUtils.remove(FAKE_ARCHIVE_PATH); + await IOUtils.remove(EXTRACTION_PATH); }); /** @@ -167,8 +167,8 @@ add_task(async function test_createArchive_encrypted() { assertExtractionsMatch(EXTRACTION_PATH); - await maybeRemovePath(FAKE_ARCHIVE_PATH); - await maybeRemovePath(EXTRACTION_PATH); + await IOUtils.remove(FAKE_ARCHIVE_PATH); + await IOUtils.remove(EXTRACTION_PATH); }); /** @@ -313,8 +313,8 @@ add_task(async function test_createArchive_encrypted_truncated() { "Extraction should have been automatically destroyed." ); - await maybeRemovePath(FAKE_ARCHIVE_PATH); - await maybeRemovePath(FAKE_COMPRESSED_FILE); + await IOUtils.remove(FAKE_ARCHIVE_PATH); + await IOUtils.remove(FAKE_COMPRESSED_FILE); }); /** @@ -388,7 +388,7 @@ add_task(async function test_createArchive_early_binary_stream_close() { "Extraction should have been automatically destroyed." ); - await maybeRemovePath(FAKE_ARCHIVE_PATH); + await IOUtils.remove(FAKE_ARCHIVE_PATH); }); /** @@ -459,7 +459,7 @@ add_task(async function test_missing_binary_block() { /Could not find binary block/ ); - await maybeRemovePath(fakeRecoveryPath); + await IOUtils.remove(fakeRecoveryPath); }); /** diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_enable_disable_encryption.js b/browser/components/backup/tests/xpcshell/test_BackupService_enable_disable_encryption.js @@ -3,6 +3,10 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; +const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" +); + const TEST_PASSWORD = "This is some test password."; add_setup(async () => { diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_recoverFromSnapshotFolder.js b/browser/components/backup/tests/xpcshell/test_BackupService_recoverFromSnapshotFolder.js @@ -3,6 +3,9 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; +const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" +); const { ArchiveUtils } = ChromeUtils.importESModule( "resource:///modules/backup/ArchiveUtils.sys.mjs" ); diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_regeneration.js b/browser/components/backup/tests/xpcshell/test_BackupService_regeneration.js @@ -3,6 +3,9 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; +const { setTimeout } = ChromeUtils.importESModule( + "resource://gre/modules/Timer.sys.mjs" +); const { NetUtil } = ChromeUtils.importESModule( "resource://gre/modules/NetUtil.sys.mjs" ); diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_resolveArchiveDestFolderPath.js b/browser/components/backup/tests/xpcshell/test_BackupService_resolveArchiveDestFolderPath.js @@ -3,6 +3,10 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; +const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" +); + const HOME_KEY = "Home"; let gTestRoot; let gFakeHomePath; diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_retryHeuristic.js b/browser/components/backup/tests/xpcshell/test_BackupService_retryHeuristic.js @@ -4,8 +4,10 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; ChromeUtils.defineESModuleGetters(this, { + setTimeout: "resource://gre/modules/Timer.sys.mjs", BackupError: "resource:///modules/backup/BackupError.mjs", ERRORS: "chrome://browser/content/backup/backup-constants.mjs", + AppConstants: "resource://gre/modules/AppConstants.sys.mjs", }); const BACKUP_RETRY_LIMIT_PREF_NAME = "browser.backup.backup-retry-limit"; diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_scheduler.js b/browser/components/backup/tests/xpcshell/test_BackupService_scheduler.js @@ -54,7 +54,7 @@ add_task(async function test_init_uninitBackupScheduler() { sandbox.stub(idleService, "addIdleObserver"); sandbox.stub(idleService, "removeIdleObserver"); - bs.initBackupScheduler(); + await bs.initBackupScheduler(); Assert.ok( idleService.addIdleObserver.calledOnce, "addIdleObserver was called" diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_takeMeasurements.js b/browser/components/backup/tests/xpcshell/test_BackupService_takeMeasurements.js @@ -193,9 +193,8 @@ add_task(async function test_BackupService_location_on_device() { ); Services.telemetry.clearScalars(); - // The default will be either the test machine's OneDrive folder, if present, - // or else the Docs folder. - bs.setParentDirPath(BackupService.DEFAULT_PARENT_DIR_PATH); + // The system "Docs" folder is considered the default location. + bs.setParentDirPath(Services.dirsvc.get("Docs", Ci.nsIFile).path); await bs.takeMeasurements(); Assert.equal( diff --git a/browser/components/profiles/SelectableProfileService.sys.mjs b/browser/components/profiles/SelectableProfileService.sys.mjs @@ -231,14 +231,6 @@ class SelectableProfileServiceClass extends EventEmitter { () => this.updateEnabledState(), "profile-after-change" ); - - Services.prefs.addObserver(PROFILES_CREATED_PREF_NAME, () => - Services.obs.notifyObservers( - null, - "sps-profile-created", - lazy.PROFILES_CREATED ? "true" : "false" - ) - ); } // Migrate any early users who created profiles before the datastore service @@ -250,10 +242,6 @@ class SelectableProfileServiceClass extends EventEmitter { } } - hasCreatedSelectableProfiles() { - return Services.prefs.getBoolPref(PROFILES_CREATED_PREF_NAME, false); - } - #getEnabledState() { if (!Services.policies.isAllowed("profileManagement")) { return false; diff --git a/modules/libjar/nsZipArchive.cpp b/modules/libjar/nsZipArchive.cpp @@ -582,8 +582,7 @@ nsresult nsZipArchive::FindInit(const char* aPattern, nsZipFind** aFind) { // nsZipFind::FindNext //--------------------------------------------- nsresult nsZipFind::FindNext(const char** aResult, uint16_t* aNameLen) { - if (!aResult || !aNameLen) return NS_ERROR_ILLEGAL_VALUE; - NS_ENSURE_TRUE(mArchive, NS_ERROR_FILE_NOT_FOUND); + if (!mArchive || !aResult || !aNameLen) return NS_ERROR_ILLEGAL_VALUE; MutexAutoLock lock(mArchive->mLock); *aResult = 0; @@ -617,9 +616,6 @@ nsresult nsZipFind::FindNext(const char** aResult, uint16_t* aNameLen) { } MMAP_FAULT_HANDLER_CATCH(NS_ERROR_FAILURE) LOG(("ZipHandle::FindNext[%p] not found %s", this, mPattern)); - // Release the archive. - mArchive = nullptr; - mItem = nullptr; return NS_ERROR_FILE_NOT_FOUND; } @@ -971,7 +967,6 @@ nsZipFind::nsZipFind(nsZipArchive* aZip, char* aPattern, bool aRegExp) mSlot(0), mRegExp(aRegExp) { MOZ_COUNT_CTOR(nsZipFind); - MOZ_ASSERT(mArchive); } nsZipFind::~nsZipFind() { diff --git a/widget/windows/WindowsTestDebug.cpp b/widget/windows/WindowsTestDebug.cpp @@ -1,146 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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 "WindowsTestDebug.h" -#include "nsCOMPtr.h" -#include "nsIFile.h" -#include "nsTArray.h" -#include "nsXPCOM.h" - -namespace { -typedef DWORD(WINAPI* RMSTARTSESSION)(DWORD*, DWORD, WCHAR[]); -typedef DWORD(WINAPI* RMREGISTERRESOURCES)(DWORD, UINT, LPCWSTR[], UINT, - RM_UNIQUE_PROCESS[], UINT, - LPCWSTR[]); -typedef DWORD(WINAPI* RMGETLIST)(DWORD, UINT*, UINT*, RM_PROCESS_INFO[], - LPDWORD); -typedef DWORD(WINAPI* RMENDSESSION)(DWORD); -} // namespace - -namespace mozilla::widget { - -class WindowsDebugProcessData : public nsIWindowsDebugProcessData { - public: - NS_DECL_ISUPPORTS - - WindowsDebugProcessData(const wchar_t* aName, const wchar_t* aFilePath, - unsigned long aPid) - : mName(aName), mFilePath(aFilePath), mPid(aPid) {} - - NS_IMETHODIMP GetName(nsAString& aOut) { - aOut = mName; - return NS_OK; - } - - NS_IMETHODIMP GetExecutablePath(nsAString& aOut) { - aOut = mFilePath; - return NS_OK; - } - - NS_IMETHODIMP GetPid(uint32_t* aOut) { - *aOut = mPid; - return NS_OK; - } - - private: - virtual ~WindowsDebugProcessData() = default; - nsString mName; - nsString mFilePath; - unsigned long mPid; -}; - -NS_IMPL_ISUPPORTS(WindowsDebugProcessData, nsIWindowsDebugProcessData) -NS_IMPL_ISUPPORTS(WindowsTestDebug, nsIWindowsTestDebug) - -WindowsTestDebug::WindowsTestDebug() {} - -WindowsTestDebug::~WindowsTestDebug() {} - -static nsReturnRef<HMODULE> MakeRestartManager() { - nsModuleHandle module(::LoadLibraryW(L"Rstrtmgr.dll")); - (void)NS_WARN_IF(!module); - return module.out(); -} - -NS_IMETHODIMP -WindowsTestDebug::ProcessesThatOpenedFile( - const nsAString& aFilename, - nsTArray<RefPtr<nsIWindowsDebugProcessData>>& aResult) { - nsModuleHandle module(MakeRestartManager()); - if (!module) { - return NS_ERROR_NOT_AVAILABLE; - } - auto RmStartSession = reinterpret_cast<RMSTARTSESSION>( - ::GetProcAddress(module, "RmStartSession")); - if (NS_WARN_IF(!RmStartSession)) { - return NS_ERROR_NOT_INITIALIZED; - } - auto RmRegisterResources = reinterpret_cast<RMREGISTERRESOURCES>( - ::GetProcAddress(module, "RmRegisterResources")); - if (NS_WARN_IF(!RmRegisterResources)) { - return NS_ERROR_NOT_INITIALIZED; - } - auto RmGetList = - reinterpret_cast<RMGETLIST>(::GetProcAddress(module, "RmGetList")); - if (NS_WARN_IF(!RmGetList)) { - return NS_ERROR_NOT_INITIALIZED; - } - auto RmEndSession = - reinterpret_cast<RMENDSESSION>(::GetProcAddress(module, "RmEndSession")); - if (NS_WARN_IF(!RmEndSession)) { - return NS_ERROR_NOT_INITIALIZED; - } - - WCHAR sessionKey[CCH_RM_SESSION_KEY + 1] = {0}; - DWORD handle; - if (NS_WARN_IF(FAILED(RmStartSession(&handle, 0, sessionKey)))) { - return NS_ERROR_FAILURE; - } - - auto endSession = MakeScopeExit( - [&, handle]() { (void)NS_WARN_IF(FAILED(RmEndSession(handle))); }); - - LPCWSTR resources[] = {PromiseFlatString(aFilename).get()}; - if (NS_WARN_IF(FAILED( - RmRegisterResources(handle, 1, resources, 0, nullptr, 0, nullptr)))) { - return NS_ERROR_FAILURE; - } - - AutoTArray<RM_PROCESS_INFO, 1> info; - UINT numEntries; - UINT numEntriesNeeded = 1; - DWORD error = ERROR_MORE_DATA; - DWORD reason = RmRebootReasonNone; - while (error == ERROR_MORE_DATA) { - info.SetLength(numEntriesNeeded); - numEntries = numEntriesNeeded; - error = - RmGetList(handle, &numEntriesNeeded, &numEntries, &info[0], &reason); - } - if (NS_WARN_IF(error != ERROR_SUCCESS)) { - return NS_ERROR_FAILURE; - } - - for (UINT i = 0; i < numEntries; ++i) { - nsAutoHandle otherProcess(::OpenProcess( - PROCESS_QUERY_LIMITED_INFORMATION, FALSE, info[i].Process.dwProcessId)); - if (NS_WARN_IF(!otherProcess)) { - continue; - } - WCHAR imageName[MAX_PATH]; - DWORD imageNameLen = MAX_PATH; - if (NS_WARN_IF(FAILED(::QueryFullProcessImageNameW( - otherProcess, 0, imageName, &imageNameLen)))) { - continue; - } - aResult.AppendElement(new WindowsDebugProcessData( - info[i].strAppName, imageName, info[i].Process.dwProcessId)); - } - - return NS_OK; -} - -} // namespace mozilla::widget diff --git a/widget/windows/WindowsTestDebug.h b/widget/windows/WindowsTestDebug.h @@ -1,31 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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/. */ - -#ifndef mozilla_widget_WindowsTestDebug_h__ -#define mozilla_widget_WindowsTestDebug_h__ - -#include <windows.h> -#include <restartmanager.h> - -#include "nsIWindowsTestDebug.h" -#include "nsString.h" -#include "nsWindowsHelpers.h" - -namespace mozilla::widget { - -class WindowsTestDebug final : public nsIWindowsTestDebug { - public: - NS_DECL_ISUPPORTS - NS_DECL_NSIWINDOWSTESTDEBUG - WindowsTestDebug(); - - private: - ~WindowsTestDebug(); -}; - -} // namespace mozilla::widget - -#endif // mozilla_widget_WindowsTestDebug_h__ diff --git a/widget/windows/components.conf b/widget/windows/components.conf @@ -135,14 +135,6 @@ Classes = [ 'headers': ['/widget/windows/SystemStatusBar.h'], 'processes': ProcessSelector.MAIN_PROCESS_ONLY, }, - { - 'name': 'nsIWindowsTestDebug', - 'cid': '{2d63d37b-d0d8-4427-b2ee-3ad706fa923f}', - 'contract_ids': ['@mozilla.org/win-test-debug;1'], - 'type': 'mozilla::widget::WindowsTestDebug', - 'headers': ['/widget/windows/WindowsTestDebug.h'], - 'processes': ProcessSelector.MAIN_PROCESS_ONLY, - }, ] if buildconfig.substs['CC_TYPE'] == 'clang-cl': diff --git a/widget/windows/moz.build b/widget/windows/moz.build @@ -114,7 +114,6 @@ UNIFIED_SOURCES += [ "WinCompositorWindowThread.cpp", "WindowHook.cpp", "WindowsConsole.cpp", - "WindowsTestDebug.cpp", "WinEventObserver.cpp", "WinIMEHandler.cpp", "WinMouseScrollHandler.cpp", @@ -173,7 +172,6 @@ if CONFIG["MOZ_ENABLE_SKIA_PDF"]: XPIDL_SOURCES += [ "nsIAlertsServiceRust.idl", - "nsIWindowsTestDebug.idl", ] XPIDL_MODULE = "widget_windows" diff --git a/widget/windows/nsIWindowsTestDebug.idl b/widget/windows/nsIWindowsTestDebug.idl @@ -1,40 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- - * - * This Source Code Form is subject to the terms of the Mozilla Public - * 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 "nsISupports.idl" - -[scriptable, uuid(19904b57-e6b8-4c0f-86eb-5ffce4c8d675)] -interface nsIWindowsDebugProcessData : nsISupports -{ - // "User-friendly name" of the process. - readonly attribute AString name; - - // Full path to the executable of the process. - readonly attribute AString executablePath; - - // PID of the process. - readonly attribute unsigned long pid; -}; - -// Useful for debugging Windows errors related to resources in use. This class -// should only be used for debugging purposes. -[scriptable, uuid(15725b2d-08d1-4b18-9e83-0cccfb87b982)] -interface nsIWindowsTestDebug : nsISupports -{ - /** - * Get the list of processes that currently have an open handle to aFilename. - * This can be used to determine what other processess (if any) are - * preventing access to a file, or even if this process still has open - * handles to it. Currently, this list will not include file handles - * obtained via memory mapping, which can be kept beyond closing the handle - * that they mapped on Windows. This behavior is obviously racy. - * - * @param aFilepath Full path to the file we are querying. - * @returns An array of the processes that held handles to aFilename at the - * time the function was executed. - */ - Array<nsIWindowsDebugProcessData> processesThatOpenedFile(in AString aFilename); -}; diff --git a/widget/windows/tests/unit/test_keep_file_open.worker.js b/widget/windows/tests/unit/test_keep_file_open.worker.js @@ -1,43 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/. */ - -/* import-globals-from /toolkit/components/workerloader/require.js */ -importScripts("resource://gre/modules/workers/require.js"); - -const PromiseWorker = require("resource://gre/modules/workers/PromiseWorker.js"); - -/** - * - */ -class OpenFileWorker extends PromiseWorker.AbstractWorker { - constructor() { - super(); - - this._file = null; - } - - postMessage(message, ...transfers) { - self.postMessage(message, transfers); - } - - dispatch(method, args) { - return this[method](...args); - } - - open(path) { - this._file = IOUtils.openFileForSyncReading(path); - } - - close() { - if (this._file) { - this._file.close(); - } - } -} - -const worker = new OpenFileWorker(); - -self.addEventListener("message", msg => worker.handleMessage(msg)); -self.addEventListener("unhandledrejection", err => { - throw err.reason; -}); diff --git a/widget/windows/tests/unit/test_windowsTestDebug.js b/widget/windows/tests/unit/test_windowsTestDebug.js @@ -1,43 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -/* - * Test nsIWindowsTestDebug. - */ - -const { BasePromiseWorker } = ChromeUtils.importESModule( - "resource://gre/modules/PromiseWorker.sys.mjs" -); - -add_task(async () => { - const path = await IOUtils.createUniqueFile(PathUtils.tempDir, "openedFile"); - await IOUtils.writeUTF8(path, ""); - Assert.ok(await IOUtils.exists(path), path + " should have been created"); - registerCleanupFunction(async () => { - await IOUtils.remove(path); - }); - - // Use a worker to keep the testFile open. - const worker = new BasePromiseWorker( - "resource://test/test_keep_file_open.worker.js" - ); - await worker.post("open", [path]); - - // Check that nsIWindowsTestDebug tells us that we have the file open. - const WindowsTestDebug = Cc["@mozilla.org/win-test-debug;1"].getService( - Ci.nsIWindowsTestDebug - ); - let procs = WindowsTestDebug.processesThatOpenedFile(path); - info(JSON.stringify(procs)); - Assert.equal(procs.length, 1, "Process list contains one process"); - const ProcessTools = Cc["@mozilla.org/processtools-service;1"].getService( - Ci.nsIProcessToolsService - ); - Assert.equal( - procs[0].pid, - ProcessTools.pid, - "Process list contains this process" - ); - await worker.post("close", []); - await IOUtils.remove(path); -}); diff --git a/widget/windows/tests/unit/xpcshell.toml b/widget/windows/tests/unit/xpcshell.toml @@ -1,7 +1,4 @@ [DEFAULT] run-if = ["os == 'win'"] -["test_windowsTestDebug.js"] -support-files=["test_keep_file_open.worker.js"] - ["test_windows_alert_service.js"]