Closed Bug 1230196 Opened 4 years ago Closed 4 years ago

Release channel should support e10s prefs

Categories

(Firefox :: General, defect)

42 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
e10s m8+ ---
firefox45 --- wontfix
firefox46 + wontfix
firefox47 + fixed

People

(Reporter: vladan, Assigned: Felipe)

References

Details

Attachments

(2 files)

Currently e10s is not allowed on Release channel, making the browser.tabs.remote.autostart prefs no-ops.

Bug 1226487 added support for e10s on the Beta channel.
Blocks: 1229947
Jeff, would you like to allow adventurous users of release 45 to run e10s if they fiddle with the prefs? Currently there's code to block the release channel from running it, so to do so it must be removed.

We need to remove it for 46, and I was wondering if that should be uplifted to 45
Flags: needinfo?(jgriffiths)
(In reply to :Felipe Gomes (needinfo me!) from comment #1)
> Jeff, would you like to allow adventurous users of release 45 to run e10s if
> they fiddle with the prefs? Currently there's code to block the release
> channel from running it, so to do so it must be removed.
> 
> We need to remove it for 46, and I was wondering if that should be uplifted
> to 45

I can't think of a good reason why we would allow it. What would we do if someone reported a bug in e10s 45 in release that was fixed in 46? Resolve it fixed, then tell them to turn it off. That's not great for that user. I would rather we promoted Beta 46 or DE 47 to 'adventurous users', if they come asking.
Flags: needinfo?(jgriffiths)
Yeah, agreed!
Status: NEW → ASSIGNED
[Tracking Requested - why for this release]:
There's code that blocks the e10s prefs from taking effect into release. If we plan to rollout for some users on 46, it needs to be removed.  Note that this doesn't enable e10s (i.e., actually enabling it for users would be another bug), it just lets the pref take effect.
Attached patch patchSplinter Review
I wondered if I should go with a version that just removes the condition instead of removing this other code involved (for an easier rollback if necessary), but I guess this wouldn't be too hard to back out either.. what you think?
Attachment #8715365 - Flags: review?(wmccloskey)
Comment on attachment 8715365 [details] [diff] [review]
patch

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

Please remove the OMTC testing pref from all.js as well.
Attachment #8715365 - Flags: review?(wmccloskey) → review+
Approval Request Comment
[Feature/regressing bug #]: this lets the e10s pref take effect on release channel, targeted to 46 (Note that this doesn't enable e10s by default)
[User impact if declined]: can't run e10s
[Describe test coverage new/current, TreeHerder]: -
[Risks and why]: -
[String/UUID change made/needed]: -
Attachment #8716322 - Flags: review+
Attachment #8716322 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e46699511082
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Tracking since this is a feature we are aiming at 46. I'd like to hold back on the uplift until we're certain 46 is going to release.
After discussion with Felipe, we're leaving this up in the air for now, and I will switch the approval flag to approval-mozilla-beta:? next week after the merge.
Comment on attachment 8716322 [details] [diff] [review]
patch as landed, r=billm

Adding a beta approval ? flag here in case we may need to enable this for 46. 
For now, I'd like to let this stay in the queue until we're sure we need it.
Attachment #8716322 - Flags: approval-mozilla-beta?
Comment on attachment 8716322 [details] [diff] [review]
patch as landed, r=billm

This is already in 47 so clearing out the Aurora uplift request.
Attachment #8716322 - Flags: approval-mozilla-aurora?
Wontfix for 46 as the plan is still to go with 47 release channel rollout at the earliest.
Attachment #8716322 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.