1167600 - [e10s] Cannot print preview "about:" pages in e10s windows

Status: RESOLVED FIXED

(Core :: Print Preview, defect)

Description

[dw-dev]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150513174244

Steps to reproduce:

1. Start Nightly 41.0a1.
2. Open new tab and load "about:credits" page.
3. Select File > Print Preview.
4. The Print Preview "Preparing..." popup is displayed.
5. The "about:credits" page is not print previewed.

In fact, only the "about:home" and "about:plugins" can be printed previewed.  None of the other "about:" pages can be print previewed.  In more detail ....

There are currently 35 "about:" pages. Of these, 8 are XUL pages, 21 are XML pages, 5 are HTML pages, and 1 is a PNG image.

The print preview engine cannot handle XUL pages, and presumably this would be difficult to fix.

Of the remaining 27 XML/HTML/PNG pages, 25 are loaded in local browsers (chrome process) and 2 are loaded in remote browsers (content process).

The 2 pages loaded in remote browsers ("about:home" and "about:plugins") can be print previewed.

The 25 pages loaded in local browsers cannot be print previewed.

What is stopping pages loaded in local browsers being print previewed?

Is it because the browsers were originally remote and then switched to local when the "About:" pages were loaded?




Actual results:

The "about:credits" page is not print previewed.


Expected results:

The "about:credits" page should be print previewed.

Comment 1

[Gingerbread Man]

(In reply to dw-dev from comment #0)
> The "about:credits" page is not print previewed.

Browser Console shows
NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMessageSender.sendAsyncMessage] printUtils.js:413:0

No problem printing.

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150522030225

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Mike, is this something we need to track?

Comment 3

[Mike Conley (:mconley) (:⚙️)]

The problem is that the about: pages that dw-dev have identified are all blacklisted from running remotely.

The print preview browser that opens doesn't know this, and opens remote by default. Attempting to pass the non-remote contentWindow to the newly created remote print preview browser causes the content script to receive the contentWindow as a CPOW. This causes an exception to be thrown.

I know that we don't really want to expend too much effort supporting non-remote, blacklisted tabs in e10s windows. We will, eventually, need to have print preview open a browser in the correct content process when we start supporting dom.ipc.processCount > 1.

I think this is pretty easy to fix. I suggest M8.

Comment 4

[Chirag Bhatia]

Hi Mike,

I can work on this. Can you please point me in the right direction as to where the code to fix this is?

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Hey Chirag!

(In reply to Chirag Bhatia from comment #4)
> Hi Mike,
> 
> I can work on this. Can you please point me in the right direction as to
> where the code to fix this is?

Sure thing - and thanks for picking this up.

The place I would look is here:

https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#3149

The getPrintPreviewBrowser method is assuming that the remoteness of the browser that is being opened is going to match the remoteness of the currently selected tab browser.

What we want to do here is:

* Check to see if gMultiProcessBrowser is true, and then
* Check to see if the currently selected browser is remote (the gBrowser.selectedTab.linkedBrowser.isRemoteBrowser can be used - though we might want to break that up a bit)
* If the currently selected browser is NOT remote, then we should add:

forceNotRemote: true

to the object of arguments that we're passing to loadOneTab.

Let me know if you have any questions!

Comment 6

[Chirag Bhatia]

Attachment: 1167600.1.patch

Hi Mike,

Thanks for assigning the bug.

I've attached the code patch and tested it with a few about pages, seems to work fine on my end. I'm passing the object with the parameters based on the if conditions, or do you think saving the parameter in a variable first would be a better idea? Let me know if the patch needs any change.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8625141 [details] [diff] [review]
1167600.1.patch

Review of attachment 8625141 [details] [diff] [review]:
-----------------------------------------------------------------

This definitely looks like the right idea - thanks! Just one suggestion below...

::: browser/base/content/browser.js
@@ +3137,5 @@
>  
>    getPrintPreviewBrowser: function () {
>      if (!this._printPreviewTab) {
>        this._tabBeforePrintPreview = gBrowser.selectedTab;
> +      if (gMultiProcessBrowser && !gBrowser.selectedTab.linkedBrowser.isRemoteBrowser) {

How about outside of this, we create an variable "browser" as a shortcut for gBrowser.selectedTab.linkedBrowser. Then, define a new variable forceNotRemote, such that:

let forceNotRemote = gMultiProcessBrowser &&!browser.isRemoteBrowser;

Then we can call loadOneTab and pass in forceNotRemote, regardless of it's value:

this._printPreviewTab = gBrowser.loadOneTab("about:blank", {
  inBackground: false,
  forceNotRemote,
});

Comment 8

[Chirag Bhatia]

Attachment: 1167600.2.patch

Good idea, Mike. I've implemented this in the patch, let me know what you think.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8625780 [details] [diff] [review]
1167600.2.patch

Review of attachment 8625780 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a75d22b8f23e

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Thanks so much Chirag! Were you interested in working on more printing bugs?

Comment 11

[Chirag Bhatia]

Thanks for your help, Mike.

I don't have anything specific in mind, but yes, I can work on more printing bugs if you have any under your belt :)

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d9a98adbac2

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2d9a98adbac2

Comment 14

[dw-dev]

Unfortunately, it appears that this patch does not fix the problem I reported.

If you follow the 5 steps to reproduce this problem (given in my original problem report), using Nightly 41.0a1, then it is still not possible to print preview the "about:credits" page.

The Web Console shows the following error:

    NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE)
        [nsIMessageSender.sendAsyncMessage] printUtils.js:413:0

Also, there appears to be an error in this patch:

    this._printPreviewTab = gBrowser.loadOneTab("about:blank",
                                                { inBackground: false,
                                                  forceNotRemote });

which I think should read:

    this._printPreviewTab = gBrowser.loadOneTab("about:blank",
                                                { inBackground: false,
                                                  forceNotRemote: forceNotRemote });

Comment 15

[Chirag Bhatia]

Hi dw-dev,

I did a hg pull, mach clobber and rebuilt firefox, but was unable to reproduce this bug.

I tried it for about:credits, about:about, about:memory, about:robots, about:cache and about:healthreport pages, and the Print preview seems to work fine for them all. Is there any other 'about:' page you are able to reproduce this on?

Comment 16

[Jonathan Howard]

Need to wait for next nightly update, patch not in today's.

Comment 17

[dw-dev]

Apologies, I saw the patch in browser.js on DXR and incorrectly assumed it was already in Nightly.

I have tested with Nightly 41.0a1 (2015-06-27) and all of the "about:..." pages referenced in the "about:" page can be print previewed, except for the 8 XUL pages (which is to be expected).

I have made a similar patch to my "Print Edit" add-on and that also works fine.

So, I am pleased to confirm that this bug is fixed.