diff --git a/browserscripts/browser/appConstants.js b/browserscripts/browser/appConstants.js index a2fb45312..e3dbf2047 100644 --- a/browserscripts/browser/appConstants.js +++ b/browserscripts/browser/appConstants.js @@ -1,6 +1,6 @@ module.exports = { requires: { privilege: true }, - function: function() { + collect: function() { const { AppConstants } = ChromeUtils.import( 'resource://gre/modules/AppConstants.jsm' ); diff --git a/browserscripts/browser/asyncAppConstants.js b/browserscripts/browser/asyncAppConstants.js index 7e57a7031..68adc179e 100644 --- a/browserscripts/browser/asyncAppConstants.js +++ b/browserscripts/browser/asyncAppConstants.js @@ -1,6 +1,7 @@ module.exports = { requires: { privilege: true }, - function: async function() { + pre: function() {}, + collect: async function() { return new Promise(resolve => { const { AppConstants } = ChromeUtils.import( 'resource://gre/modules/AppConstants.jsm' diff --git a/docs/new-style-script-bundles.md b/docs/new-style-script-bundles.md new file mode 100644 index 000000000..b2fb3b3a5 --- /dev/null +++ b/docs/new-style-script-bundles.md @@ -0,0 +1,56 @@ +# New-style script bundles: + +A new-style script bundle allows the user to write a script that executes in an environment that meets certain requirements. The script writer specifies the requirements for their script and the browser will meet those requirements before executing the script's functionality. If the requirements cannot be met, then the script's functionality will not be executed. + +A new-style script bundle is defined in a file with a new-style script bundle object that is assigned to `module.exports`. A new-style script bundle object has the following keys: + + * *requires*: An object whose keys define the requirements for executing this script object's functionality. + * *pre*: A possibly asynchronous function that configures the browser and/or test environment to meet the preconditions of the data collection function, `collect`. The function will be invoked with no parameters. If the function returns a promise, the promise (and its children promises, if any) will be resolved. The *pre* function may return `false` to disable execution of the *collect* and *post* functions from this new-style script bundle. All other return values will be discarded. + * *collect*: A possibly asynchronous function that actually performs the script object's data collection. The function will be invoked with no parameters. If the function is asynchronous, it must return a `Promise`. The value `resolve`d by that promise is serialized into browsertime's result output. If the function is not asynchronous, any value returned by this function is serialized into browsertime's result output. + * *post*: A possibly asynchronous function that restores the browser and/or test environment to its status before executing this script bundle. The function will be invoked with no parameters. If the function returns a `Promise`, it (and its children promises, if any) will be resolved. + +At the moment, browsertime only supports one requirement in new-style script objects, the `privilege` requirement. If the `privilege` requirement is set to `true`, the new-style script object's functionality will execute with access to the browser's privileged APIs (which is currently only available in Firefox). If the browser cannot guarantee that the script can execute with access to the browser's privileged API (for whatever reason), it will not be executed. + +For example, + + module.exports = { + requires: { privilege: true }, + collect: function() { + const { AppConstants } = ChromeUtils.import( + 'resource://gre/modules/AppConstants.jsm' + ); + return AppConstants; + } + }; + +defines a new-style script bundle that requires access to the browser's privileged API in order to perform its measurements. + +The equivalent functionality can be accomplished asynchronously: + + module.exports = { + requires: { privilege: true }, + collect: async function() { + return new Promise(resolve => { + const { AppConstants } = ChromeUtils.import( + 'resource://gre/modules/AppConstants.jsm' + ); + resolve(AppConstants); + }); + } + }; + +As an example of the utility of pre/post functionality in a new-style script +bundle, here is an example of how to use `ChromeUtils.collectPerfStats`: + + module.exports = { + requires: { privilege: true }, + pre: function() { + ChromeUtils.setPerfStatsCollectionMask(255); + }, + collect: async function() { + const stats = await ChromeUtils.collectPerfStats(); + return new Promise(resolve => { + resolve(JSON.parse(stats)); + }); + } + }; diff --git a/docs/new-style-script-objects.md b/docs/new-style-script-objects.md deleted file mode 100644 index 03a23d535..000000000 --- a/docs/new-style-script-objects.md +++ /dev/null @@ -1,38 +0,0 @@ -# New-style script objects: - -A new-style object allows the user to write a script that executes in an environment that meets certain requirements. The script writer specifies the requirements for their script and the browser will meet those requirements before executing the script's functionality. If the requirements cannot be met, then the script's functionality will not be executed. - -A new-style script object is defined in a script file by a single object that is assigned to `module.exports`. A new-style script object has the following keys: - - * *requires*: An object whose keys define the requirements for executing this script object's functionality. - * *function*: A possibly asynchronous function that actually performs the script object's actions. The function will be invoked with no parameters. If the function is asynchronous, it must return a `Promise`. The value `resolve`d by that promise is serialized into browsertime's result output. If the function is not asynchronous, any value returned by this function is written into browsertime's result output. - -At the moment, browsertime only supports one requirement in new-style script objects, the `privilege` requirement. If the `privilege` requirement is set to `true`, the new-style script object's functionality will execute with access to the browser's privileged APIs (which is currently only available in Firefox). If the browser cannot guarantee that the script can execute with access to the browser's privileged API (for whatever reason), it will not be executed. - -For example, - - module.exports = { - requires: { privilege: true }, - function: function() { - const { AppConstants } = ChromeUtils.import( - 'resource://gre/modules/AppConstants.jsm' - ); - return AppConstants; - } - }; - -defines a new-style script object that requires access to the browser's privileged API in order to fetch its constants. - -The equivalent functionality can be accomplished asynchronously: - - module.exports = { - requires: { privilege: true }, - function: async function() { - return new Promise(resolve => { - const { AppConstants } = ChromeUtils.import( - 'resource://gre/modules/AppConstants.jsm' - ); - resolve(AppConstants); - }); - } - }; diff --git a/lib/core/engine/command/measure.js b/lib/core/engine/command/measure.js index 1dd95f6bc..5b342fe01 100644 --- a/lib/core/engine/command/measure.js +++ b/lib/core/engine/command/measure.js @@ -9,7 +9,10 @@ const setOrangeBackground = require('../../../video/screenRecording/setOrangeBac const delay = ms => new Promise(res => setTimeout(res, ms)); const filterWhitelisted = require('../../../support/userTiming') .filterWhitelisted; -const { isAndroidConfigured, createAndroidConnection } = require('../../../android'); +const { + isAndroidConfigured, + createAndroidConnection +} = require('../../../android'); // Make this configurable const ANDROID_DELAY_TIME = 2000; @@ -247,10 +250,18 @@ class Measure { this.result[this.numberOfMeasuredPages].url = url; const syncScripts = this.scriptsByCategory - ? await this.browser.runScripts(this.scriptsByCategory) + ? await this.browser.runScripts( + this.scriptsByCategory, + false, + 'collect' + ) : {}, asyncScripts = this.asyncScriptsByCategory - ? await this.browser.runScripts(this.asyncScriptsByCategory, true) + ? await this.browser.runScripts( + this.asyncScriptsByCategory, + true, + 'collect' + ) : {}; this.result[this.numberOfMeasuredPages].browserScripts = merge( @@ -293,7 +304,11 @@ class Measure { if (isAndroidConfigured(this.options) && this.options.processStartTime) { var packageName; - if (this.options.firefox && this.options.firefox.android && this.options.firefox.android.package) { + if ( + this.options.firefox && + this.options.firefox.android && + this.options.firefox.android.package + ) { packageName = this.options.firefox.android.package; } else { packageName = this.options.chrome.android.package; @@ -306,7 +321,9 @@ class Measure { if (pid) { const processStartTime = await android.processStartTime(pid); - this.result[this.numberOfMeasuredPages].browserScripts.browser.processStartTime = processStartTime; + this.result[ + this.numberOfMeasuredPages + ].browserScripts.browser.processStartTime = processStartTime; } } } diff --git a/lib/core/engine/iteration.js b/lib/core/engine/iteration.js index d43f357a6..2c6c85e8f 100644 --- a/lib/core/engine/iteration.js +++ b/lib/core/engine/iteration.js @@ -22,6 +22,7 @@ const ChromeDevToolsProtocol = require('./command/chromeDevToolsProtocol.js'); const path = require('path'); const fs = require('fs'); const util = require('../../support/util.js'); +const ScriptType = require('../../support/browserScript.js').ScriptType; const { isAndroidConfigured } = require('../../android'); @@ -164,6 +165,18 @@ class Iteration { await preURL(browser, options); } + // Look through the scripts and try to do our pre scripts. + if (this.scriptsByCategory) { + await browser.runScripts(this.scriptsByCategory, false, ScriptType.PRE); + } + if (this.asyncScriptsByCategory) { + await browser.runScripts( + this.asyncScriptsByCategory, + true, + ScriptType.PRE + ); + } + try { await navigationScript(context, commands); } catch (e) { @@ -179,6 +192,22 @@ class Iteration { } await this.engineDelegate.onStopIteration(browser, index, result); + // Look through the scripts and try to do our post scripts. + if (this.scriptsByCategory) { + await browser.runScripts( + this.scriptsByCategory, + false, + ScriptType.POST + ); + } + if (this.asyncScriptsByCategory) { + await browser.runScripts( + this.asyncScriptsByCategory, + true, + ScriptType.POST + ); + } + for (const postScript of this.postScripts) { await postScript(context, commands); } diff --git a/lib/core/seleniumRunner.js b/lib/core/seleniumRunner.js index 70e67af2d..e60ddb8ae 100644 --- a/lib/core/seleniumRunner.js +++ b/lib/core/seleniumRunner.js @@ -7,6 +7,7 @@ const log = require('intel').getLogger('browsertime'), isAndroidConfigured = require('../android').isAndroidConfigured, UrlLoadError = require('../support/errors').UrlLoadError, BrowserError = require('../support/errors').BrowserError, + ScriptType = require('../support/browserScript').ScriptType, getViewPort = require('../support/getViewPort'); const defaultPageCompleteCheck = require('./defaultPageCompleteCheck'); @@ -506,28 +507,43 @@ class SeleniumRunner { * @param {*} scriptsByCategory * @param {boolean} isAsync - is the script synchrously or async? */ - async runScripts(scriptsByCategory, isAsync) { + async runScripts(scriptsByCategory, isAsync, type = ScriptType.COLLECT) { const categoryNames = Object.keys(scriptsByCategory); const results = {}; for (let categoryName of categoryNames) { const category = scriptsByCategory[categoryName]; try { - results[categoryName] = await this.runScriptInCategory( + const categoryResults = await this.runScriptsInCategory( category, - isAsync + isAsync, + type ); + if (type === ScriptType.COLLECT) { + results[categoryName] = categoryResults; + } else if (type === ScriptType.PRE) { + // Look through the results. Any of the scripts whose results + // are false, we set to disabled. + for (let scriptName in categoryResults) { + if (categoryResults[scriptName] === false) { + log.info('script' + scriptName + ' of category ' + categoryName + + ' marked itself as disabled.'); + scriptsByCategory[categoryName][scriptName].disabled = true; + } + } + } } catch (e) { - if (e.extra.cause === 'PrivilegeError') { + if (e.extra && e.extra.cause && e.extra.cause === 'PrivilegeError') { // Ignore those scripts that fail to execute because they // wanted privileges that the browser cannot provide. log.verbose( - 'Did not have enough privileges to execute user script: ' + - e + - '; ignoring.' + 'Did not have enough privileges to execute user script: ' + e + '; ' + + 'ignoring.' ); } else { log.error('Failed to execute user script: ' + e); - results[categoryName] = undefined; + if (type === ScriptType.COLLECT) { + results[categoryName] = undefined; + } } } } @@ -538,8 +554,10 @@ class SeleniumRunner { * Run scripts in category. * @param {*} category * @param {boolean} isAsync - is the script synchrously or async? + * @param {ScriptType} type - the type of the script (pre, post or + * measurement) */ - async runScriptInCategory(category, isAsync) { + async runScriptsInCategory(category, isAsync, type) { const scriptNames = Object.keys(category); const results = {}; let requires = []; @@ -551,21 +569,56 @@ class SeleniumRunner { // the function is not null and we want to run // the function. if (!script) { - let func = category[scriptName].function; - if (!func) { - throw 'Function and script cannot both be null in ' + - scriptName + - '.'; + let func = category[scriptName][type]; + // If either + // a) there is no function defined for this type, or + // b) the function is defined but the entire script object is marked as + // disabled, + // then we simply skip this function. + if ( + !func || + (category[scriptName].disabled && + category[scriptName].disabled === true) + ) { + log.info( + 'Skipping ' + type + ' function invocation in ' + scriptName + '.' + ); + continue; } + + log.info( + 'Executing ' + type + ' function invocation in ' + scriptName + '.' + ); // We wrap the source code of the function in parenthesis // "(...)" to contain it in a separate scope. We add a - // "();" to the source of the function to do the actual + // "()" to the source of the function to do the actual // invocation. The script writer cannot do this in their // source because it would force the evaluation of the // function too soon and cause undefined reference errors. script = '( ' + func + ' )()'; requires = category[scriptName].requires; - isAsyncOverride = category[scriptName].isAsync; + isAsyncOverride = + Object.getPrototypeOf(func) === + Object.getPrototypeOf(async function() {}); + if (log.isEnabledFor(log.TRACE)) { + log.verbose( + scriptName + + ' is a new-style script object and we are executing it when we are not collecting.' + ); + } + } else if (type !== ScriptType.COLLECT) { + // We are here because we have an old-style script + // but we are doing an iteration of this execution + // loop when we do not want to collect statistics. In + // other words, we do not want to execute that script + // here. + if (log.isEnabledFor(log.TRACE)) { + log.verbose( + scriptName + + ' is an old-style script object and we are not executing it when we are not collecting.' + ); + } + continue; } const result = await this.runScriptFromCategory( diff --git a/lib/support/browserScript.js b/lib/support/browserScript.js index b0fbb48fb..a1033c5b5 100644 --- a/lib/support/browserScript.js +++ b/lib/support/browserScript.js @@ -10,6 +10,12 @@ const stat = promisify(fs.stat); const rootFolder = path.resolve(__dirname, '..', '..', 'browserscripts'); const log = require('intel').getLogger('browsertime'); +const ScriptType = { + PRE: 'pre', + POST: 'post', + COLLECT: 'collect' +}; + function toFullPath(filename, root) { return path.join(root, filename); } @@ -28,7 +34,11 @@ async function scriptsFromDirectory(dirpath) { for (const filepath of dir) { const name = path.basename(filepath, '.js'); const script = await readFile(filepath, 'utf8'); - result[name] = generateScriptObject(name, filepath, script); + try { + result[name] = generateScriptObject(name, filepath, script); + } catch (e) { + log.error('Could not parse user script ' + name + ': ' + e); + } } return result; } @@ -65,37 +75,56 @@ async function getScriptsForCategories(categories) { * @returns {ScriptObject} */ function generateScriptObject(name, path, contents) { + let scriptAndMetadataObject = undefined; try { - const scriptAndMetadataObject = require(path); + scriptAndMetadataObject = require(path); + } catch (error) { + // Use this as a signal to fall back to an old-style script, but don't + // perform any action + } + if (scriptAndMetadataObject) { if ( - typeof scriptAndMetadataObject.function === 'function' && + typeof scriptAndMetadataObject.collect === 'function' && typeof scriptAndMetadataObject.requires === 'object' ) { log.info(name + ' is a new-style script object.'); + if ( + (scriptAndMetadataObject.pre && + typeof scriptAndMetadataObject.pre !== 'function') || + (scriptAndMetadataObject.post && + typeof scriptAndMetadataObject.post !== 'function') + ) { + throw Error( + "If defined, new-style script objects require 'pre'/'post' keys be functions." + ); + } return { name: name, requires: scriptAndMetadataObject.requires, - function: scriptAndMetadataObject.function, - content: null, - isAsync: - Object.getPrototypeOf(scriptAndMetadataObject.function) === - Object.getPrototypeOf(async function() {}) + pre: scriptAndMetadataObject.pre, + collect: scriptAndMetadataObject.collect, + post: scriptAndMetadataObject.post, + content: null }; + } else { + throw Error( + "New-style script object requires a function in the 'collect' key and an object in the 'requires' key." + ); } - } catch (error) { - // Use this as a signal to fall back to an old-style script, but don't - // perform any action - } - if (log.isEnabledFor(log.TRACE)) { - log.verbose(name + ' is an old-style script object.'); + } else { + if (log.isEnabledFor(log.TRACE)) { + log.verbose(name + ' is an old-style script object.'); + } + return { + name: name, + content: contents, + function: null, + pre: null, + post: null, + requires: {} + }; } - return { - name: name, - content: contents, - function: null, - requires: {} - }; } /** @@ -126,7 +155,11 @@ async function findAndParseScripts(root, category) { const name = path.basename(filepath, '.js'); const script = await readFile(filepath, 'utf8'); - result[name] = generateScriptObject(name, root, script); + try { + result[name] = generateScriptObject(name, root, script); + } catch (e) { + log.error('Could not parse user script ' + name + ': ' + e); + } } return result; } @@ -146,8 +179,11 @@ async function findAndParseScripts(root, category) { return readFile(root, 'utf8').then(content => { const name = path.basename(root, '.js'); let scripts = {}; - - scripts[name] = generateScriptObject(name, root, content); + try { + scripts[name] = generateScriptObject(name, root, content); + } catch (e) { + log.error('Could not parse user script ' + name + ': ' + e); + } let categories = {}; categories[category] = scripts; @@ -181,5 +217,6 @@ module.exports = { }); }, getScriptsForCategories: getScriptsForCategories, - findAndParseScripts: findAndParseScripts + findAndParseScripts: findAndParseScripts, + ScriptType: ScriptType };