1404098 - Do not force to 1 content process

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Andy McKay]

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.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

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

Comment 2

[Andy McKay]

(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 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Bug 1404098 - Don't disable multi for extensions.

Now that the addons code will refuse to allow any non-webextension that hasn't
been vetted to work with e10s-multi, we can allow all extensions in the
rollout addon.

Review commit: https://reviewboard.mozilla.org/r/184772/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184772/

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Felipe is on PTO until next week. Gabor, would you mind reviewing this? It should be pretty straightforward.

Comment 5

[Gabor Krizsanits (INACTIVE)]

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?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

(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 7

[Blake Kaplan (:mrbkap) (inactive)]

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05864e4d5340
Don't disable multi for extensions. r=krizsa

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/05864e4d5340

Comment 10

[Sylvestre Ledru [:Sylvestre]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8d3464b1fd54

Comment 12

[marius.santa]

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

Comment 13

[Luke Crouch [:groovecoder]]

When I got 57.0b5 today, it did not seem to automatically update extensions.e10sMultiBlockedByAddons to "false" ?

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

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