New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 890417 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 922657



Sign in to add a comment

OOPIFs don't print with site isolation on Android

Project Member Reported by alex...@chromium.org, Sep 28

Issue description

Repro 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.
 
I tried to see if there's a similar workaround of saving the page that we had on desktop.  I downloaded the page with an OOPIF and then viewed the page, which appears with the "offline" chip in the infobar.  Content from the OOPIF appears just fine there, but I can't find the print option in offline mode.  For a regular page it's part of the "Share..." menu item, but with an offline page it's not there, with or without strict site isolation.
Cc: fgor...@chromium.org
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?
Cc: -fgor...@chromium.org dim...@chromium.org jianli@chromium.org
I'd start from asking dimich/jianli about comment #2
Cc: petewil@chromium.org
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.
#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.
Cc: ctzsm@chromium.org
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.
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.
Cc: changwan@chromium.org
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.
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.
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?
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.
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.
Cc: boliu@chromium.org
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.
Webview UMA data ends up in an entirely different dashboard, separate from chrome.
Project Member

Comment 15 by bugdroid1@chromium.org, 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

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.
Cc: -thestig@chromium.org
Owner: thestig@chromium.org
thestig@ will try to carve out some time to take a look at this in the M73 timeframe.  Thanks Lei!
Status: Assigned (was: Available)
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.
Labels: Proj-SiteIsolationAndroid-BlockingLaunch
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.
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.
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.
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?
#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.
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.

Comment 26 by thestig@chromium.org, Jan 16 (6 days ago)

Blockedon: 922657

Comment 27 by thestig@chromium.org, Jan 18 (5 days ago)

Status: Started (was: Assigned)
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