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

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jim Mathies [:jimm]]

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.

Comment 1

[Jim Mathies [:jimm]]

[Tracking Requested - why for this release]:
need to track this to make sure it gets uplifted.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Tracking as it is important to have this before the aurora => beta merge

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 4

[Jim Mathies [:jimm]]

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.

Comment 5

[Jim Mathies [:jimm]]

(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

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

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.

Comment 8

[Dave Townsend [:mossop]]

(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?

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(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.

Comment 10

[Dave Townsend [:mossop]]

Attachment: 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/

Comment 11

[Robert Helmer [: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

Comment 12

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Thanks so much Dave. Can you land this and request uplift?

Comment 13

[Pulsebot]

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

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/90e42416b6c9

Comment 15

[Dave Townsend [:mossop]]

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

Comment 16

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/07f847a16bf1