Skip to content

Commit

Permalink
core(oopifs): skip OOPIF network records in some gatherers (#7640)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored and paulirish committed Mar 29, 2019
1 parent 39d877f commit bed02b4
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 32 deletions.
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
* 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

0 comments on commit bed02b4

Please sign in to comment.