1662336 - Merge print recording targets from cross-origin iframes

Status: RESOLVED FIXED

(Core :: Printing: Output, task, P2)

Description

[Sean Voisen (:svoisen)]

Tracks work needed to merge print recordings of out-of-process iframes in order to support printing in Fission.

Comment 1

[Neha Kochar [:neha]]

Sean said fission printing work will most probably not be ready in time for nightly experiment and we shouldn't block on it. It is expected to be fixed in a couple of weeks following the experiment launch.

Comment 2

[Sean Voisen (:svoisen)]

Per out-of-band conversation, Matt agreed to start looking into this. Thanks Matt!

Comment 3

[Matt Woodrow (:mattwoodrow)]

The rough plan at this point is:

• Add support to the print recorder to handle 'dependent' (OOP) surfaces, and send the final list of dependencies across to the parent process with the recording for the page.
• Refactor CrossProcessPaint (the fission-compatible screenshot recorder) to take a list of dependencies and asynchronously resolve a set of recordings.
• Make the parent process side of printing resolve recordings and merge those into the main recording before forwarding the result to the printer.

The existing printing code uses file handles to share recordings between processes, whereas CrossProcessPaint uses shmem.

The reasoning for this (according to the author, Bob Owen) is that printing frequently encountered OOM crashes, and switched to file handles to avoid that.

I believe it'll be fine to keep using the shmem-based CrossProcessPaint for printing iframes because:

• We're only recording iframes this way (the main page stays using the existing code), so it should be much smaller recordings.
• We now handle failed allocations without crashing, so you'd only lose the iframe contents.
• Many more users have 64bit, and more content processes.

It's more likely that other consumers of CrossProcessPaint will have OOM issues (when trying to screenshot a large document), so if we ever do run into issues we can convert that code to file handles and benefit with all consumers.

I have a rough version of code that implements this, just waiting on static cloning to be complete before I can test and finish.

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Add API to InlineTranslator to allow allocation of backing buffers for dependent surfaces during playback. r?lsalzman

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Refactor CrossProcessPaint internals to return a MozPromise of recordings. r?emilio

This moves the code for converting the set of recordings into a single bitmap into the static Start function, and will allow for other consumers to skip this.

Depends on D90802

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Add CrossProcessPaint API to just resolve the set of recordings without rasterizing to a bitmap. r?emilio

Depends on D90803

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Implement dependent surface support in the print recording code. r?bobowen

Depends on D90804

Comment 8

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Make PRemotePrintJob refcounted. r?bobowen

Depends on D90805

Comment 9

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Pass print recording dependency list across to parent process and use CrossProcessPaint to resolve it. r?bobowen

Depends on D90806

Comment 10

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Use static clone from print preview when printing. r?emilio

Depends on D90807

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Allow dependent surface drawing to be fallible. r?lsalzman

Dependent surfaces are used for recording OOP iframes, and these can go away at any time (if they navigate and switch process while we're trying to capture them).
Making these draws fallible means we can still screenshot/print the outer document (and sibling OOP iframes), rather than failing the entire operation.

Depends on D90808

Comment 12

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1662336 - Add test for printing with a cross-origin iframe. r?emilio

Depends on D90809

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e5f3b35d55d
Add API to InlineTranslator to allow allocation of backing buffers for dependent surfaces during playback. r=lsalzman,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/ec3503bf866f
Refactor CrossProcessPaint internals to return a MozPromise of recordings. r=emilio
https://hg.mozilla.org/integration/autoland/rev/4f1bb0fc80a4
Add CrossProcessPaint API to just resolve the set of recordings without rasterizing to a bitmap. r=emilio
https://hg.mozilla.org/integration/autoland/rev/9348d2cc590a
Implement dependent surface support in the print recording code. r=bobowen
https://hg.mozilla.org/integration/autoland/rev/f165b0ca5bc1
Make PRemotePrintJob refcounted. r=bobowen
https://hg.mozilla.org/integration/autoland/rev/4c8635b0e73d
Pass print recording dependency list across to parent process and use CrossProcessPaint to resolve it. r=bobowen
https://hg.mozilla.org/integration/autoland/rev/f7fdeb88c5d6
Use static clone from print preview when printing. r=emilio
https://hg.mozilla.org/integration/autoland/rev/402741812e28
Allow dependent surface drawing to be fallible. r=lsalzman,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/478eb5e81118
Add test for printing with a cross-origin iframe. r=emilio

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/26056 for changes under testing/web-platform/tests

Comment 16

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1e5f3b35d55d
https://hg.mozilla.org/mozilla-central/rev/ec3503bf866f
https://hg.mozilla.org/mozilla-central/rev/4f1bb0fc80a4
https://hg.mozilla.org/mozilla-central/rev/9348d2cc590a
https://hg.mozilla.org/mozilla-central/rev/f165b0ca5bc1
https://hg.mozilla.org/mozilla-central/rev/4c8635b0e73d
https://hg.mozilla.org/mozilla-central/rev/f7fdeb88c5d6
https://hg.mozilla.org/mozilla-central/rev/402741812e28
https://hg.mozilla.org/mozilla-central/rev/478eb5e81118

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot