From c67c7c94aae07b212a6b1cc890f0cab817c9221b Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 3 Oct 2019 18:13:31 -0700 Subject: [PATCH 01/16] flatten --- .../gather/connections/connection.js | 14 +- lighthouse-core/gather/driver.js | 127 ++++++------------ .../gatherers/dobetterweb/optimized-images.js | 2 +- .../dobetterweb/response-compression.js | 2 +- .../gather/gatherers/script-elements.js | 5 + lighthouse-core/lib/network-recorder.js | 36 ++--- lighthouse-core/lib/network-request.js | 22 +-- types/protocol.d.ts | 9 +- 8 files changed, 83 insertions(+), 134 deletions(-) diff --git a/lighthouse-core/gather/connections/connection.js b/lighthouse-core/gather/connections/connection.js index 2661554a5604..b7040bbdf812 100644 --- a/lighthouse-core/gather/connections/connection.js +++ b/lighthouse-core/gather/connections/connection.js @@ -52,17 +52,21 @@ class Connection { * Call protocol methods * @template {keyof LH.CrdpCommands} C * @param {C} method + * @param {string=} sessionId * @param {LH.CrdpCommands[C]['paramsType']} paramArgs, * @return {Promise} */ - 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; + const messageJson = JSON.stringify(message); + this.sendRawMessage(messageJson); return new Promise(resolve => { this._callbacks.set(id, {method, resolve}); @@ -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; } @@ -117,7 +121,7 @@ class Connection { if (callback) { this._callbacks.delete(object.id); - return callback.resolve(Promise.resolve().then(_ => { + callback.resolve(Promise.resolve().then(_ => { if (object.error) { log.formatProtocol('method <= browser ERR', {method: callback.method}, 'error'); throw LHError.fromProtocolMessage(callback.method, object.error); diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 7512c4570314..6d3c8d5d727a 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -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 @@ -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); 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} */ - 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) { + 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} */ - sendCommand(method, ...params) { + sendCommandToSession(method, sessionId, ...params) { const timeout = this._nextProtocolTimeout; this._nextProtocolTimeout = DEFAULT_PROTOCOL_TIMEOUT; return new Promise(async (resolve, reject) => { @@ -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); @@ -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} */ - _innerSendCommand(method, ...params) { + _innerSendCommand(method, sessionId, ...params) { const domainCommand = /^(\w+)\.(enable|disable)$/.exec(method); - if (domainCommand) { + // TODO: store domain state for each session. + 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); } /** @@ -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, @@ -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(); diff --git a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js index 9f155f54fdbe..78a3630a9d6e 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js @@ -45,7 +45,7 @@ class OptimizedImages extends Gatherer { /** @type {Set} */ 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. if (seenUrls.has(record.url) || !record.finished || record.sessionId) { return prev; } diff --git a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js index a98c62c94851..beb2eb20bb21 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js @@ -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; const mimeType = record.mimeType; diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index cb01c66c4331..c451e08f22eb 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -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) diff --git a/lighthouse-core/lib/network-recorder.js b/lighthouse-core/lib/network-recorder.js index 70a109dd3437..6fe760ab1a4c 100644 --- a/lighthouse-core/lib/network-recorder.js +++ b/lighthouse-core/lib/network-recorder.js @@ -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; } @@ -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); } @@ -310,10 +311,10 @@ 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; @@ -321,7 +322,8 @@ class NetworkRecorder extends EventEmitter { request = request.redirectDestination; } - request.setSource(source); + request.setSession(sessionId); + return request; } diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js index 0e980bc4c079..0c8579dfed02 100644 --- a/lighthouse-core/lib/network-request.js +++ b/lighthouse-core/lib/network-request.js @@ -129,14 +129,8 @@ class NetworkRequest { 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. + * Only set for child targets (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; @@ -272,16 +266,10 @@ class NetworkRequest { } /** - * @param {LH.Protocol.RawSource|undefined} source + * @param {string=} sessionId */ - setSource(source) { - if (source) { - this.targetId = source.targetId; - this.sessionId = source.sessionId; - } else { - this.targetId = undefined; - this.sessionId = undefined; - } + setSession(sessionId) { + this.sessionId = sessionId; } /** diff --git a/types/protocol.d.ts b/types/protocol.d.ts index 61c3ec5804ef..c57881f2c110 100644 --- a/types/protocol.d.ts +++ b/types/protocol.d.ts @@ -12,11 +12,6 @@ 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. */ @@ -70,8 +65,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 + // If sessionId is not set, it means the event was from the root target. + sessionId?: string; }; } From 67b9acecdfd2115a4f3b6f56ab392028fd101e92 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 4 Oct 2019 10:17:32 -0700 Subject: [PATCH 02/16] undo ws --- lighthouse-core/gather/connections/connection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/gather/connections/connection.js b/lighthouse-core/gather/connections/connection.js index b7040bbdf812..e64a0387614d 100644 --- a/lighthouse-core/gather/connections/connection.js +++ b/lighthouse-core/gather/connections/connection.js @@ -112,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; } From 922947859869bade49c8876dadd552103ce61c90 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 4 Oct 2019 10:21:52 -0700 Subject: [PATCH 03/16] sendRawMessage --- lighthouse-core/gather/connections/connection.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/gather/connections/connection.js b/lighthouse-core/gather/connections/connection.js index e64a0387614d..c5e12f34677f 100644 --- a/lighthouse-core/gather/connections/connection.js +++ b/lighthouse-core/gather/connections/connection.js @@ -62,11 +62,8 @@ class Connection { log.formatProtocol('method => browser', {method, params}, 'verbose'); const id = ++this._lastCommandId; - const message = {id, method, params}; - // @ts-ignore - if (sessionId) message.sessionId = sessionId; - const messageJson = JSON.stringify(message); - this.sendRawMessage(messageJson); + const message = JSON.stringify({id, sessionId, method, params}); + this.sendRawMessage(message); return new Promise(resolve => { this._callbacks.set(id, {method, resolve}); From f6a2831fef7b728d6284f4ace926f0e4a8472d00 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 4 Oct 2019 10:25:46 -0700 Subject: [PATCH 04/16] use sendCommandToSession --- lighthouse-core/gather/driver.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 6d3c8d5d727a..f4f1be479a49 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -295,16 +295,16 @@ class Driver { // 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._innerSendCommand('Runtime.runIfWaitingForDebugger', event.sessionId); + 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._innerSendCommand('Network.enable', event.sessionId); + 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._innerSendCommand('Target.setAutoAttach', event.sessionId, { + await this.sendCommandToSession('Target.setAutoAttach', event.sessionId, { autoAttach: true, flatten: true, // Pause targets on startup so we don't miss anything @@ -312,7 +312,7 @@ class Driver { }); // We suspended the target when we auto-attached, so make sure it goes back to being normal. - await this._innerSendCommand('Runtime.runIfWaitingForDebugger', event.sessionId); + await this.sendCommandToSession('Runtime.runIfWaitingForDebugger', event.sessionId); } /** From de0c3c61e9226979f09cf402e2f391c400fc1d04 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 8 Oct 2019 10:29:13 -0700 Subject: [PATCH 05/16] rm test code --- lighthouse-core/gather/gatherers/script-elements.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index c451e08f22eb..cb01c66c4331 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -62,11 +62,6 @@ 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) From 06c2fe894327db9b4481198c4b970da21793a566 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 8 Oct 2019 11:51:26 -0700 Subject: [PATCH 06/16] fix driver tests --- lighthouse-core/gather/driver.js | 2 +- lighthouse-core/test/gather/driver-test.js | 82 ++++---------------- lighthouse-core/test/gather/mock-commands.js | 16 ++-- 3 files changed, 25 insertions(+), 75 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index f4f1be479a49..3c794a99fe97 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -311,7 +311,7 @@ class Driver { waitForDebuggerOnStart: true, }); - // We suspended the target when we auto-attached, so make sure it goes back to being normal. + // // We suspended the target when we auto-attached, so make sure it goes back to being normal. await this.sendCommandToSession('Runtime.runIfWaitingForDebugger', event.sessionId); } diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index d78ba2c9757a..6b566b2b16ae 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -198,7 +198,7 @@ describe('.evaluateAsync', () => { it('uses a high default timeout', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Runtime.evaluate', {result: {value: 2}}, 65000); + .mockResponse('Runtime.evaluate', {result: {value: 2}}, 65000); const evaluatePromise = makePromiseInspectable(driver.evaluateAsync('1 + 1')); jest.advanceTimersByTime(30000); @@ -213,7 +213,7 @@ describe('.evaluateAsync', () => { it('uses the specific timeout given', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Runtime.evaluate', {result: {value: 2}}, 10000); + .mockResponse('Runtime.evaluate', {result: {value: 2}}, 10000); driver.setNextProtocolTimeout(5000); const evaluatePromise = makePromiseInspectable(driver.evaluateAsync('1 + 1')); @@ -343,6 +343,7 @@ describe('.setExtraHTTPHeaders', () => { expect(connectionStub.sendCommand).toHaveBeenCalledWith( 'Network.setExtraHTTPHeaders', + undefined, expect.anything() ); }); @@ -873,9 +874,9 @@ describe('.goOnline', () => { describe('Multi-target management', () => { it('enables the Network domain for iframes', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Target.sendMessageToTarget', {}) - .mockResponse('Target.sendMessageToTarget', {}) - .mockResponse('Target.sendMessageToTarget', {}); + .mockResponseToSession('Network.enable', 123, {}) + .mockResponseToSession('Target.setAutoAttach', 123, {}) + .mockResponseToSession('Runtime.runIfWaitingForDebugger', 123, {}); driver._eventEmitter.emit('Target.attachedToTarget', { sessionId: 123, @@ -883,63 +884,11 @@ describe('Multi-target management', () => { }); await flushAllTimersAndMicrotasks(); - const sendMessageArgs = connectionStub.sendCommand - .findInvocation('Target.sendMessageToTarget'); - expect(sendMessageArgs).toEqual({ - message: '{"id":1,"method":"Network.enable"}', - sessionId: 123, - }); - }); - - it('enables the Network domain for iframes of iframes of iframes', async () => { - connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Target.sendMessageToTarget', {}) - .mockResponse('Target.sendMessageToTarget', {}) - .mockResponse('Target.sendMessageToTarget', {}); - - driver._eventEmitter.emit('Target.receivedMessageFromTarget', { - sessionId: 'Outer', - message: JSON.stringify({ - method: 'Target.receivedMessageFromTarget', - params: { - sessionId: 'Middle', - message: JSON.stringify({ - method: 'Target.attachedToTarget', - params: { - sessionId: 'Inner', - targetInfo: {type: 'iframe'}, - }, - }), - }, - }), - }); - - await flushAllTimersAndMicrotasks(); - - const sendMessageArgs = connectionStub.sendCommand - .findInvocation('Target.sendMessageToTarget'); - const stringified = `{ - "id": 3, - "method": "Target.sendMessageToTarget", - "params": { - "sessionId": "Middle", - "message": "{ - \\"id\\": 2, - \\"method\\": \\"Target.sendMessageToTarget\\", - \\"params\\": { - \\"sessionId\\": \\"Inner\\", - \\"message\\":\\ "{ - \\\\\\"id\\\\\\":1, - \\\\\\"method\\\\\\":\\\\\\"Network.enable\\\\\\" - }\\" - }}" - } - }`.replace(/\s+/g, ''); - - expect(sendMessageArgs).toEqual({ - message: stringified, - sessionId: 'Outer', - }); + expect(connectionStub.sendCommand).toHaveBeenNthCalledWith(1, 'Network.enable', 123); + expect(connectionStub.sendCommand) + .toHaveBeenNthCalledWith(2, 'Target.setAutoAttach', 123, expect.anything()); + expect(connectionStub.sendCommand) + .toHaveBeenNthCalledWith(3, 'Runtime.runIfWaitingForDebugger', 123); }); it('ignores other target types, but still resumes them', async () => { @@ -952,12 +901,7 @@ describe('Multi-target management', () => { }); await flushAllTimersAndMicrotasks(); - - const sendMessageArgs = connectionStub.sendCommand - .findInvocation('Target.sendMessageToTarget'); - expect(sendMessageArgs).toEqual({ - message: JSON.stringify({id: 1, method: 'Runtime.runIfWaitingForDebugger'}), - sessionId: 'SW1', - }); + expect(connectionStub.sendCommand) + .toHaveBeenCalledWith('Runtime.runIfWaitingForDebugger', 'SW1'); }); }); diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index 32d7872634cb..773935e04222 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -22,8 +22,9 @@ */ function createMockSendCommandFn() { const mockResponses = []; - const mockFn = jest.fn().mockImplementation((command, ...args) => { - const indexOfResponse = mockResponses.findIndex(entry => entry.command === command); + const mockFn = jest.fn().mockImplementation((command, sessionId, ...args) => { + const indexOfResponse = mockResponses + .findIndex(entry => entry.command === command && entry.sessionId === sessionId); if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); const {response, delay} = mockResponses[indexOfResponse]; mockResponses.splice(indexOfResponse, 1); @@ -37,9 +38,14 @@ function createMockSendCommandFn() { return mockFn; }; - mockFn.findInvocation = command => { - expect(mockFn).toHaveBeenCalledWith(command, expect.anything()); - return mockFn.mock.calls.find(call => call[0] === command)[1]; + mockFn.mockResponseToSession = (command, sessionId, response, delay) => { + mockResponses.push({command, sessionId, response, delay}); + return mockFn; + }; + + mockFn.findInvocation = (command, sessionId) => { + expect(mockFn).toHaveBeenCalledWith(command, sessionId, expect.anything()); + return mockFn.mock.calls.find(call => call[0] === command && call[1] === sessionId)[2]; }; return mockFn; From a040b1a44451c1d175cb088ebf9ecd7bf799179b Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 8 Oct 2019 11:54:03 -0700 Subject: [PATCH 07/16] fix gather runner --- lighthouse-core/test/gather/gather-runner-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index a05e49d4ecd8..8a504ca04164 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -63,7 +63,7 @@ function getMockedEmulationDriver(emulationFn, netThrottleFn, cpuThrottleFn, clearDataForOrigin() {} }; const EmulationMock = class extends Connection { - sendCommand(command, params) { + sendCommand(command, sessionId, params) { let fn = null; switch (command) { case 'Network.emulateNetworkConditions': From f503fdb6a8abaa007216f4a3df09af6851a0c141 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 8 Oct 2019 12:56:53 -0700 Subject: [PATCH 08/16] fix gather runner test. skip extension test --- clients/test/extension/extension-test.js | 2 +- lighthouse-core/test/lib/network-recorder-test.js | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clients/test/extension/extension-test.js b/clients/test/extension/extension-test.js index 63e86e88cd6d..6911fab87801 100644 --- a/clients/test/extension/extension-test.js +++ b/clients/test/extension/extension-test.js @@ -18,7 +18,7 @@ const config = require(path.resolve(__dirname, const getAuditsOfCategory = category => config.categories[category].auditRefs; -describe('Lighthouse chrome extension', function() { +describe.skip('Lighthouse chrome extension', function() { // eslint-disable-next-line no-console console.log('\n✨ Be sure to have recently run this: yarn build-extension'); diff --git a/lighthouse-core/test/lib/network-recorder-test.js b/lighthouse-core/test/lib/network-recorder-test.js index bb318434e659..2c1c8c5def76 100644 --- a/lighthouse-core/test/lib/network-recorder-test.js +++ b/lighthouse-core/test/lib/network-recorder-test.js @@ -195,18 +195,18 @@ describe('network recorder', function() { ).params.requestId; for (const log of devtoolsLogs) { - if (log.params.requestId === requestId1) log.source = {sessionId: '1', targetId: 'a'}; + if (log.params.requestId === requestId1) log.sessionId = '1'; if (log.params.requestId === requestId2 && log.method === 'Network.loadingFinished') { - log.source = {sessionId: '2', targetId: 'b'}; + log.sessionId = '2'; } } 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'}, + {url: 'http://example.com', sessionId: undefined}, + {url: 'http://iframe.com', sessionId: '1'}, + {url: 'http://other-iframe.com', sessionId: '2'}, ]); }); From 0175d0a03ab989560bd833c90f4c0fb722d68143 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 8 Oct 2019 13:14:28 -0700 Subject: [PATCH 09/16] reuse impl --- lighthouse-core/gather/driver.js | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 3c794a99fe97..cc974dbe939e 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -320,10 +320,11 @@ class Driver { * To configure the timeout for the next call, use 'setNextProtocolTimeout'. * @template {keyof LH.CrdpCommands} C * @param {C} method + * @param {string=} sessionId * @param {LH.CrdpCommands[C]['paramsType']} params * @return {Promise} */ - sendCommand(method, ...params) { + sendCommandToSession(method, sessionId, ...params) { const timeout = this._nextProtocolTimeout; this._nextProtocolTimeout = DEFAULT_PROTOCOL_TIMEOUT; return new Promise(async (resolve, reject) => { @@ -335,7 +336,7 @@ class Driver { reject(err); }), timeout); try { - const result = await this._innerSendCommand(method, undefined, ...params); + const result = await this._innerSendCommand(method, sessionId, ...params); resolve(result); } catch (err) { reject(err); @@ -346,33 +347,14 @@ class Driver { } /** - * Same as `sendCommand`, but for a specific session. + * Alias for 'sendCommandToSession(method, undefined, ...params)' * @template {keyof LH.CrdpCommands} C * @param {C} method - * @param {string} sessionId * @param {LH.CrdpCommands[C]['paramsType']} params * @return {Promise} */ - sendCommandToSession(method, sessionId, ...params) { - 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, sessionId, ...params); - resolve(result); - } catch (err) { - reject(err); - } finally { - clearTimeout(asyncTimeout); - } - }); + sendCommand(method, ...params) { + return this.sendCommandToSession(method, undefined, ...params); } /** From 6181350665112bc987bac412c438ef64262ad772 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 9 Oct 2019 10:22:43 -0700 Subject: [PATCH 10/16] ts --- lighthouse-core/gather/connections/connection.js | 2 +- lighthouse-core/gather/driver.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/gather/connections/connection.js b/lighthouse-core/gather/connections/connection.js index c5e12f34677f..3655ecb80326 100644 --- a/lighthouse-core/gather/connections/connection.js +++ b/lighthouse-core/gather/connections/connection.js @@ -52,7 +52,7 @@ class Connection { * Call protocol methods * @template {keyof LH.CrdpCommands} C * @param {C} method - * @param {string=} sessionId + * @param {string|undefined} sessionId * @param {LH.CrdpCommands[C]['paramsType']} paramArgs, * @return {Promise} */ diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index cc974dbe939e..33c1089759a8 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -318,9 +318,10 @@ class Driver { /** * 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=} sessionId + * @param {string|undefined} sessionId * @param {LH.CrdpCommands[C]['paramsType']} params * @return {Promise} */ @@ -362,7 +363,7 @@ class Driver { * @private * @template {keyof LH.CrdpCommands} C * @param {C} method - * @param {string=} sessionId + * @param {string|undefined} sessionId * @param {LH.CrdpCommands[C]['paramsType']} params * @return {Promise} */ From ee95987244c239130b29e28eaf69451e82700f63 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 9 Oct 2019 10:51:35 -0700 Subject: [PATCH 11/16] domain state mgmt for sessions --- lighthouse-core/gather/driver.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 33c1089759a8..cded976b30c8 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -234,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) { + 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)) { @@ -369,10 +371,9 @@ class Driver { */ _innerSendCommand(method, sessionId, ...params) { const domainCommand = /^(\w+)\.(enable|disable)$/.exec(method); - // TODO: store domain state for each session. - if (domainCommand && !sessionId) { + if (domainCommand) { const enable = domainCommand[2] === 'enable'; - if (!this._shouldToggleDomain(domainCommand[1], enable)) { + if (!this._shouldToggleDomain(domainCommand[1], sessionId, enable)) { return Promise.resolve(); } } From 717fd95809afd41ab3e511bd5f60dda20c0e498d Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 11 Oct 2019 16:58:20 -0700 Subject: [PATCH 12/16] test --- lighthouse-core/test/gather/driver-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 6b566b2b16ae..3185d6d5d9bf 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -902,6 +902,6 @@ describe('Multi-target management', () => { await flushAllTimersAndMicrotasks(); expect(connectionStub.sendCommand) - .toHaveBeenCalledWith('Runtime.runIfWaitingForDebugger', 'SW1'); + .toHaveBeenNthCalledWith(1, 'Runtime.runIfWaitingForDebugger', 'SW1'); }); }); From e506a63b4e68a39b2b304d29f2de632ce6a8aa7c Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 14 Oct 2019 16:36:30 -0700 Subject: [PATCH 13/16] Apply suggestions from code review Co-Authored-By: Patrick Hulce --- lighthouse-core/gather/driver.js | 2 +- lighthouse-core/lib/network-request.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index cded976b30c8..89d6e408f005 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -313,7 +313,7 @@ class Driver { waitForDebuggerOnStart: true, }); - // // We suspended the target when we auto-attached, so make sure it goes back to being normal. + // We suspended the target when we auto-attached, so make sure it goes back to being normal. await this.sendCommandToSession('Runtime.runIfWaitingForDebugger', event.sessionId); } diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js index 0c8579dfed02..cce715d7f437 100644 --- a/lighthouse-core/lib/network-request.js +++ b/lighthouse-core/lib/network-request.js @@ -129,8 +129,8 @@ class NetworkRequest { this.frameId = ''; /** * @type {string|undefined} - * Only set for child targets (OOPIFS). This is the sessionId of the protocol connection on - * which this request was discovered. Undefined means it came from the root. + * Only set for child targets (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; From 9274b8a2e5346203afd96d77337ced9848ba7e27 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 15 Oct 2019 11:41:42 -0700 Subject: [PATCH 14/16] supress extension errors --- lighthouse-core/gather/connections/extension.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/gather/connections/extension.js b/lighthouse-core/gather/connections/extension.js index 0c1a20a57f13..a38dd7969715 100644 --- a/lighthouse-core/gather/connections/extension.js +++ b/lighthouse-core/gather/connections/extension.js @@ -19,6 +19,9 @@ class ExtensionConnection extends Connection { this._onEvent = this._onEvent.bind(this); this._onUnexpectedDetach = this._onUnexpectedDetach.bind(this); + + // The extension won't work anymore, will be removed post-haste. + throw new Error('ExtensionConnection is defunct.'); } /** @@ -111,10 +114,11 @@ class ExtensionConnection extends Connection { * Call protocol methods. * @template {keyof LH.CrdpCommands} C * @param {C} method + * @param {string} _ * @param {LH.CrdpCommands[C]['paramsType']} paramArgs, * @return {Promise} */ - sendCommand(method, ...paramArgs) { + sendCommand(method, _, ...paramArgs) { // Reify params since we need it as a property so can't just spread again. const params = paramArgs.length ? paramArgs[0] : undefined; From 5e3c161efdcc5de7d27cf205f4eb680084989757 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Oct 2019 16:07:21 -0700 Subject: [PATCH 15/16] domain test --- lighthouse-core/test/gather/driver-test.js | 70 +++++++++++++++++++--- 1 file changed, 63 insertions(+), 7 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 9b9d17ae8561..6fb7d7854c03 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -871,24 +871,80 @@ describe('.goOnline', () => { }); }); +describe('Domain.enable/disable State', () => { + it('dedupes (simple)', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Network.enable') + .mockResponse('Network.disable') + .mockResponse('Fetch.enable') + .mockResponse('Fetch.disable'); + + await driver.sendCommand('Network.enable', {}); + await driver.sendCommand('Network.enable', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(1); + + await driver.sendCommand('Network.disable', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(1); + // Network still has one enable. + + await driver.sendCommand('Fetch.enable', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); + + await driver.sendCommand('Network.disable', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(3); + // Network is now disabled. + + await driver.sendCommand('Fetch.disable', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(4); + }); + + it('dedupes (sessions)', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Network.enable') + .mockResponseToSession('Network.enable', '123') + .mockResponse('Network.disable') + .mockResponseToSession('Network.disable', '123'); + + await driver.sendCommand('Network.enable', {}); + await driver.sendCommandToSession('Network.enable', '123', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); + + await driver.sendCommand('Network.enable', {}); + await driver.sendCommandToSession('Network.enable', '123', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); + + await driver.sendCommandToSession('Network.disable', '123', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); + + await driver.sendCommand('Network.disable', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); + + await driver.sendCommandToSession('Network.disable', '123', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(3); + + await driver.sendCommand('Network.disable', {}); + expect(connectionStub.sendCommand).toHaveBeenCalledTimes(4); + }); +}); + describe('Multi-target management', () => { it('enables the Network domain for iframes', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponseToSession('Network.enable', 123, {}) - .mockResponseToSession('Target.setAutoAttach', 123, {}) - .mockResponseToSession('Runtime.runIfWaitingForDebugger', 123, {}); + .mockResponseToSession('Network.enable', '123', {}) + .mockResponseToSession('Target.setAutoAttach', '123', {}) + .mockResponseToSession('Runtime.runIfWaitingForDebugger', '123', {}); driver._eventEmitter.emit('Target.attachedToTarget', { - sessionId: 123, + sessionId: '123', targetInfo: {type: 'iframe'}, }); await flushAllTimersAndMicrotasks(); - expect(connectionStub.sendCommand).toHaveBeenNthCalledWith(1, 'Network.enable', 123); + expect(connectionStub.sendCommand).toHaveBeenNthCalledWith(1, 'Network.enable', '123'); expect(connectionStub.sendCommand) - .toHaveBeenNthCalledWith(2, 'Target.setAutoAttach', 123, expect.anything()); + .toHaveBeenNthCalledWith(2, 'Target.setAutoAttach', '123', expect.anything()); expect(connectionStub.sendCommand) - .toHaveBeenNthCalledWith(3, 'Runtime.runIfWaitingForDebugger', 123); + .toHaveBeenNthCalledWith(3, 'Runtime.runIfWaitingForDebugger', '123'); }); it('ignores other target types, but still resumes them', async () => { From 65795cc46f515bf805d2f2be781effd6064cbfa6 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 4 Nov 2019 13:26:34 -0800 Subject: [PATCH 16/16] comment --- .../gather/gatherers/dobetterweb/optimized-images.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js index 2299cc77bbcc..c924991cb6a0 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js @@ -46,7 +46,7 @@ class OptimizedImages extends Gatherer { /** @type {Set} */ const seenUrls = new Set(); return networkRecords.reduce((prev, record) => { - // Skip records that we've seen before, never finished, or came from child targets. + // 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; }