diff --git a/lighthouse-core/fraggle-rock/config/default-config.js b/lighthouse-core/fraggle-rock/config/default-config.js index 6050ef52c2c5..e487492f940e 100644 --- a/lighthouse-core/fraggle-rock/config/default-config.js +++ b/lighthouse-core/fraggle-rock/config/default-config.js @@ -31,6 +31,7 @@ const artifacts = { RobotsTxt: '', Stacks: '', TapTargets: '', + TraceElements: '', ViewportDimensions: '', WebAppManifest: '', devtoolsLogs: '', @@ -68,6 +69,7 @@ const defaultConfig = { {id: artifacts.RobotsTxt, gatherer: 'seo/robots-txt'}, {id: artifacts.Stacks, gatherer: 'stacks'}, {id: artifacts.TapTargets, gatherer: 'seo/tap-targets'}, + {id: artifacts.TraceElements, gatherer: 'trace-elements'}, {id: artifacts.ViewportDimensions, gatherer: 'viewport-dimensions'}, {id: artifacts.WebAppManifest, gatherer: 'web-app-manifest'}, /* eslint-enable max-len */ @@ -103,6 +105,7 @@ const defaultConfig = { artifacts.RobotsTxt, artifacts.Stacks, artifacts.TapTargets, + artifacts.TraceElements, artifacts.ViewportDimensions, artifacts.WebAppManifest, diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js index a41fcbc936fd..bd8ca6d4f57f 100644 --- a/lighthouse-core/gather/gatherers/trace-elements.js +++ b/lighthouse-core/gather/gatherers/trace-elements.js @@ -13,12 +13,13 @@ * We take the backend nodeId from the trace and use it to find the corresponding element in the DOM. */ -const Gatherer = require('./gatherer.js'); -const dom = require('../driver/dom.js'); +const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js'); +const {resolveNodeIdToObjectId} = require('../driver/dom.js'); const pageFunctions = require('../../lib/page-functions.js'); const TraceProcessor = require('../../lib/tracehouse/trace-processor.js'); const RectHelpers = require('../../lib/rect-helpers.js'); const Sentry = require('../../lib/sentry.js'); +const Trace = require('./trace.js'); /** @typedef {{nodeId: number, score?: number, animations?: {name?: string, failureReasonsMask?: number, unsupportedProperties?: string[]}[]}} TraceElementData */ @@ -37,7 +38,26 @@ function getNodeDetailsData() { } /* c8 ignore stop */ -class TraceElements extends Gatherer { +class TraceElements extends FRGatherer { + /** @type {LH.Gatherer.GathererMeta<'Trace'>} */ + meta = { + supportedModes: ['timespan', 'navigation'], + dependencies: {Trace: Trace.symbol}, + } + + /** @type {Map} */ + animationIdToName = new Map(); + + constructor() { + super(); + this._onAnimationStarted = this._onAnimationStarted.bind(this); + } + + /** @param {LH.Crdp.Animation.AnimationStartedEvent} args */ + _onAnimationStarted({animation: {id, name}}) { + if (name) this.animationIdToName.set(id, name); + } + /** * @param {LH.TraceEvent | undefined} event * @return {number | undefined} @@ -88,34 +108,6 @@ class TraceElements extends Gatherer { return RectHelpers.addRectTopAndBottom(rectArgs); } - /** - * @param {LH.Gatherer.PassContext} passContext - * @param {string} animationId - * @return {Promise} - */ - static async resolveAnimationName(passContext, animationId) { - const driver = passContext.driver; - try { - const result = await driver.sendCommand('Animation.resolveAnimation', {animationId}); - const objectId = result.remoteObject.objectId; - if (!objectId) return undefined; - const response = await driver.sendCommand('Runtime.getProperties', { - objectId, - }); - const nameProperty = response.result.find((property) => property.name === 'animationName'); - const animationName = nameProperty && nameProperty.value && nameProperty.value.value; - if (animationName === '') return undefined; - return animationName; - } catch (err) { - // Animation name is not mission critical information and can be evicted, so don't throw fatally if we can't find it. - Sentry.captureException(err, { - tags: {gatherer: TraceElements.name}, - level: 'error', - }); - return undefined; - } - } - /** * This function finds the top (up to 5) elements that contribute to the CLS score of the page. * Each layout shift event has a 'score' which is the amount added to the CLS as a result of the given shift(s). @@ -186,11 +178,10 @@ class TraceElements extends Gatherer { /** * Find the node ids of elements which are animated using the Animation trace events. - * @param {LH.Gatherer.PassContext} passContext * @param {Array} mainThreadEvents * @return {Promise>} */ - static async getAnimatedElements(passContext, mainThreadEvents) { + async getAnimatedElements(mainThreadEvents) { /** @type {Map} */ const animationPairs = new Map(); for (const event of mainThreadEvents) { @@ -214,10 +205,10 @@ class TraceElements extends Gatherer { /** @type {Map>} */ const elementAnimations = new Map(); for (const {begin, status} of animationPairs.values()) { - const nodeId = this.getNodeIDFromTraceEvent(begin); - const animationId = this.getAnimationIDFromTraceEvent(begin); - const failureReasonsMask = this.getFailureReasonsFromTraceEvent(status); - const unsupportedProperties = this.getUnsupportedPropertiesFromTraceEvent(status); + const nodeId = TraceElements.getNodeIDFromTraceEvent(begin); + const animationId = TraceElements.getAnimationIDFromTraceEvent(begin); + const failureReasonsMask = TraceElements.getFailureReasonsFromTraceEvent(status); + const unsupportedProperties = TraceElements.getUnsupportedPropertiesFromTraceEvent(status); if (!nodeId || !animationId) continue; const animationIds = elementAnimations.get(nodeId) || new Set(); animationIds.add({animationId, failureReasonsMask, unsupportedProperties}); @@ -229,7 +220,7 @@ class TraceElements extends Gatherer { for (const [nodeId, animationIds] of elementAnimations) { const animations = []; for (const {animationId, failureReasonsMask, unsupportedProperties} of animationIds) { - const animationName = await this.resolveAnimationName(passContext, animationId); + const animationName = this.animationIdToName.get(animationId); animations.push({name: animationName, failureReasonsMask, unsupportedProperties}); } animatedElementData.push({nodeId, animations}); @@ -238,30 +229,39 @@ class TraceElements extends Gatherer { } /** - * @param {LH.Gatherer.PassContext} passContext + * @param {LH.Gatherer.FRTransitionalContext} context */ - async beforePass(passContext) { - await passContext.driver.sendCommand('Animation.enable'); + async startInstrumentation(context) { + await context.driver.defaultSession.sendCommand('Animation.enable'); + context.driver.defaultSession.on('Animation.animationStarted', this._onAnimationStarted); } /** - * @param {LH.Gatherer.PassContext} passContext - * @param {LH.Gatherer.LoadData} loadData + * @param {LH.Gatherer.FRTransitionalContext} context + */ + async stopInstrumentation(context) { + context.driver.defaultSession.off('Animation.animationStarted', this._onAnimationStarted); + await context.driver.defaultSession.sendCommand('Animation.disable'); + } + + /** + * @param {LH.Gatherer.FRTransitionalContext} context + * @param {LH.Trace|undefined} trace * @return {Promise} */ - async afterPass(passContext, loadData) { - const driver = passContext.driver; - if (!loadData.trace) { + async _getArtifact(context, trace) { + const session = context.driver.defaultSession; + if (!trace) { throw new Error('Trace is missing!'); } const {largestContentfulPaintEvt, mainThreadEvents} = - TraceProcessor.computeTraceOfTab(loadData.trace); + TraceProcessor.computeTraceOfTab(trace); const lcpNodeId = TraceElements.getNodeIDFromTraceEvent(largestContentfulPaintEvt); const clsNodeData = TraceElements.getTopLayoutShiftElements(mainThreadEvents); const animatedElementData = - await TraceElements.getAnimatedElements(passContext, mainThreadEvents); + await this.getAnimatedElements(mainThreadEvents); /** @type {Map} */ const backendNodeDataMap = new Map([ @@ -276,9 +276,9 @@ class TraceElements extends Gatherer { const backendNodeId = backendNodeData[i].nodeId; let response; try { - const objectId = await dom.resolveNodeIdToObjectId(driver.defaultSession, backendNodeId); + const objectId = await resolveNodeIdToObjectId(session, backendNodeId); if (!objectId) continue; - response = await driver.sendCommand('Runtime.callFunctionOn', { + response = await session.sendCommand('Runtime.callFunctionOn', { objectId, functionDeclaration: `function () { ${getNodeDetailsData.toString()}; @@ -308,10 +308,27 @@ class TraceElements extends Gatherer { } } - await driver.sendCommand('Animation.disable'); - return traceElements; } + + /** + * @param {LH.Gatherer.FRTransitionalContext<'Trace'>} context + * @return {Promise} + */ + async getArtifact(context) { + return this._getArtifact(context, context.dependencies.Trace); + } + + /** + * @param {LH.Gatherer.PassContext} passContext + * @param {LH.Gatherer.LoadData} loadData + * @return {Promise} + */ + async afterPass(passContext, loadData) { + const context = {...passContext, dependencies: {}}; + await this.stopInstrumentation(context); + return this._getArtifact(context, loadData.trace); + } } module.exports = TraceElements; diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js index 42536ede532a..5f60136cbe0c 100644 --- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js +++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js @@ -159,7 +159,7 @@ describe('Fraggle Rock API', () => { const {lhr} = result; const {auditResults, failedAudits, erroredAudits} = getAuditsBreakdown(lhr); // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached. - expect(auditResults.length).toMatchInlineSnapshot(`103`); + expect(auditResults.length).toMatchInlineSnapshot(`105`); expect(erroredAudits).toHaveLength(0); const failedAuditIds = failedAudits.map(audit => audit.id); diff --git a/lighthouse-core/test/gather/gatherers/trace-elements-test.js b/lighthouse-core/test/gather/gatherers/trace-elements-test.js index d65a8c137a4b..629a7cc2b0fd 100644 --- a/lighthouse-core/test/gather/gatherers/trace-elements-test.js +++ b/lighthouse-core/test/gather/gatherers/trace-elements-test.js @@ -11,10 +11,13 @@ const TraceElementsGatherer = require('../../../gather/gatherers/trace-elements. const Driver = require('../../../gather/driver.js'); const Connection = require('../../../gather/connections/connection.js'); const createTestTrace = require('../../create-test-trace.js'); -const {createMockSendCommandFn} = require('../mock-commands.js'); +const {createMockSendCommandFn, createMockOnFn} = require('../mock-commands.js'); +const {flushAllTimersAndMicrotasks} = require('../../test-utils.js'); const animationTrace = require('../../fixtures/traces/animation.json'); +jest.useFakeTimers(); + function makeLayoutShiftTraceEvent(score, impactedNodes, had_recent_input = false) { return { name: 'LayoutShift', @@ -318,24 +321,6 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { describe('Trace Elements gatherer - Animated Elements', () => { it('gets animated node ids with non-composited animations', async () => { - const connectionStub = new Connection(); - connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 1}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'alpha'}, - }]}) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 2}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: ''}, - }]}) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 3}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'beta'}, - }]}); - const driver = new Driver(connectionStub); const traceEvents = [ makeAnimationTraceEvent('0x363db876c1', 'b', {id: '1', nodeId: 5}), makeAnimationTraceEvent('0x363db876c1', 'n', { @@ -354,7 +339,11 @@ describe('Trace Elements gatherer - Animated Elements', () => { }), ]; - const result = await TraceElementsGatherer.getAnimatedElements({driver}, traceEvents); + const gatherer = new TraceElementsGatherer(); + gatherer.animationIdToName.set('1', 'alpha'); + gatherer.animationIdToName.set('3', 'beta'); + + const result = await gatherer.getAnimatedElements(traceEvents); expect(result).toEqual([ {nodeId: 5, animations: [ {name: 'alpha', failureReasonsMask: 8192, unsupportedProperties: ['height']}, @@ -367,19 +356,6 @@ describe('Trace Elements gatherer - Animated Elements', () => { }); it('get non-composited animations with no unsupported properties', async () => { - const connectionStub = new Connection(); - connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 1}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'alpha'}, - }]}) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 2}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: ''}, - }]}); - const driver = new Driver(connectionStub); const traceEvents = [ makeAnimationTraceEvent('0x363db876c1', 'b', {id: '1', nodeId: 5}), makeAnimationTraceEvent('0x363db876c1', 'n', { @@ -393,7 +369,10 @@ describe('Trace Elements gatherer - Animated Elements', () => { }), ]; - const result = await TraceElementsGatherer.getAnimatedElements({driver}, traceEvents); + const gatherer = new TraceElementsGatherer(); + gatherer.animationIdToName.set('1', 'alpha'); + + const result = await gatherer.getAnimatedElements(traceEvents); expect(result).toEqual([ {nodeId: 5, animations: [ {name: 'alpha', failureReasonsMask: 2048, unsupportedProperties: []}, @@ -403,24 +382,6 @@ describe('Trace Elements gatherer - Animated Elements', () => { }); it('gets animated node ids with composited animations', async () => { - const connectionStub = new Connection(); - connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 1}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'alpha'}, - }]}) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 2}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: ''}, - }]}) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 3}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'beta'}, - }]}); - const driver = new Driver(connectionStub); const traceEvents = [ makeAnimationTraceEvent('0x363db876c1', 'b', {id: '1', nodeId: 5}), makeAnimationTraceEvent('0x363db876c1', 'n', {compositeFailed: 0, unsupportedProperties: []}), @@ -430,7 +391,11 @@ describe('Trace Elements gatherer - Animated Elements', () => { makeAnimationTraceEvent('0x363db876c3', 'n', {compositeFailed: 0, unsupportedProperties: []}), ]; - const result = await TraceElementsGatherer.getAnimatedElements({driver}, traceEvents); + const gatherer = new TraceElementsGatherer(); + gatherer.animationIdToName.set('1', 'alpha'); + gatherer.animationIdToName.set('3', 'beta'); + + const result = await gatherer.getAnimatedElements(traceEvents); expect(result).toEqual([ {nodeId: 5, animations: [ {name: 'alpha', failureReasonsMask: 0, unsupportedProperties: []}, @@ -497,11 +462,6 @@ describe('Trace Elements gatherer - Animated Elements', () => { .mockResponse('DOM.resolveNode', () => { // 2nd CLS node throw Error('No node found'); }) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 4}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'example'}, - }]}) .mockResponse('DOM.resolveNode', {object: {objectId: 3}}) .mockResponse('Runtime.callFunctionOn', {result: {value: animationNodeData}}); const driver = new Driver(connectionStub); @@ -529,7 +489,9 @@ describe('Trace Elements gatherer - Animated Elements', () => { trace.traceEvents.push(makeLCPTraceEvent(6)); const gatherer = new TraceElementsGatherer(); - const result = await gatherer.afterPass({driver}, {trace}); + gatherer.animationIdToName.set('1', 'example'); + + const result = await gatherer._getArtifact({driver}, trace); expect(result).toEqual([ { @@ -603,36 +565,19 @@ describe('Trace Elements gatherer - Animated Elements', () => { .mockResponse('DOM.resolveNode', {object: {objectId: 1}}) .mockResponse('Runtime.callFunctionOn', {result: {value: LCPNodeData}}) // Animated node - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 2}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: ''}, - }]}) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 3}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'alpha'}, - }]}) - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 4}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'beta'}, - }]}) .mockResponse('DOM.resolveNode', {object: {objectId: 5}}) .mockResponse('Runtime.callFunctionOn', {result: {value: animationNodeData}}) // Composited node - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 6}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'gamma'}, - }]}) .mockResponse('DOM.resolveNode', {object: {objectId: 7}}) .mockResponse('Runtime.callFunctionOn', {result: {value: compositedNodeData}}); const driver = new Driver(connectionStub); const gatherer = new TraceElementsGatherer(); + gatherer.animationIdToName.set('2', 'alpha'); + gatherer.animationIdToName.set('3', 'beta'); + gatherer.animationIdToName.set('4', 'gamma'); - const result = await gatherer.afterPass({driver}, {trace: animationTrace}); + const result = await gatherer._getArtifact({driver}, animationTrace); expect(result).toEqual([ { @@ -694,18 +639,10 @@ describe('Trace Elements gatherer - Animated Elements', () => { .mockResponse('DOM.resolveNode', {object: {objectId: 1}}) .mockResponse('Runtime.callFunctionOn', {result: {value: LCPNodeData}}) // Animation 1 - .mockResponse('Animation.resolveAnimation', () => { - throw Error(); - }) .mockResponse('DOM.resolveNode', () => { throw Error(); }) // Animation 2 - .mockResponse('Animation.resolveAnimation', {remoteObject: {objectId: 4}}) - .mockResponse('Runtime.getProperties', {result: [{ - name: 'animationName', - value: {type: 'string', value: 'example'}, - }]}) .mockResponse('DOM.resolveNode', {object: {objectId: 5}}) .mockResponse('Runtime.callFunctionOn', {result: {value: animationNodeData}}); const driver = new Driver(connectionStub); @@ -724,7 +661,10 @@ describe('Trace Elements gatherer - Animated Elements', () => { trace.traceEvents.push(makeLCPTraceEvent(7)); const gatherer = new TraceElementsGatherer(); - const result = await gatherer.afterPass({driver}, {trace}); + gatherer.animationIdToName.set('1', 'notgunnamatter'); + gatherer.animationIdToName.set('2', 'example'); + + const result = await gatherer._getArtifact({driver}, trace); expect(result).toEqual([ { @@ -741,3 +681,75 @@ describe('Trace Elements gatherer - Animated Elements', () => { ]); }); }); + +describe('instrumentation', () => { + it('resolves animation name', async () => { + const connectionStub = new Connection(); + connectionStub.on = createMockOnFn() + .mockEvent('protocolevent', { + method: 'Animation.animationStarted', + params: {animation: {id: '1', name: 'example'}}, + }); + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Animation.enable') + .mockResponse('Animation.disable'); + const driver = new Driver(connectionStub); + const gatherer = new TraceElementsGatherer(); + await gatherer.startInstrumentation({driver}); + + await flushAllTimersAndMicrotasks(); + + await gatherer.stopInstrumentation({driver}); + + expect(gatherer.animationIdToName.size).toEqual(1); + expect(gatherer.animationIdToName.get('1')).toEqual('example'); + }); + + it('ignores empty name', async () => { + const connectionStub = new Connection(); + connectionStub.on = createMockOnFn() + .mockEvent('protocolevent', { + method: 'Animation.animationStarted', + params: {animation: {id: '1', name: ''}}, + }); + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Animation.enable') + .mockResponse('Animation.disable'); + const driver = new Driver(connectionStub); + const gatherer = new TraceElementsGatherer(); + await gatherer.startInstrumentation({driver}); + + await flushAllTimersAndMicrotasks(); + + await gatherer.stopInstrumentation({driver}); + + expect(gatherer.animationIdToName.size).toEqual(0); + }); +}); + +describe('FR compat', () => { + it('uses loadData in legacy mode', async () => { + const trace = ['1', '2']; + const gatherer = new TraceElementsGatherer(); + gatherer._getArtifact = jest.fn(); + gatherer.stopInstrumentation = jest.fn(); + + await gatherer.afterPass({}, {trace}); + + expect(gatherer._getArtifact).toHaveBeenCalledWith({dependencies: {}}, trace); + expect(gatherer.stopInstrumentation).toHaveBeenCalledWith({dependencies: {}}); + }); + + it('uses dependency in legacy mode', async () => { + const trace = ['1', '2']; + const gatherer = new TraceElementsGatherer(); + gatherer._getArtifact = jest.fn(); + gatherer.stopInstrumentation = jest.fn(); + + const context = {dependencies: {Trace: trace}}; + await gatherer.getArtifact(context); + + expect(gatherer._getArtifact).toHaveBeenCalledWith(context, trace); + expect(gatherer.stopInstrumentation).not.toHaveBeenCalled(); + }); +}); diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 18ca6142befe..f2c81cd18936 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -37,7 +37,6 @@ declare global { | 'ServiceWorker' | 'SourceMaps' | 'TagsBlockingFirstPaint' - | 'TraceElements' | keyof FRBaseArtifacts >;