From 76204ec6308a235eb0b77557d0a52657952572f4 Mon Sep 17 00:00:00 2001
From: Varo <122426901+varo-yang@users.noreply.github.com>
Date: Wed, 8 Jul 2026 04:21:59 +0800
Subject: [PATCH] fix(engine): pre-create __render_frame__ siblings in
 initializeSession (#2006)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(engine): commit render-frame siblings with a visual BeginFrame at init

Chunk-lambda renders drop a periodic near-black frame — one every
chunk_frames/worker_count frames (every 60 on a 4-worker single-video chunk),
YAVG ~22 against YMAX ~240 in signalstats. Local single-process renders don't
show it because they don't run under BeginFrame.

It's the isNewImage branch in injectVideoFramesBatch: the first time a session
paints a given videoId there's no __render_frame__ sibling yet, so it creates
the <img> on the spot (createElement + insertBefore) right before capture.
Under HeadlessExperimental.BeginFrame the compositor doesn't have that fresh
layer in the immediately-next frame, so the first captured frame per session
paints only body background + already-composited overlays. Each lambda worker
is its own session, hence the worker-boundary periodicity.

Pre-create the hidden sibling at the end of initializeSession, then drive one
non-capture visual BeginFrame (noDisplayUpdates: false) to composite the new
layers before the first real capture. The warmup ticks are noDisplayUpdates:
true (they advance the clock but don't paint) and the per-frame seek doesn't
tick, so this explicit visual frame is what actually commits the layers; its
tick sits in the gap between warmup and frame 0 so ticks stay monotonic and no
render frame is consumed. Every subsequent inject then takes the hasImg=true
(src-update) path; the isNewImage branch stays as a fallback for callers that
don't go through initializeSession.

* fix(engine): place the render-frame commit tick before the liveness probe

The commit tick at init sends its BeginFrame at `beginFrameTimeTicks - 1·interval`.
The producer's liveness probe then fires right after init at
`beginFrameTimeTicks - 5·interval` — an earlier tick. Per-session BeginFrame time
has to be monotonic, so the probe running backwards past the commit tick stalls
chrome-headless-shell indefinitely; the engine reads that timeout as a SwiftShader
heavy-layer stall and routes the render to screenshot capture, which then dies
relaunching and hangs the shard to the job timeout.

Reproduced on a native x86 SwiftShader host and bisected: with the commit tick
present the probe times out even with zero render-frame siblings created, so it's
the tick ordering, not layer count. Moving the commit tick to `-6·interval` (below
the probe, above the warmup ticks) keeps warmup < commit < probe < capture
monotonic and clears the stall on every affected comp — sub-composition-video,
chat, style-5-prod — while a healthy comp (style-18-prod) is unchanged. The commit
tick itself is untouched, so the black-frame fix it exists for still holds.
---
 packages/engine/src/services/frameCapture.ts  | 48 ++++++++--
 .../src/services/screenshotService.test.ts    | 88 +++++++++++++++++++
 .../engine/src/services/screenshotService.ts  | 40 +++++++++
 3 files changed, 170 insertions(+), 6 deletions(-)

diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts
index b52113532..d017ea920 100644
--- a/packages/engine/src/services/frameCapture.ts
+++ b/packages/engine/src/services/frameCapture.ts
@@ -25,6 +25,7 @@ import {
 } from "./browserManager.js";
 import {
   beginFrameCapture,
+  ensureRenderFrameSiblings,
   getCdpSession,
   pageScreenshotCapture,
   initTransparentBackground,
@@ -1518,6 +1519,7 @@ export async function initializeSession(session: CaptureSession): Promise<void>
     await initDrawElementOrTransparentBackground(session, page, logInitPhase);
 
     await armStaticDedup(session, session.page, logInitPhase);
+    await ensureRenderFrameSiblings(session.page);
     session.isInitialized = true;
     return;
   }
@@ -1648,13 +1650,16 @@ export async function initializeSession(session: CaptureSession): Promise<void>
 
   await recordSessionInitTelemetry(session, initStart);
 
-  // Stop warmup. Unlocked mode exits on this flag; locked mode keeps ticking
-  // until LOCKED_WARMUP_TICKS, so we await its promise to ensure the count is
-  // exact before deriving the baseline.
+  // Stop warmup, then drain the loop in BOTH modes before any further
+  // BeginFrame on this session (drawElement init, the render-frame commit tick
+  // at the end of init, and frame 0). Clearing the flag only stops new
+  // iterations — a warmup `HeadlessExperimental.beginFrame` may still be in
+  // flight, and a second beginFrame on the same session while one is pending
+  // fails with "Another frame is pending". Locked mode additionally needs the
+  // await to reach the exact LOCKED_WARMUP_TICKS count before deriving the
+  // baseline below.
   warmupState.running = false;
-  if (lockWarmupTicks) {
-    await warmupLoopPromise.catch(() => {});
-  }
+  await warmupLoopPromise.catch(() => {});
 
   // Set base frame time ticks past warmup range. Locked mode pins to the
   // constant so chunk workers on different hosts compute the same baseline.
@@ -1672,6 +1677,37 @@ export async function initializeSession(session: CaptureSession): Promise<void>
   await initDrawElementOrTransparentBackground(session, page, logInitPhase);
 
   await armStaticDedup(session, session.page, logInitPhase);
+
+  // Pre-create the hidden `__render_frame__` siblings so the first per-session
+  // `injectVideoFramesBatch` takes the `hasImg = true` (src-update) path instead
+  // of inserting a fresh `<img>` mid-capture. Then drive ONE non-capture,
+  // *visual* BeginFrame (`noDisplayUpdates: false`) to actually composite the
+  // new layers before the first real capture. The warmup ticks above are
+  // `noDisplayUpdates: true` (they advance the clock but don't paint), and the
+  // seek in `prepareFrameForCapture` doesn't tick, so without this commit tick
+  // the first *display-producing* BeginFrame would be the capture itself — and
+  // the freshly-inserted layers would miss it (the per-session worker-boundary
+  // near-black flash). The tick time sits in the gap between the last warmup
+  // tick and frame 0 (`beginFrameTimeTicks` carries +10 intervals of headroom),
+  // so ticks stay monotonic and no render frame is consumed.
+  //
+  // It must land BELOW the producer's BeginFrame liveness probe, which fires
+  // from probeStage right after init at `beginFrameTimeTicks - 5·interval`
+  // (screenshotService.probeBeginFrameLiveness). This commit tick is sent during
+  // init — temporally before the probe — so if its tick were above the probe's,
+  // the probe would run backwards in BeginFrame time (non-monotonic) and
+  // chrome-headless-shell stalls it indefinitely (surfaced as a "SwiftShader
+  // heavy-layer" probe timeout, then routed to screenshot capture). At the
+  // original `-1·interval` that stalled every comp reaching the probe; at
+  // `-6·interval` the order stays warmup < commit < probe < capture.
+  await ensureRenderFrameSiblings(page);
+  const commitCdp = await getCdpSession(page);
+  await commitCdp.send("HeadlessExperimental.beginFrame", {
+    frameTimeTicks: session.beginFrameTimeTicks - 6 * session.beginFrameIntervalMs,
+    interval: session.beginFrameIntervalMs,
+    noDisplayUpdates: false,
+  });
+
   session.isInitialized = true;
 }
 
diff --git a/packages/engine/src/services/screenshotService.test.ts b/packages/engine/src/services/screenshotService.test.ts
index 9b7c78d20..c59b73669 100644
--- a/packages/engine/src/services/screenshotService.test.ts
+++ b/packages/engine/src/services/screenshotService.test.ts
@@ -6,6 +6,7 @@ import { type Page } from "puppeteer-core";
 import {
   pageScreenshotCapture,
   cdpSessionCache,
+  ensureRenderFrameSiblings,
   applyDomLayerMask,
   removeDomLayerMask,
   injectVideoFramesBatch,
@@ -744,3 +745,90 @@ describe("video-frame injection respects ancestor visibility", () => {
     }
   });
 });
+
+describe("ensureRenderFrameSiblings", () => {
+  function passthroughPage(): Page {
+    return {
+      evaluate: async (fn: (...args: unknown[]) => unknown, ...args: unknown[]) =>
+        Promise.resolve((fn as (...a: unknown[]) => unknown)(...args)),
+    } as unknown as Page;
+  }
+
+  function withGlobalDom(setup: { window: Window; document: Document }): { teardown: () => void } {
+    const globals = globalThis as unknown as { window?: Window; document?: Document };
+    const previousWindow = globals.window;
+    const previousDocument = globals.document;
+    globals.window = setup.window;
+    globals.document = setup.document;
+    return {
+      teardown: () => {
+        globals.window = previousWindow;
+        globals.document = previousDocument;
+      },
+    };
+  }
+
+  it("creates a hidden __render_frame__ sibling for every video[data-start]", async () => {
+    const { window, document } = parseHTML(
+      `<html><body>
+        <div id="root">
+          <video id="v1" data-start="0" data-duration="2"></video>
+          <video id="v2" data-start="2" data-duration="2"></video>
+        </div>
+      </body></html>`,
+    );
+    const { teardown } = withGlobalDom({ window, document });
+    try {
+      await ensureRenderFrameSiblings(passthroughPage());
+    } finally {
+      teardown();
+    }
+
+    for (const id of ["v1", "v2"]) {
+      const video = document.getElementById(id) as HTMLVideoElement;
+      const sibling = video.nextElementSibling as HTMLElement | null;
+      expect(sibling).not.toBeNull();
+      expect(sibling?.tagName.toLowerCase()).toBe("img");
+      expect(sibling?.classList.contains("__render_frame__")).toBe(true);
+      expect(sibling?.id).toBe(`__render_frame_${id}__`);
+      expect(sibling?.style.visibility).toBe("hidden");
+    }
+  });
+
+  it("skips videos that already have a __render_frame__ sibling", async () => {
+    const { window, document } = parseHTML(
+      `<html><body>
+        <div id="root">
+          <video id="clip" data-start="0" data-duration="2"></video>
+          <img id="__render_frame_clip__" class="__render_frame__">
+        </div>
+      </body></html>`,
+    );
+    const preExisting = document.getElementById("__render_frame_clip__") as HTMLImageElement;
+    const { teardown } = withGlobalDom({ window, document });
+    try {
+      await ensureRenderFrameSiblings(passthroughPage());
+    } finally {
+      teardown();
+    }
+
+    const video = document.getElementById("clip") as HTMLVideoElement;
+    expect(video.nextElementSibling).toBe(preExisting);
+    expect(document.querySelectorAll(".__render_frame__").length).toBe(1);
+  });
+
+  it("does not touch <video> elements without data-start", async () => {
+    const { window, document } = parseHTML(
+      `<html><body><div id="root"><video id="raw"></video></div></body></html>`,
+    );
+    const { teardown } = withGlobalDom({ window, document });
+    try {
+      await ensureRenderFrameSiblings(passthroughPage());
+    } finally {
+      teardown();
+    }
+
+    const video = document.getElementById("raw") as HTMLVideoElement;
+    expect(video.nextElementSibling).toBeNull();
+  });
+});
diff --git a/packages/engine/src/services/screenshotService.ts b/packages/engine/src/services/screenshotService.ts
index ac5199b6e..1662c9246 100644
--- a/packages/engine/src/services/screenshotService.ts
+++ b/packages/engine/src/services/screenshotService.ts
@@ -542,6 +542,46 @@ export async function removeDomLayerMask(page: Page, _extraHideIds: string[]): P
   );
 }
 
+/**
+ * Pre-create hidden `__render_frame__` sibling `<img>`s for every
+ * `video[data-start]` in the page. Idempotent — videos that already
+ * have a sibling are skipped.
+ *
+ * `injectVideoFramesBatch` creates the sibling on the fly the first time
+ * it paints a given videoId (the `isNewImage = !hasImg` branch below).
+ * Under chrome-headless-shell's deterministic + `HeadlessExperimental.
+ * BeginFrame` mode, the immediately-next BeginFrame captures before the
+ * freshly-inserted `<img>` layer lands in the compositor's layer tree;
+ * the layer arrives a frame later. That single frame paints only the
+ * body background + previously-composed overlays.
+ *
+ * Called from `initializeSession`: in the screenshot path at the end (that
+ * capture path flushes paint, so timing doesn't matter), and in the BeginFrame
+ * path followed by one explicit visual `HeadlessExperimental.beginFrame`
+ * (`noDisplayUpdates: false`) that composites the new layers before the first
+ * capture — the warmup ticks are `noDisplayUpdates: true` and don't paint.
+ * Every subsequent `injectVideoFramesBatch` then takes the `hasImg = true` path
+ * (just an `img.src` update). The `isNewImage` branch stays as a fallback for
+ * callers that don't run through `initializeSession`.
+ */
+export async function ensureRenderFrameSiblings(page: Page): Promise<void> {
+  await page.evaluate(() => {
+    for (const video of Array.from(
+      document.querySelectorAll<HTMLVideoElement>("video[data-start]"),
+    )) {
+      const next = video.nextElementSibling;
+      if (next !== null && next.classList.contains("__render_frame__")) continue;
+      const img = document.createElement("img");
+      img.classList.add("__render_frame__");
+      img.id = `__render_frame_${video.id}__`;
+      img.style.pointerEvents = "none";
+      img.style.position = "absolute";
+      img.style.visibility = "hidden";
+      video.parentNode?.insertBefore(img, video.nextSibling);
+    }
+  });
+}
+
 /**
  * Returns the subset of `updates.videoId`s that were actually painted in
  * this call. Videos skipped because of a hidden visual ancestor are NOT