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
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/byte-efficiency-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class ByteEfficiencyAudit extends Audit {
const originalTransferSizes = new Map();
graph.traverse(node => {
if (node.type !== 'network') return;
const wastedBytes = wastedBytesByUrl.get(node.record.url);
const wastedBytes = wastedBytesByUrl.get(node.request.url);
if (!wastedBytes) return;

const original = node.request.transferSize;
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
if (node.type === 'cpu' && timing.duration >= 50) {
lastLongTaskStartTime = Math.max(lastLongTaskStartTime, timing.startTime);
} else if (node.type === 'network') {
startTimesByURL.set(node.record.url, timing.startTime);
startTimesByURL.set(node.request.url, timing.startTime);
}
}

Expand Down
10 changes: 5 additions & 5 deletions core/audits/byte-efficiency/render-blocking-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function computeStackSpecificTiming(node, nodeTiming, Stacks) {
// https://github.com/ampproject/amphtml/blob/8e03ac2f315774070651584a7e046ff24212c9b1/src/font-stylesheet-timeout.js#L54-L59
// Any potential savings must only include time spent on AMP stylesheet nodes before 2.1 seconds.
if (node.type === BaseNode.TYPES.NETWORK &&
node.record.resourceType === NetworkRequest.TYPES.Stylesheet &&
node.request.resourceType === NetworkRequest.TYPES.Stylesheet &&
nodeTiming.endTime > 2100) {
stackSpecificTiming.endTime = Math.max(nodeTiming.startTime, 2100);
stackSpecificTiming.duration = stackSpecificTiming.endTime - stackSpecificTiming.startTime;
Expand Down Expand Up @@ -228,11 +228,11 @@ class RenderBlockingResources extends Audit {
if (node.type !== BaseNode.TYPES.NETWORK) return !canDeferRequest;

const isStylesheet =
node.record.resourceType === NetworkRequest.TYPES.Stylesheet;
node.request.resourceType === NetworkRequest.TYPES.Stylesheet;
if (canDeferRequest && isStylesheet) {
// We'll inline the used bytes of the stylesheet and assume the rest can be deferred
const wastedBytes = wastedCssBytesByUrl.get(node.record.url) || 0;
totalChildNetworkBytes += (node.record.transferSize || 0) - wastedBytes;
const wastedBytes = wastedCssBytesByUrl.get(node.request.url) || 0;
totalChildNetworkBytes += (node.request.transferSize || 0) - wastedBytes;
}
return !canDeferRequest;
});
Expand All @@ -247,7 +247,7 @@ class RenderBlockingResources extends Audit {
));

// Add the inlined bytes to the HTML response
const originalTransferSize = minimalFCPGraph.record.transferSize;
const originalTransferSize = minimalFCPGraph.request.transferSize;
const safeTransferSize = originalTransferSize || 0;
minimalFCPGraph.request.transferSize = safeTransferSize + totalChildNetworkBytes;
const estimateAfterInline = simulator.simulate(minimalFCPGraph).timeInMs;
Expand Down
4 changes: 2 additions & 2 deletions core/audits/dobetterweb/uses-http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ class UsesHTTP2Audit extends Audit {
const originalProtocols = new Map();
graph.traverse(node => {
if (node.type !== 'network') return;
if (!urlsToChange.has(node.record.url)) return;
if (!urlsToChange.has(node.request.url)) return;

originalProtocols.set(node.request.requestId, node.record.protocol);
originalProtocols.set(node.request.requestId, node.request.protocol);
node.request.protocol = 'h2';
});

Expand Down
2 changes: 1 addition & 1 deletion core/audits/prioritize-lcp-image.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class PrioritizeLcpImage extends Audit {
wastedMs,
results: [{
node: Audit.makeNodeItem(lcpElement.node),
url: lcpNode.record.url,
url: lcpNode.request.url,
wastedMs,
}],
};
Expand Down
4 changes: 2 additions & 2 deletions core/audits/uses-rel-preconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,14 @@ class UsesRelPreconnectAudit extends Audit {
/** @type {Set<string>} */
const lcpGraphURLs = new Set();
lcpGraph.traverse(node => {
if (node.type === 'network') lcpGraphURLs.add(node.record.url);
if (node.type === 'network') lcpGraphURLs.add(node.request.url);
});

const fcpGraph =
await LanternFirstContentfulPaint.getPessimisticGraph(pageGraph, processedNavigation);
const fcpGraphURLs = new Set();
fcpGraph.traverse(node => {
if (node.type === 'network') fcpGraphURLs.add(node.record.url);
if (node.type === 'network') fcpGraphURLs.add(node.request.url);
});

/** @type {Map<string, LH.Artifacts.NetworkRequest[]>} */
Expand Down
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
1 change: 1 addition & 0 deletions core/computed/metrics/lantern-metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ async function getComputationDataParamsFromTrace(data, context) {
}

const {trace, URL} = data;
// TODO(15841): use computed artifact.
const {graph} = await createGraphFromTrace(URL, trace, context);
const processedNavigation = await ProcessedNavigation.request(data.trace, context);
const simulator = data.simulator || (await LoadSimulator.request(data, context));
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
7 changes: 3 additions & 4 deletions core/lib/lantern/network-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const NON_NETWORK_SCHEMES = [
];

/**
* Use `NetworkRequest.isNonNetworkRequest(req)` if working with a request.
* Note: the `protocol` field from CDP can be 'h2', 'http', (not 'https'!) or it'll be url's scheme.
* https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/network_handler.cc;l=598-611;drc=56d4a9a9deb30be73adcee8737c73bcb2a5ab64f
* However, a `new URL(href).protocol` has a colon suffix.
Expand Down Expand Up @@ -69,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 @@ -105,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
Loading
Loading