about:support should have a section for multiprocess browsing

RESOLVED FIXED in mozilla33

Status

()

Toolkit
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

(Blocks: 1 bug)

Trunk
mozilla33
Points:
---

Firefox Tracking Flags

(e10s?)

Details

Attachments

(1 attachment)

A couple people have asked for a section in about:support saying whether the current window has useRemoteTabs set (i.e., whether we're opening tabs in a separate process).
tracking-e10s: --- → ?

Updated

4 years ago
Blocks: 594464
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
Created attachment 8455458 [details] [diff] [review]
about-support

This adds a row in the "Application Basics" section called "Multiprocess Windows". If you have one e10s window and one non-e10s window, it would read "1/2".
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8455458 - Flags: review?(davemgarrett)

Comment 2

4 years ago
Comment on attachment 8455458 [details] [diff] [review]
about-support

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

It's pretty straightforward with the minor things noted. The N/M format for the value is the same as for the graphics acceleration section, so it's probably fine, though this has confused a few users in the past. You could possibly switch both over to show as "N (M total)", but it doesn't really matter as the people providing support using this data can figure things out fine.

That being said, I don't know if my review is sufficient to land. I don't think I'm not qualified to review this level of code, but I haven't before and my OK is probably not enough. I'm not an official reviewer for anything here. I'll mark r+ with the caveats noted here.

::: toolkit/content/aboutSupport.xhtml
@@ +112,5 @@
> +
> +            <td id="multiprocess-box">
> +            </td>
> +          </tr>
> +

I'd stick this at the end of the section instead of after the user agent.

::: toolkit/modules/Troubleshoot.jsm
@@ +154,5 @@
> +      let remote = winEnumer.getNext().
> +                   QueryInterface(Ci.nsIInterfaceRequestor).
> +                   getInterface(Ci.nsIWebNavigation).
> +                   QueryInterface(Ci.nsILoadContext).
> +                   useRemoteTabs;

Are all windows returned via nsIWindowWatcher.getWindowEnumerator() guaranteed to QI to all of these? If there's the possibility of some other non-browser window being open that won't, then encapsulating this in a try...catch might be warranted.
Attachment #8455458 - Flags: review?(davemgarrett) → review+
Comment on attachment 8455458 [details] [diff] [review]
about-support

I think that every window should be an nsGlobalWindow, so the GetInterface/QI should work. But I can add a try/catch just to be sure.
Attachment #8455458 - Flags: review?(felipc)
Comment on attachment 8455458 [details] [diff] [review]
about-support

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

You can change it to Services.ww.getWindowEnumerator("navigator:browser") which will guaranteed only return browser.xul windows.
Attachment #8455458 - Flags: review?(felipc) → review+

Comment 5

4 years ago
Just depends on if you want the total to count non-browser windows, I guess. Try...catch the check to count them all or get only "navigator:browser" to just count the browsers. The accelerated windows count counts all windows using Services.ww.getWindowEnumerator() so I'd use the try...catch to match.

Comment 6

4 years ago
(& include the remote check and increment in the try{} too due to the let scoping)
https://hg.mozilla.org/mozilla-central/rev/6094dd25da95
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.