1314429 - Beta 51 target all add-ons except those marked MPC=False or specifically excluded

Status: RESOLVED FIXED

(Firefox :: Extension Compatibility, defect, P2)

Description

[:shell escalante]

update the system add-on for the "go big" test.  Very interested in % of population that qualifies. 

Make e10s available to all add-ons except those marked MPC=False or specifically excluded (same list as in 50 - checking now for additional specific exclusions).

This will test how well the shims cover the compatibility gap - and how large it still is.  From Soft Vision work we have a good idea that we'll be safe on featured add-ons with over 75,000 users.  Theory is add-ons with fewer users tend to be less complex.

Expect to get more insight into unlisted add-ons 

Will file a related metrics bug - to see if there are other items we need to be ready for.

Asking in email if we need to throttle the experiment - since it could be as many as 40% (so instead of 160,000 in experiment and control - it could be 1M)

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1314429 - Allow every add-on to run on e10s, except those explictly marked with multiprocessCompatible=false.

There's also an existing blocklist of add-ons in bootstrap.js that currently only includes TabMixPlus

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

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8809524 [details]
Bug 1314429 - Allow every add-on to run on e10s, except those explictly marked with multiprocessCompatible=false.

https://reviewboard.mozilla.org/r/92086/#review92356

Thanks!

Do you know if the Add-ons team is watching the spinner probe for regressions along with things like crashy-ness and other performance metrics?

Comment 3

[:Felipe Gomes (needinfo for replies!)]

(In reply to Mike Conley (:mconley) (PTO - Nov 11 - Nov 14) from comment #2)
> Do you know if the Add-ons team is watching the spinner probe for
> regressions along with things like crashy-ness and other performance metrics?

I don't know. I'll leave that question for shell or dzeber

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/faf82a6f0dd4
Allow every add-on to run on e10s, except those explictly marked with multiprocessCompatible=false. r=mconley

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/faf82a6f0dd4

Comment 6

[Dave Zeber [:dzeber]]

(In reply to Mike Conley (:mconley) (PTO - Nov 11 - Nov 14) from comment #2)
> Do you know if the Add-ons team is watching the spinner probe for
> regressions along with things like crashy-ness and other performance metrics?

As far as I know, we're not, but Shell may have more info.

Comment 7

[:shell escalante]

[Tracking Requested - why for this release]:
we are expanding add-ons/e10s experiment in Beta for all MPC=False.  This is important for e10s

Comment 8

[:shell escalante]

sorry - saved before i was done.

we are going to use this experiment to limit audience some when we go live in 51.  If there are no issues - we will limit expansion to add-ons that were newly qualified with Beta users (in addition to the existing MPC=true and webextension users).

beta add-on use data gives us the ability for a middle ground step before going to all not marked MPC=false.  this is the safest way to expand with the least change required to the system add-on.

Comment 9

[:Felipe Gomes (needinfo for replies!)]

So, this patch didn't work, and there's a problem: turns out that when we read the data from install.rdf, the information about whether false was explicitly set or not is lost, and we only store a true or false about multiprocessCompatibility. See [1]

[1] http://searchfox.org/mozilla-central/rev/904bf9addd03b03d4cad11b82f19f43d875b7f27/toolkit/mozapps/extensions/internal/XPIProvider.jsm#1198


rhelmer or andym, do you know of any way that I can trigger a re-read of this? Or does one by any chance already happen automatically at, say, Firefox upgrade time?

Comment 10

[Robert Helmer [:rhelmer]]

Discussed in IRC - Felipe and I discussed storing whether the multiprocessCompatible property is explicitly set in a new property in the addons DB, and bumping the schema version.

The bump to the schema version should cause all manifests to be re-read when Firefox upgrades, and the addons DB to be rebuilt and populate this new property.

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Attachment: WIP, doesn't seem to work

Trying to get the addons manifest to be re-read to include the necessary information, but bumping the DB Schema doesn't appear to trigger it

Comment 12

[:shell escalante]

so comment 11 we'll need to keep looking for solution for release - but for Beta 51...

Jorge - can you pull a list of "listed" and "unlisted" add-ons marked mpc=false?  we'd need ID and name (name is for human readability only).  

The reason: see comment 9
  
The action for Beta: we can do a static blocklist for MPC=False for the Beta 51 experiment (testing allowing e10s to go to all but those marked MPC=False).  For beta i'd rather be overly blocking, than tackle the issue of versions - since versioning on the blocklist could get out of date.

In the Beta experiment anyone that had ever marked themselves as MPC=False would stay that way. will start  email with rhelmer, jorge, felipe for Release ideas, if other solution can't be worked out (comment 10 and comment 11).

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Gerry, just FYI this is a system add-on update planned for 51. This came up in the GoFaster meeting today.

Comment 14

[Gerry Chang [:gchang]]

Track 51+ as we will make e10s available to all add-ons except those marked MPC=False or specifically excluded in 51.

Comment 15

[Jorge Villalobos [:jorgev] (he/him)]

I'll send you the list. FTR, there are 43 add-ons with MPC=false, about half of them listed.

Comment 16

[:Felipe Gomes (needinfo for replies!)]

Attachment: Add new mpcOptedOut field and bump schema version to re-read it

This works now! It depends on bug 1272446

Comment 17

[Robert Helmer [:rhelmer]]

Comment on attachment 8814174 [details] [diff] [review]
Add new mpcOptedOut field and bump schema version to re-read it

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

::: toolkit/mozapps/extensions/internal/E10SAddonsRollout.jsm
@@ +222,5 @@
>      }
>  
>      let policy = RolloutPolicy[policyId];
>  
> +    dump("\n\nFLEIPE----\n");

Remember to remove this

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7bf172373ff
Bump XPIProvider's schema version to re-read info about multiprocessCompatible. Store information about whether that is false in a new field in the DB to avoid conflicts with any existing code. r=rhelmer

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f7bf172373ff

Comment 20

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8814174 [details] [diff] [review]
Add new mpcOptedOut field and bump schema version to re-read it

Approval Request Comment
[Feature/Bug causing the regression]: new e10s-addons experiment for beta 51 where every addon (except those marked as mpc=false) will get activated
[User impact if declined]: won't run the experiment on beta so we can't roll out  e10s for more users on release
[Is this code covered by automated tests?]: yes, by the next patch
[Has the fix been verified in Nightly?]: no, this code is only active on beta
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: the other patch here and bug 1272446 are required to land together
[Is the change risky?]: somewhat
[Why is the change risky/not risky?]: the code change itself is not risky, but this will enable e10s for many addon users. There will still be a 50/50 control/test group which will be used for comparisons.
[String changes made/needed]: none

Comment 21

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8809524 [details]
Bug 1314429 - Allow every add-on to run on e10s, except those explictly marked with multiprocessCompatible=false.

Approval Request Comment
Same as above, the two patches go together. That one contains the major code changes, and this one just flips the toggle to activate it

Comment 22

[Gerry Chang [:gchang]]

Comment on attachment 8809524 [details]
Bug 1314429 - Allow every add-on to run on e10s, except those explictly marked with multiprocessCompatible=false.

For e10s add-on rollout on Beta51. Beta51+ and Aurora52+. Should be in 51 beta 6.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

This needs rebasing for Beta. Maybe just a roll-up patch at this point?

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/38176b5357f0

Comment 25

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/releases/mozilla-beta/rev/fda168823ea2e8102ded10dbb3de97975826a50e
https://hg.mozilla.org/releases/mozilla-beta/rev/9c9278793a1526d65c5cc3437d030d880f14cfe2