Skip to content

Commit b6ed610

Browse files
Connor ClarkDevtools-frontend LUCI CQ
Connor Clark
authored and
Devtools-frontend LUCI CQ
committed
[RPP] Display final redirect/history URL for insight sets
Adds `finalDisplayUrlByNavigationId` to MetaHandler, which resolves a navigation id to the final redirect / history push state URL. This matches the `finalDisplayedUrl` semantics from Lighthouse. Fixed: 402743677 Change-Id: I30e223c14a95b628780097de0be0b7b79969b2a4 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6353987 Auto-Submit: Connor Clark <[email protected]> Commit-Queue: Paul Irish <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent b2de95f commit b6ed610

File tree

8 files changed

+96
-2
lines changed

8 files changed

+96
-2
lines changed

front_end/models/trace/Processor.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -450,11 +450,12 @@ export class TraceProcessor extends EventTarget {
450450
let id, urlString, navigation;
451451
if (context.navigation) {
452452
id = context.navigationId;
453-
urlString = context.navigation.args.data?.documentLoaderURL ?? parsedTrace.Meta.mainFrameURL;
453+
urlString =
454+
parsedTrace.Meta.finalDisplayUrlByNavigationId.get(context.navigationId) ?? parsedTrace.Meta.mainFrameURL;
454455
navigation = context.navigation;
455456
} else {
456457
id = Types.Events.NO_NAVIGATION;
457-
urlString = parsedTrace.Meta.mainFrameURL;
458+
urlString = parsedTrace.Meta.finalDisplayUrlByNavigationId.get('') ?? parsedTrace.Meta.mainFrameURL;
458459
}
459460

460461
const model = {} as Insights.Types.InsightSet['model'];

front_end/models/trace/handlers/MetaHandler.test.ts

+31
Original file line numberDiff line numberDiff line change
@@ -604,4 +604,35 @@ describe('MetaHandler', function() {
604604
// But they are not in the primary page.
605605
assert.strictEqual(data.mainFrameId, '07B7D55F5BE0ADB8AAD6502F2D3859FF');
606606
});
607+
608+
it('will detect the final redirect URL for a main frame navigation', async function() {
609+
// See crbug.com/402743677 for context.
610+
const events = await TraceLoader.rawEvents(this, 'redirects-http-to-https.json.gz');
611+
for (const event of events) {
612+
Trace.Handlers.ModelHandlers.Meta.handleEvent(event);
613+
}
614+
await Trace.Handlers.ModelHandlers.Meta.finalize();
615+
const data = Trace.Handlers.ModelHandlers.Meta.data();
616+
617+
assert.strictEqual(data.mainFrameURL, 'https://example.com/');
618+
assert.strictEqual(data.mainFrameNavigations[0].args.data?.documentLoaderURL, 'http://paulirish.com/');
619+
assert.deepEqual([...data.finalDisplayUrlByNavigationId], [
620+
['832038B69FE671709DE9655B8161EB9A', 'https://www.paulirish.com/'],
621+
]);
622+
});
623+
624+
it('will detect history API navigations for the start of the trace even w/o any navigation', async function() {
625+
// See crbug.com/402743677 for context.
626+
const events = await TraceLoader.rawEvents(this, 'history-api-no-nav.json.gz');
627+
for (const event of events) {
628+
Trace.Handlers.ModelHandlers.Meta.handleEvent(event);
629+
}
630+
await Trace.Handlers.ModelHandlers.Meta.finalize();
631+
const data = Trace.Handlers.ModelHandlers.Meta.data();
632+
633+
assert.strictEqual(data.mainFrameURL, 'http://localhost:10325/test.html');
634+
assert.deepEqual([...data.finalDisplayUrlByNavigationId], [
635+
['', 'http://localhost:10325/testing/me?0.7574185139653986'],
636+
]);
637+
});
607638
});

front_end/models/trace/handlers/MetaHandler.ts

+45
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ const traceBounds: Types.Timing.TraceWindowMicro = {
5151
*/
5252
const navigationsByFrameId = new Map<string, Types.Events.NavigationStart[]>();
5353
const navigationsByNavigationId = new Map<string, Types.Events.NavigationStart>();
54+
const finalDisplayUrlByNavigationId = new Map<string, string>();
5455
const mainFrameNavigations: Types.Events.NavigationStart[] = [];
5556

5657
// Represents all the threads in the trace, organized by process. This is mostly for internal
@@ -82,6 +83,7 @@ const CHROME_WEB_TRACE_EVENTS = new Set([
8283
export function reset(): void {
8384
navigationsByFrameId.clear();
8485
navigationsByNavigationId.clear();
86+
finalDisplayUrlByNavigationId.clear();
8587
processNames.clear();
8688
mainFrameNavigations.length = 0;
8789

@@ -303,6 +305,7 @@ export function handleEvent(event: Types.Events.Event): void {
303305
return;
304306
}
305307
navigationsByNavigationId.set(navigationId, event);
308+
finalDisplayUrlByNavigationId.set(navigationId, event.args.data.documentLoaderURL);
306309

307310
const frameId = event.args.frame;
308311
const existingFrameNavigations = navigationsByFrameId.get(frameId) || [];
@@ -313,6 +316,34 @@ export function handleEvent(event: Types.Events.Event): void {
313316
}
314317
return;
315318
}
319+
320+
// Update `finalDisplayUrlByNavigationId` to reflect the latest redirect for each navigation.
321+
if (Types.Events.isResourceSendRequest(event)) {
322+
if (event.args.data.resourceType !== 'Document') {
323+
return;
324+
}
325+
326+
const maybeNavigationId = event.args.data.requestId;
327+
const navigation = navigationsByNavigationId.get(maybeNavigationId);
328+
if (!navigation) {
329+
return;
330+
}
331+
332+
finalDisplayUrlByNavigationId.set(maybeNavigationId, event.args.data.url);
333+
return;
334+
}
335+
336+
// Update `finalDisplayUrlByNavigationId` to reflect history API navigations.
337+
if (Types.Events.isDidCommitSameDocumentNavigation(event)) {
338+
if (event.args.render_frame_host.frame_type !== 'PRIMARY_MAIN_FRAME') {
339+
return;
340+
}
341+
342+
const navigation = mainFrameNavigations.at(-1);
343+
const key = navigation?.args.data?.navigationId ?? '';
344+
finalDisplayUrlByNavigationId.set(key, event.args.url);
345+
return;
346+
}
316347
}
317348

318349
export async function finalize(): Promise<void> {
@@ -405,6 +436,19 @@ export interface MetaHandlerData {
405436
gpuProcessId: Types.Events.ProcessID;
406437
navigationsByFrameId: Map<string, Types.Events.NavigationStart[]>;
407438
navigationsByNavigationId: Map<string, Types.Events.NavigationStart>;
439+
/**
440+
* The user-visible URL displayed to users in the address bar.
441+
* This captures:
442+
* - resolving all redirects
443+
* - history API pushState
444+
*
445+
* Given no redirects or history API usages, this is just the navigation event's documentLoaderURL.
446+
*
447+
* Note: empty string special case denotes the duration of the trace between the start
448+
* and the first navigation. If there is no history API navigation during this time,
449+
* there will be no value for empty string.
450+
**/
451+
finalDisplayUrlByNavigationId: Map<string, string>;
408452
threadsInProcess: Map<Types.Events.ProcessID, Map<Types.Events.ThreadID, Types.Events.ThreadName>>;
409453
mainFrameId: string;
410454
mainFrameURL: string;
@@ -457,6 +501,7 @@ export function data(): MetaHandlerData {
457501
mainFrameURL,
458502
navigationsByFrameId,
459503
navigationsByNavigationId,
504+
finalDisplayUrlByNavigationId,
460505
threadsInProcess,
461506
rendererProcessesByFrame: rendererProcessesByFrameId,
462507
topLevelRendererIds,

front_end/models/trace/types/TraceEvents.ts

+14
Original file line numberDiff line numberDiff line change
@@ -2248,6 +2248,20 @@ export function isNavigationStart(event: Event): event is NavigationStart {
22482248
return event.name === 'navigationStart' && (event as NavigationStart).args?.data?.documentLoaderURL !== '';
22492249
}
22502250

2251+
export interface DidCommitSameDocumentNavigation extends Complete {
2252+
name: 'RenderFrameHostImpl::DidCommitSameDocumentNavigation';
2253+
args: Args&{
2254+
url: string,
2255+
render_frame_host: {
2256+
frame_type: string,
2257+
},
2258+
};
2259+
}
2260+
2261+
export function isDidCommitSameDocumentNavigation(event: Event): event is DidCommitSameDocumentNavigation {
2262+
return event.name === 'RenderFrameHostImpl::DidCommitSameDocumentNavigation';
2263+
}
2264+
22512265
export function isMainFrameViewport(
22522266
event: Event,
22532267
): event is MainFrameViewport {

front_end/panels/timeline/fixtures/traces/BUILD.gn

+2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ copy_to_gen("traces") {
3434
"forced-layouts-and-no-gpu.json.gz",
3535
"forced-reflow.json.gz",
3636
"generic-about-tracing.json.gz",
37+
"history-api-no-nav.json.gz",
3738
"idle-callback.json.gz",
3839
"idle-tasks.json.gz",
3940
"iframe-shift.json.gz",
@@ -86,6 +87,7 @@ copy_to_gen("traces") {
8687
"react-hello-world.json.gz",
8788
"recursive-blocking-js.json.gz",
8889
"recursive-counting-js.json.gz",
90+
"redirects-http-to-https.json.gz",
8991
"redirects-subresource-multiple.json.gz",
9092
"redirects.json.gz",
9193
"reload-and-trace-page.json.gz",
Binary file not shown.
Binary file not shown.

front_end/testing/TraceHelpers.ts

+1
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,7 @@ export function getBaseTraceParseModelData(overrides: Partial<ParsedTrace> = {})
630630
threadsInProcess: new Map(),
631631
navigationsByFrameId: new Map(),
632632
navigationsByNavigationId: new Map(),
633+
finalDisplayUrlByNavigationId: new Map(),
633634
mainFrameId: '',
634635
mainFrameURL: '',
635636
rendererProcessesByFrame: new Map(),

0 commit comments

Comments
 (0)