Autofill:Autocheckout: Enable Autocheckout for finch experimenting

Autofill:Autocheckout: In addition to "Enable Experimental Form filling" flag, support second method of enabling Autocheckout: using finch experiment. Autocheckout will be enabled if either of the conditions is true. Will remove "Enable experimental form filling" flag, once finch experiment is setup completely.


BUG=230026
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194767

[Raman Kakilate, 2013-04-12 17:46:22]

Albert/Ilya - PTAL. Thanks.

[Albert Bodenhamer, 2013-04-12 17:54:14]

Please attach a more detailed description.  It looks like the intent is to be
able to turn this on either with the existing switch or finch experiment.  That
should be in the description.

Do we still need the switch in chrome://flags?  It seems just just having access
to it from the command line should be enough.

[Raman Kakilate, 2013-04-12 18:05:05]

On 2013/04/12 17:54:14, Albert Bodenhamer wrote:
> Please attach a more detailed description.  It looks like the intent is to be
> able to turn this on either with the existing switch or finch experiment. 
That
> should be in the description.
> 
> Do we still need the switch in chrome://flags?  It seems just just having
access
> to it from the command line should be enough.

Wanted to test the finch experiment before removing it from chrome://flags. Will
be doing it in future CL.

[Ilya Sherman, 2013-04-13 01:12:09]

LGTM

On 2013/04/12 18:05:05, Raman Kakilate wrote:
> On 2013/04/12 17:54:14, Albert Bodenhamer wrote:
> > Please attach a more detailed description.  It looks like the intent is to
be
> > able to turn this on either with the existing switch or finch experiment. 
> That
> > should be in the description.
> > 
> > Do we still need the switch in chrome://flags?  It seems just just having
> access
> > to it from the command line should be enough.
> 
> Wanted to test the finch experiment before removing it from chrome://flags.
Will
> be doing it in future CL.

In general, the recommended approach is to have a tri-state flag, with states
"Default", "Enable", and "Disable".  The field trial should only take effect
when the flag is in the "Default" state.

But, if we don't want external developers to be able to mess around with this
feature for now, then removing the flag is ok.

https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
File components/autofill/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
components/autofill/renderer/form_autofill_util.cc:85: return
base::FieldTrialList::FindFullName("Autocheckout") == "Yes" ||
I'm not sure that all field trials are automatically sent to the renderer
process.  You might want to double-check that this works correctly.

[Raman Kakilate, 2013-04-15 04:37:41]

On 2013/04/13 01:12:09, Ilya Sherman wrote:
> LGTM
> 
> On 2013/04/12 18:05:05, Raman Kakilate wrote:
> > On 2013/04/12 17:54:14, Albert Bodenhamer wrote:
> > > Please attach a more detailed description.  It looks like the intent is to
> be
> > > able to turn this on either with the existing switch or finch experiment. 
> > That
> > > should be in the description.
> > > 
> > > Do we still need the switch in chrome://flags?  It seems just just having
> > access
> > > to it from the command line should be enough.
> > 
> > Wanted to test the finch experiment before removing it from chrome://flags.
> Will
> > be doing it in future CL.
> 
> In general, the recommended approach is to have a tri-state flag, with states
> "Default", "Enable", and "Disable".  The field trial should only take effect
> when the flag is in the "Default" state.
> 
> But, if we don't want external developers to be able to mess around with this
> feature for now, then removing the flag is ok.
> 
>
https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
> File components/autofill/renderer/form_autofill_util.cc (right):
> 
>
https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
> components/autofill/renderer/form_autofill_util.cc:85: return
> base::FieldTrialList::FindFullName("Autocheckout") == "Yes" ||
> I'm not sure that all field trials are automatically sent to the renderer
> process.  You might want to double-check that this works correctly.

I am not aware of testing this locally. Will submit it for now along with server
config and will fix if needed.

[commit-bot: I haz the power, 2013-04-15 04:38:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ramankk@chromium.org/13811045/1

[Alexei Svitkine (slow), 2013-04-15 04:47:44]

On Mon, Apr 15, 2013 at 12:37 AM, <ramankk@chromium.org> wrote:

> On 2013/04/13 01:12:09, Ilya Sherman wrote:
>
>> LGTM
>>
>
>  On 2013/04/12 18:05:05, Raman Kakilate wrote:
>> > On 2013/04/12 17:54:14, Albert Bodenhamer wrote:
>> > > Please attach a more detailed description.  It looks like the intent
>> is to
>> be
>> > > able to turn this on either with the existing switch or finch
>> experiment.
>> > That
>> > > should be in the description.
>> > >
>> > > Do we still need the switch in chrome://flags?  It seems just just
>> having
>> > access
>> > > to it from the command line should be enough.
>> >
>> > Wanted to test the finch experiment before removing it from
>> chrome://flags.
>> Will
>> > be doing it in future CL.
>>
>
>  In general, the recommended approach is to have a tri-state flag, with
>> states
>> "Default", "Enable", and "Disable".  The field trial should only take
>> effect
>> when the flag is in the "Default" state.
>>
>
>  But, if we don't want external developers to be able to mess around with
>> this
>> feature for now, then removing the flag is ok.
>>
>
>
> https://codereview.chromium.**org/13811045/diff/1/**
>
components/autofill/renderer/**form_autofill_util.cc<https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/form_autofill_util.cc>
>
>> File components/autofill/renderer/**form_autofill_util.cc (right):
>>
>
>
> https://codereview.chromium.**org/13811045/diff/1/**
>
components/autofill/renderer/**form_autofill_util.cc#**newcode85<https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/form_autofill_util.cc#newcode85>
>
>> components/autofill/renderer/**form_autofill_util.cc:85: return
>> base::FieldTrialList::**FindFullName("Autocheckout") == "Yes" ||
>> I'm not sure that all field trials are automatically sent to the renderer
>> process.  You might want to double-check that this works correctly.
>>
>
> I am not aware of testing this locally. Will submit it for now along with
> server
> config and will fix if needed.
>

You can test this locally using by passing the command-line
--force-fieldtrials=Autocheckout/Yes/ to Chrome.


>
>
https://codereview.chromium.**org/13811045/<https://codereview.chromium.org/1...
>

[Raman Kakilate, 2013-04-15 05:00:37]

On 2013/04/15 04:47:44, Alexei Svitkine wrote:
> On Mon, Apr 15, 2013 at 12:37 AM, <mailto:ramankk@chromium.org> wrote:
> 
> > On 2013/04/13 01:12:09, Ilya Sherman wrote:
> >
> >> LGTM
> >>
> >
> >  On 2013/04/12 18:05:05, Raman Kakilate wrote:
> >> > On 2013/04/12 17:54:14, Albert Bodenhamer wrote:
> >> > > Please attach a more detailed description.  It looks like the intent
> >> is to
> >> be
> >> > > able to turn this on either with the existing switch or finch
> >> experiment.
> >> > That
> >> > > should be in the description.
> >> > >
> >> > > Do we still need the switch in chrome://flags?  It seems just just
> >> having
> >> > access
> >> > > to it from the command line should be enough.
> >> >
> >> > Wanted to test the finch experiment before removing it from
> >> chrome://flags.
> >> Will
> >> > be doing it in future CL.
> >>
> >
> >  In general, the recommended approach is to have a tri-state flag, with
> >> states
> >> "Default", "Enable", and "Disable".  The field trial should only take
> >> effect
> >> when the flag is in the "Default" state.
> >>
> >
> >  But, if we don't want external developers to be able to mess around with
> >> this
> >> feature for now, then removing the flag is ok.
> >>
> >
> >
> > https://codereview.chromium.**org/13811045/diff/1/**
> >
>
components/autofill/renderer/**form_autofill_util.cc<https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/form_autofill_util.cc>
> >
> >> File components/autofill/renderer/**form_autofill_util.cc (right):
> >>
> >
> >
> > https://codereview.chromium.**org/13811045/diff/1/**
> >
>
components/autofill/renderer/**form_autofill_util.cc#**newcode85<https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/form_autofill_util.cc#newcode85>
> >
> >> components/autofill/renderer/**form_autofill_util.cc:85: return
> >> base::FieldTrialList::**FindFullName("Autocheckout") == "Yes" ||
> >> I'm not sure that all field trials are automatically sent to the renderer
> >> process.  You might want to double-check that this works correctly.
> >>
> >
> > I am not aware of testing this locally. Will submit it for now along with
> > server
> > config and will fix if needed.
> >
> 
> You can test this locally using by passing the command-line
> --force-fieldtrials=Autocheckout/Yes/ to Chrome.
> 
> 
Thanks for the tip Alexei.

Having trouble verifying over NX. removed the commit bit for now. Will test it
and submit.

> >
> >
>
https://codereview.chromium.**org/13811045/%3Chttps://codereview.chromium.org...>
> >

[Alexei Svitkine (slow), 2013-04-15 15:24:13]

https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
File components/autofill/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
components/autofill/renderer/form_autofill_util.cc:85: return
base::FieldTrialList::FindFullName("Autocheckout") == "Yes" ||
On 2013/04/13 01:12:09, Ilya Sherman wrote:
> I'm not sure that all field trials are automatically sent to the renderer
> process.  You might want to double-check that this works correctly.

I believe we only tell the renderer of studies that have had their group value
queried by the browser process. So to make sure its visible in the renderer,
ensure that you do a base::FieldTrialList::FindFullName("Autocheckout") in the
browser prior to that.

[Raman Kakilate, 2013-04-15 15:49:52]

https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
File components/autofill/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/13811045/diff/1/components/autofill/renderer/...
components/autofill/renderer/form_autofill_util.cc:85: return
base::FieldTrialList::FindFullName("Autocheckout") == "Yes" ||
On 2013/04/15 15:24:13, Alexei Svitkine wrote:
> On 2013/04/13 01:12:09, Ilya Sherman wrote:
> > I'm not sure that all field trials are automatically sent to the renderer
> > process.  You might want to double-check that this works correctly.
> 
> I believe we only tell the renderer of studies that have had their group value
> queried by the browser process. So to make sure its visible in the renderer,
> ensure that you do a base::FieldTrialList::FindFullName("Autocheckout") in the
> browser prior to that.

Done. Verified that Autocheckout field trial is in "Yes" state in renderer
process.

[Alexei Svitkine (slow), 2013-04-15 15:54:28]

LGTM

[commit-bot: I haz the power, 2013-04-17 23:24:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ramankk@chromium.org/13811045/19001

[commit-bot: I haz the power, 2013-04-18 04:52:34]

Change committed as 194767