OOPIFs don't print with site isolation on Android |
||||||||||||
Issue descriptionRepro steps: on an Android device: 1. Enable strict site isolation via chrome://flags/#enable-site-per-process 2. Go to http://csreis.github.io/tests/cross-site-iframe.html 3. Click one of the two "Go cross-site" buttons. 3. Try to print the page. Expected: The print preview, as well as the output, includes the contents of the subframe. This is the case without strict site isolation. Actual: The subframe is blank. Tested this on Android Canary 71.0.3563.0 on a Pixel C. It seems like kUsePdfCompositorServiceForPrint feature is enabled by default on all platforms, but I don't know if additional work is required to get the print compositor working on Android. I also noticed the "use-pdf-compositor-service-for-print" flag is desktop-only. weili@ and thestig@, can you please help us understand what's involved in fixing this? This would be important to support as we're planning to ship some form of site isolation on Android.
,
Sep 28
fgorski@ - is there an existing way to print an "offline" page (see #c1)? If not, then we probably should open a separate bug for this, right?
,
Sep 28
I'd start from asking dimich/jianli about comment #2
,
Sep 28
We set intent type to "multipart/related" when sharing an offline page. Google Cloud Print needs the intent type to be "text/plain" or "text/html". If you disable offline page sharing, you can see the print option.
,
Sep 28
#4: Thanks. I can confirm that after disabling chrome://flags/#offline-pages-sharing the Print option shows up, and so the printing workaround in #1 works.
,
Oct 1
Android does not follow the same path as desktop platforms. See components/printing/renderer/print_render_frame_helper_linux.cc. Right now Android grabs the PDF from the renderer directly. It probably needs to instead send the metadata to the PDF compositor, like desktop platforms, and save the resulting PDF.
,
Oct 18
weili@ or thestig@ - given that you're already very familiar with the desktop OOPIF printing paths, would either one of you have time to work on this in the next couple of months? I imagine we'll want to support this eventually for launching site isolation on Android. We have a couple of Android site isolation trials on canary and dev, and I'll try to add metrics to see how frequently pages with OOPIFs are printed there to help judge the priority of this.
,
Oct 18
alexmos@, Android WebView is using the same code path, so you probably want to distinguish the usage from WebView and Chrome to have better understanding. Looks like we should be careful about not breaking WebView printing for the isolation effort, since we have almost no Chromium printing test for WebView. We only have CTS tests currently.
,
Oct 18
There is a way to fix the issue that offline page can't be printed. We can add the following line to PrintShareActivity section in AndroidManifest.xml: <data android:mimeType="multipart/related" /> If we want this to be fixed, please file a new bug since offline page printing issue seems to be different what is reported as in #1.
,
Oct 18
Oh, that's interesting about Android WebView. I was going to just log something platform-independent in PrintRenderFrameHelper::PrintPages(), if the printed frame has any RemoteFrame descendants, and then look at the stats for Android on the UMA dashboard. Do you know if there'll be a way to break down webview vs Chrome cases there, or if not, do you have any advice on how to distinguish the two?
,
Oct 18
Comment 9: sure, seems like that's worth fixing, and it will simplify the workaround for this issue. I've just filed issue 896860 for it.
,
Oct 18
Re #10, sorry I wasn't clear about UMA, so I just asked around, UMA will handle platform info by itself, no need to do anything specific in code.
,
Oct 18
Checking some other metrics, I don't actually see Android Webview separated out under Platforms on the dashboard, but maybe there's some other way to distinguish. +boliu@ - do you know? One thing we'll need to do differently for it though is that we can't just look at remote frames, as Android Webview doesn't have any yet. We can still estimate things by looking at number of cross-site subframes for strict site isolation, though we wouldn't be able to easily estimate things for subset-of-sites policies we're considering. Probably good enough to start with.
,
Oct 18
Webview UMA data ends up in an entirely different dashboard, separate from chrome.
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f567d06d23644bf3960c78083f46780b290944e commit 6f567d06d23644bf3960c78083f46780b290944e Author: Alex Moshchuk <alexmos@chromium.org> Date: Thu Oct 25 21:20:01 2018 Add site isolation printing metrics. This CL adds some site isolation printing metrics, with goal of measuring two things for printing on Android and Android Webview: 1. How often printed frames contain OOPIFs. Combined with our ongoing public site isolation Android trials, this will let us estimate how often users with site isolation will be affected by not being able to print OOPIFs. 2. How often printed frames contain cross-site descendant frames. This can be checked on Android Webview, which doesn't support OOPIFs yet, to estimate how often we'd need OOPIF printing support once we start supporting site isolation there. We also record how many of those frames are visible. Bug: 890417 Change-Id: I5ba543587c398c304a79e8fe071d72522e7795ab Reviewed-on: https://chromium-review.googlesource.com/c/1292835 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#602857} [modify] https://crrev.com/6f567d06d23644bf3960c78083f46780b290944e/components/printing/renderer/print_render_frame_helper.cc [modify] https://crrev.com/6f567d06d23644bf3960c78083f46780b290944e/tools/metrics/histograms/histograms.xml
,
Nov 2
Some early data for PrintPreview.SiteIsolation.RemoteFrameCount from ~5 days on Android canary: of the 471 attempts to print, 53 (11%) had remote subframes. Although that's a fairly small percentage, it does show that people will print things that have OOPIFs on Android, so we'll want to fix this.
,
Nov 2
thestig@ will try to carve out some time to take a look at this in the M73 timeframe. Thanks Lei!
,
Nov 2
,
Dec 4
thestig@: now that we're in M73, just a gentle reminder to see if you can start looking at this some time soon. I think we'll need this for shipping site isolation on Android, and it's one of the more potentially non-trivial blockers we have.
,
Jan 8
thestig@: any updates on this, since we're getting close to M73 branch cut? BTW, we have Beta data for metrics from #c15. Although there aren't SI trials there, roughly 17% of print attempts have cross-site visible iframes. That's a large number, though it'll overcount the actual number of OOPIFs, since we won't isolate everything on Android. For canary/dev, we're seeing at least one OOPIF in 13%+ of print attempts in the full --site-per-process trial, and in 10%+ of print attempts in our trial for isolating a smaller set of sites where users enter passwords. 10% is probably the most representative of these, and it's still a pretty significant number. Adding a label to track that we'll want to fix this before shipping site isolation on Android.
,
Jan 9
Haven't had time to work on it. One piece of good news is most of the PDF compositor code is already building on Android. So at least we won't have to do any additional work on that.
,
Jan 16
I worked on this today and got something that's not 100% right, but works for chrome_public_apk. It mainly involved removing Android-specific IPCs that work with file descriptors (PrintHostMsg_AllocateTempFileForPrinting, PrintHostMsg_TempFileForPrintingWritten) and making Chrome Android printing work more like desktop printing. I knew system_webview_apk is broken so I looked into fixing that next. However, AwPdfExporter::ExportToPdf() builds on top of the existing code, which works with file descriptors. So I got to find a way to accommodate this use case.
,
Jan 16
The mental model I have for WebView's AwPdfExporter is that it's more or less similar to Chrome for Android's browser side initiated printing task. If removing these file descriptor related IPCs works for Clank, there should be no big difference for WebView I think?
,
Jan 16
#c22: great to hear about the progress, thank you! Just noting that site isolation on Android Webview doesn't exist yet -- that's something we'll need to figure out this quarter, but right now there isn't a way to even get OOPIFs there, I think. So right now the regular Android case is more important, but of course if whatever we build works for AW too, that's even better.
,
Jan 16
ctzsm: With my local changes, I've encapsulated the file descriptor inside PrintingContextAndroid and stopped exposing it to PrintViewManager. AwPdfExporter still expects AwPrintManager to take the file descriptor. Maybe I'll undo that for now to keep the two PrintManager classes from diverging too much.
,
Jan 16
(6 days ago)
,
Jan 18
(5 days ago)
Fixing bug 922657 is actually the hard part. Fixing this isn't actually that bad. https://chromium-review.googlesource.com/1420302 |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by alex...@chromium.org
, Sep 28