TraceEngine: update render_blocking with preload status change

This CL fixes a bug where a blocking resource (e.g. stylesheet) is not
marked as blocking in RPP because it is first preloaded.

This was because we did not pay attention to the
PreloadRenderBlockingStatusChange event which exists for this exact
reason. With this CL when we construct the final synthetic network
request for a resource, we will use the render blocking status from the
last PreloadRenderBlockingStatusChange event (if we have any), before
falling back to the render blocking status on the SendRequest event.

[email protected]

Fixed: 457323832
Change-Id: I996a3372a4ec8f9cbf8d1c387b2ec44a80aa9c14
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7112841
Auto-Submit: Jack Franklin <[email protected]>
Reviewed-by: Connor Clark <[email protected]>
Commit-Queue: Connor Clark <[email protected]>

Author: jackfranklin

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

@@ -471,6 +471,24 @@ describe('NetworkRequestsHandler', function() {
       assert.deepEqual(actualLinks, expectedLinks);
     });
   });
+
+  it('updates render blocking request statuses if they were updated with a preloadRenderBlockingStatusChange event',
+     async function() {
+       const traceEvents = await TraceLoader.rawEvents(this, 'render-blocking-preload.json.gz');
+       for (const event of traceEvents) {
+         Trace.Handlers.ModelHandlers.Meta.handleEvent(event);
+         Trace.Handlers.ModelHandlers.NetworkRequests.handleEvent(event);
+       }
+       await Trace.Handlers.ModelHandlers.Meta.finalize();
+       await Trace.Handlers.ModelHandlers.NetworkRequests.finalize();
+       const url = 'https://andydavies.github.io/agent-tests/render-blocking/css/styles.css';
+       const data = Trace.Handlers.ModelHandlers.NetworkRequests.data();
+
+       const request = data.byTime.find(e => e.args.data.url === url);
+       assert.isOk(request);
+
+       assert.strictEqual(request.args.data.renderBlocking, 'blocking');
+     });
 });
 
 function assertDataArgsStats<D extends keyof DataArgs>(

front_end/models/trace/handlers/NetworkRequestsHandler.ts

@@ -32,6 +32,7 @@ export interface TraceEventsForNetworkRequest {
   resourceFinish?: Types.Events.ResourceFinish;
   receivedData?: Types.Events.ResourceReceivedData[];
   resourceMarkAsCached?: Types.Events.ResourceMarkAsCached;
+  preloadRenderBlockingStatusChange?: Types.Events.PreloadRenderBlockingStatusChangeEvent[];
 }
 
 export interface WebSocketTraceDataForFrame {
@@ -164,6 +165,10 @@ export function handleEvent(event: Types.Events.Event): void {
     return;
   }
 
+  if (Types.Events.isPreloadRenderBlockingStatusChangeEvent(event)) {
+    storeTraceEventWithRequestId(event.args.data.requestId, 'preloadRenderBlockingStatusChange', [event]);
+  }
+
   if (Types.Events.isWebSocketCreate(event) || Types.Events.isWebSocketInfo(event) ||
       Types.Events.isWebSocketTransfer(event)) {
     const identifier = event.args.data.identifier;
@@ -487,13 +492,24 @@ export async function finalize(): Promise<void> {
         Types.Timing.Micro(0);
 
     // Finally get some of the general data from the trace events.
-    const {frame, url, renderBlocking} = finalSendRequest.args.data;
+    const {frame, url, renderBlocking: sendRequestIsRenderBlocking} = finalSendRequest.args.data;
     const {encodedDataLength, decodedBodyLength} =
         request.resourceFinish ? request.resourceFinish.args.data : {encodedDataLength: 0, decodedBodyLength: 0};
     const parsedUrl = new URL(url);
     const isHttps = parsedUrl.protocol === 'https:';
     const requestingFrameUrl =
         Helpers.Trace.activeURLForFrameAtTime(frame, finalSendRequest.ts, rendererProcessesByFrame) || '';
+
+    // A resource that is preloaded (and not marked as render blocking) can
+    // become render blocked later via a PreloadRenderBlockingStatusChange. In
+    // this case, we take the render blocking value of the last
+    // PreloadRenderBlockingStatusChange for this request.
+    const preloadRenderBlockingStatusChange =
+        request.preloadRenderBlockingStatusChange?.at(-1)?.args.data.renderBlocking;
+
+    // In the event the property isn't set, assume non-blocking.
+    const isRenderBlocking = preloadRenderBlockingStatusChange ?? sendRequestIsRenderBlocking ?? 'non_blocking';
+
     // Construct a synthetic trace event for this network request.
     const networkEvent =
         Helpers.SyntheticEvents.SyntheticEventsManager.registerSyntheticEvent<Types.Events.SyntheticNetworkRequest>({
@@ -535,8 +551,7 @@ export async function finalize(): Promise<void> {
               initialPriority,
               protocol: request.receiveResponse?.args.data.protocol ?? 'unknown',
               redirects,
-              // In the event the property isn't set, assume non-blocking.
-              renderBlocking: renderBlocking ?? 'non_blocking',
+              renderBlocking: isRenderBlocking,
               requestId,
               requestingFrameUrl,
               requestMethod: finalSendRequest.args.data.requestMethod,

front_end/models/trace/types/TraceEvents.ts

@@ -3142,6 +3142,8 @@ export const enum Name {
   USER_TIMING_MEASURE = 'UserTiming::Measure',
 
   LINK_PRECONNECT = 'LinkPreconnect',
+
+  PRELOAD_RENDER_BLOCKING_STATUS_CHANGE = 'PreloadRenderBlockingStatusChange',
 }
 
 /**
@@ -3275,3 +3277,20 @@ export function isAnyScriptSourceEvent(event: Event): event is RundownScriptSour
     RundownScriptStub {
   return event.cat === 'disabled-by-default-devtools.v8-source-rundown-sources';
 }
+
+export interface PreloadRenderBlockingStatusChangeEvent extends Instant {
+  name: Name.PRELOAD_RENDER_BLOCKING_STATUS_CHANGE;
+  cat: 'devtools.timeline';
+  args: Args&{
+    data: {
+      requestId: string,
+      url: string,
+      renderBlocking?: RenderBlocking,
+    },
+  };
+}
+
+export function isPreloadRenderBlockingStatusChangeEvent(event: Event):
+    event is PreloadRenderBlockingStatusChangeEvent {
+  return event.name === Name.PRELOAD_RENDER_BLOCKING_STATUS_CHANGE;
+}

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

@@ -110,6 +110,7 @@ copy_to_gen("traces") {
     "reload-no-tti.json.gz",
     "render-blocking-body.json.gz",
     "render-blocking-in-iframe.json.gz",
+    "render-blocking-preload.json.gz",
     "render-blocking-requests.json.gz",
     "scheduler-post-task.json.gz",
     "scripts/coursehero-bundle-1.js.gz",

front_end/panels/timeline/fixtures/traces/README.md

@@ -365,3 +365,7 @@ A trace that has a layout shift with a root cause that is a non-composited anima
 ### cls-with-iframes
 
 A trace of https://astro-news-1026410574114.us-central1.run.app/ used to test CLS + iframes and what our AI output looks like.
+
+### render-blocking-preload
+
+A trace of https://andydavies.github.io/agent-tests/render-blocking/css-preload.html that highlighted a bug where we do not update render blocking status based on PreloadRenderBlockingStatus events (crbug.com/457323832).