1250744 - Enable blocking of e10s by add-ons for beta/release

Status: RESOLVED FIXED

Description

[:Felipe Gomes (needinfo me!)]

[Tracking Requested - why for this release]:

This was enabled by the experiment in 45, but for 46 onwards we should let the pref ride the trains.

#ifdef'ed to RELEASE_BUILD so that we keep Nightly/Aurora users enabled regardless of add-ons.

Comment 1

[:Felipe Gomes (needinfo me!)]

Created attachment 8722780 [details]
MozReview Request: Bug 1250744 - Enable blocking of e10s by add-ons for beta/release. r=Mossop

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

Comment 2

[Dave Townsend [:mossop] (he/him)]

Comment on attachment 8722780 [details]
MozReview Request: Bug 1250744 - Enable blocking of e10s by add-ons for beta/release. r=Mossop

https://reviewboard.mozilla.org/r/36231/#review32889

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3ed513cef91

Comment 4

[:Felipe Gomes (needinfo me!)]

Comment on attachment 8722780 [details]
MozReview Request: Bug 1250744 - Enable blocking of e10s by add-ons for beta/release. r=Mossop

Approval Request Comment
[Feature/regressing bug #]: This pref enables the blocking of e10s for add-ons users
[User impact if declined]: Rollout of e10s to beta46
[Describe test coverage new/current, TreeHerder]: Feature was tested in the beta45 experiment, and has automated tests
[Risks and why]: small, contained to the disabling of e10s
[String/UUID change made/needed]: none

Comment 5

[Carsten Book [:Tomcat]]

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

Comment 6

[Liz Henry (:lizzard) (use needinfo)]

Tracking since we need this to roll out e10s in 46

Comment 7

[Liz Henry (:lizzard) (use needinfo)]

So, will this mean that e10s users on release won't be able to install  addons or themes, and so on? 
What result will pref("extensions.e10sBlocksEnabling", true) have for users?

Comment 8

[:Felipe Gomes (needinfo me!)]

No, it just means that installing a restartless add-on still require a restart to get the add-on enabled (in order for e10s to be deactivated)

Comment 9

[Liz Henry (:lizzard) (use needinfo)]

Comment on attachment 8722780 [details]
MozReview Request: Bug 1250744 - Enable blocking of e10s by add-ons for beta/release. r=Mossop

Lets e10s block addon enabling, please uplift to aurora.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Pretty sure this patch is the reason why my Trunk-as-Beta simulation pushes are burning e10s tests like crazy at the moment (i.e. unable to activate e10s = mass failure).

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0dcdb53c422a

Comment 12

[:Felipe Gomes (needinfo me!)]

Created attachment 8724855 [details] [diff] [review]
Unset this pref on testing harnesses

Some of the test harnesses uses add-ons (like for SpecialPowers) to drive or support the testing process. Ryan has done a test uplift to beta where he noticed this, so we need to set these to false on the testing profiles.

I don't know if all of this is necessary, or if I missed something, but I think this is all that I found on the tree. Ryan, could you do a test with this patch to see if anything else remains?

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8724855 [details] [diff] [review]
Unset this pref on testing harnesses

crashtests/reftests are still broken. I assume it's because the patch needs s/e10sBlocksSinging/e10sBlocksEnabling in runreftest.py? :)

Comment 14

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #13)
> crashtests/reftests are still broken. I assume it's because the patch needs
> s/e10sBlocksSinging/e10sBlocksEnabling in runreftest.py? :)

Try is much happier with this change.

Comment 15

[:Felipe Gomes (needinfo me!)]

what a typo :S

Did everything work well with that fixed? If so I'm gonna update the patch and request review :)

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Yes, working great with that typo fixed.

Comment 17

[:Felipe Gomes (needinfo me!)]

Created attachment 8725677 [details] [diff] [review]
Unset this pref on testing harnesses

Hey Joel, on beta/release there's a pref that makes add-ons block e10s being enabled. However, since some of the harnesses uses add-ons, it has the effect of blocking e10s during testing. This patch unsets this pref for e10s runs on testing profiles.

Comment 18

[Joel Maher ( :jmaher ) (UTC-4)]

Comment on attachment 8725677 [details] [diff] [review]
Unset this pref on testing harnesses

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

this is nice to see, are there concerns with Talos at all?  What about web platform tests?

Comment 19

[:Felipe Gomes (needinfo me!)]

I believe talos is already covered by this patch. I'm not sure about web platform tests, but RyanVM ran a try build with this patch and things looked good, so I'm guessing this doesn't affect them (no add-on is used?)

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6606ab35633b

Comment 21

[Joel Maher ( :jmaher ) (UTC-4)]

yes, talos is convered, my fault for asking redundant questions.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d663d8db5a96

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6606ab35633b