support CC expiration dates in imperative autocomplete dialog.

support CC expiration dates in imperative autocomplete dialog.

BUG=157270, 157273
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170796

[Evan Stade, 2012-11-29 04:53:50]

isherman: autofill
sky: views

[sky, 2012-11-29 16:30:24]

LGTM if you want to wait on making tf/combobox the same height.

https://codereview.chromium.org/11428071/diff/9001/chrome/browser/ui/views/au...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://codereview.chromium.org/11428071/diff/9001/chrome/browser/ui/views/au...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:341: // TODO(estade):
TextFields and Comboboxes need to be the same height.
You should be able to do that by specifying a vertical alignment of FILL.

[Evan Stade, 2012-11-29 19:50:52]

https://codereview.chromium.org/11428071/diff/9001/chrome/browser/ui/views/au...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://codereview.chromium.org/11428071/diff/9001/chrome/browser/ui/views/au...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:341: // TODO(estade):
TextFields and Comboboxes need to be the same height.
On 2012/11/29 16:30:24, sky wrote:
> You should be able to do that by specifying a vertical alignment of FILL.

yea, but that would only fix comboboxes and textfields on the same line, right?
I think it will look weird if all textfields aren't the same height.

[Ilya Sherman, 2012-11-30 01:01:32]

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/autofill/fo...
File chrome/browser/autofill/form_structure.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/autofill/fo...
chrome/browser/autofill/form_structure.cc:194: return
CREDIT_CARD_VERIFICATION_CODE;
You'll need to update
chrome/test/data/autofill/heuristics/output/01_autocomplete_attribute_basic.out
to reflect this change.

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_comboboxes.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_comboboxes.cc:34: int index) {
nit: Looks like this will fit on the previous line.

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_comboboxes.cc:54:
YearComboboxModel::YearComboboxModel() : this_year_(0) {
nit: No need to init this_year_ here since you init it within the body.

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_controller.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller.cc:21: // Returns true if
|input| should be shown when |field| has been requeted.
nit: "requeted" -> "requested"

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller.cc:297:
cc_exp_month_combobox_model_.reset(new MonthComboboxModel());
nit: Why lazy-init and heap-allocate rather than just always having one
allocated during construction, as a direct member of the class (rather than a
pointer)?

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/views/au...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/views/au...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:346:
combobox->SetSelectedIndex(starting_value);
This looks like it will try to use values like "2015" as an index into a
10-element list for the case of credit card expiration years.  Am I
misunderstanding how this works?

[Evan Stade, 2012-11-30 22:55:25]

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/autofill/fo...
File chrome/browser/autofill/form_structure.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/autofill/fo...
chrome/browser/autofill/form_structure.cc:194: return
CREDIT_CARD_VERIFICATION_CODE;
On 2012/11/30 01:01:32, Ilya Sherman wrote:
> You'll need to update
>
chrome/test/data/autofill/heuristics/output/01_autocomplete_attribute_basic.out
> to reflect this change.

Done.

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_comboboxes.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_comboboxes.cc:34: int index) {
On 2012/11/30 01:01:32, Ilya Sherman wrote:
> nit: Looks like this will fit on the previous line.

Done.

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_comboboxes.cc:54:
YearComboboxModel::YearComboboxModel() : this_year_(0) {
On 2012/11/30 01:01:32, Ilya Sherman wrote:
> nit: No need to init this_year_ here since you init it within the body.

I think coverity still complains if you don't init it.

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_controller.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller.cc:21: // Returns true if
|input| should be shown when |field| has been requeted.
On 2012/11/30 01:01:32, Ilya Sherman wrote:
> nit: "requeted" -> "requested"

Done.

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller.cc:297:
cc_exp_month_combobox_model_.reset(new MonthComboboxModel());
On 2012/11/30 01:01:32, Ilya Sherman wrote:
> nit: Why lazy-init and heap-allocate rather than just always having one
> allocated during construction, as a direct member of the class (rather than a
> pointer)?

good point

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/views/au...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://codereview.chromium.org/11428071/diff/1011/chrome/browser/ui/views/au...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:346:
combobox->SetSelectedIndex(starting_value);
On 2012/11/30 01:01:32, Ilya Sherman wrote:
> This looks like it will try to use values like "2015" as an index into a
> 10-element list for the case of credit card expiration years.  Am I
> misunderstanding how this works?

no, you're not.

[Ilya Sherman, 2012-12-01 01:17:37]

LGTM, thanks