Merge print recording targets from cross-origin iframes
Categories
(Core :: Printing: Output, task, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox83 | --- | fixed |
People
(Reporter: svoisen, Assigned: mattwoodrow)
References
Details
(Whiteboard: [print2020_v83], [wptsync upstream])
Crash Data
Attachments
(9 files)
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details | |
|
(deleted),
text/x-phabricator-request
|
Details |
Tracks work needed to merge print recordings of out-of-process iframes in order to support printing in Fission.
|
Reporter | |
Updated•4 years ago
|
|
||
Comment 1•4 years ago
|
||
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.
|
|
Reporter | |
Comment 2•4 years ago
|
||
Per out-of-band conversation, Matt agreed to start looking into this. Thanks Matt!
|
Assignee | |
Comment 3•4 years ago
|
||
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.
|
|
Assignee | |
Comment 4•4 years ago
|
||
|
|
Assignee | |
Comment 5•4 years ago
|
||
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
|
|
Assignee | |
Comment 6•4 years ago
|
||
Depends on D90803
|
|
Assignee | |
Comment 7•4 years ago
|
||
Depends on D90804
|
|
Assignee | |
Comment 8•4 years ago
|
||
Depends on D90805
|
|
Assignee | |
Comment 9•4 years ago
|
||
Depends on D90806
|
|
Assignee | |
Comment 10•4 years ago
|
||
Depends on D90807
|
|
Assignee | |
Comment 11•4 years ago
|
||
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
|
|
Assignee | |
Comment 12•4 years ago
|
||
Depends on D90809
|
||
Updated•4 years ago
|
|
||
Comment 14•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: 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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/26056 for changes under testing/web-platform/tests
|
||
Comment 16•4 years ago
|
||
| bugherder | ||
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
|
||
Updated•4 years ago
|
Upstream PR merged by moz-wptsync-bot
|
||
Updated•3 years ago
|
Description
•