Currently e10s is not allowed on Beta, making the browser.tabs.remote.autostart prefs no-ops: https://hg.mozilla.org/releases/mozilla-beta/annotate/f34e97d70cfe/configure.in#l3405 Can we just change this line to check for release only?

Passing this off to felipe, he understand what impact this would have.

We actually don't want to change that line because that will enable all the e10s UI. We just need to change the conditions in nsAppRunner.cpp. But there are no compile checks to differentiate between Beta and Release. We may just allow release users to toggle the pref, or check based on the channel. I'll take the bug.

So this basically removes the E10S_TESTING_ONLY parts of nsAppRunner.cpp. The nsIObserver that was ifdef'ed is used in the Preferences dialog to display the possible reason e10s was disabled. It won't be used on Beta right now, but eventually I want to show that information in about:support, so that's why I'm leaving it there

In order to allow e10s to be used on beta, we need to also enable the code that checks for a11y and disables it. This code is all duplicated from the E10S_TESTING_UI block above in the same file. Instead of removing the parts that can be removed, I duplicated the parts that I want to keep for simplicity. Everything else not used will be removed in a separate bug for clarity: bug 1227230

Note: in order to run the e10s beta experiment (bug 1222894), we need to uplift these two patches to Beta

Comment on attachment 8690916 [details] [diff] [review] allow-e10s-beta Review of attachment 8690916 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me, but seems like an e10s peer should review this.

Comment on attachment 8690916 [details] [diff] [review] allow-e10s-beta Review of attachment 8690916 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this looks right. Thanks felipe!

Comment on attachment 8690928 [details] [diff] [review] e10s-a11ychecks Review of attachment 8690928 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for pointing out bug 1227230. It'll be nice to close that one out. :) ::: browser/components/nsBrowserGlue.js @@ +3219,5 @@ > }; > + > +#else // E10S_TESTING_ONLY > + > +var E10SAccessibilityCheck = { Can you add a quick docstring above this object talking about its responsibilities?

https://hg.mozilla.org/integration/fx-team/rev/a59d57130e96 https://hg.mozilla.org/integration/fx-team/rev/0ed0b39c3541

https://hg.mozilla.org/mozilla-central/rev/a59d57130e96 https://hg.mozilla.org/mozilla-central/rev/0ed0b39c3541

Comment on attachment 8690916 [details] [diff] [review] allow-e10s-beta Approval Request Comment [Feature/regressing bug #]: Needed for e10s experiment on Beta [User impact if declined]: Can't run e10s experiment (bug 1222894) [Describe test coverage new/current, TreeHerder]: landed on central [Risks and why]: this patch allows the e10s pref to take effect on beta [String/UUID change made/needed]: -

Comment on attachment 8690928 [details] [diff] [review] e10s-a11ychecks Approval Request Comment [Feature/regressing bug #]: Needed for e10s experiment on Beta [User impact if declined]: Can't run e10s experiment (bug 1222894) [Describe test coverage new/current, TreeHerder]: landed on central [Risks and why]: this patch enables on beta the same a11y checks that e10s has been running on nightly/aurora. It is required to disable e10s for a11y users. [String/UUID change made/needed]: -

Felipe, have we made a decision on whether to conduct an A/B experiment on e10s on Beta44? I know it was planned for Beta43 but did it really happen. I am just trying to confirm the plan because bug 1222894 mentions Beta43.

Ritu: yeah, the plan is to conduct a short A/B with a small population on Beta 43 (7.5%, which is bug 1222894), and then a larger one on Beta 44 (bug 1225496)

Comment on attachment 8690928 [details] [diff] [review] e10s-a11ychecks Please uplift to aurora and beta, needed for planned a/b test.

Felipe, are we doing a11y checks on nightly? I'm asking because this came up in the channel meeting this morning, as many of the top crashes on nightly are a11y-related.

Comment on attachment 8690916 [details] [diff] [review] allow-e10s-beta Please uplift both patches on this bug for e10s a/b testing to aurora and beta.

https://hg.mozilla.org/releases/mozilla-aurora/rev/cb3cd0920e76 https://hg.mozilla.org/releases/mozilla-aurora/rev/4a24c951617a

https://hg.mozilla.org/releases/mozilla-beta/rev/20905912e6e0 https://hg.mozilla.org/releases/mozilla-beta/rev/6adfcf56bd53

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #16) > Felipe, are we doing a11y checks on nightly? I'm asking because this came up > in the channel meeting this morning, as many of the top crashes on nightly > are a11y-related. Yeah, there are checks, but on Windows there's a blacklist of a11y clients known to be problematic, and if it doesn't match that blacklist (like remote desktop), e10s is still enabled together with a11y

Comment on attachment 8692005 [details] [diff] [review] Don't use the blocklist for the a11y check on beta Review of attachment 8692005 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/nsBrowserGlue.js @@ +3396,5 @@ > > observe: function(subject, topic, data) { > if (topic == "a11y-init-or-shutdown" > && data == "1" && > + Services.appinfo.accessibilityEnabled) { nit - you don't need this one, you're in the a11y notification observer.

Approval Request Comment [Feature/regressing bug #]: follow-up here with the question raised in comment 16. [User impact if declined]: The a11y check for e10s would skip some known software that activates a11y but are not really a11y related. For the beta experiment we should really not skip them to avoid related crashes [Describe test coverage new/current, TreeHerder]: landed on central [Risks and why]: Given the changes here we'll need to QA the beta experiment well to ensure everything is working as expected (i.e., e10s can be enabled on beta and can be disabled by a11y tools) [String/UUID change made/needed]: none

Comment on attachment 8692626 [details] [diff] [review] Don't use the blocklist for the a11y check on beta, r=jmathies This change has no impact when e10s is disabled (which is how 43 and 44 will ship) but is required for a Telemetry experiment. Vladan will land this himself after QA has finished verifying that this fix is sufficient for the experiment. Aurora+ Beta+

needinfo for myself to land this after Kamil's testing

https://hg.mozilla.org/releases/mozilla-aurora/rev/c38e578502c0 https://hg.mozilla.org/releases/mozilla-beta/rev/04f89d19556d