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(lantern): rename NetworkRequest record to rawRequest #16037

Merged
merged 13 commits into from
Jun 4, 2024
18 changes: 9 additions & 9 deletions core/audits/uses-rel-preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ class UsesRelPreloadAudit extends Audit {
if (node.type !== 'network') return;
// Don't include the node itself or any CPU nodes in the initiatorPath
const path = traversalPath.slice(1).filter(initiator => initiator.type === 'network');
if (!UsesRelPreloadAudit.shouldPreloadRequest(node.record, mainResource, path)) return;
urls.add(node.record.url);
if (!UsesRelPreloadAudit.shouldPreloadRequest(node.rawRequest, mainResource, path)) return;
urls.add(node.rawRequest.url);
});

return urls;
Expand All @@ -77,7 +77,7 @@ class UsesRelPreloadAudit extends Audit {
static getURLsFailedToPreload(graph) {
/** @type {Array<LH.Artifacts.NetworkRequest>} */
const requests = [];
graph.traverse(node => node.type === 'network' && requests.push(node.record));
graph.traverse(node => node.type === 'network' && requests.push(node.rawRequest));

const preloadRequests = requests.filter(req => req.isLinkPreload);
const preloadURLsByFrame = new Map();
Expand Down Expand Up @@ -157,7 +157,7 @@ class UsesRelPreloadAudit extends Audit {

if (node.isMainDocument()) {
mainDocumentNode = node;
} else if (node.record && urls.has(node.record.url)) {
} else if (node.rawRequest && urls.has(node.rawRequest.url)) {
nodesToPreload.push(node);
}
});
Expand All @@ -176,20 +176,20 @@ class UsesRelPreloadAudit extends Audit {

// Once we've modified the dependencies, simulate the new graph.
const simulationAfterChanges = simulator.simulate(modifiedGraph);
const originalNodesByRecord = Array.from(simulationBeforeChanges.nodeTimings.keys())
// @ts-expect-error we don't care if all nodes without a record collect on `undefined`
.reduce((map, node) => map.set(node.record, node), new Map());
const originalNodesByRequest = Array.from(simulationBeforeChanges.nodeTimings.keys())
// @ts-expect-error we don't care if all nodes without a request collect on `undefined`
.reduce((map, node) => map.set(node.request, node), new Map());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

rawRequest is not necessarily unique (well it may be, but let's not require it. how redirects are handled may not adhere to this constraint atm). so just use request as a key.


const results = [];
for (const node of nodesToPreload) {
const originalNode = originalNodesByRecord.get(node.record);
const originalNode = originalNodesByRequest.get(node.request);
const timingAfter = simulationAfterChanges.nodeTimings.get(node);
const timingBefore = simulationBeforeChanges.nodeTimings.get(originalNode);
if (!timingBefore || !timingAfter) throw new Error('Missing preload node');

const wastedMs = Math.round(timingBefore.endTime - timingAfter.endTime);
if (wastedMs < THRESHOLD_IN_MS) continue;
results.push({url: node.record.url, wastedMs});
results.push({url: node.rawRequest.url, wastedMs});
}

if (!results.length) {
Expand Down
6 changes: 3 additions & 3 deletions core/computed/critical-request-chains.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,19 @@ class CriticalRequestChains {
graph.traverse((node, traversalPath) => {
seenNodes.add(node);
if (node.type !== 'network') return;
if (!CriticalRequestChains.isCritical(node.record, mainResource)) return;
if (!CriticalRequestChains.isCritical(node.rawRequest, mainResource)) return;

const networkPath = traversalPath
.filter(/** @return {n is LH.Gatherer.Simulation.GraphNetworkNode} */
n => n.type === 'network')
.reverse()
.map(node => node.record);
.map(node => node.rawRequest);

// Ignore if some ancestor is not a critical request.
if (networkPath.some(r => !CriticalRequestChains.isCritical(r, mainResource))) return;

// Ignore non-network things (like data urls).
if (NetworkRequest.isNonNetworkRequest(node.record)) return;
if (NetworkRequest.isNonNetworkRequest(node.rawRequest)) return;

addChain(networkPath);
}, getNextNodes);
Expand Down
2 changes: 1 addition & 1 deletion core/lib/lantern-trace-saver.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function convertNodeTimingsToTrace(nodeTimings) {
traceEvents.push(...createFakeTaskEvents(node, timing));
} else {
/** @type {LH.Artifacts.NetworkRequest} */
const record = node.record;
const record = node.rawRequest;
// Ignore data URIs as they don't really add much value
if (/^data/.test(record.url)) continue;
traceEvents.push(...createFakeNetworkEvents(requestId, record, timing));
Expand Down
6 changes: 3 additions & 3 deletions core/lib/lantern/network-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ class NetworkNode extends BaseNode {
/**
* @return {Readonly<T>}
*/
get record() {
return /** @type {Required<T>} */ (this._request.record);
get rawRequest() {
return /** @type {Required<T>} */ (this._request.rawRequest);
}

/**
Expand Down Expand Up @@ -104,7 +104,7 @@ class NetworkNode extends BaseNode {
}

/**
* Returns whether this network record can be downloaded without a TCP connection.
* Returns whether this network request can be downloaded without a TCP connection.
* During simulation we treat data coming in over a network connection separately from on-device data.
* @return {boolean}
*/
Expand Down
88 changes: 44 additions & 44 deletions core/lib/lantern/page-dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@ const IGNORED_MIME_TYPES_REGEX = /^video/;

class PageDependencyGraph {
/**
* @param {Lantern.NetworkRequest} record
* @param {Lantern.NetworkRequest} request
* @return {Array<string>}
*/
static getNetworkInitiators(record) {
if (!record.initiator) return [];
if (record.initiator.url) return [record.initiator.url];
if (record.initiator.type === 'script') {
static getNetworkInitiators(request) {
if (!request.initiator) return [];
if (request.initiator.url) return [request.initiator.url];
if (request.initiator.type === 'script') {
// Script initiators have the stack of callFrames from all functions that led to this request.
// If async stacks are enabled, then the stack will also have the parent functions that asynchronously
// led to this request chained in the `parent` property.
/** @type {Set<string>} */
const scriptURLs = new Set();
let stack = record.initiator.stack;
let stack = request.initiator.stack;
while (stack) {
const callFrames = stack.callFrames || [];
for (const frame of callFrames) {
Expand All @@ -61,10 +61,10 @@ class PageDependencyGraph {
}

/**
* @param {Array<Lantern.NetworkRequest>} networkRecords
* @param {Array<Lantern.NetworkRequest>} networkRequests
* @return {NetworkNodeOutput}
*/
static getNetworkNodeOutput(networkRecords) {
static getNetworkNodeOutput(networkRequests) {
/** @type {Array<NetworkNode>} */
const nodes = [];
/** @type {Map<string, NetworkNode>} */
Expand All @@ -74,35 +74,35 @@ class PageDependencyGraph {
/** @type {Map<string, NetworkNode|null>} */
const frameIdToNodeMap = new Map();

networkRecords.forEach(record => {
if (IGNORED_MIME_TYPES_REGEX.test(record.mimeType)) return;
if (record.fromWorker) return;
networkRequests.forEach(request => {
if (IGNORED_MIME_TYPES_REGEX.test(request.mimeType)) return;
if (request.fromWorker) return;

// Network record requestIds can be duplicated for an unknown reason
// Suffix all subsequent records with `:duplicate` until it's unique
// Network requestIds can be duplicated for an unknown reason
// Suffix all subsequent requests with `:duplicate` until it's unique
// NOTE: This should never happen with modern NetworkRequest library, but old fixtures
// might still have this issue.
while (idToNodeMap.has(record.requestId)) {
record.requestId += ':duplicate';
while (idToNodeMap.has(request.requestId)) {
request.requestId += ':duplicate';
}

const node = new NetworkNode(record);
const node = new NetworkNode(request);
nodes.push(node);

const urlList = urlToNodeMap.get(record.url) || [];
const urlList = urlToNodeMap.get(request.url) || [];
urlList.push(node);

idToNodeMap.set(record.requestId, node);
urlToNodeMap.set(record.url, urlList);
idToNodeMap.set(request.requestId, node);
urlToNodeMap.set(request.url, urlList);

// If the request was for the root document of an iframe, save an entry in our
// map so we can link up the task `args.data.frame` dependencies later in graph creation.
if (record.frameId &&
record.resourceType === NetworkRequestTypes.Document &&
record.documentURL === record.url) {
if (request.frameId &&
request.resourceType === NetworkRequestTypes.Document &&
request.documentURL === request.url) {
// If there's ever any ambiguity, permanently set the value to `false` to avoid loops in the graph.
const value = frameIdToNodeMap.has(record.frameId) ? null : node;
frameIdToNodeMap.set(record.frameId, value);
const value = frameIdToNodeMap.has(request.frameId) ? null : node;
frameIdToNodeMap.set(request.frameId, value);
}
});

Expand Down Expand Up @@ -427,7 +427,7 @@ class PageDependencyGraph {
}

for (const r of lanternRequests) {
delete r.record;
delete r.rawRequest;
if (r.initiatorRequest) {
// @ts-expect-error
r.initiatorRequest = {id: r.initiatorRequest.requestId};
Expand Down Expand Up @@ -508,25 +508,25 @@ class PageDependencyGraph {

/**
* @param {LH.TraceEvent[]} mainThreadEvents
* @param {Array<Lantern.NetworkRequest>} networkRecords
* @param {Lantern.NetworkRequest[]} networkRequests
* @param {URLArtifact} URL
* @return {Node}
*/
static createGraph(mainThreadEvents, networkRecords, URL) {
static createGraph(mainThreadEvents, networkRequests, URL) {
// This is for debugging trace/devtoolslog network records.
// const debug = PageDependencyGraph._debugNormalizeRequests(networkRecords);
const networkNodeOutput = PageDependencyGraph.getNetworkNodeOutput(networkRecords);
// const debug = PageDependencyGraph._debugNormalizeRequests(networkRequests);
const networkNodeOutput = PageDependencyGraph.getNetworkNodeOutput(networkRequests);
const cpuNodes = PageDependencyGraph.getCPUNodes(mainThreadEvents);
const {requestedUrl, mainDocumentUrl} = URL;
if (!requestedUrl) throw new Error('requestedUrl is required to get the root request');
if (!mainDocumentUrl) throw new Error('mainDocumentUrl is required to get the main resource');

const rootRequest = NetworkAnalyzer.findResourceForUrl(networkRecords, requestedUrl);
const rootRequest = NetworkAnalyzer.findResourceForUrl(networkRequests, requestedUrl);
if (!rootRequest) throw new Error('rootRequest not found');
const rootNode = networkNodeOutput.idToNodeMap.get(rootRequest.requestId);
if (!rootNode) throw new Error('rootNode not found');
const mainDocumentRequest =
NetworkAnalyzer.findLastDocumentForUrl(networkRecords, mainDocumentUrl);
NetworkAnalyzer.findLastDocumentForUrl(networkRequests, mainDocumentUrl);
if (!mainDocumentRequest) throw new Error('mainDocumentRequest not found');
const mainDocumentNode = networkNodeOutput.idToNodeMap.get(mainDocumentRequest.requestId);
if (!mainDocumentNode) throw new Error('mainDocumentNode not found');
Expand All @@ -543,20 +543,20 @@ class PageDependencyGraph {
}

/**
* @param {Lantern.NetworkRequest} record The record to find the initiator of
* @param {Map<string, Lantern.NetworkRequest[]>} recordsByURL
* @param {Lantern.NetworkRequest} request The request to find the initiator of
* @param {Map<string, Lantern.NetworkRequest[]>} requestsByURL
* @return {Lantern.NetworkRequest|null}
*/
static chooseInitiatorRequest(record, recordsByURL) {
if (record.redirectSource) {
return record.redirectSource;
static chooseInitiatorRequest(request, requestsByURL) {
if (request.redirectSource) {
return request.redirectSource;
}

const initiatorURL = PageDependencyGraph.getNetworkInitiators(record)[0];
let candidates = recordsByURL.get(initiatorURL) || [];
const initiatorURL = PageDependencyGraph.getNetworkInitiators(request)[0];
let candidates = requestsByURL.get(initiatorURL) || [];
// The (valid) initiator must come before the initiated request.
candidates = candidates.filter(c => {
return c.responseHeadersEndTime <= record.rendererStartTime &&
return c.responseHeadersEndTime <= request.rendererStartTime &&
c.finished && !c.failed;
});
if (candidates.length > 1) {
Expand All @@ -570,12 +570,12 @@ class PageDependencyGraph {
}
if (candidates.length > 1) {
// Disambiguate based on frame. It's likely that the initiator comes from the same frame.
const sameFrameCandidates = candidates.filter(cand => cand.frameId === record.frameId);
const sameFrameCandidates = candidates.filter(cand => cand.frameId === request.frameId);
if (sameFrameCandidates.length) {
candidates = sameFrameCandidates;
}
}
if (candidates.length > 1 && record.initiator.type === 'parser') {
if (candidates.length > 1 && request.initiator.type === 'parser') {
// Filter to just Documents when initiator type is parser.
const documentCandidates = candidates.filter(cand =>
cand.resourceType === RESOURCE_TYPES.Document);
Expand Down Expand Up @@ -728,6 +728,7 @@ class PageDependencyGraph {
}

return {
rawRequest: request,
requestId: request.args.data.requestId,
connectionId: request.args.data.connectionId,
connectionReused: request.args.data.connectionReused,
Expand All @@ -754,7 +755,6 @@ class PageDependencyGraph {
priority: request.args.data.priority,
frameId: request.args.data.frame,
fromWorker,
record: request,
// Set later.
redirects: undefined,
redirectSource: undefined,
Expand Down Expand Up @@ -805,9 +805,9 @@ class PageDependencyGraph {
// TraceEngine consolidates all redirects into a single request object, but lantern needs
// an entry for each redirected request.
for (const request of [...lanternRequests]) {
if (!request.record) continue;
if (!request.rawRequest) continue;

const redirects = request.record.args.data.redirects;
const redirects = request.rawRequest.args.data.redirects;
if (!redirects.length) continue;

const requestChain = [];
Expand Down
Loading
Loading