When e10s is enabled installing or enabling an add-on should require a restart

RESOLVED FIXED in Firefox 45

Status

()

Firefox
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jimm, Assigned: mossop)

Tracking

Trunk
Firefox 46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm8+, firefox45+ fixed, firefox46 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
e10s rollout to beta will initially target users without addons. If a user with e10s enabled installs an addon we should force a restart and disable e10s.

This is currently targeted at the fx 45 release.
(Reporter)

Comment 1

2 years ago
[Tracking Requested - why for this release]:
need to track this to make sure it gets uplifted.
tracking-firefox45: --- → ?
tracking-e10s: ? → m8+
Assignee: nobody → felipc
Tracking as it is important to have this before the aurora => beta merge
tracking-firefox45: ? → +
I think the best way to do this would be to treat every add-on as a non-restartless add-on if e10s is enabled. That way they'll be forced to restart and we can disable e10s on the next startup. Also, we probably want to put this behind a pref like "force_addon_restart_if_e10s" or something like that. Then we can set that pref only if MOZ_RELEASE.

Dave has very kindly agreed to take this.
Assignee: felipc → dtownsend
Flags: needinfo?(dtownsend)
(Reporter)

Comment 4

2 years ago
Note we also have bug 1234675 filed which is an M*, which is about making sure e10s does not get turned on for users with addons installed. We might be able to combine these two.
(Reporter)

Comment 5

2 years ago
(In reply to Jim Mathies [:jimm] from comment #4)
> Note we also have bug 1234675 filed which is an M*, which is about making
> sure e10s does not get turned on for users with addons installed. We might
> be able to combine these two.

s/M*/M8
I think we want to keep these separate. Felipe and I talked today about a way to handle the work in bug 1234675, but I didn't realize Gabor was already assigned to do that work. I'll CC Felipe on that bug (although he'll probably see this too).

As far as this bug is concerned, we just need to restart every time an add-on is installed. Bug 1234675 (or something) will notice that the list of pending add-ons has changed and set a pref saying that we shouldn't use e10s on the next startup.
One thing to keep in mind for both this bug and bug 1234675 is that the hotfix add-on, and experiment add-ons, need to be exceptions for these rules and not restart/disable e10s.
(Assignee)

Comment 8

2 years ago
(In reply to :Felipe Gomes (needinfo me!) from comment #7)
> One thing to keep in mind for both this bug and bug 1234675 is that the
> hotfix add-on, and experiment add-ons, need to be exceptions for these rules
> and not restart/disable e10s.

What about webextensions, themes, dictionaries, locales...?

Should this also apply to enabling an extension or just for installing?
Flags: needinfo?(dtownsend) → needinfo?(wmccloskey)
(Assignee)

Updated

2 years ago
Summary: Disable e10s and force a restart if a user without addons installs an addon → When e10s is enabled installing an add-on should require a restart
Blocks: 1225496
(In reply to Dave Townsend [:mossop] from comment #8)
> (In reply to :Felipe Gomes (needinfo me!) from comment #7)
> > One thing to keep in mind for both this bug and bug 1234675 is that the
> > hotfix add-on, and experiment add-ons, need to be exceptions for these rules
> > and not restart/disable e10s.
> 
> What about webextensions, themes, dictionaries, locales...?

Let's not force a restart for those and we'll keep e10s enabled.

> Should this also apply to enabling an extension or just for installing?

Yes, also for enabling.
Flags: needinfo?(wmccloskey)
(Assignee)

Updated

2 years ago
Summary: When e10s is enabled installing an add-on should require a restart → When e10s is enabled installing or enabling an add-on should require a restart
(Assignee)

Comment 10

2 years ago
Created attachment 8708138 [details]
MozReview Request: Bug 1232274: Make installing or enabling an add-on require a restart if e10s is on and a pref is set. r?rhelmer

Review commit: https://reviewboard.mozilla.org/r/30999/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30999/
Attachment #8708138 - Flags: review?(rhelmer)
Comment on attachment 8708138 [details]
MozReview Request: Bug 1232274: Make installing or enabling an add-on require a restart if e10s is on and a pref is set. r?rhelmer

https://reviewboard.mozilla.org/r/30999/#review27949
Attachment #8708138 - Flags: review?(rhelmer) → review+
Whiteboard: [e10s-45-uplift]
Thanks so much Dave. Can you land this and request uplift?
Flags: needinfo?(dtownsend)
Whiteboard: [e10s-45-uplift]
Whiteboard: [e10s-45-uplift]

Comment 13

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/90e42416b6c9

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/90e42416b6c9
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
(Assignee)

Updated

2 years ago
Flags: needinfo?(dtownsend)
(Assignee)

Comment 15

2 years ago
Comment on attachment 8708138 [details]
MozReview Request: Bug 1232274: Make installing or enabling an add-on require a restart if e10s is on and a pref is set. r?rhelmer

Approval Request Comment
[Feature/regressing bug #]: E10S
[User impact if declined]: The feature calls for E10S disabled when add-ons are enabled and this code stops users from enabling add-ons without disabling E10S.
[Describe test coverage new/current, TreeHerder]: Running on m-c with an automated test
[Risks and why]: Low to moderate risk. This is a change to the behaviour of restartless add-ons, mostly that is fine though as it just pushes them through the same paths that non-restartless add-ons have used for some time. The tests are good and once turned on we'd hear pretty quickly if there was any issue from users.
[String/UUID change made/needed]: None
Attachment #8708138 - Flags: approval-mozilla-aurora?
Comment on attachment 8708138 [details]
MozReview Request: Bug 1232274: Make installing or enabling an add-on require a restart if e10s is on and a pref is set. r?rhelmer

Needed for e10s, taking it.
Attachment #8708138 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 17

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/07f847a16bf1
status-firefox45: affected → fixed
Whiteboard: [e10s-45-uplift]
(Assignee)

Updated

2 years ago
Depends on: 1243935
You need to log in before you can comment on or make changes to this bug.