Skip to content

Commit

Permalink
remove workarounds for lack of domain state tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny committed Jan 17, 2017
1 parent 2988a55 commit 2cb1636
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 37 deletions.
24 changes: 2 additions & 22 deletions lighthouse-core/gather/connections/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ class Connection {
// handleRawError returns or throws synchronously; wrap to put into promise chain.
return callback.resolve(Promise.resolve().then(_ => {
if (object.error) {
return this.handleRawError(object.error, callback.method);
log.formatProtocol('method <= browser ERR', {method: callback.method}, 'error');
throw new Error(`Protocol error (${callback.method}): ${object.error.message}`);
}

log.formatProtocol('method <= browser OK',
Expand All @@ -113,27 +114,6 @@ class Connection {
this.emitNotification(object.method, object.params);
}

/**
* Handles error responses from the protocol, absorbing errors we don't care
* about and throwing on the rest.
*
* Currently the only error ignored is from defensive calls of `DOM.disable`
* when already disabled.
* @param {{message: string}} error
* @param {string} method Protocol method that received the error response.
* @throws {Error}
* @protected
*/
handleRawError(error, method) {
// We proactively disable the DOM domain. Ignore any errors.
if (error.message && error.message.includes('DOM agent hasn\'t been enabled')) {
return;
}

log.formatProtocol('method <= browser ERR', {method}, 'error');
throw new Error(`Protocol error (${method}): ${error.message}`);
}

/**
* @param {!string} method
* @param {!Object} params
Expand Down
22 changes: 13 additions & 9 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -559,12 +559,19 @@ class Driver {
options: 'sampling-frequency=10000' // 1000 is default and too slow.
};

// Disable any domains that could interfere or add overhead to the trace
return this.sendCommand('Debugger.disable')
.then(_ => this.sendCommand('CSS.disable'))
.then(_ => this.sendCommand('DOM.disable'))
// Enable Page domain to wait for Page.loadEventFired
.then(_ => this.sendCommand('Page.enable'))
// Check any domains that could interfere with or add overhead to the trace.
if (this.isDomainEnabled('Debugger')) {
throw new Error('Debugger domain enabled when starting trace');
}
if (this.isDomainEnabled('CSS')) {
throw new Error('CSS domain enabled when starting trace');
}
if (this.isDomainEnabled('DOM')) {
throw new Error('DOM domain enabled when starting trace');
}

// Enable Page domain to wait for Page.loadEventFired
return this.sendCommand('Page.enable')
.then(_ => this.sendCommand('Tracing.start', tracingOpts));
}

Expand All @@ -583,9 +590,6 @@ class Driver {

_readTraceFromStream(streamHandle) {
return new Promise((resolve, reject) => {
// COMPAT: We've found `result` not retaining its value in this scenario when it's
// declared with `let`. Observed in Chrome 50 and 52. While investigating the V8 bug
// further, we'll use a plain `var` declaration.
let isEOF = false;
let result = '';

Expand Down
10 changes: 4 additions & 6 deletions lighthouse-core/gather/gatherers/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,10 @@ class Styles extends Gatherer {
Promise.all(contentPromises).then(styleHeaders => {
driver.off('CSS.styleSheetAdded', this._onStyleSheetAdded);
driver.off('CSS.styleSheetRemoved', this._onStyleSheetRemoved);
resolve(styleHeaders);
// Currently both CSSUsage and Styles use these domains, so let it disable there.
// TODO: have a better way to specify used domains
// return driver.sendCommand('CSS.disable')
// .then(_ => driver.sendCommand('DOM.disable'))
// .then(_ => resolve(styleHeaders));

return driver.sendCommand('CSS.disable')
.then(_ => driver.sendCommand('DOM.disable'))
.then(_ => resolve(styleHeaders));
}).catch(err => reject(err));
});
}
Expand Down

0 comments on commit 2cb1636

Please sign in to comment.