1685961 - Hide "Content process limit" preference UI when Fission pref is enabled

Status: RESOLVED FIXED

(Firefox :: Settings UI, task, P3)

Description

[Chris Peterson [:cpeterson]]

The "Content process limits" preferences are not relevant when Fission is enabled.

UI strings in about:preferences's "Performance" section when you uncheck "Use recommended performance settings":

Content process limit
Additional content processes can improve performance when using multiple tabs, but will also use more memory.
Modifying the number of content processes is only possible with multiprocess { -brand-short-name }.

Strings:
https://searchfox.org/mozilla-central/rev/692a10e624a02fe79bf45046b586e50ea0ff17f1/browser/locales/en-US/browser/preferences/preferences.ftl#498-502

XUL:
https://searchfox.org/mozilla-central/rev/692a10e624a02fe79bf45046b586e50ea0ff17f1/browser/components/preferences/main.inc.xhtml#587-605

We should also remove the unused link to the ancient e10s wiki in the XUL code.

Comment 1

[:Gijs (he/him)]

Gonna mark things disabled as there's no way to enable fission on release branches.

:cpeterson, is someone on the fission team likely to tackle this, or is this a request for frontend folks to do this work?

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to :Gijs (he/him) from comment #1)

:cpeterson, is someone on the fission team likely to tackle this, or is this a request for frontend folks to do this work?

I think the Fission team can fix this, unless someone on the frontend team has time to work on it soon. I just wanted to file the bug in the appropriate Bugzilla component.

Until we enabled Fission by default for everyone, this bug fix should just dynamically hide this UI. After Fission is enabled by default for everyone, remove this UI and all the content process code that reads these preferences.

Comment 3

[:Gijs (he/him)]

Could you explain why you changed this back to a task? This has user-facing consequences, right?

Comment 4

[Neha Kochar [:neha]]

My rationale for making this a task was: this was anyway ignored when Fission is enabled, so hiding it is merely a task with no change in actual behavior.

Comment 5

[:Gijs (he/him)]

(In reply to Neha Kochar [:neha] from comment #4)

My rationale for making this a task was: this was anyway ignored when Fission is enabled, so hiding it is merely a task with no change in actual behavior.

OK. I think from a user perspective, having UI that claims to do something and then doesn't do it is a bug. :-)

Comment 6

[Neha Kochar [:neha]]

Gijs, is there someone from the frontend team who can do this? Hopefully this will be trivial.

Comment 7

[:Gijs (he/him)]

(In reply to Neha Kochar [:neha] from comment #6)

Gijs, is there someone from the frontend team who can do this? Hopefully this will be trivial.

Blake?

Comment 8

[Blake Winton (:bwinton) (:☕️)]

Jared's the module owner, so I'll let him find someone (or take it on himself)…

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Bernard, can you take this?

Comment 10

[Bernard Igiri]

Attachment: Bug 1685961 - Hide Content Process Limit UI r=jaws,gijs

Hides the "Content Process Limit" UI when Fission is enabled.

Comment 11

[Bernard Igiri]

I have a WIP patch, but I'm not sure how to test this. What are the steps to testing this?

Comment 12

[:Gijs (he/him)]

(In reply to Bernard Igiri from comment #11)

I have a WIP patch, but I'm not sure how to test this. What are the steps to testing this?

You can use ./mach run --enable-fission to test what thinks look like with fission enabled.

In terms of the automated test, you could check that the element's visibility matches gFissionBrowser (which is defined in the main browser window). You can check some of the other tests in browser/components/preferences/tests/ to see how to open the prefs.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca970620edf2
Hide Content Process Limit UI r=Gijs

Comment 14

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures in browser_performance

Backout link: https://hg.mozilla.org/integration/autoland/rev/6182294d50ecc2b0edacd0278acd3755b87b2b37

Push with failures

Failure log

Comment 15

[Bernard Igiri]

I updated the patch with a fix for the test failure.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11374d11a36c
Hide Content Process Limit UI r=Gijs

Comment 17

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/11374d11a36c

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.