Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

always construct networkRecords from devtoolsLog #2133

Merged
merged 5 commits into from
May 5, 2017
Merged

always construct networkRecords from devtoolsLog #2133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43d4da6
always construct networkRecords from devtoolsLog
brendankenny May 3, 2017
62949d4
feedback
brendankenny May 4, 2017
253e41d
startsWith
brendankenny May 4, 2017
b9a62b7
get network record content directly from driver
brendankenny May 5, 2017
faafddf
clarification
brendankenny May 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 96 additions & 69 deletions lighthouse-core/gather/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,28 @@ class Driver {
this._connection = connection;
// currently only used by WPT where just Page and Network are needed
this._devtoolsLog = new DevtoolsLog(/^(Page|Network)\./);
this.online = true;
this._domainEnabledCounts = new Map();

/**
* Used for monitoring network status events during gotoURL.
* @private {?NetworkRecorder}
*/
this._networkStatusMonitor = null;

/**
* Used for monitoring url redirects during gotoURL.
* @private {?string}
*/
this._monitoredUrl = null;

connection.on('notification', event => {
this._devtoolsLog.record(event);
this.recordNetworkEvent(event.method, event.params);
if (this._networkStatusMonitor) {
this._networkStatusMonitor.dispatch(event.method, event.params);
}
this._eventEmitter.emit(event.method, event.params);
});
this.online = true;
this._domainEnabledCounts = new Map();
}

static get traceCategories() {
Expand All @@ -73,13 +88,6 @@ class Driver {
];
}

/**
* @return {!Array<{method: string, params: !Object}>}
*/
get devtoolsLog() {
return this._devtoolsLog.messages;
}

/**
* @return {!Promise<string>}
*/
Expand Down Expand Up @@ -346,25 +354,6 @@ class Driver {
});
}

/**
* If our main document URL redirects, we will update options.url accordingly
* As such, options.url will always represent the post-redirected URL.
* options.initialUrl is the pre-redirect URL that things started with
* @param {!Object} opts
*/
enableUrlUpdateIfRedirected(opts) {
this._networkRecorder.on('requestloaded', redirectRequest => {
// Quit if this is not a redirected request
if (!redirectRequest.redirectSource) {
return;
}
const earlierRequest = redirectRequest.redirectSource;
if (earlierRequest.url === opts.url) {
opts.url = redirectRequest.url;
}
});
}

/**
* Returns a promise that resolves when the network has been idle for
* `pauseAfterLoadMs` ms and a method to cancel internal network listeners and
Expand All @@ -380,25 +369,25 @@ class Driver {
const promise = new Promise((resolve, reject) => {
const onIdle = () => {
// eslint-disable-next-line no-use-before-define
this._networkRecorder.once('networkbusy', onBusy);
this._networkStatusMonitor.once('networkbusy', onBusy);
idleTimeout = setTimeout(_ => {
cancel();
resolve();
}, pauseAfterLoadMs);
};

const onBusy = () => {
this._networkRecorder.once('networkidle', onIdle);
this._networkStatusMonitor.once('networkidle', onIdle);
clearTimeout(idleTimeout);
};

cancel = () => {
clearTimeout(idleTimeout);
this._networkRecorder.removeListener('networkbusy', onBusy);
this._networkRecorder.removeListener('networkidle', onIdle);
this._networkStatusMonitor.removeListener('networkbusy', onBusy);
this._networkStatusMonitor.removeListener('networkidle', onIdle);
};

if (this._networkRecorder.isIdle()) {
if (this._networkStatusMonitor.isIdle()) {
onIdle();
} else {
onBusy();
Expand Down Expand Up @@ -490,14 +479,55 @@ class Driver {
}

/**
* Navigate to the given URL. Use of this method directly isn't advised: if
* Set up listener for network quiet events and URL redirects. Passed in URL
* will be monitored for redirects, with the final loaded URL passed back in
* _endNetworkStatusMonitoring.
* @param {string} startingUrl
* @return {!Promise}
* @private
*/
_beginNetworkStatusMonitoring(startingUrl) {
this._networkStatusMonitor = new NetworkRecorder([]);

// Update startingUrl if it's ever redirected.
this._monitoredUrl = startingUrl;
this._networkStatusMonitor.on('requestloaded', redirectRequest => {
// Ignore if this is not a redirected request.
if (!redirectRequest.redirectSource) {
return;
}
const earlierRequest = redirectRequest.redirectSource;
if (earlierRequest.url === this._monitoredUrl) {
this._monitoredUrl = redirectRequest.url;
}
});

return this.sendCommand('Network.enable');
}

/**
* End network status listening. Returns the final, possibly redirected,
* loaded URL starting with the one passed into _endNetworkStatusMonitoring.
* @return {string}
* @private
*/
_endNetworkStatusMonitoring() {
this._networkStatusMonitor = null;
const finalUrl = this._monitoredUrl;
this._monitoredUrl = null;
return finalUrl;
}

/**
* Navigate to the given URL. Direct use of this method isn't advised: if
* the current page is already at the given URL, navigation will not occur and
* so the returned promise will only resolve after the MAX_WAIT_FOR_FULLY_LOADED
* timeout. See https://github.com/GoogleChrome/lighthouse/pull/185 for one
* possible workaround.
* Resolves on the url of the loaded page, taking into account any redirects.
* @param {string} url
* @param {!Object} options
* @return {!Promise}
* @return {!Promise<string>}
*/
gotoURL(url, options = {}) {
const waitForLoad = options.waitForLoad || false;
Expand All @@ -506,16 +536,18 @@ class Driver {
const maxWaitMs = (options.flags && options.flags.maxWaitForLoad) ||
Driver.MAX_WAIT_FOR_FULLY_LOADED;

return this.sendCommand('Page.enable')
return this._beginNetworkStatusMonitoring(url)
.then(_ => this.sendCommand('Page.enable'))
.then(_ => this.sendCommand('Emulation.setScriptExecutionDisabled', {value: disableJS}))
.then(_ => this.sendCommand('Page.navigate', {url}))
.then(_ => waitForLoad && this._waitForFullyLoaded(pauseAfterLoadMs, maxWaitMs));
.then(_ => waitForLoad && this._waitForFullyLoaded(pauseAfterLoadMs, maxWaitMs))
.then(_ => this._endNetworkStatusMonitoring());
}

/**
* @param {string} objectId Object ID for the resolved DOM node
* @param {string} propName Name of the property
* @return {!Promise<string>} The property value, or null, if property not found
* @param {string} objectId Object ID for the resolved DOM node
* @param {string} propName Name of the property
* @return {!Promise<string>} The property value, or null, if property not found
*/
getObjectProperty(objectId, propName) {
return new Promise((resolve, reject) => {
Expand All @@ -538,6 +570,18 @@ class Driver {
});
}

/**
* Return the body of the response with the given ID.
* @param {string} requestId
* @return {string}
*/
getRequestContent(requestId) {
return this.sendCommand('Network.getResponseBody', {
requestId,
// Ignoring result.base64Encoded, which indicates if body is already encoded
}).then(result => result.body);
}

/**
* @param {string} name The name of API whose permission you wish to query
* @return {!Promise<string>} The state of permissions, resolved in a promise.
Expand Down Expand Up @@ -618,9 +662,6 @@ class Driver {
throw new Error('DOM domain enabled when starting trace');
}

this._devtoolsLog.reset();
this._devtoolsLog.beginRecording();

// Enable Page domain to wait for Page.loadEventFired
return this.sendCommand('Page.enable')
.then(_ => this.sendCommand('Tracing.start', tracingOpts));
Expand All @@ -633,7 +674,6 @@ class Driver {
return new Promise((resolve, reject) => {
// When the tracing has ended this will fire with a stream handle.
this.once('Tracing.tracingComplete', streamHandle => {
this._devtoolsLog.endRecording();
this._readTraceFromStream(streamHandle)
.then(traceContents => resolve(traceContents), reject);
});
Expand Down Expand Up @@ -672,34 +712,21 @@ class Driver {
});
}

beginNetworkCollect(opts) {
return new Promise((resolve, reject) => {
this._networkRecords = [];
this._networkRecorder = new NetworkRecorder(this._networkRecords, this);
this.enableUrlUpdateIfRedirected(opts);

this.sendCommand('Network.enable').then(resolve, reject);
});
}

/**
* @param {!string} method
* @param {!Object<string, *>=} params
* Begin recording devtools protocol messages.
*/
recordNetworkEvent(method, params) {
if (!this._networkRecorder) return;

const regexFilter = /^Network\./;
if (!regexFilter.test(method)) return;
this._networkRecorder.dispatch(method, params);
beginDevtoolsLog() {
this._devtoolsLog.reset();
this._devtoolsLog.beginRecording();
}

endNetworkCollect() {
return new Promise((resolve, reject) => {
resolve(this._networkRecords);
this._networkRecorder = null;
this._networkRecords = [];
});
/**
* Stop recording to devtoolsLog and return log contents.
* @return {!Array<{method: string, params: (!Object<string, *>|undefined)}>}
*/
endDevtoolsLog() {
this._devtoolsLog.endRecording();
return this._devtoolsLog.messages;
}

enableRuntimeEvents() {
Expand Down
71 changes: 40 additions & 31 deletions lighthouse-core/gather/gather-runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
const log = require('../lib/log.js');
const Audit = require('../audits/audit');
const URL = require('../lib/url-shim');
const NetworkRecorder = require('../lib/network-recorder.js');

/**
* @typedef {!Object<string, !Array<!Promise<*>>>}
Expand All @@ -44,15 +45,15 @@ let GathererResults; // eslint-disable-line no-unused-vars
* 2. For each pass in the config:
* A. GatherRunner.beforePass()
* i. navigate to about:blank
* ii. all gatherer's beforePass()
* ii. all gatherers' beforePass()
* B. GatherRunner.pass()
* i. GatherRunner.loadPage()
* b. beginTrace (if requested) & beginNetworkCollect
* c. navigate to options.url (and wait for onload)
* ii. all gatherer's pass()
* i. beginTrace (if requested) & beginDevtoolsLog
* ii. GatherRunner.loadPage()
* a. navigate to options.url (and wait for onload)
* iii. all gatherers' pass()
* C. GatherRunner.afterPass()
* i. endTrace (if requested) & endNetworkCollect & endThrottling
* ii. all gatherer's afterPass()
* i. endTrace (if requested) & endDevtoolsLog & endThrottling
* ii. all gatherers' afterPass()
*
* 3. Teardown
* A. GatherRunner.disposeDriver()
Expand All @@ -76,23 +77,22 @@ class GatherRunner {
}

/**
* Loads options.url with specified options.
* Loads options.url with specified options. If the main document URL
* redirects, options.url will be updated accordingly. As such, options.url
* will always represent the post-redirected URL. options.initialUrl is the
* pre-redirect starting URL.
* @param {!Driver} driver
* @param {!Object} options
* @return {!Promise}
*/
static loadPage(driver, options) {
return Promise.resolve()
// Begin tracing only if requested by config.
.then(_ => options.config.recordTrace && driver.beginTrace(options.flags))
// Network is always recorded for gatherer use
.then(_ => driver.beginNetworkCollect(options))
// Navigate.
.then(_ => driver.gotoURL(options.url, {
waitForLoad: true,
disableJavaScript: !!options.disableJavaScript,
flags: options.flags,
}));
return driver.gotoURL(options.url, {
waitForLoad: true,
disableJavaScript: !!options.disableJavaScript,
flags: options.flags,
}).then(finalUrl => {
options.url = finalUrl;
});
}

/**
Expand Down Expand Up @@ -200,9 +200,15 @@ class GatherRunner {
const status = 'Loading page & waiting for onload';
log.log('status', status, gatherernames);

const pass = GatherRunner.loadPage(driver, options).then(_ => {
log.log('statusEnd', status);
});
// Always record devtoolsLog.
driver.beginDevtoolsLog();

const pass = Promise.resolve()
// Begin tracing only if requested by config.
.then(_ => config.recordTrace && driver.beginTrace(options.flags))
// Navigate.
.then(_ => GatherRunner.loadPage(driver, options))
.then(_ => log.log('statusEnd', status));

return gatherers.reduce((chain, gatherer) => {
return chain.then(_ => {
Expand Down Expand Up @@ -239,20 +245,21 @@ class GatherRunner {
// an object with a traceEvents property. Normalize to object form.
passData.trace = Array.isArray(traceContents) ?
{traceEvents: traceContents} : traceContents;
passData.devtoolsLog = driver.devtoolsLog;
log.verbose('statusEnd', 'Retrieving trace');
});
}

const status = 'Retrieving network records';
pass = pass.then(_ => {
const status = 'Retrieving devtoolsLog and network records';
log.log('status', status);
return driver.endNetworkCollect();
}).then(networkRecords => {
const devtoolsLog = driver.endDevtoolsLog();
const networkRecords = NetworkRecorder.recordsFromLogs(devtoolsLog);
GatherRunner.assertPageLoaded(options.url, driver, networkRecords);
// Expose networkRecords to gatherers
passData.networkRecords = networkRecords;
log.verbose('statusEnd', status);

// Expose devtoolsLog and networkRecords to gatherers
passData.devtoolsLog = devtoolsLog;
passData.networkRecords = networkRecords;
});

// Disable throttling so the afterPass analysis isn't throttled
Expand Down Expand Up @@ -351,12 +358,14 @@ class GatherRunner {
.then(_ => GatherRunner.pass(runOptions, gathererResults))
.then(_ => GatherRunner.afterPass(runOptions, gathererResults))
.then(passData => {
// If requested by config, merge trace -> tracingData
// passData.networkRecords is now discarded and not added onto artifacts
const passName = config.passName || Audit.DEFAULT_PASS;

// networkRecords are discarded and not added onto artifacts.
tracingData.devtoolsLogs[passName] = passData.devtoolsLog;

// If requested by config, add trace to pass's tracingData
if (config.recordTrace) {
tracingData.traces[passName] = passData.trace;
tracingData.devtoolsLogs[passName] = passData.devtoolsLog;
}

if (passIndex === 0) {
Expand Down
Loading