e10s beta experiment uses incorrect prefs

RESOLVED FIXED in Firefox 44

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

34 Branch
Firefox 46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s?, firefox44 fixed, firefox45 fixed, firefox46 fixed)

Details

Attachments

(2 attachments)

I was looking into why tests are failing on beta and I noticed that a number of important prefs are still gated on E10S_TESTING_ONLY, which is disabled on beta:

extensions.interposition.enabled
extensions.interposition.prefetching
dom.compartment_per_addon
dom.ipc.plugins.asyncInit.enabled

So, on beta, the people with e10s enabled are not getting any add-on shims and they're getting async plugin init (which is still broken in e10s AFAIK).
Actually, it looks like async plugin init is unconditionally disabled on beta.

Updated

3 years ago
tracking-e10s: --- → ?
In the meeting we decided we'll have these prefs ride the trains with 45.
Assignee: nobody → wmccloskey
Created attachment 8704371 [details] [diff] [review]
fix-shims

Actually, we decided to get this change into 44 so that we can still get some data out of the next beta.
Attachment #8704371 - Flags: review?(mconley)
Comment on attachment 8704371 [details] [diff] [review]
fix-shims

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

I assume we've evaluated the potential for increased memory usage from the new compartments, as well as increased overhead over the interposition layer, and are okay with it?

If so, r=me.
Attachment #8704371 - Flags: review?(mconley) → review+
Created attachment 8704440 [details] [diff] [review]
change-default

Sorry, Mike. Now they want us to enable this stuff only for people with e10s.

It turns out that the compartment_per_addon pref is already enabled if you have e10s:
https://dxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCWrappedNativeScope.cpp#239

And the prefetching pref can be enabled unconditionally since the prefetcher is only used if shims are already enabled.

I change each callsite that uses extensions.interposition.enabled so that it also checks BrowserTabsRemoteAutostart.
Attachment #8704440 - Flags: review?(mconley)
By the way, we want to get this into the beta, which builds tomorrow.
Gotcha - reviewing now.
Comment on attachment 8704440 [details] [diff] [review]
change-default

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

Looks good - let's roll.
Attachment #8704440 - Flags: review?(mconley) → review+
Comment on attachment 8704440 [details] [diff] [review]
change-default

This is the patch we want to take on beta so that the e10s experiment has more validity. It only enables interposition if e10s is enabled already.
Attachment #8704440 - Flags: approval-mozilla-beta?
Comment on attachment 8704371 [details] [diff] [review]
fix-shims

I'd like to land this patch on Aurora. It ensures that these prefs will be enabled in the next beta, with or without e10s.

These prefs have been tested on Nightly and Aurora for many releases already.
Attachment #8704371 - Flags: approval-mozilla-aurora?
Comment on attachment 8704440 [details] [diff] [review]
change-default

Taking it because it's needed to get e10s add-on telemetry data for FF45, beta44+
Attachment #8704440 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Updated

3 years ago
status-firefox44: --- → affected

Updated

3 years ago
status-firefox45: --- → affected

Comment 15

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a2a9829434d3
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-v2.5: fixed → ---
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment on attachment 8704371 [details] [diff] [review]
fix-shims

aurora too
Attachment #8704371 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 17

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/d295cbbdaf5d
status-firefox45: affected → fixed

Updated

3 years ago
status-firefox44: affected → fixed
Depends on: 1238802
Hi Bill, could you please backout the patch we landed in this bug to enable the e10s experiment? We've agreed to postpone this experiment from Fx44 to Fx45 beta cycle. Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1238802#c10. Thanks!
Flags: needinfo?(wmccloskey)

Comment 20

3 years ago
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected
You need to log in before you can comment on or make changes to this bug.