Closed
Bug 1250744
Opened 9 years ago
Closed 9 years ago
Enable blocking of e10s by add-ons for beta/release
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: Felipe, Assigned: Felipe)
Details
Attachments
(2 files, 1 obsolete file)
58 bytes,
text/x-review-board-request
|
mossop
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details |
9.17 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
[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•9 years ago
|
||
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 2•9 years ago
|
||
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+
Assignee | ||
Comment 4•9 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•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 6•9 years ago
|
||
Tracking since we need this to roll out e10s in 46
tracking-firefox47:
--- → +
Comment 7•9 years ago
|
||
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•9 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 9•9 years ago
|
||
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+
Comment 10•9 years ago
|
||
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•9 years ago
|
||
bugherder uplift |
Assignee | ||
Comment 12•9 years ago
|
||
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 13•9 years ago
|
||
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-
Comment 14•9 years ago
|
||
(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•9 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)
Comment 16•9 years ago
|
||
Yes, working great with that typo fixed.
Assignee | ||
Comment 17•9 years ago
|
||
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 18•9 years ago
|
||
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•9 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•9 years ago
|
||
Comment 21•9 years ago
|
||
yes, talos is convered, my fault for asking redundant questions.
Comment 22•9 years ago
|
||
Comment 23•9 years ago
|
||
bugherder |
You need to log in
before you can comment on or make changes to this bug.
Description
•