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 all commits
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
7 changes: 4 additions & 3 deletions lighthouse-core/gather/connections/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,17 @@ class Connection {
* Call protocol methods
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {string|undefined} sessionId
* @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});
const message = JSON.stringify({id, sessionId, method, params});
this.sendRawMessage(message);

return new Promise(resolve => {
Expand Down Expand Up @@ -117,7 +118,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
129 changes: 34 additions & 95 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 @@ -247,14 +234,16 @@ class Driver {
* user of that domain (e.g. two gatherers have enabled a domain, both need to
* disable it for it to be disabled). Returns true otherwise.
* @param {string} domain
* @param {string|undefined} sessionId
* @param {boolean} enable
* @return {boolean}
* @private
*/
_shouldToggleDomain(domain, enable) {
const enabledCount = this._domainEnabledCounts.get(domain) || 0;
_shouldToggleDomain(domain, sessionId, enable) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

time to add tests for this functionality? :D

should hopefully be pretty easy with the new mock capabilities and can just sendCommand('Network.enable') or whatever to make sure it doesn't turn off to early :)

Copy link
Member

Choose a reason for hiding this comment

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

time to add tests for this functionality? :D

should hopefully be pretty easy with the new mock capabilities and can just sendCommand('Network.enable') or whatever to make sure it doesn't turn off to early :)

+1 to this. After a certain number of enables and disables, watching when the actual command is sent shouldn't end up too bad, I hope.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

const key = domain + (sessionId || '');
const enabledCount = this._domainEnabledCounts.get(key) || 0;
const newCount = enabledCount + (enable ? 1 : -1);
this._domainEnabledCounts.set(domain, Math.max(0, newCount));
this._domainEnabledCounts.set(key, Math.max(0, newCount));

// Switching to enabled or disabled, respectively.
if ((enable && newCount === 1) || (!enable && newCount === 0)) {
Expand Down Expand Up @@ -300,108 +289,45 @@ 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.sendCommandToSession('Runtime.runIfWaitingForDebugger', event.sessionId);
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.sendCommandToSession('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.sendCommandToSession('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');
}

/**
* Send protocol commands to a subtarget, wraps the message in as many `Target.sendMessageToTarget`
* commands as necessary.
*
* @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);
await this.sendCommandToSession('Runtime.runIfWaitingForDebugger', event.sessionId);
}

/**
* Call protocol methods, with a timeout.
* To configure the timeout for the next call, use 'setNextProtocolTimeout'.
* If 'sessionId' is undefined, the message is sent to the main session.
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {string|undefined} 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 +339,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 @@ -423,23 +349,35 @@ class Driver {
});
}

/**
* Alias for 'sendCommandToSession(method, undefined, ...params)'
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {LH.CrdpCommands[C]['paramsType']} params
* @return {Promise<LH.CrdpCommands[C]['returnType']>}
*/
sendCommand(method, ...params) {
return this.sendCommandToSession(method, undefined, ...params);
}

/**
* Call protocol methods.
* @private
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {string|undefined} 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) {
const enable = domainCommand[2] === 'enable';
if (!this._shouldToggleDomain(domainCommand[1], enable)) {
if (!this._shouldToggleDomain(domainCommand[1], sessionId, enable)) {
return Promise.resolve();
}
}
return this._connection.sendCommand(method, ...params);
return this._connection.sendCommand(method, sessionId, ...params);
}

/**
Expand Down Expand Up @@ -1133,6 +1071,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 @@ -1142,7 +1081,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 @@ -46,7 +46,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 (OOPIFS).
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
36 changes: 19 additions & 17 deletions lighthouse-core/lib/network-recorder.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,16 @@ class NetworkRecorder extends EventEmitter {
// DevTools SDK network layer.

/**
* @param {{params: LH.Crdp.Network.RequestWillBeSentEvent, source?: LH.Protocol.RawSource}} event
* @param {{params: LH.Crdp.Network.RequestWillBeSentEvent, sessionId?: string}} event
*/
onRequestWillBeSent(event) {
const data = event.params;
const originalRequest = this._findRealRequestAndSetSource(data.requestId, event.source);
const originalRequest = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
// This is a simple new request, create the NetworkRequest object and finish.
if (!originalRequest) {
const request = new NetworkRequest();
request.onRequestWillBeSent(data);
request.sessionId = event.sessionId;
this.onRequestStarted(request);
return;
}
Expand Down Expand Up @@ -225,63 +226,63 @@ class NetworkRecorder extends EventEmitter {
}

/**
* @param {{params: LH.Crdp.Network.RequestServedFromCacheEvent, source?: LH.Protocol.RawSource}} event
* @param {{params: LH.Crdp.Network.RequestServedFromCacheEvent, sessionId?: string}} event
*/
onRequestServedFromCache(event) {
const data = event.params;
const request = this._findRealRequestAndSetSource(data.requestId, event.source);
const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
if (!request) return;
request.onRequestServedFromCache();
}

/**
* @param {{params: LH.Crdp.Network.ResponseReceivedEvent, source?: LH.Protocol.RawSource}} event
* @param {{params: LH.Crdp.Network.ResponseReceivedEvent, sessionId?: string}} event
*/
onResponseReceived(event) {
const data = event.params;
const request = this._findRealRequestAndSetSource(data.requestId, event.source);
const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
if (!request) return;
request.onResponseReceived(data);
}

/**
* @param {{params: LH.Crdp.Network.DataReceivedEvent, source?: LH.Protocol.RawSource}} event
* @param {{params: LH.Crdp.Network.DataReceivedEvent, sessionId?: string}} event
*/
onDataReceived(event) {
const data = event.params;
const request = this._findRealRequestAndSetSource(data.requestId, event.source);
const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
if (!request) return;
request.onDataReceived(data);
}

/**
* @param {{params: LH.Crdp.Network.LoadingFinishedEvent, source?: LH.Protocol.RawSource}} event
* @param {{params: LH.Crdp.Network.LoadingFinishedEvent, sessionId?: string}} event
*/
onLoadingFinished(event) {
const data = event.params;
const request = this._findRealRequestAndSetSource(data.requestId, event.source);
const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
if (!request) return;
request.onLoadingFinished(data);
this.onRequestFinished(request);
}

/**
* @param {{params: LH.Crdp.Network.LoadingFailedEvent, source?: LH.Protocol.RawSource}} event
* @param {{params: LH.Crdp.Network.LoadingFailedEvent, sessionId?: string}} event
*/
onLoadingFailed(event) {
const data = event.params;
const request = this._findRealRequestAndSetSource(data.requestId, event.source);
const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
if (!request) return;
request.onLoadingFailed(data);
this.onRequestFinished(request);
}

/**
* @param {{params: LH.Crdp.Network.ResourceChangedPriorityEvent, source?: LH.Protocol.RawSource}} event
* @param {{params: LH.Crdp.Network.ResourceChangedPriorityEvent, sessionId?: string}} event
*/
onResourceChangedPriority(event) {
const data = event.params;
const request = this._findRealRequestAndSetSource(data.requestId, event.source);
const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId);
if (!request) return;
request.onResourceChangedPriority(data);
}
Expand Down Expand Up @@ -310,18 +311,19 @@ class NetworkRecorder extends EventEmitter {
* message is referring.
*
* @param {string} requestId
* @param {LH.Protocol.RawSource|undefined} source
* @param {string|undefined} sessionId
* @return {NetworkRequest|undefined}
*/
_findRealRequestAndSetSource(requestId, source) {
_findRealRequestAndSetSession(requestId, sessionId) {
let request = this._recordsById.get(requestId);
if (!request || !request.isValid) return undefined;

while (request.redirectDestination) {
request = request.redirectDestination;
}

request.setSource(source);
request.setSession(sessionId);

return request;
}

Expand Down
Loading