Thanks for filing this. I will take a look. My first guess would be post messaging issues.

This works without --isolate-extensions, right? Can we come up with a test extension, so we can go straight to step 8?

This is happening for me on 56.0.02924.0 without --site-per-process or --isolate-extensions.

I did a bisect and the CL is within the range 432298 (known good) - 432374 (known bad). I will try to build locally and find the potential culprit.

https://chromium.googlesource.com/chromium/src/+log/7f225c70775269836739cbd735f7d84919b505d2..e75795096a81f406c92afce687ef4aa54900845f

Thanks for looking into this, Ehsan.  I'll assign to you for now to finish the bisect.

Here are my findings (running without --isolate-extensions).

The regression apparently first shows up at #432353 which lands OOPIF printing
(CL: https://codereview.chromium.org/2426503002)

The change is then reverted at #432507. But when I tried to print on that revision renderer crashed:
Received signal 11 SEGV_MAPERR 000000000008
#0 0x7ffb70065587 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#1 0x7ffb6db95330 <unknown>
#2 0x7ffb7057e894 SkCanvas::getDevice()
#3 0x7ffb7073f556 skia::GetMetaData()
#4 0x7ffb7335e76a printing::MetafileSkiaWrapper::GetMetafileFromCanvas()
#5 0x7ffb72b1c351 content::PepperPluginInstanceImpl::PrintPDFOutput()
#6 0x7ffb72b1c400 content::PepperPluginInstanceImpl::PrintEnd()
#7 0x7ffb71d45fe1 blink::WebLocalFrameImpl::printEnd()
#8 0x7ffb72c32d01 printing::PrepareFrameAndViewForPrint::FinishPrinting()
#9 0x7ffb72c36d22 printing::PrintWebViewHelper::CreatePreviewDocument()
#10 0x7ffb72c3665b printing::PrintWebViewHelper::OnFramePreparedForPreviewDocume
..

The same crash exists on #432765 (assuming it was never OK in between).

At #432775 OOPIF printing relands (https://codereview.chromium.org/2508923003) which takes us to now as I understand.

The SkCanvas bug is  bug 666154 . There's a couple possible workarounds on the bug, but they both require patching your local build.

Here is what I found so far:
Ctrl+P and pressing the extension button have two different codepaths to print preview.

With Ctrl+P I end up with the renderer crash in  bug 666154 . I assume fixing that should fix Ctrl+P printing.

However, there seems to be another issue with the window closing so early (this bug here). I noticed here:
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/print_preview/print_preview_handler.cc?rcl=1479284732&l=703

When the call is from inside the PDF extension by pressing the button, the initiator is the WebContentsImpl associated with MimeHandlerViewGuest which will lead to a null RenderFrameHost and the window exits early.

I also noticed that pressing Ctrl+P will call PrintViewManager::PrintPreviewNow() to setup the |print_preview_rfh_| properly. This does not happen when pressing the print button.

What the the print button do exactly? Does it call window.print() ? If so, that is indeed a completely different renderer initiated code path.

I don't know yet and I guess I have to investigate the PDF extension for it.

Calling window.print() will lead to a call to:
PrintViewManager::OnShowScriptedPrintPreview()
which is fine as in it sets the |print_preview_rfh_| (and then of course  bug 666154 ).

I submitted a CL which seemingly fixes the issue. But probably not the right way (or maybe breaks something else). 

The issue is that PrintHostMsg_RequestPrintPreview is not handled properly as in |print_preview_rfh_| is not set properly. This is not a GuestView bug as far as I understand.

Link to CL just in case someone wants to take a look: https://codereview.chromium.org/2517493002/.

So can this be reduced down to an out of process iframe that loads a PDF, or something like that? If I can play around with it and repro, I may be able to give you better suggestions on the CL.

Thanks. For me this issue occurs even when the top-level/tab is loading a PDF so it does not have to be an OOPIF.

Updating the title since the issue here only happens through pressing the print button on extension and also is reproducible on non-OOPIF pages.

I rebased the CL above to ToT today and everything seems to be working as normal.

I tried 1) the steps above with --site-per-process, 2) navigate tab to a PDF URL and print, and 3) navigate OOPIF to PDF URL which embeds a PDF and print.

Printing here is by pressing the print button as per comment #1 instructions.

Does this mean that the issue is fixed? If so, can we close it?

No. I meant the CL I have apparently fixes the issue. I need thestig@ or someone more familiar with PrintPreview to review it.

 Issue 666675  has been merged into this issue.

The sample.pdf attached to bug 539354 may be the simplest repro case. I'll poke at it.

Thanks thestig@. Just for FYI, for the PDF extension here, pressing print button leads to a post message to the plugin to print. This will call PepperPDFHost::OnHostMsgPrint which eventually ends up with PrintWebViewHelper::PrintNode.

Assigning the bug to thestig@ who has a patch which fixes both this and the sheets/  bug 666675 .

Both the Flash and PDF plugins have Print() PPAPI calls they can invoke. They both end up in PepperPDFHost::OnHostMsgPrint() eventually.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4665756d5cdece234b38a2c722f2489a5c53a647

commit 4665756d5cdece234b38a2c722f2489a5c53a647
Author: thestig <thestig@chromium.org>
Date: Mon Nov 21 21:35:22 2016

Make printing work for PDF and Flash.

BUG= 666432 

Review-Url: https://codereview.chromium.org/2518993002
Cr-Commit-Position: refs/heads/master@{#433657}

[modify] https://crrev.com/4665756d5cdece234b38a2c722f2489a5c53a647/chrome/browser/printing/print_preview_message_handler.cc
[modify] https://crrev.com/4665756d5cdece234b38a2c722f2489a5c53a647/chrome/browser/printing/print_preview_message_handler.h
[modify] https://crrev.com/4665756d5cdece234b38a2c722f2489a5c53a647/chrome/browser/printing/print_view_manager.cc
[modify] https://crrev.com/4665756d5cdece234b38a2c722f2489a5c53a647/chrome/browser/printing/print_view_manager.h

Let's verify it's fixed in Canary tomorrow and then request the M56 merge.

Latest canary with the fix will not be available to due to Issue 667625 for verification.

Issue is still reproducible on today's Dev release candidate(56.0.2924.3) on Mac OS 10.11.6 and Linux Ubuntu 14.04.(Windows PGO build still building for 56.0.2924.3).

We are unable to print as print preview dialog closes abruptly on pdfs/sheets( Issue 666675 ). La-belling this issue accordingly as this Blocks the first Dev branch release.

You will probably need to wait for a revision after #433657 which is the CL above. Current Canary as I see is at #433437.

The latest canary is still building- Chrome Version: 57.0.2928.0. I can verify the fix once its available.

Wondering whether this is really a Dev blocker since the issue happens while launching chrome with command line flag --isolate-extensions. Lei, please suggest.

Isolate Extensions is enabled at 90% on Dev and Canary channels through a field trial, so it is very close to being the default configuration for those channels. 

56.0.2924.3 is the first Dev build after M56 branching,we need to get the Dev out today to gather information about overall stability of the branch.

Will it be possible to disable the filed trial on Dev? We can take the merge before the next Dev release which is scheduled on 11/29.

Yeah, bustamante@ and I had a chat and I'll disable the trial on Dev until after the next Dev release.

https://critique.corp.google.com/#review/139969584 disables Isolate Extensions in Dev- 56.0.2924.3

The 57.0.2928.0 build should exist at this point for testing.

Fix is working fine on Windows-10 & Linux Ubuntu 14.04 on chrome version: 57.0.2928.0(Mac canary version: 57.0.2928.0 is not available due to Issue 667830 for verification).


However I am still able to repro the issue the latest Dev(56.0.2924.3) of Windows-10, Mac OS 10.11.6 and Linux Ubuntu 14.04. Looks like the disable of this field trial has not reached to the Dev users yet. 

Your change meets the bar and is auto-approved for M56 (branch: 2924)

The trial should be off for Dev channel. If you are still hitting issues, can you share the state of your field trials (variations under chrome://version)? 

Just as FYI, this original issue was not --isolate-extensions related AFAIK. It was reproducible without the flag.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1780aa52d15b9a07d47d1a35763bc5f5d2233a7

commit b1780aa52d15b9a07d47d1a35763bc5f5d2233a7
Author: Lei Zhang <thestig@chromium.org>
Date: Wed Nov 23 19:26:09 2016

M56: Make printing work for PDF and Flash.

BUG= 666432 

Review-Url: https://codereview.chromium.org/2518993002
Cr-Commit-Position: refs/heads/master@{#433657}
(cherry picked from commit 4665756d5cdece234b38a2c722f2489a5c53a647)

Review URL: https://codereview.chromium.org/2523283002 .

Cr-Commit-Position: refs/branch-heads/2924@{#83}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/b1780aa52d15b9a07d47d1a35763bc5f5d2233a7/chrome/browser/printing/print_preview_message_handler.cc
[modify] https://crrev.com/b1780aa52d15b9a07d47d1a35763bc5f5d2233a7/chrome/browser/printing/print_preview_message_handler.h
[modify] https://crrev.com/b1780aa52d15b9a07d47d1a35763bc5f5d2233a7/chrome/browser/printing/print_view_manager.cc
[modify] https://crrev.com/b1780aa52d15b9a07d47d1a35763bc5f5d2233a7/chrome/browser/printing/print_view_manager.h

Comment 40: Yikes, thanks for pointing that out!  Seems we overlooked that in comments 30-34, and that explains why the bug still repro'd in comment 36.

TPMs: Ehsan points out that this bug is present in 56.0.2924.3, even with the --isolate-extensions trial disabled.  Is this something we're ok to live with until the next Dev channel push?

Also, since the Finch trial doesn't seem to affect the bug, is it ok to re-enable it on Dev?

Yeah that's unfortunate :( per discussion offline we verified that this can be reproduced with the isolate extensions experiment disabled.  Re-enabling the Finch trial sounds fine.

Verified the merge on the latest M-56(56.0.2924.5) on Windows-10, Mac OS 10.11.6 and Linux Ubuntu 14.04. This is working as intended.

Adding the verified label therefore.

 Issue 669099  has been merged into this issue.