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(fr): convert script-elements gatherer #12621

Merged
merged 3 commits into from
Jun 4, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions lighthouse-core/fraggle-rock/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const artifacts = {
PasswordInputsWithPreventedPaste: '',
ResponseCompression: '',
RobotsTxt: '',
ScriptElements: '',
SourceMaps: '',
Stacks: '',
TagsBlockingFirstPaint: '',
Expand Down Expand Up @@ -91,6 +92,7 @@ const defaultConfig = {
{id: artifacts.PasswordInputsWithPreventedPaste, gatherer: 'dobetterweb/password-inputs-with-prevented-paste'},
{id: artifacts.ResponseCompression, gatherer: 'dobetterweb/response-compression'},
{id: artifacts.RobotsTxt, gatherer: 'seo/robots-txt'},
{id: artifacts.ScriptElements, gatherer: 'script-elements'},
{id: artifacts.SourceMaps, gatherer: 'source-maps'},
{id: artifacts.Stacks, gatherer: 'stacks'},
{id: artifacts.TagsBlockingFirstPaint, gatherer: 'dobetterweb/tags-blocking-first-paint'},
Expand Down Expand Up @@ -144,6 +146,7 @@ const defaultConfig = {
artifacts.PasswordInputsWithPreventedPaste,
artifacts.ResponseCompression,
artifacts.RobotsTxt,
artifacts.ScriptElements,
artifacts.SourceMaps,
artifacts.Stacks,
artifacts.TagsBlockingFirstPaint,
Expand Down
55 changes: 44 additions & 11 deletions lighthouse-core/gather/gatherers/script-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
*/
'use strict';

const Gatherer = require('./gatherer.js');
const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
const NetworkRecords = require('../../computed/network-records.js');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
const NetworkRequest = require('../../lib/network-request.js');
const pageFunctions = require('../../lib/page-functions.js');
const {fetchResponseBodyFromCache} = require('../driver/network.js');
const DevtoolsLog = require('./devtools-log.js');
const HostFormFactor = require('./host-form-factor.js');

/* global getNodeDetails */

Expand Down Expand Up @@ -62,17 +66,25 @@ async function runInSeriesOrParallel(values, promiseMapper, runInSeries) {
/**
* @fileoverview Gets JavaScript file contents.
*/
class ScriptElements extends Gatherer {
class ScriptElements extends FRGatherer {
/** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'|'HostFormFactor'>} */
meta = {
supportedModes: ['timespan', 'navigation'],
dependencies: {DevtoolsLog: DevtoolsLog.symbol, HostFormFactor: HostFormFactor.symbol},
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @param {LH.Artifacts['HostFormFactor']} formFactor
* @return {Promise<LH.Artifacts['ScriptElements']>}
*/
async afterPass(passContext, loadData) {
const driver = passContext.driver;
const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url);
async _getArtifact(context, networkRecords, formFactor) {
const session = context.driver.defaultSession;
const executionContext = context.driver.executionContext;
const mainResource = NetworkAnalyzer.findMainDocument(networkRecords, context.url);

const scripts = await driver.executionContext.evaluate(collectAllScriptElements, {
const scripts = await executionContext.evaluate(collectAllScriptElements, {
args: [],
useIsolation: true,
deps: [
Expand All @@ -85,7 +97,7 @@ class ScriptElements extends Gatherer {
if (script.content) script.requestId = mainResource.requestId;
}

const scriptRecords = loadData.networkRecords
const scriptRecords = networkRecords
// Ignore records from OOPIFs
.filter(record => !record.sessionId)
// Only get the content of script requests
Expand All @@ -95,8 +107,9 @@ class ScriptElements extends Gatherer {
// record at a time.
const scriptRecordContents = await runInSeriesOrParallel(
scriptRecords,
record => driver.getRequestContent(record.requestId).catch(() => ''),
passContext.baseArtifacts.HostFormFactor === 'mobile' /* runInSeries*/ );
record => fetchResponseBodyFromCache(session, record.requestId).catch(() => ''),
formFactor === 'mobile' /* runInSeries */
);

for (let i = 0; i < scriptRecords.length; i++) {
const record = scriptRecords[i];
Expand All @@ -123,6 +136,26 @@ class ScriptElements extends Gatherer {
}
return scripts;
}

/**
* @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'|'HostFormFactor'>} context
*/
async getArtifact(context) {
const devtoolsLog = context.dependencies.DevtoolsLog;
const formFactor = context.dependencies.HostFormFactor;
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
return this._getArtifact(context, networkRecords, formFactor);
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
*/
async afterPass(passContext, loadData) {
const networkRecords = loadData.networkRecords;
const formFactor = passContext.baseArtifacts.HostFormFactor;
return this._getArtifact({...passContext, dependencies: {}}, networkRecords, formFactor);
}
}

module.exports = ScriptElements;
4 changes: 2 additions & 2 deletions lighthouse-core/test/fraggle-rock/api-test-pptr.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ describe('Fraggle Rock API', () => {

const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
expect(auditResults.length).toMatchInlineSnapshot(`35`);
expect(auditResults.length).toMatchInlineSnapshot(`36`);

expect(erroredAudits).toHaveLength(0);
expect(failedAudits.map(audit => audit.id)).toContain('errors-in-console');
Expand Down Expand Up @@ -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(`145`);
expect(auditResults.length).toMatchInlineSnapshot(`151`);
expect(erroredAudits).toHaveLength(1); // FIXME: MainDocumentContent broken from merge

const failedAuditIds = failedAudits.map(audit => audit.id);
Expand Down
169 changes: 169 additions & 0 deletions lighthouse-core/test/gather/gatherers/script-elements-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/**
* @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/* eslint-env jest */

const {
createMockContext,
mockDriverSubmodules,
} = require('../../fraggle-rock/gather/mock-driver.js');
const mocks = mockDriverSubmodules();
const ScriptElements = require('../../../gather/gatherers/script-elements.js');
const NetworkRequest = require('../../../lib/network-request.js');

/**
* @param {Partial<LH.Artifacts.NetworkRequest>=} partial
* @return {LH.Artifacts.NetworkRequest}
*/
function mockRecord(partial) {
const request = new NetworkRequest();
request.resourceType = NetworkRequest.TYPES.Script;
return Object.assign(request, partial);
}

/**
* @param {Partial<LH.Artifacts.ScriptElement>=} partial
* @return {LH.Artifacts.ScriptElement}
*/
function mockElement(partial) {
return {
type: null,
src: null,
id: null,
async: false,
defer: false,
source: 'head',
content: null,
requestId: null,
node: null,
...partial,
};
}

describe('_getArtifact', () => {
let mockContext = createMockContext();
/** @type {ScriptElements} */
let gatherer;
/** @type {LH.Artifacts.ScriptElement[]} */
let scriptElements;
/** @type {LH.Artifacts.NetworkRequest[]} */
let networkRecords;
/** @type {Record<string, string>} */
let scriptRecordContents;
/** @type {LH.Artifacts.NetworkRequest} */
let mainDocument;

beforeEach(() => {
mocks.reset();
mockContext = createMockContext();
gatherer = new ScriptElements();
scriptElements = [];
mainDocument = mockRecord({resourceType: NetworkRequest.TYPES.Document, requestId: '0'});
networkRecords = [mainDocument];
scriptRecordContents = {};
mockContext.driver._executionContext.evaluate.mockImplementation(() => scriptElements);
mocks.networkMock.fetchResponseBodyFromCache
.mockImplementation((_, id) => Promise.resolve(scriptRecordContents[id]));
});

it('collects script elements', async () => {
networkRecords = [
mainDocument,
mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
];
scriptRecordContents = {
'1': '// SOURCED',
};
scriptElements = [
mockElement({src: 'https://example.com/script.js'}),
mockElement({content: '// INLINE'}),
];

const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');

expect(artifact).toEqual([
mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED'}),
mockElement({content: '// INLINE', requestId: '0'}),
]);
});

it('ignore OOPIF records', async () => {
networkRecords = [
mainDocument,
mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
mockRecord({url: 'https://oopif.com/script.js', requestId: '2', sessionId: 'OOPIF'}),
];
scriptRecordContents = {
'1': '// SOURCED',
'2': '// OOPIF',
};
// OOPIF would not produce script element
scriptElements = [
mockElement({src: 'https://example.com/script.js'}),
mockElement({content: '// INLINE'}),
];

const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');

expect(artifact).toEqual([
mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED'}),
mockElement({content: '// INLINE', requestId: '0'}),
]);
});

it('null content for sourced script with empty content', async () => {
networkRecords = [
mainDocument,
mockRecord({url: 'https://example.com/empty.js', requestId: '1'}),
];
scriptRecordContents = {
'1': '',
};
scriptElements = [
mockElement({src: 'https://example.com/empty.js'}),
];

const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');

expect(artifact).toEqual([
mockElement({src: 'https://example.com/empty.js'}),
]);
});

it('handle erroneous network content', async () => {
networkRecords = [
mainDocument,
mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
];
mocks.networkMock.fetchResponseBodyFromCache.mockRejectedValue('Error');
scriptElements = [
mockElement({src: 'https://example.com/script.js'}),
];

const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');

expect(artifact).toEqual([
mockElement({src: 'https://example.com/script.js'}),
]);
});

it('create element if none found', async () => {
networkRecords = [
mainDocument,
mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
];
scriptRecordContents = {
'1': '// SOURCED',
};

const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');

expect(artifact).toEqual([
mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED', source: 'network'}),
]);
});
});
1 change: 0 additions & 1 deletion types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ declare global {
| 'HTTPRedirect'
| 'Manifest'
| 'MixedContent'
| 'ScriptElements'
| 'ServiceWorker'
| keyof FRBaseArtifacts
>;
Expand Down