1275386 - Determine impact of stub installer default browser checkbox

Status: RESOLVED WONTFIX

(Firefox :: Installer, defect, P2)

Description

[Justin Dolske [:Dolske]]

Before we completely remove this checkbox (bug 1272162), I'd like to have a clearer understanding of its direct impact. Usually, the way to measure this kind of thing would be to do an A/B test (eg Funnelcake), and compare the test to a control group. Changes to the stub installer require a full-up build and release, so a Funnelcake repack isn't possible here. Instead, we'd like to do the following, which is basically doing an A/B test in a normal release:

1) The stub installer will hide the checkbox for a small, randomly chosen test cohort. EG:  if(random() < 0.01) { hidden = true; }

2) When the stub installer does this, it will tag the install as part of the test cohort. Probably with a registry key, similar to what was done in bug 1041514.

3) Firefox will record that registry key in opt-out telemetry.

After this hits Release and soaks for a while, we'll run a telemetry analysis to compare user retention, usage hours, etc for the two cohorts. (IE, Win 7/XP users who got the checkbox as normal vs those who didn't as part of this test.)

If there's no significant difference, we should proceed with bug 1272162. If there's a small difference, we should discuss if the qualitative benefits mean we should should still proceed with bug 1272162. This will give us the clearest understanding of the specific impact of what we're doing, and enable us to make an informed decision on deliberate change.

[Verdi suggested we could arrange for a heartbeat survey to see if users who didn't get the checkbox are happier as a result, which would help build the case for such qualitative benefits; good idea, separate bug.]

This is a minimal change to the stub installer, and since Jared's already done similar work in bug 1041514 he'll pick this up.

Comment 1

[Robert Strong (they/them - no direct email)]

Flag me for review on this.

Comment 2

[Robert Strong (they/them - no direct email)]

I don't think this has to ride all of the channels and can be uplifted. Is there agreement on this?

Comment 3

[Robert Strong (they/them - no direct email)]

We've also recompiled the stub installer for funnelcake studies in the past and there should be no reason it can't be done for this study. There are also defines so the two different builds / installer can be differentiated in the stub ping.

http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/defines.nsi.in#6

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #2)
> I don't think this has to ride all of the channels and can be uplifted. Is
> there agreement on this?

Yeah, the plan is to uplift it as there is no string change and we can get the results sooner.

Comment 5

[Robert Strong (they/them - no direct email)]

Just checked with nthomas and we can do a funnelcake study with two stub installers as we have done in the past.

Task list
The telemetry probes would need to be added to Firefox.
The defines would need to be set by releng (I can supply patches as I have done previously) before compiling the stub for each stub installer at
http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/defines.nsi.in#6
The code to add the study ID would need to be added by releng (I can supply patches as I have done previously) to the registry by the stub installer before compiling.
The patch in Bug 1272162 would need to be applied by releng before compiling the stub without the checkbox.
The stub would need to be served to users at 50 (with checkbox) / 50 (without checkbox) as we have done in the past.
We have run these types of studies without a hard end date until we felt comfortable with the amount of data collected. I suggest a week should be enough.

Note: in case it isn't obvious, I'd very much prefer to stick with the funnelcake methodology vs. an ad hoc methodology.

Comment 6

[Robert Strong (they/them - no direct email)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #5)
> Just checked with nthomas and we can do a funnelcake study with two stub
> installers as we have done in the past.
> 
> Task list
> The telemetry probes would need to be added to Firefox.
> The defines would need to be set by releng (I can supply patches as I have
> done previously) before compiling the stub for each stub installer at
> http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/
> defines.nsi.in#6
> The code to add the study ID would need to be added by releng (I can supply
> patches as I have done previously) to the registry by the stub installer
> before compiling.
This isn't needed since it will be in the funnelcake distribution. We might want to add whether the user set as default for the non-modified stub so it can be sent back with telemetry.

> The patch in Bug 1272162 would need to be applied by releng before compiling
> the stub without the checkbox.
> The stub would need to be served to users at 50 (with checkbox) / 50
> (without checkbox) as we have done in the past.
> We have run these types of studies without a hard end date until we felt
> comfortable with the amount of data collected. I suggest a week should be
> enough.
> 
> Note: in case it isn't obvious, I'd very much prefer to stick with the
> funnelcake methodology vs. an ad hoc methodology.

Comment 7

[Justin Dolske [:Dolske]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #2)
> I don't think this has to ride all of the channels and can be uplifted. Is
> there agreement on this?

I think we'd all like to uplift it as much as possible, and it seems low risk.

I assume 47 is too early (since it hits RC in 6 days), leaving 48 as the earliest train to do this.

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #3)
> We've also recompiled the stub installer for funnelcake studies in the past
> and there should be no reason it can't be done for this study. There are
> also defines so the two different builds / installer can be differentiated
> in the stub ping.

Oh, my understanding from bug 1191060 comment 14 / 16 was that this wasn't realistically possible. But if it is, fine! The implementation doesn't matter so long as we can get a reliable analysis between the two groups.

Do we need still need to add telemetry probes for this? If not, maybe this is doable for 47?

Comment 8

[Robert Strong (they/them - no direct email)]

We've done it before and it shouldn't take much. The comparison will be more of an apples to apples comparison and hence the data should be more accurate.

Not sure if we need telemetry probes and we might not if the funnelcake id is already collected. I think it would be a good thing to add telemetry so we have the stub identify systems that set as default via the stub (e.g. already default, can't set as default, opted out, and set as default) and hand that off to the browser to send via telemetry as well as the same or some subset of the same for using the Firefox code for setting as default, etc.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

Review commit: https://reviewboard.mozilla.org/r/56942/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56942/

Comment 10

[Robert Strong (they/them - no direct email)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

https://reviewboard.mozilla.org/r/56942/#review53718

Please separate out the patch I wrote in bug 1272162 from this patch since I should not be r+ing code I wrote.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56942/diff/1-2/

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

https://reviewboard.mozilla.org/r/56942/#review53818

Discussed additional telemetry data that should be captured with jaws over irc which will obsolete this patch

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56942/diff/2-3/

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56942/diff/3-4/

Comment 15

[Robert Strong (they/them - no direct email)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

https://reviewboard.mozilla.org/r/56942/#review55734

::: browser/installer/windows/nsis/stub.nsi:782
(Diff revision 4)
>      SetAutoClose true
>      StrCpy $R9 "2"
>      Call RelativeGotoPage
>  !else
> +
> +    ${LogHeader} "Writing set-as-default status"

I don't think it is valuable to write this to the install log file. Also, the way this is written the first error will cause the other error checks to be true since you don't call ClearErrors.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56942/diff/4-5/

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Note, the only difference between this patch and the one for the funnelcake is adding `true` or `false` to the STUB_INSTALLER_HIDE_DEFAULT_BROWSER_CHECKBOX histogram.

mozreview will show many changes in the interdiff on this patch due to rebasing. This updated patch does not make those changes, it just removes the extra logs and calls ClearError as you requested in comment 15.

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56942/diff/5-6/

Comment 19

[Robert Strong (they/them - no direct email)]

Comment on attachment 8758794 [details]
Bug 1275386 - Report through telemetry if the stub installer default browser checkbox was used and its results.

https://reviewboard.mozilla.org/r/56942/#review59944

Looks fine with the latest changes.

When deploying it would be a good thing (though not required since everything needed will be reported via telemetry) to set FunnelcakeVersion in defines.nsi.in for both stubs used in the experiment.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

We've decided that this isn't useful now since we would want to get at least bug 1286651 fixed first before considering removing this checkbox.

Comment 21

[Robert Strong (they/them - no direct email)]

Is there agreement that bug 1272162 can land after bug 1286651 is fixed?

Comment 22

[Justin Dolske [:Dolske]]

I'd more generally say that we don't want to remove the stub installer checkbox until we have a in-product default browser experience that we're satisfied with (i.e. in terms of both UX and effect on user retention). That may or may not be bug 1286651 specifically, and we need to be sure it's something that will actually ship and not just be temporarily enabled for testing.