From a78b44be00142d5e7827777d95b0e6d9a6718a44 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Fri, 8 Jul 2022 00:59:02 +0000 Subject: [PATCH] Bug 1777886 - Check that denylist/intermittent files actually exist in startup perf tests. r=florian This will require that entries for renamed files be updated so that they don't accidentally start being loaded under the new name later. It also prevents dead code entries from sticking around after their targets are removed. Using `throttledMapPromises` is probably not strictly necessary given the small number of entries in most lists, but since it already exists, we may as well use it here. Differential Revision: https://phabricator.services.mozilla.com/D150921 --- .../test/performance/PerfTestHelpers.jsm | 78 +++++++++++++++++++ .../test/performance/browser_startup.js | 37 +++++++-- .../performance/browser_startup_content.js | 2 +- .../browser_startup_content_subframe.js | 2 +- .../performance/browser_startup_images.js | 13 ++++ browser/base/content/test/performance/head.js | 33 +++++++- .../base/content/test/performance/moz.build | 17 ++++ .../static/browser_all_files_referenced.js | 33 +++----- .../test/static/browser_parsable_css.js | 4 +- .../test/static/browser_parsable_script.js | 2 +- browser/base/content/test/static/head.js | 20 +---- browser/base/moz.build | 8 +- 12 files changed, 191 insertions(+), 58 deletions(-) create mode 100644 browser/base/content/test/performance/PerfTestHelpers.jsm create mode 100644 browser/base/content/test/performance/moz.build diff --git a/browser/base/content/test/performance/PerfTestHelpers.jsm b/browser/base/content/test/performance/PerfTestHelpers.jsm new file mode 100644 index 000000000000..bf5a1543f7ab --- /dev/null +++ b/browser/base/content/test/performance/PerfTestHelpers.jsm @@ -0,0 +1,78 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +var EXPORTED_SYMBOLS = ["PerfTestHelpers"]; + +const lazy = {}; + +ChromeUtils.defineModuleGetter( + lazy, + "NetUtil", + "resource://gre/modules/NetUtil.jsm" +); + +var PerfTestHelpers = { + /** + * Maps the entries in the given iterable to the given + * promise-returning task function, and waits for all returned + * promises to have resolved. At most `limit` promises may remain + * unresolved at a time. When the limit is reached, the function will + * wait for some to resolve before spawning more tasks. + */ + async throttledMapPromises(iterable, task, limit = 64) { + let pending = new Set(); + let promises = []; + for (let data of iterable) { + while (pending.size >= limit) { + await Promise.race(pending); + } + + let promise = task(data); + promises.push(promise); + if (promise) { + promise.finally(() => pending.delete(promise)); + pending.add(promise); + } + } + + return Promise.all(promises); + }, + + /** + * Returns a promise which resolves to true if the resource at the + * given URI exists, false if it doesn't. This should only be used + * with local resources, such as from resource:/chrome:/jar:/file: + * URIs. + */ + checkURIExists(uri) { + return new Promise(resolve => { + try { + let channel = lazy.NetUtil.newChannel({ + uri, + loadUsingSystemPrincipal: true, + }); + + channel.asyncOpen({ + onStartRequest(request) { + resolve(Components.isSuccessCode(request.status)); + request.cancel(Cr.NS_BINDING_ABORTED); + }, + + onStopRequest(request, status) { + // We should have already resolved from `onStartRequest`, but + // we resolve again here just as a failsafe. + resolve(Components.isSuccessCode(status)); + }, + }); + } catch (e) { + if ( + e.result != Cr.NS_ERROR_FILE_NOT_FOUND && + e.result != Cr.NS_ERROR_NOT_AVAILABLE + ) { + throw e; + } + resolve(false); + } + }); + }, +}; diff --git a/browser/base/content/test/performance/browser_startup.js b/browser/base/content/test/performance/browser_startup.js index c7bf95851d69..dddd5fe61c75 100644 --- a/browser/base/content/test/performance/browser_startup.js +++ b/browser/base/content/test/performance/browser_startup.js @@ -57,8 +57,6 @@ const startupPhases = { "before first paint": { denylist: { modules: new Set([ - "chrome://webcompat/content/data/ua_overrides.jsm", - "chrome://webcompat/content/lib/ua_overrider.jsm", "resource:///modules/AboutNewTab.jsm", "resource:///modules/BrowserUsageTelemetry.jsm", "resource:///modules/ContentCrashHandlers.jsm", @@ -86,7 +84,6 @@ const startupPhases = { "resource://gre/modules/BookmarkHTMLUtils.jsm", "resource://gre/modules/Bookmarks.jsm", "resource://gre/modules/ContextualIdentityService.jsm", - "resource://gre/modules/CrashSubmit.jsm", "resource://gre/modules/FxAccounts.jsm", "resource://gre/modules/FxAccountsStorage.jsm", "resource://gre/modules/PlacesBackups.jsm", @@ -94,10 +91,7 @@ const startupPhases = { "resource://gre/modules/PlacesSyncUtils.jsm", "resource://gre/modules/PushComponents.jsm", ]), - services: new Set([ - "@mozilla.org/browser/annotation-service;1", - "@mozilla.org/browser/nav-bookmarks-service;1", - ]), + services: new Set(["@mozilla.org/browser/nav-bookmarks-service;1"]), }, }, @@ -128,6 +122,12 @@ if ( ); } +if (AppConstants.MOZ_CRASHREPORTER) { + startupPhases["before handling user events"].denylist.modules.add( + "resource://gre/modules/CrashSubmit.jsm" + ); +} + add_task(async function() { if ( !AppConstants.NIGHTLY_BUILD && @@ -219,6 +219,29 @@ add_task(async function() { } } } + + if (denylist.modules) { + let results = await PerfTestHelpers.throttledMapPromises( + denylist.modules, + async uri => ({ + uri, + exists: await PerfTestHelpers.checkURIExists(uri), + }) + ); + + for (let { uri, exists } of results) { + ok(exists, `denylist entry ${uri} for phase "${phase}" must exist`); + } + } + + if (denylist.services) { + for (let contract of denylist.services) { + ok( + contract in Cc, + `denylist entry ${contract} for phase "${phase}" must exist` + ); + } + } } } }); diff --git a/browser/base/content/test/performance/browser_startup_content.js b/browser/base/content/test/performance/browser_startup_content.js index 539061157a0a..ec63d9bf009a 100644 --- a/browser/base/content/test/performance/browser_startup_content.js +++ b/browser/base/content/test/performance/browser_startup_content.js @@ -170,7 +170,7 @@ add_task(async function() { loadedInfo.processScripts[uri] = ""; } - checkLoadedScripts({ + await checkLoadedScripts({ loadedInfo, known: known_scripts, intermittent: intermittently_loaded_scripts, diff --git a/browser/base/content/test/performance/browser_startup_content_subframe.js b/browser/base/content/test/performance/browser_startup_content_subframe.js index 9b7ae5d43323..cb3c40feb329 100644 --- a/browser/base/content/test/performance/browser_startup_content_subframe.js +++ b/browser/base/content/test/performance/browser_startup_content_subframe.js @@ -141,7 +141,7 @@ add_task(async function() { loadedInfo.processScripts[uri] = ""; } - checkLoadedScripts({ + await checkLoadedScripts({ loadedInfo, known: known_scripts, intermittent: intermittently_loaded_scripts, diff --git a/browser/base/content/test/performance/browser_startup_images.js b/browser/base/content/test/performance/browser_startup_images.js index 0d713394f3eb..600b336dbbaa 100644 --- a/browser/base/content/test/performance/browser_startup_images.js +++ b/browser/base/content/test/performance/browser_startup_images.js @@ -82,6 +82,19 @@ add_task(async function() { return el.platforms.includes(AppConstants.platform); }); + { + let results = await PerfTestHelpers.throttledMapPromises( + knownImagesForPlatform, + async image => ({ + uri: image.file, + exists: await PerfTestHelpers.checkURIExists(image.file), + }) + ); + for (let { uri, exists } of results) { + ok(exists, `Unshown image entry ${uri} must exist`); + } + } + let loadedImages = data["image-loading"]; let shownImages = data["image-drawing"]; diff --git a/browser/base/content/test/performance/head.js b/browser/base/content/test/performance/head.js index 53d489d6059e..43fb0582d1bc 100644 --- a/browser/base/content/test/performance/head.js +++ b/browser/base/content/test/performance/head.js @@ -2,6 +2,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { AboutNewTab: "resource:///modules/AboutNewTab.jsm", + PerfTestHelpers: "resource://testing-common/PerfTestHelpers.jsm", PlacesTestUtils: "resource://testing-common/PlacesTestUtils.jsm", PlacesUtils: "resource://gre/modules/PlacesUtils.jsm", UrlbarTestUtils: "resource://testing-common/UrlbarTestUtils.jsm", @@ -843,7 +844,7 @@ async function runUrlbarTest( * If true, dump the stacks for all loaded modules. Makes the output * noisy. */ -function checkLoadedScripts({ +async function checkLoadedScripts({ loadedInfo, known, intermittent, @@ -852,6 +853,32 @@ function checkLoadedScripts({ }) { let loadedList = {}; + async function checkAllExist(scriptType, list, listType) { + if (scriptType == "services") { + for (let contract of list) { + ok( + contract in Cc, + `${listType} entry ${contract} for content process startup must exist` + ); + } + } else { + let results = await PerfTestHelpers.throttledMapPromises( + list, + async uri => ({ + uri, + exists: await PerfTestHelpers.checkURIExists(uri), + }) + ); + + for (let { uri, exists } of results) { + ok( + exists, + `${listType} entry ${uri} for content process startup must exist` + ); + } + } + } + for (let scriptType in known) { loadedList[scriptType] = Object.keys(loadedInfo[scriptType]).filter(c => { if (!known[scriptType].has(c)) { @@ -880,6 +907,8 @@ function checkLoadedScripts({ ); } + await checkAllExist(scriptType, intermittent[scriptType], "intermittent"); + is( known[scriptType].size, 0, @@ -919,5 +948,7 @@ function checkLoadedScripts({ ); } } + + await checkAllExist(scriptType, forbidden[scriptType], "forbidden"); } } diff --git a/browser/base/content/test/performance/moz.build b/browser/base/content/test/performance/moz.build new file mode 100644 index 000000000000..5fc779f7c750 --- /dev/null +++ b/browser/base/content/test/performance/moz.build @@ -0,0 +1,17 @@ +# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*- +# vim: set filetype=python: +# 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/. + + +BROWSER_CHROME_MANIFESTS += [ + "browser.ini", + "hidpi/browser.ini", + "io/browser.ini", + "lowdpi/browser.ini", +] + +TESTING_JS_MODULES += [ + "PerfTestHelpers.jsm", +] diff --git a/browser/base/content/test/static/browser_all_files_referenced.js b/browser/base/content/test/static/browser_all_files_referenced.js index 04fb6dbe2dd6..111da88f801d 100644 --- a/browser/base/content/test/static/browser_all_files_referenced.js +++ b/browser/base/content/test/static/browser_all_files_referenced.js @@ -744,30 +744,13 @@ function convertToCodeURI(fileUri) { } } -function chromeFileExists(aURI) { - let available = 0; +async function chromeFileExists(aURI) { try { - let channel = NetUtil.newChannel({ - uri: aURI, - loadUsingSystemPrincipal: true, - contentPolicyType: Ci.nsIContentPolicy.TYPE_FETCH, - }); - let stream = channel.open(); - let sstream = Cc["@mozilla.org/scriptableinputstream;1"].createInstance( - Ci.nsIScriptableInputStream - ); - sstream.init(stream); - available = sstream.available(); - sstream.close(); + return await PerfTestHelpers.checkURIExists(aURI); } catch (e) { - if ( - e.result != Cr.NS_ERROR_FILE_NOT_FOUND && - e.result != Cr.NS_ERROR_NOT_AVAILABLE - ) { - todo(false, "Failed to check if " + aURI + "exists: " + e); - } + todo(false, `Failed to check if ${aURI} exists: ${e}`); + return false; } - return available > 0; } function findChromeUrlsFromArray(array, prefix) { @@ -875,7 +858,7 @@ add_task(async function checkAllTheFiles() { }); // Wait for all manifest to be parsed - await throttledMapPromises(manifestURIs, parseManifest); + await PerfTestHelpers.throttledMapPromises(manifestURIs, parseManifest); for (let jsm of Components.manager.getComponentJSMs()) { gReferencesFromCode.set(jsm, null); @@ -908,7 +891,9 @@ add_task(async function checkAllTheFiles() { } // Wait for all the files to have actually loaded: - await throttledMapPromises(allPromises, ([task, uri]) => task(uri)); + await PerfTestHelpers.throttledMapPromises(allPromises, ([task, uri]) => + task(uri) + ); // Keep only chrome:// files, and filter out either the devtools paths or // the non-devtools paths: @@ -1069,7 +1054,7 @@ add_task(async function checkAllTheFiles() { if ( (file.startsWith("chrome://") || file.startsWith("resource://")) && - !chromeFileExists(file) + !(await chromeFileExists(file)) ) { // Ignore chrome prefixes that have been automatically expanded. let pathParts = diff --git a/browser/base/content/test/static/browser_parsable_css.js b/browser/base/content/test/static/browser_parsable_css.js index dde51220c601..d15a61c3f426 100644 --- a/browser/base/content/test/static/browser_parsable_css.js +++ b/browser/base/content/test/static/browser_parsable_css.js @@ -437,7 +437,7 @@ add_task(async function checkAllTheCSS() { return true; }); // Wait for all manifest to be parsed - await throttledMapPromises(manifestURIs, parseManifest); + await PerfTestHelpers.throttledMapPromises(manifestURIs, parseManifest); // filter out either the devtools paths or the non-devtools paths: let isDevtools = SimpleTest.harnessParameters.subsuite == "devtools"; @@ -488,7 +488,7 @@ add_task(async function checkAllTheCSS() { } // Wait for all the files to have actually loaded: - await throttledMapPromises(allPromises, loadCSS); + await PerfTestHelpers.throttledMapPromises(allPromises, loadCSS); // Check if all the files referenced from CSS actually exist. // Files in browser/ should never be referenced outside browser/. diff --git a/browser/base/content/test/static/browser_parsable_script.js b/browser/base/content/test/static/browser_parsable_script.js index 23bbaafd03e4..fb7ac946353d 100644 --- a/browser/base/content/test/static/browser_parsable_script.js +++ b/browser/base/content/test/static/browser_parsable_script.js @@ -153,7 +153,7 @@ add_task(async function checkAllTheJS() { // We create an array of promises so we can parallelize all our parsing // and file loading activity: - await throttledMapPromises(uris, uri => { + await PerfTestHelpers.throttledMapPromises(uris, uri => { if (uriIsWhiteListed(uri)) { info("Not checking whitelisted " + uri.spec); return undefined; diff --git a/browser/base/content/test/static/head.js b/browser/base/content/test/static/head.js index 5be974349d7e..52828401249a 100644 --- a/browser/base/content/test/static/head.js +++ b/browser/base/content/test/static/head.js @@ -17,6 +17,9 @@ const ZipReader = new Components.Constructor( const IS_ALPHA = /^[a-z]+$/i; +var { PerfTestHelpers } = ChromeUtils.import( + "resource://testing-common/PerfTestHelpers.jsm" +); var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); var { OS, require } = ChromeUtils.import("resource://gre/modules/osfile.jsm"); @@ -157,23 +160,6 @@ function fetchFile(uri) { }); } -async function throttledMapPromises(iterable, task, limit = 64) { - let promises = new Set(); - for (let data of iterable) { - while (promises.size >= limit) { - await Promise.race(promises); - } - - let promise = task(data); - if (promise) { - promise.finally(() => promises.delete(promise)); - promises.add(promise); - } - } - - await Promise.all(promises); -} - /** * Returns whether or not a word (presumably in en-US) is capitalized per * expectations. diff --git a/browser/base/moz.build b/browser/base/moz.build index ef7f61710464..ce1780aa0240 100644 --- a/browser/base/moz.build +++ b/browser/base/moz.build @@ -39,10 +39,6 @@ BROWSER_CHROME_MANIFESTS += [ "content/test/pageActions/browser.ini", "content/test/pageinfo/browser.ini", "content/test/pageStyle/browser.ini", - "content/test/performance/browser.ini", - "content/test/performance/hidpi/browser.ini", - "content/test/performance/io/browser.ini", - "content/test/performance/lowdpi/browser.ini", "content/test/permissions/browser.ini", "content/test/plugins/browser.ini", "content/test/popupNotifications/browser.ini", @@ -71,6 +67,10 @@ BROWSER_CHROME_MANIFESTS += [ "content/test/zoom/browser.ini", ] +DIRS += [ + "content/test/performance/", +] + PERFTESTS_MANIFESTS += ["content/test/perftest.ini"] DEFINES["MOZ_APP_VERSION"] = CONFIG["MOZ_APP_VERSION"]