Fix cohort name edge cases in the beta 54 multi experiement

RESOLVED FIXED in Firefox 54

Status

()

Firefox
General
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 months ago
The most important problem is that an extension that blocks multi but not e10s (e.g. a bootstrapped extension) will show up as it did for the e10s single experiment, which won't be super clear.
(Assignee)

Comment 1

4 months ago
Created attachment 8859808 [details] [diff] [review]
Fix cohort names in edge cases for the beta experiment

There are a few cases where the cohort names aren't as clear as they could be.
In those cases, we can recuperate the data by looking at a combination of the
prefs that were set, but it would be nice to fix them up.

This also fixes a bug where if the user explicitly opted into e10s (that is,
they set browser.tabs.remote.force-enable) they wouldn't be placed in the
multi experiment.

MozReview-Commit-ID: 5wpfQDvbJJ3
Attachment #8859808 - Flags: review?(felipc)
(Assignee)

Comment 2

4 months ago
Created attachment 8860202 [details] [diff] [review]
Updated patch

After talking to Felipe on IRC, we decided to tweak a couple of things here. In particular, the suffix -test in the cohort will mean "eligible for e10s but disqualified by an addon from multi." -control will mean "selected to not be in e10s at all" and to reduce confusion, we're not going to bother with the cohort prefix for the multi buckets since we don't have different addon policies for multi.
Attachment #8860202 - Flags: review?(felipc)
(Assignee)

Updated

4 months ago
Attachment #8859808 - Attachment is obsolete: true
Attachment #8859808 - Flags: review?(felipc)
Attachment #8860202 - Flags: review?(felipc) → review+

Comment 3

4 months ago
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bc74de54b5b
Put people who forced into e10s into the multi test group. r=Felipe
(Assignee)

Comment 4

4 months ago
Comment on attachment 8860202 [details] [diff] [review]
Updated patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1349363
[User impact if declined]: Some users will not be included in the e10s-multi experiment.
[Is this code covered by automated tests?]: No :(
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: See bug 1352388
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This simply changes a couple of strings and sets a variable that should have been set.
[String changes made/needed]: n/a
Attachment #8860202 - Flags: approval-mozilla-beta?

Comment 5

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4bc74de54b5b
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Updated

4 months ago
status-firefox54: --- → affected

Comment 6

4 months ago
Comment on attachment 8860202 [details] [diff] [review]
Updated patch

This patch is for e10s-multi experiment. Beta54+. Should be in 54 beta 2.
Attachment #8860202 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 7

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/7f717abab712
status-firefox54: affected → fixed
You need to log in before you can comment on or make changes to this bug.