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

RESOLVED FIXED in Firefox 46

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Felipe, Assigned: Felipe)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

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

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
[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.
(Assignee)

Comment 1

2 years ago
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/
Attachment #8722780 - Flags: review?(dtownsend)
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 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3ed513cef91
(Assignee)

Comment 4

2 years ago
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a3ed513cef91
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Tracking since we need this to roll out e10s in 46
tracking-firefox46: ? → +
tracking-firefox47: --- → +
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)
(Assignee)

Comment 8

2 years ago
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).

Comment 11

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/0dcdb53c422a
status-firefox46: affected → fixed
(Assignee)

Comment 12

2 years ago
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?
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.
(Assignee)

Comment 15

2 years ago
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.
(Assignee)

Comment 17

2 years ago
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.
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+
(Assignee)

Comment 19

2 years ago
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

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/6606ab35633b
yes, talos is convered, my fault for asking redundant questions.
https://hg.mozilla.org/releases/mozilla-aurora/rev/d663d8db5a96

Comment 23

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6606ab35633b
You need to log in before you can comment on or make changes to this bug.