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

RESOLVED FIXED in Firefox 46

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Felipe, Assigned: Felipe)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(e10s?, firefox46+ fixed, firefox47+ fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

[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 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
Attachment #8722780 - Flags: review?(dtownsend) → review+
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
Attachment #8722780 - Flags: approval-mozilla-aurora?

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a3ed513cef91
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Tracking since we need this to roll out e10s in 46
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?
Flags: needinfo?(felipc)
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)
Flags: needinfo?(felipc)
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.
Attachment #8722780 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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).
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?
Attachment #8724855 - Flags: feedback?(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? :)
Flags: needinfo?(felipc)
Attachment #8724855 - Flags: feedback?(ryanvm) → feedback-
(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.
what a typo :S

Did everything work well with that fixed? If so I'm gonna update the patch and request review :)
Flags: needinfo?(felipc)
Yes, working great with that typo fixed.
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.
Attachment #8724855 - Attachment is obsolete: true
Attachment #8725677 - Flags: review?(jmaher)
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?
Attachment #8725677 - Flags: review?(jmaher) → review+
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?)
yes, talos is convered, my fault for asking redundant questions.
You need to log in before you can comment on or make changes to this bug.