1038245 - about:support should have a section for multiprocess browsing

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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).

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: 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".

Comment 2

[Dave Garrett]

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.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

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.

Comment 5

[Dave Garrett]

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

[Dave Garrett]

(& include the remote check and increment in the try{} too due to the let scoping)

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6094dd25da95

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6094dd25da95