Closed Bug 1348576 Opened 7 years ago Closed 7 years ago

e10s is never enabled for non-official release builds

Categories

(Firefox :: Extension Compatibility, defect)

Product:
Firefox
Component:
Extension Compatibility
Version:
52 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox52 --- wontfix
firefox-esr52 --- fixed
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: jbeich, Assigned: Felipe)

References

Details

Attachments

(1 file)

[mq]: Bug 1348576 - e10s is never enabled for non-official release builds.
59 bytes, text/x-review-board-request
mconley
: review+
gchang
: approval-mozilla-aurora+
ritu
: approval-mozilla-esr52+
Details 
e10srollout addon controls how many users end up with e10s enabled by default. It relies on app.update.channel to figure out restrictions. Downstream doesn't use or even disables updater, so the channel remains "default". However, it's unclear when the system addon is going to be phased out.

Steps-To-Reproduce:

  $ hg clone https://hg.mozilla.org/mozilla-unified firefox
  $ cd firefox
  $ hg up release
  $ ./mach bootstrap
  $ echo "ac_add_options --enable-release" >>.mozconfig
  $ echo "ac_add_options --enable-official-branding" >>.mozconfig

  $ ./mach build
  $ ./mach run about:support
  Multiprocess Windows 	0/1 (Disabled)

vs.

  $ echo "ac_add_options --enable-update-channel=release" >>.mozconfig
  $ ./mach build
  $ ./mach run about:support
  Multiprocess Windows 	1/1 (Enabled)
Martin, does Fedora pass --enable-update-channel or set browser.tabs.remote.autostart? e10s is supposed to be enabled since FF48 per bug 1299247. Check about:support on a fresh profile.
Flags: needinfo?(stransky)
We talked about this with Felipe some time last week or the week before.

--enable-update-channel is only supposed to be used for the updater. Downstream is not supposed to have to set it.

... but it ended up being used for the e10s rollout.

Felipe, can you dupe this to the bug you were responding to?
Flags: needinfo?(felipc)
(In reply to Jan Beich from comment #1)
> Martin, does Fedora pass --enable-update-channel or set
> browser.tabs.remote.autostart? e10s is supposed to be enabled since FF48 per
> bug 1299247. Check about:support on a fresh profile.

Fedora users has to set browser.tabs.remote.autostart to true to get e10s activated. Thanks for looking at it, I was wondering why e10s is not enabled by default for our builds.
Flags: needinfo?(stransky)
(In reply to Mike Hommey [:glandium] from comment #2)
> We talked about this with Felipe some time last week or the week before.
> 
> --enable-update-channel is only supposed to be used for the updater.
> Downstream is not supposed to have to set it.
> 
> ... but it ended up being used for the e10s rollout.
> 
> Felipe, can you dupe this to the bug you were responding to?

The bug I was responding was on GitHub, so let's use this one to track this here on Bugzilla.

I still don't know what a good alternative we could use here considering the needs to have different rollout thresholds for users on the beta and release channels. Maybe I'll be able to set this for the "default" channel too, but it's unlikely I can get rid of using the update channel for this. I'll think more about it, and if anyone has suggestions I'm all ears.

Note that it's likely that this will no longer be a problem starting from 57 where I'm expecting to rip all this code out and directly set browser.tabs.remote.autostart to true by default.
Flags: needinfo?(felipc)
Assignee: nobody → felipc
Status: NEW → ASSIGNED
I am seeing the same with the Debian builds.

> Fedora users has to set browser.tabs.remote.autostart to true to get e10s activated.

Unfortunately, even doing this does not seem to help:  e10s is still deactivated when any plugin is enabled/disabled.
Comment on attachment 8852666 [details]
[mq]: Bug 1348576 - e10s is never enabled for non-official release builds.

https://reviewboard.mozilla.org/r/124852/#review127396

::: browser/extensions/e10srollout/bootstrap.js:35
(Diff revision 1)
> +  // the e10s rollout is controlled by the channel name, which
> +  // is the only way to distinguish between Beta and Release.
> +  // However, non-official release builds (like the ones done by distros
> +  // to ship Firefox on their package managers) do not set a value
> +  // for the release channel, which gets them to the default value
> +  // of.. (drumroll) "default".

lol

::: browser/extensions/e10srollout/bootstrap.js:36
(Diff revision 1)
> +  // But we can't just always configure the same settings for the
> +  // "default" channel because that's also the name that a locally
> +  // built Firefox gets, and e10s is managed in a different way
> +  // there (directly by prefs, on Nightly and Aurora).

This implies that locally built beta and release builds will get the release policy, right?

If so, we should maybe call that out. Dunno if that needs wider messaging - I'm not sure how many times people tend to build and run local beta or release.
Attachment #8852666 - Flags: review?(mconley) → review+
(In reply to Mike Conley (:mconley) (High latency until March 31) from comment #8)
> ::: browser/extensions/e10srollout/bootstrap.js:36
> (Diff revision 1)
> > +  // But we can't just always configure the same settings for the
> > +  // "default" channel because that's also the name that a locally
> > +  // built Firefox gets, and e10s is managed in a different way
> > +  // there (directly by prefs, on Nightly and Aurora).
> 
> This implies that locally built beta and release builds will get the release
> policy, right?
> 
> If so, we should maybe call that out. Dunno if that needs wider messaging -
> I'm not sure how many times people tend to build and run local beta or
> release.

Correct, unless they use --enable-update-channel=beta. I thought about it, but
the differences are minimal since the addons policy now match on both channels.
And it's actually a positive "downside": now every beta build will consistently
get e10s, instead of 50% of the time.
Comment on attachment 8852666 [details]
[mq]: Bug 1348576 - e10s is never enabled for non-official release builds.

https://reviewboard.mozilla.org/r/124852/#review127396

> This implies that locally built beta and release builds will get the release policy, right?
> 
> If so, we should maybe call that out. Dunno if that needs wider messaging - I'm not sure how many times people tend to build and run local beta or release.

Okay, I'm into it. Thanks!
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/171c6ef99177
[mq]: Bug 1348576 - e10s is never enabled for non-official release builds. r=mconley
https://hg.mozilla.org/mozilla-central/rev/171c6ef99177

Nice [mq] in the commit message there :)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment on attachment 8852666 [details]
[mq]: Bug 1348576 - e10s is never enabled for non-official release builds.

Approval Request Comment
[Feature/Bug causing the regression]: E10s doesn't get enabled for distro-built FF.
[User impact if declined]: Users of non-official release builds (shipped by distros) will continue to get non-e10s
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: none from us, as it doesn't affect our builds.. 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: somewhat
[Why is the change risky/not risky?]: the risk is that Firefox will start getting e10s.. and this might or might not be something that the distro maintainers are testing well on their distros (and whatever they ship together like bundled add-ons). I'm not going to request beta uplift because that's too little time for them to test, but perhaps getting this into Aurora would be good to get e10s faster for their users.
[String changes made/needed]: none
Attachment #8852666 - Flags: approval-mozilla-aurora?
Comment on attachment 8852666 [details]
[mq]: Bug 1348576 - e10s is never enabled for non-official release builds.

[Approval Request Comment]

So the real tough question is whether we should uplift this to ESR 52. It depends on whether many distros build Firefox from ESR instead of Release. And do they consider updates to ESR (e.g., 52.1, 52.2 etc) to be "minor" or "major" updates.

On one hand, it would be strange to suddenly get a large feature like e10s on a "minor" update.  On the other hand, it would be a shame if all these users remain on non-e10s until the next ESR ships AND distros start using it AND shipping it.


If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: described above
Fix Landed on Version: 55
Risk to taking this patch (and alternatives if risky): enabling e10s for more linux users
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8852666 - Flags: approval-mozilla-esr52?
Hi Jan,
Can you help verify if this is fixed as expected in latest nightly?
Flags: needinfo?(jbeich)
browser.tabs.remote.autostart.2=true by default on Nightly/Aurora, so e10srollout is limited to release branches. Otherwise, I confirm, backporting the patch to ESR52 fixes the issue.
Flags: needinfo?(jbeich)
Comment on attachment 8852666 [details]
[mq]: Bug 1348576 - e10s is never enabled for non-official release builds.

Enable e10s for non-official release builds. Aurora54+.
Attachment #8852666 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to :Felipe Gomes (needinfo me!) from comment #13)
> [Is this code covered by automated tests?]: no
> [Has the fix been verified in Nightly?]: no
> [Needs manual test from QE? If yes, steps to reproduce]: none from us, as it
> doesn't affect our builds.. 

Setting qe-verify- based on Felipe's assessment on manual testing needs.
Flags: qe-verify-
We use this patch in Fedora distro Firefox builds (53 recently) and seems to work as expected - people start to report e10s bugs ;-)
Comment on attachment 8852666 [details]
[mq]: Bug 1348576 - e10s is never enabled for non-official release builds.

I see no harm in taking this one, this should have a positive impact and the direction we are headed. ESR52.2+
Attachment #8852666 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Dan suggested we relnote this for ESR52.2. NI myself so I remember to do that.
Flags: needinfo?(rkothari)
Needs a rebased patch for ESR52. It's just a version number conflict in install.rdf, but I'm not sure what the right solution is in this case.
Flags: needinfo?(felipc)
Let's use 1.10 for the version number
Flags: needinfo?(felipc)
Felipe: Can you suggest some good wording for a release note for ESR 52.2?
Flags: needinfo?(felipc)
Added to ESR52.2.0 release notes.
Flags: needinfo?(rkothari)
(In reply to Marcia Knous [:marcia - use ni] from comment #26)
> Felipe: Can you suggest some good wording for a release note for ESR 52.2?

I don't know exactly what to suggest, so I was gonna point to how we first introduced e10s on 48: https://www.mozilla.org/en-US/firefox/48.0/releasenotes/
Flags: needinfo?(felipc)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: