Do not force to 1 content process

VERIFIED FIXED in Firefox 57

Status

()

P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: andy+bugzilla, Assigned: mrbkap)

Tracking

unspecified
mozilla58
Points:
---

Firefox Tracking Flags

(firefox57 verified, firefox58 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
In Firefox 57 the only way a legacy extension can appear on Firefox is by being signed as a Mozilla extension. There are very few of these and we are assuming the bar that they are e10s multi compatible.

If the e10s rollout extension is going to be around in Firefox 57 then it should check to see if the extension is a Mozilla signed extension and exempt it from forcing to single process.

In fact we don't even need to do that check and in Firefox 57 onwards just stop trying to turn off e10s multi. Let's get this landed in Firefox 57.

STR:
* start 57 Beta in a new profile
* go to about:support, note that Web Content Processes is something like x/4
* install https://addons.mozilla.org/en-US/firefox/addon/multi-account-containers/
* restart 
* go to about:support, note that Web Content Processes is something like x/1

This is probably more for :felipe but I can't remember the right component.
(Reporter)

Updated

2 years ago
Priority: -- → P1
(In reply to Andy McKay [:andym] from comment #0)
> In Firefox 57 the only way a legacy extension can appear on Firefox is by
> being signed as a Mozilla extension. There are very few of these and we are
> assuming the bar that they are e10s multi compatible.

To be clear on terminology, "legacy extensions" include bootstrap extensions, right? If so, this is extremely easy to fix.
(Reporter)

Comment 2

2 years ago
(In reply to Blake Kaplan (:mrbkap) from comment #1)
> To be clear on terminology, "legacy extensions" include bootstrap
> extensions, right? If so, this is extremely easy to fix.

Correct
Comment hidden (mozreview-request)
Felipe is on PTO until next week. Gabor, would you mind reviewing this? It should be pretty straightforward.
Assignee: nobody → mrbkap

Comment 5

2 years ago
mozreview-review
Comment on attachment 8913445 [details]
Bug 1404098 - Don't disable multi for extensions.

https://reviewboard.mozilla.org/r/184772/#review190042

::: browser/extensions/e10srollout/bootstrap.js:31
(Diff revision 1)
>    "release": { buckets: { 4: 1 }, // 4 processes: 100%
>  
> -               // See above for an explanation of this: we only allow users
> -               // with no extensions or users with WebExtensions.
> +               // See the comment above the "beta" addonsDisableExperiment.
> +               addonsDisableExperiment(prefix) { return false; } }
> -               addonsDisableExperiment(prefix) { return getAddonsDisqualifyForMulti(); } }

Just double checking. This system add-on update will not reach the 56 release channel, only 57 beta and 57 release when it gets there right?
Attachment #8913445 - Flags: review?(gkrizsanits) → review+
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #5)
> Just double checking. This system add-on update will not reach the 56
> release channel, only 57 beta and 57 release when it gets there right?

Right.
Comment on attachment 8913445 [details]
Bug 1404098 - Don't disable multi for extensions.

Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: Users with approved addons (such as compartments) will be considered ineligible for multi.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Not yet (it doesn't run on Nightly).
[Needs manual test from QE? If yes, steps to reproduce]: Install compartments addon and verify that we still get multi.
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It is well understood.
[String changes made/needed]: n/a
Attachment #8913445 - Flags: approval-mozilla-beta?

Comment 8

2 years ago
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05864e4d5340
Don't disable multi for extensions. r=krizsa

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/05864e4d5340
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment on attachment 8913445 [details]
Bug 1404098 - Don't disable multi for extensions.

We want as many people as possible to have multi.
Taking it.
Should be in 57b5
Attachment #8913445 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 11

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/8d3464b1fd54
status-firefox57: affected → fixed

Comment 12

2 years ago
Posted image 4.PNG
This bug is verified on Firefox 57.0b5 (20171002181526) and Firefox 58.0a1 (20171002220204) under Windows 10 64bit/Mac OS X 10.12.3.

Please see the attached screenshot.

Updated

2 years ago
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
status-firefox58: fixed → verified
When I got 57.0b5 today, it did not seem to automatically update extensions.e10sMultiBlockedByAddons to "false" ?
(In reply to Luke Crouch [:groovecoder] from comment #13)
> When I got 57.0b5 today, it did not seem to automatically update
> extensions.e10sMultiBlockedByAddons to "false" ?

The fix here removed the last remaining piece of code that reads that pref. The code that sets it is going away in bug 1406212.
You need to log in before you can comment on or make changes to this bug.