Closed Bug 1167600 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Print Preview, defect)

38 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
e10s m8+ ---
firefox41 --- fixed

People

(Reporter: dw-dev, Assigned: chiragbhatia2006, Mentored)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
(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
Blocks: 927188
Status: UNCONFIRMED → NEW
Component: Untriaged → Print Preview
Ever confirmed: true
Product: Firefox → Core
Mike, is this something we need to track?
Assignee: nobody → mconley
Flags: needinfo?(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.
Flags: needinfo?(mconley)
Assignee: mconley → nobody
Mentor: mconley
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?
Flags: needinfo?(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!
Flags: needinfo?(mconley)
Attached patch 1167600.1.patch (obsolete) — Splinter Review
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.
Attachment #8625141 - Flags: review?(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,
});
Attachment #8625141 - Flags: review?(mconley) → review-
Attached patch 1167600.2.patchSplinter Review
Good idea, Mike. I've implemented this in the patch, let me know what you think.
Attachment #8625141 - Attachment is obsolete: true
Attachment #8625780 - Flags: review?(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
Attachment #8625780 - Flags: review?(mconley) → review+
Assignee: nobody → chiragbhatia2006
Thanks so much Chirag! Were you interested in working on more printing bugs?
Flags: needinfo?(chiragbhatia2006)
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 :)
Flags: needinfo?(chiragbhatia2006)
https://hg.mozilla.org/mozilla-central/rev/2d9a98adbac2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
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 });
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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?
Flags: needinfo?(dw-dev)
Need to wait for next nightly update, patch not in today's.
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.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Flags: needinfo?(dw-dev)
Resolution: --- → WORKSFORME
Resolution: WORKSFORME → FIXED
You need to log in before you can comment on or make changes to this bug.