diff --git a/browser/components/BrowserComponents.manifest b/browser/components/BrowserComponents.manifest index df8af7837404..43b61d763173 100644 --- a/browser/components/BrowserComponents.manifest +++ b/browser/components/BrowserComponents.manifest @@ -34,4 +34,19 @@ category browser-idle-startup resource://gre/modules/UpdateListener.sys.mjs Upda category browser-idle-startup resource:///modules/WindowsJumpLists.sys.mjs WinTaskbarJumpList.startup #endif +# App shutdown consumers +category browser-quit-application-granted resource:///modules/BrowserUsageTelemetry.sys.mjs BrowserUsageTelemetry.uninit +category browser-quit-application-granted resource:///modules/Interactions.sys.mjs Interactions.uninit +category browser-quit-application-granted resource:///modules/pagedata/PageDataService.sys.mjs PageDataService.uninit +category browser-quit-application-granted resource://gre/modules/PageThumbs.sys.mjs PageThumbs.uninit +category browser-quit-application-granted resource://gre/modules/NewTabUtils.sys.mjs NewTabUtils.uninit +category browser-quit-application-granted resource://normandy/Normandy.sys.mjs Normandy.uninit +category browser-quit-application-granted resource://gre/modules/RFPHelper.sys.mjs RFPHelper.uninit +category browser-quit-application-granted resource:///modules/ShoppingUtils.sys.mjs ShoppingUtils.uninit +category browser-quit-application-granted resource:///modules/asrouter/ASRouterNewTabHook.sys.mjs ASRouterNewTabHook.destroy +#ifdef MOZ_UPDATER +category browser-quit-application-granted resource://gre/modules/UpdateListener.sys.mjs UpdateListener.reset +#endif +category browser-quit-application-granted resource:///modules/UrlbarSearchTermsPersistence.sys.mjs UrlbarSearchTermsPersistence.uninit + category search-service-notification resource:///modules/SearchUIUtils.sys.mjs SearchUIUtils.showSearchServiceNotification diff --git a/browser/components/BrowserGlue.sys.mjs b/browser/components/BrowserGlue.sys.mjs index 10e6051ebb92..d77a228bd50b 100644 --- a/browser/components/BrowserGlue.sys.mjs +++ b/browser/components/BrowserGlue.sys.mjs @@ -74,7 +74,6 @@ ChromeUtils.defineESModuleGetters(lazy, { PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.sys.mjs", ProcessHangMonitor: "resource:///modules/ProcessHangMonitor.sys.mjs", QuickSuggest: "resource:///modules/QuickSuggest.sys.mjs", - RFPHelper: "resource://gre/modules/RFPHelper.sys.mjs", RemoteSecuritySettings: "resource://gre/modules/psm/RemoteSecuritySettings.sys.mjs", RemoteSettings: "resource://services-settings/remote-settings.sys.mjs", @@ -92,7 +91,6 @@ ChromeUtils.defineESModuleGetters(lazy, { SessionStore: "resource:///modules/sessionstore/SessionStore.sys.mjs", ShellService: "resource:///modules/ShellService.sys.mjs", ShortcutUtils: "resource://gre/modules/ShortcutUtils.sys.mjs", - ShoppingUtils: "resource:///modules/ShoppingUtils.sys.mjs", SpecialMessageActions: "resource://messaging-system/lib/SpecialMessageActions.sys.mjs", TelemetryReportingPolicy: @@ -100,8 +98,6 @@ ChromeUtils.defineESModuleGetters(lazy, { TRRRacer: "resource:///modules/TRRPerformance.sys.mjs", TabCrashHandler: "resource:///modules/ContentCrashHandlers.sys.mjs", UrlbarPrefs: "resource:///modules/UrlbarPrefs.sys.mjs", - UrlbarSearchTermsPersistence: - "resource:///modules/UrlbarSearchTermsPersistence.sys.mjs", UsageReporting: "resource://gre/modules/UsageReporting.sys.mjs", WebChannel: "resource://gre/modules/WebChannel.sys.mjs", WebProtocolHandlerRegistrar: @@ -113,24 +109,6 @@ ChromeUtils.defineESModuleGetters(lazy, { setTimeout: "resource://gre/modules/Timer.sys.mjs", }); -if (AppConstants.MOZ_UPDATER) { - ChromeUtils.defineESModuleGetters(lazy, { - UpdateListener: "resource://gre/modules/UpdateListener.sys.mjs", - }); - XPCOMUtils.defineLazyServiceGetters(lazy, { - UpdateServiceStub: [ - "@mozilla.org/updates/update-service-stub;1", - "nsIApplicationUpdateServiceStub", - ], - }); -} -if (AppConstants.MOZ_UPDATE_AGENT) { - ChromeUtils.defineESModuleGetters(lazy, { - BackgroundUpdate: "resource://gre/modules/BackgroundUpdate.sys.mjs", - }); -} - -// PluginManager is used in the listeners object below. XPCOMUtils.defineLazyServiceGetters(lazy, { BrowserHandler: ["@mozilla.org/browser/clh;1", "nsIBrowserHandler"], PushService: ["@mozilla.org/push/Service;1", "nsIPushService"], @@ -1032,6 +1010,12 @@ const listeners = { }, }; if (AppConstants.MOZ_UPDATER) { + ChromeUtils.defineESModuleGetters(lazy, { + // This listeners/observers/lazy indirection is too much for eslint: + // eslint-disable-next-line mozilla/valid-lazy + UpdateListener: "resource://gre/modules/UpdateListener.sys.mjs", + }); + listeners.observers["update-downloading"] = ["UpdateListener"]; listeners.observers["update-staged"] = ["UpdateListener"]; listeners.observers["update-downloaded"] = ["UpdateListener"]; @@ -2192,8 +2176,28 @@ BrowserGlue.prototype = { /** * Application shutdown handler. + * + * If you need new code to be called on shutdown, please use + * the category manager browser-quit-application-granted category + * instead of adding new manual code to this function. */ _onQuitApplicationGranted() { + function failureHandler(ex) { + if (Cu.isInAutomation) { + // This usually happens after the test harness is done collecting + // test errors, thus we can't easily add a failure to it. The only + // noticeable solution we have is crashing. + Cc["@mozilla.org/xpcom/debug;1"] + .getService(Ci.nsIDebug2) + .abort(ex.filename, ex.lineNumber); + } + } + + lazy.BrowserUtils.callModulesFromCategory({ + categoryName: "browser-quit-application-granted", + failureHandler, + }); + let tasks = [ // This pref must be set here because SessionStore will use its value // on quit-application. @@ -2213,28 +2217,16 @@ BrowserGlue.prototype = { } }, - () => lazy.BrowserUsageTelemetry.uninit(), + // These should also be moved to use the category manager, but ran into + // leaking issues. Bug 1949294 tracks. () => lazy.SearchSERPTelemetry.uninit(), () => lazy.SearchSERPCategorization.uninit(), - () => lazy.Interactions.uninit(), - () => lazy.PageDataService.uninit(), - () => lazy.PageThumbs.uninit(), - () => lazy.NewTabUtils.uninit(), - () => lazy.Normandy.uninit(), - () => lazy.RFPHelper.uninit(), - () => lazy.ShoppingUtils.uninit(), - () => lazy.ASRouterNewTabHook.destroy(), - () => { - if (AppConstants.MOZ_UPDATER) { - lazy.UpdateListener.reset(); - } - }, + () => { // bug 1839426 - The FOG service needs to be instantiated reliably so it // can perform at-shutdown tasks later in shutdown. Services.fog; }, - () => lazy.UrlbarSearchTermsPersistence.uninit(), ]; for (let task of tasks) { @@ -2242,14 +2234,7 @@ BrowserGlue.prototype = { task(); } catch (ex) { console.error(`Error during quit-application-granted: ${ex}`); - if (Cu.isInAutomation) { - // This usually happens after the test harness is done collecting - // test errors, thus we can't easily add a failure to it. The only - // noticeable solution we have is crashing. - Cc["@mozilla.org/xpcom/debug;1"] - .getService(Ci.nsIDebug2) - .abort(ex.filename, ex.lineNumber); - } + failureHandler(ex); } } }, @@ -3019,22 +3004,25 @@ BrowserGlue.prototype = { { name: "BackgroundUpdate", - condition: AppConstants.MOZ_UPDATE_AGENT, + condition: AppConstants.MOZ_UPDATE_AGENT && AppConstants.MOZ_UPDATER, task: async () => { + let updateServiceStub = Cc[ + "@mozilla.org/updates/update-service-stub;1" + ].getService(Ci.nsIApplicationUpdateServiceStub); // Never in automation! - if ( - AppConstants.MOZ_UPDATER && - !lazy.UpdateServiceStub.updateDisabledForTesting - ) { + if (!updateServiceStub.updateDisabledForTesting) { + let { BackgroundUpdate } = ChromeUtils.importESModule( + "resource://gre/modules/BackgroundUpdate.sys.mjs" + ); try { - await lazy.BackgroundUpdate.scheduleFirefoxMessagingSystemTargetingSnapshotting(); + await BackgroundUpdate.scheduleFirefoxMessagingSystemTargetingSnapshotting(); } catch (e) { console.error( "There was an error scheduling Firefox Messaging System targeting snapshotting: ", e ); } - await lazy.BackgroundUpdate.maybeScheduleBackgroundUpdateTask(); + await BackgroundUpdate.maybeScheduleBackgroundUpdateTask(); } }, }, diff --git a/toolkit/modules/BrowserUtils.sys.mjs b/toolkit/modules/BrowserUtils.sys.mjs index 51b5dd61ddae..0213c56058bf 100644 --- a/toolkit/modules/BrowserUtils.sys.mjs +++ b/toolkit/modules/BrowserUtils.sys.mjs @@ -499,11 +499,19 @@ export var BrowserUtils = { * @param {string} [options.profilerMarker=""] * If specified, will create a profiler marker with the provided * identifier for each consumer. + * @param {function} [options.failureHandler] + * If specified, will be called for any exceptions raised, in + * order to do custom failure handling. * @param {...any} args * Arguments to pass to the consumers. */ callModulesFromCategory( - { categoryName, profilerMarker = "", idleDispatch = false }, + { + categoryName, + profilerMarker = "", + idleDispatch = false, + failureHandler = null, + }, ...args ) { // Use an async function for profiler markers and error handling. @@ -518,6 +526,17 @@ export var BrowserUtils = { `Error in processing ${categoryName} for ${fn._descriptiveName}` ); console.error(ex); + try { + await failureHandler?.(ex); + } catch (nestedEx) { + console.error(`Error in handling failure: ${nestedEx}`); + // Crash in automation: + if (BrowserUtils._inAutomation) { + Cc["@mozilla.org/xpcom/debug;1"] + .getService(Ci.nsIDebug2) + .abort(nestedEx.filename, nestedEx.lineNumber); + } + } } if (profilerMarker) { ChromeUtils.addProfilerMarker( diff --git a/toolkit/modules/tests/xpcshell/my_catman_1.sys.mjs b/toolkit/modules/tests/xpcshell/my_catman_1.sys.mjs index e0fe0b760a37..44f50c69d5b6 100644 --- a/toolkit/modules/tests/xpcshell/my_catman_1.sys.mjs +++ b/toolkit/modules/tests/xpcshell/my_catman_1.sys.mjs @@ -6,4 +6,7 @@ export let Module1 = { arg ); }, + throwingFunction() { + throw new Error("Uh oh. I have a bad feeling about this."); + }, }; diff --git a/toolkit/modules/tests/xpcshell/test_BrowserUtils.js b/toolkit/modules/tests/xpcshell/test_BrowserUtils.js index 4d254eb90074..dc92f85d2825 100644 --- a/toolkit/modules/tests/xpcshell/test_BrowserUtils.js +++ b/toolkit/modules/tests/xpcshell/test_BrowserUtils.js @@ -399,3 +399,66 @@ add_task(async function test_callModulesFromCategory() { Services.obs.removeObserver(ob, OBSTOPIC1); }); + +// Test that errors are reported but do not throw at the callsite, +// and that custom error handlers are invoked. +add_task(async function test_callModulesFromCategory_errors() { + const OTHER_CAT = "someothercat"; + const MODULE1 = "resource://test/my_catman_1.sys.mjs"; + + // Add an item that doesn't exist, and check that although we report errors, + // the callsite doesn't throw. + let catManUpdated = TestUtils.topicObserved("xpcom-category-entry-added"); + Services.catMan.addCategoryEntry( + OTHER_CAT, + MODULE1, + `Module1.nonExistantFunction`, + false, + false + ); + await catManUpdated; + let catEntries = Array.from(Services.catMan.enumerateCategory(OTHER_CAT)); + Assert.equal(catEntries.length, 1); + + let consolePromise = TestUtils.consoleMessageObserved(m => { + let firstArg = m.wrappedJSObject.arguments?.[0]?.message; + return typeof firstArg == "string" && firstArg.includes("not a function"); + }); + BrowserUtils.callModulesFromCategory( + { + categoryName: OTHER_CAT, + }, + "Hello" + ); + let reportedError = await consolePromise; + let firstArg = reportedError.wrappedJSObject.arguments?.[0]?.message; + Assert.stringContains( + firstArg, + MODULE1, + "Error message should include module URL." + ); + Services.catMan.deleteCategoryEntry(OTHER_CAT, MODULE1, false); + + // Check that custom exception handling from extant methods works: + catManUpdated = TestUtils.topicObserved("xpcom-category-entry-added"); + Services.catMan.addCategoryEntry( + OTHER_CAT, + MODULE1, + `Module1.throwingFunction`, + false, + false + ); + await catManUpdated; + Assert.equal(catEntries.length, 1); + let exHandler = Promise.withResolvers(); + BrowserUtils.callModulesFromCategory({ + categoryName: OTHER_CAT, + failureHandler: exHandler.resolve, + }); + let caughtException = await exHandler.promise; + Assert.stringContains( + caughtException.message, + "Uh oh", + "Exceptions should be handled." + ); +});