After bug 1258609, the PDF viewer stopped producing high quality output during printing. STR: 1. Open the attached test case 2. Print it to a PDF file 3. Inspect PDF output Actual result: The "Test" text at "... loading print" sections is rasterized Expected result: The "Test" text at "... loading print" sections must be vectorized and selectable. As result any document printed by PDF Viewer with low quality. 20:08.11 LOG: MainThread Bisector INFO Last good revision: c9ff56dbb6fcdb7ce0573b93c520496a8e21f250 20:08.11 LOG: MainThread Bisector INFO First bad revision: c992422247b7b33fa4a89f891d08bfe792fc7d07 20:08.11 LOG: MainThread Bisector INFO Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c9ff56dbb6fcdb7ce0573b93c520496a8e21f250&tochange=c992422247b7b33fa4a89f891d08bfe792fc7d07

This sounds Really Not Good.

Before the fix in bug 1258609, printing via the parent was always resulting in rasterisation because canvas printing wasn't using the correct DrawTarget. From what I remember the only reason we weren't always getting rasterisation issues before that in non-e10s, was because of some magic in cairo's own recording surface that meant that snapshots were still getting played back as the original draw commands. I suspect what is happening here is that there are some other cases where canvases are using a DrawTarget/surface that hasn't been created though the printing one. Although I don't understand why that magic isn't working for 49, when we're not printing via the parent. I'll see if I can find the problem.

Is this a Mac only issue? On Windows the selectable text seems to be the same for me on Nightly and version 45.0.2, which is before any of my printing changes for sandboxing. Even on 45.0.2 some parts aren't selectable, because of the issues I described in comment 2. Are none of the "Test" text sections selectable on Mac?

I noticed that on my Mac. AFIAK we had issues before on some platforms/configurations -- I'm not aware on which platformsand when it was broken or non-implemented.

OK, hopefully I've found the issue. DrawTargetCairo::CreateSimilarDrawTarget has some specific code for WIN32 surfaces, but not QUARTZ. Whereas the override that was used before gfxQuartzSurface::CreateSimilarSurface obviously did (because that's why it exists :-) ). Here a try push with code added to DrawTargetCairo::CreateSimilarDrawTarget for QUARTZ, which will hopefully fix the regression: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00f53ec1258dba07e0adca9149320154d141be3e Yury - can you try this build, to see if it fixes the issue please, I don't have a Mac to test.

(In reply to Bob Owen (:bobowen) from comment #5) > Here a try push with code added to DrawTargetCairo::CreateSimilarDrawTarget > for QUARTZ, which will hopefully fix the regression: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=00f53ec1258dba07e0adca9149320154d141be3e > > Yury - can you try this build, to see if it fixes the issue please, I don't > have a Mac to test. It appears to be working in non-e10s mode; and no changes in e10s mode -- rasterized output.

(In reply to Yury Delendik (:yury) from comment #6) > (In reply to Bob Owen (:bobowen) from comment #5) > > Here a try push with code added to DrawTargetCairo::CreateSimilarDrawTarget > > for QUARTZ, which will hopefully fix the regression: > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=00f53ec1258dba07e0adca9149320154d141be3e > > > > Yury - can you try this build, to see if it fixes the issue please, I don't > > have a Mac to test. > > It appears to be working in non-e10s mode; and no changes in e10s mode -- > rasterized output. Thanks, would you mind trying with e10s enabled, but with print.print_via_parent=false (it should be true at the moment). You might need to have a lower setting for the sandbox to print to PDF like this (security.sandbox.content.level=1 or 0 which turns it off)).

(In reply to Bob Owen (:bobowen) from comment #7) > Thanks, would you mind trying with e10s enabled, but with > print.print_via_parent=false (it should be true at the moment). > You might need to have a lower setting for the sandbox to print to PDF like > this (security.sandbox.content.level=1 or 0 which turns it off)). Works on Mac as expected on e10s with print.print_via_parent=false and security.sandbox.content.level=0 (doesn't work with 1 or 2).

(In reply to Yury Delendik (:yury) from comment #8) > (In reply to Bob Owen (:bobowen) from comment #7) > > Thanks, would you mind trying with e10s enabled, but with > > print.print_via_parent=false (it should be true at the moment). > > You might need to have a lower setting for the sandbox to print to PDF like > > this (security.sandbox.content.level=1 or 0 which turns it off)). > > Works on Mac as expected on e10s with print.print_via_parent=false and > security.sandbox.content.level=0 (doesn't work with 1 or 2). Thanks Yury. So, it looks like this could be an acceptable fix for release. I'll file a separate bug for printing via the parent.

I also noticed that we weren't destroying the surface in the error case. MozReview-Commit-ID: F5fMfRBiOW7

Can you add a test case?

Pushed by bobowencode@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4bb87ce613c3 Add quartz surface specific code into DrawTargetCairo::CreateSimilarDrawTarget. r=jrmuizel

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]: Per bug 1310165 comment 4, we may want to uplift this fix to beta and aurora.

Hello Yury, could you please verify this issue is fixed as expected on a latest Nightly build (10-18)? Thanks!

Hello Bob, should we uplift this fix to Beta50, Aurora51?

(In reply to Ritu Kothari (:ritu) from comment #17) > Hello Bob, should we uplift this fix to Beta50, Aurora51? Yes, I was just waiting for at least one day on Nightly in case there was immediate non-printing related fallout.

(In reply to Ritu Kothari (:ritu) from comment #16) > Hello Yury, could you please verify this issue is fixed as expected on a > latest Nightly build (10-18)? Thanks! Yes, it works for me.

Comment on attachment 8801063 [details] [diff] [review] Add quartz surface specific code into DrawTargetCairo::CreateSimilarDrawTarget jrmuizel - do you think that this change is OK for uplift? (I'm not sure where DrawTargetCairo::CreateSimilarDrawTarget might be used outside of printing). Approval Request Comment [Feature/regressing bug #]: Regressed by bug 1258609. [User impact if declined]: On Mac, PDF text will continue to be rasterised when it shouldn't be. [Describe test coverage new/current, TreeHerder]: Manual testing for the issue by reporter. Automated testing, for other things on Mac using DrawTargetCairo::CreateSimilarDrawTarget. [Risks and why]: Medium - the change is to a function that could be used for things other than printing. Setting needinfo to jrmuizel to confirm the risk here. [String/UUID change made/needed]: None

Comment on attachment 8801063 [details] [diff] [review] Add quartz surface specific code into DrawTargetCairo::CreateSimilarDrawTarget Jeff said the risk is manageable here and it might be ok to uplift to Beta. Let's stabilize this on Aurora51 before taking this into Beta10 on Monday.

(In reply to Ritu Kothari (:ritu) from comment #21) > Comment on attachment 8801063 [details] [diff] [review] > Add quartz surface specific code into > DrawTargetCairo::CreateSimilarDrawTarget > > Jeff said the risk is manageable here and it might be ok to uplift to Beta. > Let's stabilize this on Aurora51 before taking this into Beta10 on Monday. Any breakage from this patch is likely going to be rare/hard to find so it's probably worth getting this in as many trees as early as possible.

Comment on attachment 8801063 [details] [diff] [review] Add quartz surface specific code into DrawTargetCairo::CreateSimilarDrawTarget This seems like a core scenario, has stabilize on Nightly52 and Aurora51 for a few days now, let's uplift to Beta50.