Skip to content

Schedule render animation frames on the canvas's own window - #21825

Open
jsg2021 wants to merge 1 commit into
mozilla:masterfrom
jsg2021:render-task-owner-window
Open

Schedule render animation frames on the canvas's own window#21825
jsg2021 wants to merge 1 commit into
mozilla:masterfrom
jsg2021:render-task-owner-window

Conversation

@jsg2021

@jsg2021 jsg2021 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

InternalRenderTask schedules each render continuation with the global
window.requestAnimationFrame and cancels with window.cancelAnimationFrame.
When the canvas belongs to a different window than the one running the render,
those calls go to the wrong window.

The case where this matters: a page opened with window.open whose content is
rendered by the opener. The canvas lives in the popup's document, but
_scheduleNext schedules on the opener. If the opener is not visible — a
full-screen popup covering it is enough — the browser pauses its animation
frames, the continuation never runs, and the render stalls partway through. It is
eventually cancelled, and cancel() then calls cancelAnimationFrame on that
same wrong window, so the handle is never actually released.

From the outside this looks like a blank or half-drawn canvas with no error:
RenderingCancelledException is what callers get, which is normally the benign
result of paging or zooming, so it tends to be ignored. Making the opener visible
again (switching windows) lets a later render complete, which makes it look
intermittent.

Browsers differ in how aggressively they throttle a non-visible window's
requestAnimationFrame, so this reproduces in Chrome, Edge and Safari but not in
Firefox.

This patch resolves the window from the canvas's own document, falling back to
the global window when there is no canvas (the canvasContext-only path) or no
owner document:

get #ownerWindow() {
  return (
    (this._canvas ?? this._canvasContext?.canvas)?.ownerDocument
      ?.defaultView ?? window
  );
}

Behaviour is unchanged for the single-window case, which is why the viewer itself
never hit this — it renders into the document that owns the canvas.

Not PDF-specific, so there is no test PDF to attach: it reproduces with any
document rendered into a window.open'd page from the opener, with the opener
occluded. Happy to add a test if you can point me at the right harness for a
second window — test/integration/ drives a single page, and the unit tests do
not have a second document to render into.

npx eslint src/display/api.js and npx prettier --check src/display/api.js are
clean. I have not been able to finish npx gulp unittestcli locally -- it stalls
fetching the test corpus from web.archive.org on my network -- so I am relying on
CI for the unit suite.

InternalRenderTask scheduled and cancelled its render continuations with the
global window.requestAnimationFrame. When the canvas belongs to another
document -- one opened with window.open and rendered into from the opener --
that is the wrong window: browsers pause requestAnimationFrame for a window
that is not visible, so an occluded opener never runs the callback and the
render stalls partway through, leaving a partially drawn canvas and a
RenderingCancelledException that reads as benign.

Resolve the window from the canvas's own document instead, falling back to the
global one when there is no canvas or no owner document.
@codecov-commenter

codecov-commenter commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.21%. Comparing base (a570239) to head (307c449).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #21825   +/-   ##
=======================================
  Coverage   90.20%   90.21%           
=======================================
  Files         264      264           
  Lines       67322    67323    +1     
=======================================
+ Hits        60729    60736    +7     
+ Misses       6593     6587    -6     
Flag Coverage Δ
browsertest 66.34% <66.66%> (+0.02%) ⬆️
fonttest 8.91% <ø> (ø)
integrationtest 69.41% <100.00%> (+<0.01%) ⬆️
unittest 58.37% <66.66%> (-0.01%) ⬇️
unittestcli 56.93% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Snuffleupagus

Snuffleupagus commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

This looks like a duplicate of #14846, but with a lot of AI generated words.

@jsg2021

jsg2021 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

This is more complete than that one. but yes. it's the same effective change. I have need to open a new window with a custom pdf viewer using pdf.js. this window is used as a portal target. The owner (correctly) idles when it is occluded. But, my portal stops redrawing.

This change just allows the pdf renderer to schedule redraws on the windows that is not throttled and is actively being used.

And, yes, it was generated with Claude Code. I verified it before opening the PR, so it's not just ai-slop.

I'm using a local patch (bun patch) in my app for now and it's working well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants