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: always use MainResource for main document #13756

Merged
merged 10 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,13 @@ class DuplicatedJavascript extends ByteEfficiencyAudit {
/** @type {number|undefined} */
let transferRatio = transferRatioByUrl.get(url);
if (transferRatio === undefined) {
const networkRecord = getRequestForScript(networkRecords, script);

if (!script || script.length === undefined) {
// This should never happen because we found the wasted bytes from bundles, which required contents in a ScriptElement.
continue;
}

const contentLength = script.length;
const networkRecord = getRequestForScript(networkRecords, script);
transferRatio = DuplicatedJavascript._estimateTransferRatio(networkRecord, contentLength);
transferRatioByUrl.set(url, transferRatio);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,12 @@ class LegacyJavascript extends ByteEfficiencyAudit {
if (transferRatio !== undefined) return transferRatio;

const script = artifacts.Scripts.find(script => script.url === url);
const networkRecord = getRequestForScript(networkRecords, script);

if (!script || script.content === null) {
// Can't find content, so just use 1.
transferRatio = 1;
} else {
const networkRecord = getRequestForScript(networkRecords, script);
const contentLength = script.length || 0;
const transferSize =
ByteEfficiencyAudit.estimateTransferSize(networkRecord, contentLength, 'Script');
Expand Down
7 changes: 4 additions & 3 deletions lighthouse-core/audits/diagnostics.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const Audit = require('./audit.js');
const MainThreadTasksComputed = require('../computed/main-thread-tasks.js');
const NetworkRecordsComputed = require('../computed/network-records.js');
const NetworkAnalysisComputed = require('../computed/network-analysis.js');
const NetworkAnalyzer = require('../lib/dependency-graph/simulator/network-analyzer.js');
const MainResource = require('../computed/main-resource.js');

class Diagnostics extends Audit {
/**
Expand All @@ -22,7 +22,7 @@ class Diagnostics extends Audit {
title: 'Diagnostics',
description: 'Collection of useful page vitals.',
supportedModes: ['navigation'],
requiredArtifacts: ['traces', 'devtoolsLogs'],
requiredArtifacts: ['URL', 'traces', 'devtoolsLogs'],
};
}

Expand All @@ -37,9 +37,10 @@ class Diagnostics extends Audit {
const tasks = await MainThreadTasksComputed.request(trace, context);
const records = await NetworkRecordsComputed.request(devtoolsLog, context);
const analysis = await NetworkAnalysisComputed.request(devtoolsLog, context);
const mainResource = await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);

const toplevelTasks = tasks.filter(t => !t.parent);
const mainDocumentTransferSize = NetworkAnalyzer.findMainDocument(records).transferSize;
const mainDocumentTransferSize = mainResource.transferSize;
const totalByteWeight = records.reduce((sum, r) => sum + (r.transferSize || 0), 0);
const totalTaskTime = toplevelTasks.reduce((sum, t) => sum + (t.duration || 0), 0);
const maxRtt = Math.max(...analysis.additionalRttByOrigin.values()) + analysis.rtt;
Expand Down
6 changes: 2 additions & 4 deletions lighthouse-core/audits/seo/http-status-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
'use strict';

const Audit = require('../audit.js');
const NetworkRecords = require('../../computed/network-records.js');
const HTTP_UNSUCCESSFUL_CODE_LOW = 400;
const HTTP_UNSUCCESSFUL_CODE_HIGH = 599;
const i18n = require('../../lib/i18n/i18n.js');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
const MainResource = require('../../computed/main-resource.js');

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on the HTTP status code a page responds with. This descriptive title is shown when the page has responded with a valid HTTP status code. */
Expand Down Expand Up @@ -47,8 +46,7 @@ class HTTPStatusCode extends Audit {
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const URL = artifacts.URL;
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const mainResource = NetworkAnalyzer.findMainDocument(networkRecords, URL.finalUrl);
const mainResource = await MainResource.request({devtoolsLog, URL}, context);

const statusCode = mainResource.statusCode;

Expand Down
19 changes: 2 additions & 17 deletions lighthouse-core/audits/server-response-time.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
const Audit = require('./audit.js');
const i18n = require('../lib/i18n/i18n.js');
const MainResource = require('../computed/main-resource.js');
const NetworkRecords = require('../computed/network-records.js');
const NetworkAnalyzer = require('../lib/dependency-graph/simulator/network-analyzer.js');

const UIStrings = {
/** Title of a diagnostic audit that provides detail on how long it took from starting a request to when the server started responding. This descriptive title is shown to users when the amount is acceptable and no user action is required. */
Expand Down Expand Up @@ -39,7 +37,7 @@ class ServerResponseTime extends Audit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
supportedModes: ['timespan', 'navigation'],
supportedModes: ['navigation'],
requiredArtifacts: ['devtoolsLogs', 'URL', 'GatherContext'],
};
}
Expand All @@ -61,20 +59,7 @@ class ServerResponseTime extends Audit {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];

/** @type {LH.Artifacts.NetworkRequest} */
let mainResource;
if (artifacts.GatherContext.gatherMode === 'timespan') {
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const optionalMainResource = NetworkAnalyzer.findOptionalMainDocument(
networkRecords,
artifacts.URL.finalUrl
);
if (!optionalMainResource) {
return {score: null, notApplicable: true};
}
mainResource = optionalMainResource;
} else {
mainResource = await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);
}
const mainResource = await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);

const responseTime = ServerResponseTime.calculateResponseTime(mainResource);
const passed = responseTime < TOO_SLOW_THRESHOLD_MS;
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/computed/page-dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ class PageDependencyGraph {
const networkNodeOutput = PageDependencyGraph.getNetworkNodeOutput(networkRecords);
const cpuNodes = PageDependencyGraph.getCPUNodes(processedTrace);

// TODO: Remove this usage of `findMainDocument` and create a new function to get the first document request.
// https://github.com/GoogleChrome/lighthouse/issues/8984
//
// The main document request is the earliest network request *of type document*.
// This will be different from the root request when there are server redirects.
const mainDocumentRequest = NetworkAnalyzer.findMainDocument(networkRecords);
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/fraggle-rock/gather/navigation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ async function _navigation(navigationContext) {
await collectPhaseArtifacts({phase: 'startInstrumentation', ...phaseState});
await collectPhaseArtifacts({phase: 'startSensitiveInstrumentation', ...phaseState});
const navigateResult = await _navigate(navigationContext);
phaseState.baseArtifacts.URL.finalUrl = navigateResult.finalUrl;
phaseState.url = navigateResult.finalUrl;
await collectPhaseArtifacts({phase: 'stopSensitiveInstrumentation', ...phaseState});
await collectPhaseArtifacts({phase: 'stopInstrumentation', ...phaseState});
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class GatherRunner {
...passContext.passConfig,
});
passContext.url = finalUrl;
passContext.baseArtifacts.URL.finalUrl = finalUrl;
if (passContext.passConfig.loadFailureMode === 'fatal') {
passContext.LighthouseRunWarnings.push(...warnings);
}
Expand Down
26 changes: 12 additions & 14 deletions lighthouse-core/gather/gatherers/link-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
const LinkHeader = require('http-link-header');
const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
const {URL} = require('../../lib/url-shim.js');
const NetworkRecords = require('../../computed/network-records.js');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
const pageFunctions = require('../../lib/page-functions.js');
const DevtoolsLog = require('./devtools-log.js');
const MainResource = require('../../computed/main-resource.js');

/* globals HTMLLinkElement getNodeDetails */

Expand Down Expand Up @@ -114,12 +113,12 @@ class LinkElements extends FRGatherer {

/**
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @return {LH.Artifacts['LinkElements']}
* @param {LH.Artifacts['DevtoolsLog']} devtoolsLog
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
static getLinkElementsInHeaders(context, networkRecords) {
const finalUrl = context.url;
const mainDocument = NetworkAnalyzer.findMainDocument(networkRecords, finalUrl);
static async getLinkElementsInHeaders(context, devtoolsLog) {
const mainDocument =
await MainResource.request({devtoolsLog, URL: context.baseArtifacts.URL}, context);

/** @type {LH.Artifacts['LinkElements']} */
const linkElements = [];
Expand All @@ -130,7 +129,7 @@ class LinkElements extends FRGatherer {
for (const link of LinkHeader.parse(header.value).refs) {
linkElements.push({
rel: link.rel || '',
href: normalizeUrlOrNull(link.uri, finalUrl),
href: normalizeUrlOrNull(link.uri, context.baseArtifacts.URL.finalUrl),
hrefRaw: link.uri || '',
hreflang: link.hreflang || '',
as: link.as || '',
Expand All @@ -146,12 +145,12 @@ class LinkElements extends FRGatherer {

/**
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @param {LH.Artifacts['DevtoolsLog']} devtoolsLog
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
async _getArtifact(context, networkRecords) {
async _getArtifact(context, devtoolsLog) {
const fromDOM = await LinkElements.getLinkElementsInDOM(context);
const fromHeaders = LinkElements.getLinkElementsInHeaders(context, networkRecords);
const fromHeaders = await LinkElements.getLinkElementsInHeaders(context, devtoolsLog);
const linkElements = fromDOM.concat(fromHeaders);

for (const link of linkElements) {
Expand All @@ -168,16 +167,15 @@ class LinkElements extends FRGatherer {
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
async afterPass(context, loadData) {
return this._getArtifact({...context, dependencies: {}}, loadData.networkRecords);
return this._getArtifact({...context, dependencies: {}}, loadData.devtoolsLog);
}

/**
* @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
async getArtifact(context) {
const records = await NetworkRecords.request(context.dependencies.DevtoolsLog, context);
return this._getArtifact(context, records);
return this._getArtifact(context, context.dependencies.DevtoolsLog);
}
}

Expand Down
16 changes: 8 additions & 8 deletions lighthouse-core/gather/gatherers/main-document-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
'use strict';

const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
const NetworkRecords = require('../../computed/network-records.js');
const DevtoolsLog = require('./devtools-log.js');
const {fetchResponseBodyFromCache} = require('../driver/network.js');
const MainResource = require('../../computed/main-resource.js');

/**
* Collects the content of the main html document.
Expand All @@ -24,23 +23,24 @@ class MainDocumentContent extends FRGatherer {
/**
*
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @param {LH.Artifacts['DevtoolsLog']} devtoolsLog
* @return {Promise<LH.Artifacts['MainDocumentContent']>}
*/
async _getArtifact(context, networkRecords) {
const mainResource = NetworkAnalyzer.findMainDocument(networkRecords, context.url);
async _getArtifact(context, devtoolsLog) {
const mainResource =
await MainResource.request({devtoolsLog, URL: context.baseArtifacts.URL}, context);
const session = context.driver.defaultSession;
return fetchResponseBodyFromCache(session, mainResource.requestId);
}

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

/**
Expand All @@ -49,7 +49,7 @@ class MainDocumentContent extends FRGatherer {
* @return {Promise<LH.Artifacts['MainDocumentContent']>}
*/
async afterPass(passContext, loadData) {
return this._getArtifact({...passContext, dependencies: {}}, loadData.networkRecords);
return this._getArtifact({...passContext, dependencies: {}}, loadData.devtoolsLog);
}
}

Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/lib/script-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@

/**
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @param {LH.Artifacts.Script|undefined} script
Copy link
Member Author

Choose a reason for hiding this comment

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

Drive-by

* @param {LH.Artifacts.Script} script
* @return {LH.Artifacts.NetworkRequest|undefined}
*/
function getRequestForScript(networkRecords, script) {
if (!script) return;
let networkRequest = networkRecords.find(request => request.url === script.url);
while (networkRequest?.redirectDestination) {
networkRequest = networkRequest.redirectDestination;
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/test/audits/diagnostics-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ describe('Diagnostics audit', () => {
const artifacts = {
traces: {defaultPass: acceptableTrace},
devtoolsLogs: {defaultPass: acceptableDevToolsLog},
URL: {
finalUrl: 'https://pwa.rocks/',
},
};

const result = await Diagnostics.audit(artifacts, {computedCache: new Map()});
Expand Down
39 changes: 1 addition & 38 deletions lighthouse-core/test/audits/server-response-time-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,44 +56,7 @@ describe('Performance: server-response-time audit', () => {
});
});

it('identifies main resource in timespan mode', async () => {
const mainResource = {
url: 'https://example.com/',
requestId: '0',
timing: {receiveHeadersEnd: 400, sendEnd: 200},
};
const devtoolsLog = networkRecordsToDevtoolsLog([mainResource]);

const artifacts = {
devtoolsLogs: {[ServerResponseTime.DEFAULT_PASS]: devtoolsLog},
URL: {finalUrl: 'https://example.com/'},
GatherContext: {gatherMode: 'timespan'},
};

const result = await ServerResponseTime.audit(artifacts, {computedCache: new Map()});
expect(result).toMatchObject({
numericValue: 200,
score: 1,
});
});

it('result is n/a if no main resource in timespan', async () => {
const devtoolsLog = networkRecordsToDevtoolsLog([]);

const artifacts = {
devtoolsLogs: {[ServerResponseTime.DEFAULT_PASS]: devtoolsLog},
URL: {finalUrl: 'https://example.com/'},
GatherContext: {gatherMode: 'timespan'},
};

const result = await ServerResponseTime.audit(artifacts, {computedCache: new Map()});
expect(result).toEqual({
score: null,
notApplicable: true,
});
});

it('throws error if no main resource in navigation', async () => {
it('throws error if no main resource', async () => {
const devtoolsLog = networkRecordsToDevtoolsLog([]);

const artifacts = {
Expand Down
Loading