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(oopifs): skip OOPIF network records in some gatherers #7640

Merged
merged 2 commits into from
Mar 29, 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
4 changes: 2 additions & 2 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ class Driver {
* @param {string[]} parentSessionIds The list of session ids of the parents from youngest to oldest.
*/
async _handleReceivedMessageFromTarget(event, parentSessionIds) {
const {sessionId, message} = event;
const {targetId, sessionId, message} = event;
/** @type {LH.Protocol.RawMessage} */
const protocolMessage = JSON.parse(message);

Expand All @@ -317,7 +317,7 @@ class Driver {
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class OptimizedImages extends Gatherer {
/** @type {Set<string>} */
const seenUrls = new Set();
return networkRecords.reduce((prev, record) => {
if (seenUrls.has(record.url) || !record.finished) {
// Skip records that we've seen before, never finished, or came from 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,6 +40,9 @@ class ResponseCompression extends Gatherer {
const unoptimizedResponses = [];

networkRecords.forEach(record => {
// Ignore records from OOPIFs
if (record.sessionId) return;

const mimeType = record.mimeType;
const resourceType = record.resourceType || NetworkRequest.TYPES.Other;
const resourceSize = record.resourceSize;
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/gather/gatherers/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class Scripts extends Gatherer {
}

const scriptRecords = loadData.networkRecords
// Ignore records from OOPIFs
.filter(record => !record.sessionId)
// Only get the content of script requests
.filter(record => record.resourceType === NetworkRequest.TYPES.Script);

for (const record of scriptRecords) {
Expand Down
67 changes: 38 additions & 29 deletions lighthouse-core/lib/network-recorder.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,11 @@ class NetworkRecorder extends EventEmitter {
// DevTools SDK network layer.

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

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

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

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

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

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

/**
* @param {LH.Crdp.Network.ResourceChangedPriorityEvent} data
* @param {{params: LH.Crdp.Network.ResourceChangedPriorityEvent, source?: LH.Protocol.RawSource}} event
*/
onResourceChangedPriority(data) {
const request = this._findRealRequest(data.requestId);
onResourceChangedPriority(event) {
const data = event.params;
const request = this._findRealRequestAndSetSource(data.requestId, event.source);
if (!request) return;
request.onResourceChangedPriority(data);
}
Expand All @@ -296,13 +303,13 @@ class NetworkRecorder extends EventEmitter {
*/
dispatch(event) {
switch (event.method) {
case 'Network.requestWillBeSent': return this.onRequestWillBeSent(event.params);
case 'Network.requestServedFromCache': return this.onRequestServedFromCache(event.params);
case 'Network.responseReceived': return this.onResponseReceived(event.params);
case 'Network.dataReceived': return this.onDataReceived(event.params);
case 'Network.loadingFinished': return this.onLoadingFinished(event.params);
case 'Network.loadingFailed': return this.onLoadingFailed(event.params);
case 'Network.resourceChangedPriority': return this.onResourceChangedPriority(event.params);
case 'Network.requestWillBeSent': return this.onRequestWillBeSent(event);
case 'Network.requestServedFromCache': return this.onRequestServedFromCache(event);
case 'Network.responseReceived': return this.onResponseReceived(event);
case 'Network.dataReceived': return this.onDataReceived(event);
case 'Network.loadingFinished': return this.onLoadingFinished(event);
case 'Network.loadingFailed': return this.onLoadingFailed(event);
case 'Network.resourceChangedPriority': return this.onResourceChangedPriority(event);
default: return;
}
}
Expand All @@ -314,16 +321,18 @@ class NetworkRecorder extends EventEmitter {
* message is referring.
*
* @param {string} requestId
* @param {LH.Protocol.RawSource|undefined} source
* @return {NetworkRequest|undefined}
*/
_findRealRequest(requestId) {
_findRealRequestAndSetSource(requestId, source) {
let request = this._recordsById.get(requestId);
if (!request || !request.isValid) return undefined;

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

request.setSource(source);
return request;
}

Expand Down
25 changes: 25 additions & 0 deletions lighthouse-core/lib/network-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ module.exports = class NetworkRequest {
this.fetchedViaServiceWorker = false;
/** @type {string|undefined} */
this.frameId = '';
/**
* @type {string|undefined}
* Only set for OOPIFs. This is the targetId of the protocol target from which this
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

technically according to the types this can be undefined even when there was a source, but I have never been able to see this happen with an OOPIF

* request came. Undefined means it came from the root.
*/
this.targetId = undefined;
/**
* @type {string|undefined}
* Only set for OOPIFs. This is the sessionId of the protocol connection on which this
* request was discovered. Undefined means it came from the root.
*/
this.sessionId = undefined;
this.isLinkPreload = false;
}

Expand Down Expand Up @@ -233,6 +245,19 @@ module.exports = class NetworkRequest {
this._updateResponseReceivedTimeIfNecessary();
}

/**
* @param {LH.Protocol.RawSource|undefined} source
*/
setSource(source) {
if (source) {
this.targetId = source.targetId;
this.sessionId = source.sessionId;
} else {
this.targetId = undefined;
this.sessionId = undefined;
}
}

/**
* @param {LH.Crdp.Network.Response} response
* @param {number} timestamp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ const traceData = {
resourceType: 'Image',
finished: true,
},
{
requestId: '1',
url: 'http://gmail.com/image.jpg',
mimeType: 'image/jpeg',
resourceSize: 15000,
transferSize: 20000,
resourceType: 'Image',
finished: true,
sessionId: 'oopif', // ignore for being an oopif
},
{
requestId: '1',
url: 'data: image/jpeg ; base64 ,SgVcAT32587935321...',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@ const traceData = {
content: '1234567',
finished: true,
},
{
url: 'http://google.com/index.json',
_statusCode: 200,
mimeType: 'application/json',
requestId: 27,
resourceSize: 7,
transferSize: 8,
resourceType: 'XHR',
responseHeaders: [],
content: '1234567',
finished: true,
sessionId: 'oopif', // ignore for being from oopif
},
{
url: 'http://google.com/index.json',
_statusCode: 304, // ignore for being a cache not modified response
Expand Down
30 changes: 30 additions & 0 deletions lighthouse-core/test/lib/network-recorder-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,36 @@ describe('network recorder', function() {
});
});

it('should set the source of network records', () => {
const devtoolsLogs = networkRecordsToDevtoolsLog([
{url: 'http://example.com'},
{url: 'http://iframe.com'},
{url: 'http://other-iframe.com'},
]);

const requestId1 = devtoolsLogs.find(
log => log.params.request && log.params.request.url === 'http://iframe.com'
).params.requestId;
const requestId2 = devtoolsLogs.find(
log => log.params.request && log.params.request.url === 'http://other-iframe.com'
).params.requestId;

for (const log of devtoolsLogs) {
if (log.params.requestId === requestId1) log.source = {sessionId: '1', targetId: 'a'};

if (log.params.requestId === requestId2 && log.method === 'Network.loadingFinished') {
log.source = {sessionId: '2', targetId: 'b'};
}
}

const records = NetworkRecorder.recordsFromLogs(devtoolsLogs);
expect(records).toMatchObject([
{url: 'http://example.com', sessionId: undefined, targetId: undefined},
{url: 'http://iframe.com', sessionId: '1', targetId: 'a'},
{url: 'http://other-iframe.com', sessionId: '2', targetId: 'b'},
]);
});

describe('#findNetworkQuietPeriods', () => {
function record(data) {
const url = data.url || 'https://example.com';
Expand Down
7 changes: 7 additions & 0 deletions types/protocol.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ declare global {
*/
export type RawEventMessage = RawEventMessageRecord[keyof RawEventMessageRecord];

export interface RawSource {
targetId?: string;
sessionId: string;
}

/**
* Raw (over the wire) message format of all possible Crdp command responses.
*/
Expand Down Expand Up @@ -65,6 +70,8 @@ type RawEventMessageRecord = {
method: K,
// Drop [] for `undefined` (so a JS value is valid).
params: LH.CrdpEvents[K] extends [] ? undefined: LH.CrdpEvents[K][number]
// If the source is not set, it means the event was from the root target.
source?: LH.Protocol.RawSource
};
}

Expand Down