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.

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

core: use flatten protocol #9783

Merged
merged 22 commits into from
Nov 4, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
14 changes: 9 additions & 5 deletions lighthouse-core/gather/connections/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,21 @@ class Connection {
* Call protocol methods
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {string=} sessionId
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
* @param {LH.CrdpCommands[C]['paramsType']} paramArgs,
* @return {Promise<LH.CrdpCommands[C]['returnType']>}
*/
sendCommand(method, ...paramArgs) {
sendCommand(method, sessionId, ...paramArgs) {
// Reify params since we need it as a property so can't just spread again.
const params = paramArgs.length ? paramArgs[0] : undefined;

log.formatProtocol('method => browser', {method, params}, 'verbose');
const id = ++this._lastCommandId;
const message = JSON.stringify({id, method, params});
this.sendRawMessage(message);
const message = {id, method, params};
// @ts-ignore
if (sessionId) message.sessionId = sessionId;
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const messageJson = JSON.stringify(message);
this.sendRawMessage(messageJson);

return new Promise(resolve => {
this._callbacks.set(id, {method, resolve});
Expand Down Expand Up @@ -108,7 +112,7 @@ class Connection {
// Responses to commands carry "id" property, while events do not.
if (!('id' in object)) {
log.formatProtocol('<= event',
{method: object.method, params: object.params}, 'verbose');
{method: object.method, params: object.params}, 'verbose');
this.emitProtocolEvent(object);
return;
}
Expand All @@ -117,7 +121,7 @@ class Connection {
if (callback) {
this._callbacks.delete(object.id);

return callback.resolve(Promise.resolve().then(_ => {
callback.resolve(Promise.resolve().then(_ => {
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
if (object.error) {
log.formatProtocol('method <= browser ERR', {method: callback.method}, 'error');
throw LHError.fromProtocolMessage(callback.method, object.error);
Expand Down
127 changes: 41 additions & 86 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,8 @@ class Driver {
*/
this._monitoredUrl = null;

/**
* Used for sending messages to subtargets. Each message needs a unique ID even if we don't bother
* reading back the result of each command.
*
* @type {number}
* @private
*/
this._targetProxyMessageId = 0;

this.on('Target.attachedToTarget', event => {
this._handleTargetAttached(event, []).catch(this._handleEventError);
});

this.on('Target.receivedMessageFromTarget', event => {
this._handleReceivedMessageFromTarget(event, []).catch(this._handleEventError);
this._handleTargetAttached(event).catch(this._handleEventError);
});

// We use isolated execution contexts for `evaluateAsync` that can be destroyed through navigation
Expand Down Expand Up @@ -300,108 +287,73 @@ class Driver {
log.error('Driver', 'Unhandled event error', error.message);
}

/**
* @param {LH.Crdp.Target.ReceivedMessageFromTargetEvent} event
* @param {string[]} parentSessionIds The list of session ids of the parents from youngest to oldest.
*/
async _handleReceivedMessageFromTarget(event, parentSessionIds) {
const {targetId, sessionId, message} = event;
/** @type {LH.Protocol.RawMessage} */
const protocolMessage = JSON.parse(message);

// Message was a response to some command, not an event, so we'll ignore it.
if ('id' in protocolMessage) return;

// We receive messages from the outermost subtarget which wraps the messages from the inner subtargets.
// We are recursively processing them from outside in, so build the list of parentSessionIds accordingly.
const sessionIdPath = [sessionId, ...parentSessionIds];

if (protocolMessage.method === 'Target.receivedMessageFromTarget') {
// Unravel any messages from subtargets by recursively processing
await this._handleReceivedMessageFromTarget(protocolMessage.params, sessionIdPath);
}

if (protocolMessage.method === 'Target.attachedToTarget') {
// Process any attachedToTarget messages from subtargets
await this._handleTargetAttached(protocolMessage.params, sessionIdPath);
}

if (protocolMessage.method.startsWith('Network')) {
this._handleProtocolEvent({...protocolMessage, source: {targetId, sessionId}});
}
}

/**
* @param {LH.Crdp.Target.AttachedToTargetEvent} event
* @param {string[]} parentSessionIds The list of session ids of the parents from youngest to oldest.
*/
async _handleTargetAttached(event, parentSessionIds) {
const sessionIdPath = [event.sessionId, ...parentSessionIds];

async _handleTargetAttached(event) {
// We're only interested in network requests from iframes for now as those are "part of the page".
// If it's not an iframe, just resume it and move on.
if (event.targetInfo.type !== 'iframe') {
// We suspended the target when we auto-attached, so make sure it goes back to being normal.
await this.sendMessageToTarget(sessionIdPath, 'Runtime.runIfWaitingForDebugger');
await this._innerSendCommand('Runtime.runIfWaitingForDebugger', event.sessionId);
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
return;
}

// Events from subtargets will be stringified and sent back on `Target.receivedMessageFromTarget`.
// We want to receive information about network requests from iframes, so enable the Network domain.
await this.sendMessageToTarget(sessionIdPath, 'Network.enable');
await this._innerSendCommand('Network.enable', event.sessionId);

// We also want to receive information about subtargets of subtargets, so make sure we autoattach recursively.
await this.sendMessageToTarget(sessionIdPath, 'Target.setAutoAttach', {
await this._innerSendCommand('Target.setAutoAttach', event.sessionId, {
autoAttach: true,
flatten: true,
// Pause targets on startup so we don't miss anything
waitForDebuggerOnStart: true,
});

// We suspended the target when we auto-attached, so make sure it goes back to being normal.
await this.sendMessageToTarget(sessionIdPath, 'Runtime.runIfWaitingForDebugger');
await this._innerSendCommand('Runtime.runIfWaitingForDebugger', event.sessionId);
}

/**
* Send protocol commands to a subtarget, wraps the message in as many `Target.sendMessageToTarget`
* commands as necessary.
*
* Call protocol methods, with a timeout.
* To configure the timeout for the next call, use 'setNextProtocolTimeout'.
* @template {keyof LH.CrdpCommands} C
* @param {string[]} sessionIdPath List of session ids to send to, from youngest-oldest/inside-out.
* @param {C} method
* @param {LH.CrdpCommands[C]['paramsType']} params
* @return {Promise<LH.CrdpCommands[C]['returnType']>}
*/
sendMessageToTarget(sessionIdPath, method, ...params) {
this._targetProxyMessageId++;
/** @type {LH.Crdp.Target.SendMessageToTargetRequest} */
let payload = {
sessionId: sessionIdPath[0],
message: JSON.stringify({id: this._targetProxyMessageId, method, params: params[0]}),
};

for (const sessionId of sessionIdPath.slice(1)) {
this._targetProxyMessageId++;
payload = {
sessionId,
message: JSON.stringify({
id: this._targetProxyMessageId,
method: 'Target.sendMessageToTarget',
params: payload,
}),
};
}

return this.sendCommand('Target.sendMessageToTarget', payload);
sendCommand(method, ...params) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const timeout = this._nextProtocolTimeout;
this._nextProtocolTimeout = DEFAULT_PROTOCOL_TIMEOUT;
return new Promise(async (resolve, reject) => {
const asyncTimeout = setTimeout((() => {
const err = new LHError(
LHError.errors.PROTOCOL_TIMEOUT,
{protocolMethod: method}
);
reject(err);
}), timeout);
try {
const result = await this._innerSendCommand(method, undefined, ...params);
resolve(result);
} catch (err) {
reject(err);
} finally {
clearTimeout(asyncTimeout);
}
});
}

/**
* Call protocol methods, with a timeout.
* To configure the timeout for the next call, use 'setNextProtocolTimeout'.
* Same as `sendCommand`, but for a specific session.
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {string} sessionId
* @param {LH.CrdpCommands[C]['paramsType']} params
* @return {Promise<LH.CrdpCommands[C]['returnType']>}
*/
sendCommand(method, ...params) {
sendCommandToSession(method, sessionId, ...params) {
const timeout = this._nextProtocolTimeout;
this._nextProtocolTimeout = DEFAULT_PROTOCOL_TIMEOUT;
return new Promise(async (resolve, reject) => {
Expand All @@ -413,7 +365,7 @@ class Driver {
reject(err);
}), timeout);
try {
const result = await this._innerSendCommand(method, ...params);
const result = await this._innerSendCommand(method, sessionId, ...params);
resolve(result);
} catch (err) {
reject(err);
Expand All @@ -428,18 +380,20 @@ class Driver {
* @private
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {string=} sessionId
* @param {LH.CrdpCommands[C]['paramsType']} params
* @return {Promise<LH.CrdpCommands[C]['returnType']>}
*/
_innerSendCommand(method, ...params) {
_innerSendCommand(method, sessionId, ...params) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const domainCommand = /^(\w+)\.(enable|disable)$/.exec(method);
if (domainCommand) {
// TODO: store domain state for each session.
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
if (domainCommand && !sessionId) {
const enable = domainCommand[2] === 'enable';
if (!this._shouldToggleDomain(domainCommand[1], enable)) {
return Promise.resolve();
}
}
return this._connection.sendCommand(method, ...params);
return this._connection.sendCommand(method, sessionId, ...params);
}

/**
Expand Down Expand Up @@ -1129,6 +1083,7 @@ class Driver {

// Enable auto-attaching to subtargets so we receive iframe information
await this.sendCommand('Target.setAutoAttach', {
flatten: true,
autoAttach: true,
// Pause targets on startup so we don't miss anything
waitForDebuggerOnStart: true,
Expand All @@ -1138,7 +1093,7 @@ class Driver {
await this.sendCommand('Page.setLifecycleEventsEnabled', {enabled: true});
await this.sendCommand('Emulation.setScriptExecutionDisabled', {value: disableJS});
// No timeout needed for Page.navigate. See https://github.com/GoogleChrome/lighthouse/pull/6413.
const waitforPageNavigateCmd = this._innerSendCommand('Page.navigate', {url});
const waitforPageNavigateCmd = this._innerSendCommand('Page.navigate', undefined, {url});

if (waitForNavigated) {
await this._waitForFrameNavigated();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class OptimizedImages extends Gatherer {
/** @type {Set<string>} */
const seenUrls = new Set();
return networkRecords.reduce((prev, record) => {
// Skip records that we've seen before, never finished, or came from OOPIFs.
// Skip records that we've seen before, never finished, or came from child targets.
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
if (seenUrls.has(record.url) || !record.finished || record.sessionId) {
return prev;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ResponseCompression extends Gatherer {
const unoptimizedResponses = [];

networkRecords.forEach(record => {
// Ignore records from OOPIFs
// Ignore records from child targets (OOPIFS).
if (record.sessionId) return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requests from the main target still won't have a sessionId will they?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.


const mimeType = record.mimeType;
Expand Down
5 changes: 5 additions & 0 deletions lighthouse-core/gather/gatherers/script-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ class ScriptElements extends Gatherer {
if (script.content) script.requestId = mainResource.requestId;
}

// console.log('CONNOR DEBUG networkRecords',
// JSON.stringify(loadData.networkRecords
// .filter(r => r.sessionId)
// .map(r => ({requestId: r.requestId, sessionId: r.sessionId, url: r.url})), null, 2));

const scriptRecords = loadData.networkRecords
// Ignore records from OOPIFs
.filter(record => !record.sessionId)
Expand Down
Loading