[Extensions] Add a field trial for reporting abuse on uninstall

[Extensions] Add a field trial for reporting abuse on uninstall

Add a field trial, currently disabled (i.e., 100% control group) for allowing
users to report abuse on extension uninstallation.

BUG=441377
Committed: https://crrev.com/627e9db9c59e90bfb2e922cf54388f4aab5369a8
Cr-Commit-Position: refs/heads/master@{#321822}

[Devlin, 2015-03-20 22:42:58]

rdevlin.cronin@chromium.org changed reviewers:
+ finnur@chromium.org

[Devlin, 2015-03-20 22:42:58]

Having never done a FieldTrial before, I think this is all it takes to set one
up - but an extra pair of eyes would be great. :)

[Finnur, 2015-03-23 11:08:55]

It's been a while since I did a field trial and the code looks like it has
changed since, but this looks more or less in line with what the docs state you
should do -- if that's any consolation. :)

LGTM with a couple of comments/question below.

https://codereview.chromium.org/1025613006/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_uninstall_dialog.cc (right):

https://codereview.chromium.org/1025613006/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_uninstall_dialog.cc:140: kControlGroupName,
It is unusual to see a control group named here. Usually people put "Disabled"
or "Default" here. Also, are you sure a control group is not created explicitly
for you (because then you'd have two, right?)

This may be perfectly fine. Just wanted to mention it.

https://codereview.chromium.org/1025613006/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_uninstall_dialog.cc:144: int
experiment_group = trial->AppendGroup(kShowCheckboxGroup, 0);
Don't you want this to be > 0 and remove the TODO?

[Devlin, 2015-03-23 15:43:24]

https://codereview.chromium.org/1025613006/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_uninstall_dialog.cc (right):

https://codereview.chromium.org/1025613006/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_uninstall_dialog.cc:140: kControlGroupName,
On 2015/03/23 11:08:55, Finnur wrote:
> It is unusual to see a control group named here. Usually people put "Disabled"
> or "Default" here. Also, are you sure a control group is not created
explicitly
> for you (because then you'd have two, right?)
> 
> This may be perfectly fine. Just wanted to mention it.

I figured that the default would be the control (since right now they are
synonymous - no checkbox), but maybe it makes more sense to call it "Default" in
case this changes.  (Of course, if it changes, we may then need to add a real
control group.)

https://codereview.chromium.org/1025613006/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_uninstall_dialog.cc:144: int
experiment_group = trial->AppendGroup(kShowCheckboxGroup, 0);
On 2015/03/23 11:08:55, Finnur wrote:
> Don't you want this to be > 0 and remove the TODO? 

Not quite yet - we're still waiting on UI feedback and exact numbers.  Just
instrumenting all this so that it should (hopefully) be a simple "Turn it on"
patch. :)

[Devlin, 2015-03-23 18:46:40]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2015-03-23 18:46:41]

The patchset sent to the CQ was uploaded after l-g-t-m from finnur@chromium.org
Link to the patchset: https://codereview.chromium.org/1025613006/#ps20001
(title: " ")

[commit-bot: I haz the power, 2015-03-23 18:48:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1025613006/20001

[commit-bot: I haz the power, 2015-03-23 18:52:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-23 18:53:35]

Patchset 2 (id:??) landed as
https://crrev.com/627e9db9c59e90bfb2e922cf54388f4aab5369a8
Cr-Commit-Position: refs/heads/master@{#321822}

[Ilya Sherman, 2015-03-25 18:23:10]

isherman@chromium.org changed reviewers:
+ isherman@chromium.org

[Ilya Sherman, 2015-03-25 18:23:11]

Why are you defining this field trial in the client code, rather than via the
Finch server?

[Devlin, 2015-03-25 18:27:11]

On 2015/03/25 18:23:11, Ilya Sherman wrote:
> Why are you defining this field trial in the client code, rather than via the
> Finch server?

Ignorance -- I was under the impression that this was how it was done, and
looking around the codebase, there were other experiments which did the same.  I
take it from your question that this is the incorrect approach?

[Ilya Sherman, 2015-03-25 18:31:58]

On 2015/03/25 18:27:11, Devlin wrote:
> On 2015/03/25 18:23:11, Ilya Sherman wrote:
> > Why are you defining this field trial in the client code, rather than via
the
> > Finch server?
> 
> Ignorance -- I was under the impression that this was how it was done, and
> looking around the codebase, there were other experiments which did the same. 
I
> take it from your question that this is the incorrect approach?

Okay, fair enough.  I'll try to go through and clean up old code that uses this
approach -- I didn't think there were any live trials that did this.

The current best practice is to follow the steps at [ http://go/finch ] :)