1359823 - Firefox All Aboard Add-On Disables e10s

Status: RESOLVED FIXED

(Firefox :: General, enhancement)

Description

[Mason Chang [Inactive] [:mchang]]

This is bad. And it looks like it is installed by default sometimes.

Comment 1

[Schalk Neethling [:espressive]]

(In reply to Mason Chang [:mchang] from comment #0)
> This is bad. And it looks like it is installed by default sometimes.

Hmmm, this is most likely because the add-on is SDK based. This is only meant to be installed in a specific funnelcake that should no longer be live.

Chris can provide more details here.

Comment 2

[Chris More [:cmore]]

The Firefox All Aboard add-ons were part of 4 past funnelcake experiments during these months:

v1.0: July 2016 (en-US)
v1.2: September 2016 (en-US)
v1.5: November 2016 (en-US)
v1.6: March 2017 (en-US + de)

v1.0 through v1.5 are long over and if we wanted to, we could blocklist the add-ons to ensure they are disabled in any of the funnelcake users.

v1.6 is still within a cohort of users that we want to let alone for a while longer.

For v1.0-v1.5, is it possible to block these on AMO, have them disabled from Firefox, and have them not block e10s for these cohorts?

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Is it possible to update 1.6 so that it doesn't disable e10s?

Comment 4

[Chris More [:cmore]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> Is it possible to update 1.6 so that it doesn't disable e10s?

I am pretty sure there is nothing special about these add-ons that would prevent e10s from functioning. I think just were not reviewed and approved to be allowed to work with e10s, but I would confirm with someone on the AMO team or more familiar with e10s requirements to be certain. Maybe it is just the add-on sdk is a blocker just because it is the sdk.

Who can we ask to get confirmation?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Maybe mconley knows or knows who knows.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

I don't see anything in the add-on's package.json[1] that would qualify it for multiprocess compatible. I _believe_ this needs to be added to the package.json in the "permissions" object, under "multiprocess". See [2].

I think this means that Firefox is going to interpret the All Aboard add-on as not compatible, and disable e10s by default.

[1]: https://github.com/mozilla/all-aboard/blob/master/package.json
[2]: https://developer.mozilla.org/en-US/Add-ons/SDK/Tools/package_json

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Because of this, I'd take any data gained by using the All Aboard add-on with a grain of salt, as the targeted users were put into the less optimal configuration.

Comment 8

[Chris More [:cmore]]

(In reply to Mike Conley (:mconley) - PTO on April 28th. from comment #7)
> Because of this, I'd take any data gained by using the All Aboard add-on
> with a grain of salt, as the targeted users were put into the less optimal
> configuration.

Interesting enough, the onboarding cohorts with the onboarding add-on have statistically improved user retention rates compared to the control, which was a vanilla Firefox build. But yeah, I do agree, the non-e10s nature of the onboarding cohorts could be dragging down the retention impact or eliminating it in some cases.

Comment 9

[Chris More [:cmore]]

(In reply to Mike Conley (:mconley) - PTO on April 28th. from comment #6)
> I don't see anything in the add-on's package.json[1] that would qualify it
> for multiprocess compatible. I _believe_ this needs to be added to the
> package.json in the "permissions" object, under "multiprocess". See [2].
> 
> I think this means that Firefox is going to interpret the All Aboard add-on
> as not compatible, and disable e10s by default.
> 
> [1]: https://github.com/mozilla/all-aboard/blob/master/package.json
> [2]: https://developer.mozilla.org/en-US/Add-ons/SDK/Tools/package_json

Anyway to tell Firefox to not take the default and block these add-ons anyway?

We could in theory push updates to the add-ons remotely to add the permissions/multiprocess to the package.json and when the clients come online, they would pull down the latest XPI from AMO.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

That would likely involve updating another system add-on anyway (the e10s rollout add-on, to add this add-on to the whitelist), in which case you're _probably_ doing the same amount of effort as updating the package.json and shipping a new release of All Aboard.

ni?ing felipe to make sure my math is right on this one. felipe, am I correct on this?

Comment 11

[:Felipe Gomes (needinfo for replies!)]

The whitelist has been removed, so no addons that aren't mpc=true can allow e10s.

But I was told on IRC that the multiprocess flag that Mike pointed out in link [2] works fine, so it should be just a matter of updating the package.json and posting a new xpi on AMO. You can verify if it's working by checking if the generated install.rdf contains:

<em:multiprocessCompatible>true</em:multiprocessCompatible>

Comment 12

[Chris More [:cmore]]

Schalk: I know you are not on this project anymore, but how much would it take to update the old all-board repo for v1.0, 1.2, 1.5 and 1.6 to include the e10 flag from comment 11? If you could do that, I could upload the 4 new XPIs to AMO to get them pushed their review.

Comment 13

[Schalk Neethling [:espressive]]

Attachment: all-aboard-v1.xpi

Unsigned XPI for v1.0

Comment 14

[Schalk Neethling [:espressive]]

Attachment: all-aboard-v1-2.xpi

Unsigned XPI for v1.2

Comment 15

[Schalk Neethling [:espressive]]

Attachment: all-aboard-v1-5.xpi

Unsigned XPI for v1.5

Comment 16

[Schalk Neethling [:espressive]]

Attachment: all-aboard-v1-6.xpi

Unsigned XPI for v1.6

Comment 17

[Chris More [:cmore]]

(In reply to Schalk Neethling [:espressive] from comment #15)
> Created attachment 8863727 [details]
> all-aboard-v1-5.xpi
> 
> Unsigned XPI for v1.5

Schalk: can you increment the version number to 1.5.8? This was 1.5.7 and that is the version already on the website.

Comment 18

[Chris More [:cmore]]

Looks like v1.0 was already uploaded to AMO and I just uploaded v1.2 to AMO for review and mentioned the e10s flag.

Comment 19

[Chris More [:cmore]]

Uploaded v1.6 for AMO review too.

Comment 20

[Schalk Neethling [:espressive]]

Attachment: all-aboard-v1-5.xpi

New unsigned v1.5 XPI with version bumped to 1.5.8

Comment 21

[Chris More [:cmore]]

(In reply to Schalk Neethling [:espressive] from comment #20)
> Created attachment 8866708 [details]
> all-aboard-v1-5.xpi
> 
> New unsigned v1.5 XPI with version bumped to 1.5.8

Submitted! Thanks

Comment 22

[Chris More [:cmore]]

Schalk: do you know if the https://github.com/mozilla/onboard XPI had the same flag flipped to allow it to be e10s?

Comment 23

[Schalk Neethling [:espressive]]

(In reply to Chris More [:cmore] from comment #22)
> Schalk: do you know if the https://github.com/mozilla/onboard XPI had the
> same flag flipped to allow it to be e10s?

Yup.

Comment 24

[Chris More [:cmore]]

(In reply to Schalk Neethling [:espressive] from comment #23)
> (In reply to Chris More [:cmore] from comment #22)
> > Schalk: do you know if the https://github.com/mozilla/onboard XPI had the
> > same flag flipped to allow it to be e10s?
> 
> Yup.

and that's the one that was shipped in the funnelcake, right?

Comment 25

[Chris More [:cmore]]

All versions updated to e10s and clients should have them the next time they fire up Firefox and restart.