Skip to content

Commit 7d502af

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[DocumentLatency] Get server response time from header for Lightrider
I thought I fixed this, but I overlooked that the trace processor does not set this field. Only the Lantern usage from Lighthouse was getting the correct server response time in Lightrider, but the document lantency insight was still wrong. Bug: 352244434 Change-Id: I5eb711df4ff791618248426154a8e6a64c923223 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7001852 Reviewed-by: Paul Irish <paulirish@chromium.org> Commit-Queue: Connor Clark <cjamcl@chromium.org>
1 parent 40d68f6 commit 7d502af

5 files changed

Lines changed: 67 additions & 10 deletions

File tree

front_end/models/trace/LanternComputationData.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ function createLanternRequest(
192192
priority: request.args.data.priority,
193193
frameId: request.args.data.frame,
194194
fromWorker,
195+
serverResponseTime: request.args.data.lrServerResponseTime ?? undefined,
195196
// Set later.
196197
redirects: undefined,
197198
redirectSource: undefined,

front_end/models/trace/handlers/NetworkRequestsHandler.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,7 @@ export async function finalize(): Promise<void> {
295295
*
296296
* See `_updateTimingsForLightrider` in Lighthouse for more detail.
297297
*/
298+
let lrServerResponseTime;
298299
if (isLightrider && request.receiveResponse?.args.data.headers) {
299300
timing = {
300301
requestTime: Helpers.Timing.microToSeconds(request.sendRequests.at(0)?.ts ?? 0 as Types.Timing.Micro),
@@ -330,6 +331,14 @@ export async function finalize(): Promise<void> {
330331
timing.connectEnd = TCPMs as Types.Timing.Milli;
331332
timing.sslEnd = TCPMs as Types.Timing.Milli;
332333
}
334+
335+
// Lightrider does not have any equivalent for `sendEnd` timing values. The
336+
// closest we can get to the server response time is from a header that
337+
// Lightrider sets.
338+
const ResponseMsHeader = request.receiveResponse.args.data.headers.find(h => h.name === 'X-ResponseMs');
339+
if (ResponseMsHeader) {
340+
lrServerResponseTime = Math.max(0, parseInt(ResponseMsHeader.value, 10)) as Types.Timing.Milli;
341+
}
333342
}
334343

335344
// TODO: consider allowing chrome / about.
@@ -526,6 +535,7 @@ export async function finalize(): Promise<void> {
526535
initiator: finalSendRequest.args.data.initiator,
527536
stackTrace: finalSendRequest.args.data.stackTrace,
528537
timing,
538+
lrServerResponseTime,
529539
url,
530540
failed: request.resourceFinish?.args.data.didFail ?? false,
531541
finished: Boolean(request.resourceFinish),

front_end/models/trace/insights/DocumentLatency.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,51 @@ describeWithEnvironment('DocumentLatency', function() {
6464
assert.deepEqual(insight.metricSavings, {FCP: 943, LCP: 943} as Trace.Insights.Types.MetricSavings);
6565
});
6666

67+
describe('Lightrider', () => {
68+
before(() => {
69+
// @ts-expect-error
70+
globalThis.isLightrider = true;
71+
});
72+
73+
after(() => {
74+
// @ts-expect-error
75+
delete globalThis.isLightrider;
76+
});
77+
78+
it('reports savings for server with slow server response time (Lightrider)', async function() {
79+
const traceEvents = [...await TraceLoader.rawEvents(this, 'lantern/paul/trace.json.gz')];
80+
const processor = Trace.Processor.TraceProcessor.createWithAllHandlers();
81+
82+
const mainRequestEventIndex = traceEvents.findIndex(e => e.name === 'ResourceReceiveResponse');
83+
const mainRequestEvent = structuredClone(traceEvents[mainRequestEventIndex]);
84+
assert(Types.Events.isResourceReceiveResponse(mainRequestEvent));
85+
assert.strictEqual(mainRequestEvent.args.data.requestId, '1000C0FDC0A75327167272FC7438E999');
86+
if (!mainRequestEvent.args.data.timing) {
87+
throw new Error('missing timing field');
88+
}
89+
90+
delete mainRequestEvent.args.data.timing;
91+
mainRequestEvent.args.data.headers = [
92+
{name: 'X-ResponseMs', value: '1234'},
93+
];
94+
95+
traceEvents[mainRequestEventIndex] = mainRequestEvent;
96+
97+
await processor.parse(traceEvents, {isCPUProfile: false, isFreshRecording: true});
98+
const data = processor.data;
99+
if (!data) {
100+
throw new Error('missing data');
101+
}
102+
103+
const navigation = getFirstOrError(data.Meta.navigationsByNavigationId.values());
104+
const context = createContextForNavigation(data, navigation, data.Meta.mainFrameId);
105+
const insight = Trace.Insights.Models.DocumentLatency.generateInsight(data, context);
106+
assert.strictEqual(insight.data?.serverResponseTime, 1234);
107+
assert.isFalse(insight.data?.checklist.serverResponseIsFast.value);
108+
assert.deepEqual(insight.metricSavings, {FCP: 1134, LCP: 1134} as Trace.Insights.Types.MetricSavings);
109+
});
110+
});
111+
67112
it('reports no compression savings for compressed text', async function() {
68113
const {data, insights} = await processTrace(this, 'lantern/paul/trace.json.gz');
69114
const insight =

front_end/models/trace/insights/DocumentLatency.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,14 @@ export type DocumentLatencyInsightModel = InsightModel<typeof UIStrings, {
9595
},
9696
}>;
9797

98-
function getServerResponseTime(
99-
request: Types.Events.SyntheticNetworkRequest, context: InsightSetContext): Types.Timing.Milli|null {
100-
// Prefer the value as given by the Lantern provider.
101-
// For PSI, Lighthouse uses this to set a better value for the server response
102-
// time. For technical reasons, in Lightrider we do not have `sendEnd` timing
103-
// values. See Lighthouse's `asLanternNetworkRequest` function for more.
104-
const lanternRequest = context.navigation && context.lantern?.requests.find(r => r.rawRequest === request);
105-
if (lanternRequest?.serverResponseTime !== undefined) {
106-
return lanternRequest.serverResponseTime as Types.Timing.Milli;
98+
function getServerResponseTime(request: Types.Events.SyntheticNetworkRequest): Types.Timing.Milli|null {
99+
// For technical reasons, Lightrider does not have `sendEnd` timing values. The
100+
// closest we can get to the server response time is from a header that Lightrider
101+
// sets.
102+
// @ts-expect-error
103+
const isLightrider = globalThis.isLightrider;
104+
if (isLightrider) {
105+
return request.args.data.lrServerResponseTime ?? null;
107106
}
108107

109108
const timing = request.args.data.timing;
@@ -202,7 +201,7 @@ export function generateInsight(
202201
return finalize({warnings: [InsightWarning.NO_DOCUMENT_REQUEST]});
203202
}
204203

205-
const serverResponseTime = getServerResponseTime(documentRequest, context);
204+
const serverResponseTime = getServerResponseTime(documentRequest);
206205
if (serverResponseTime === null) {
207206
throw new Error('missing document request timing');
208207
}

front_end/models/trace/types/TraceEvents.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,8 @@ export interface SyntheticNetworkRequest extends Complete, SyntheticBased<Phase.
415415
initiator?: Initiator,
416416
requestMethod?: string,
417417
timing?: ResourceReceiveResponseTimingData,
418+
/** Server response time according to Lightrider. */
419+
lrServerResponseTime?: Milli,
418420
},
419421
};
420422
cat: 'loading';

0 commit comments

Comments
 (0)