[Payments] Add combobox support to editors.

[Payments] Add combobox support to editors.

* Refactored the ValidationDelegate to support Textfield and Combobox.
* Added the COMBOBOX type for editors to specify when returning field definitions.
* Added a constructor to Combobox which takes ownership of its model.
* Added new ValidatingCombobox type which validates on first blur and subsequent changes.
* Re-enable flaky interactive_ui_tests PaymentRequest*CreditCard*Valid

BUG=687601, 691185
TEST=CreditCardEditorViewController unit_tests, PaymentRequest* interactive_ui_tests

Review-Url: https://codereview.chromium.org/2689363004
Cr-Commit-Position: refs/heads/master@{#451005}
Committed: https://chromium.googlesource.com/chromium/src/+/0e33868f81c5c8a299e14c96b1ce6b49ce325ee5

[Mathieu, 2017-02-14 21:21:44]

Patchset #1 (id:1) has been deleted

[Mathieu, 2017-02-14 21:23:21]

Description was changed from

==========
[Payments] Add combobox support to editors.

BUG=687601
==========

to

==========
[Payments] Add combobox support to editors.

BUG=687601
==========

[Mathieu, 2017-02-14 21:23:21]

mathp@chromium.org changed reviewers:
+ sky@chromium.org

[Mathieu, 2017-02-14 21:23:54]

Hi Scott, not a review yet, but see the following two comments because I need
design advice. Thanks

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.h
(right):

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.h:53:
std::unique_ptr<ui::SimpleComboboxModel> exp_month_model_;
Under the current code, it's problematic to have this here because the
controller is destroyed before the Combobox, and ComboboxMenuModel needs a valid
reference to this model in its destructor [1]. I think the controller should be
destroyed after the view (see other comment)

[1]
https://cs.chromium.org/chromium/src/ui/views/controls/combobox/combobox.cc?r...

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/payment_request_dialog_view.cc (right):

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_dialog_view.cc:165: // DO NOT
SUBMIT Note to reviewer: When we are here, I believe the View is
See this comment

[sky, 2017-02-14 23:19:24]

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.h
(right):

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.h:53:
std::unique_ptr<ui::SimpleComboboxModel> exp_month_model_;
On 2017/02/14 21:23:54, Mathieu Perreault wrote:
> Under the current code, it's problematic to have this here because the
> controller is destroyed before the Combobox, and ComboboxMenuModel needs a
valid
> reference to this model in its destructor [1]. I think the controller should
be
> destroyed after the view (see other comment)
> 
> [1]
>
https://cs.chromium.org/chromium/src/ui/views/controls/combobox/combobox.cc?r...

Indeed. You'll have to tie the lifetime of the model to the combobox. Can you
explicitly delete the combobox before the model? If not, you'll need to transfer
ownership to an object that lives as long as the combobox. One option for that
moving ownership to PaymentRequestDialogView as it lasts at least as long as the
combobox. Another option is to use class properties so that the combobox
effectively owns the model. See
https://cs.chromium.org/chromium/src/ui/base/class_property.h?q=classproperty...
. A last option is to add a constructor to Combobox() that takes a
unique_ptr<ComboboxModel>. This is likely best option given others have
encountered this in the past and you won't be the last that wants the combobox
to own the model.

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/payment_request_dialog_view.cc (right):

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_dialog_view.cc:172: // whether
observing the Widget is equivalent to observing the View.
Is the only way you get here if the ViewStack removes an item? Would it make
sense to have more direct linkage, meaning viewstack calls a function on this
class and then deletes the view?
Another option is to add a OnViewDestroyed() function to ViewObserver and call
it from ~View.

A Widget may have any number of Views contained in it, and views can be removed
from the widget at anytime, not just for deletion. So, it doesn't make sense to
try to tie the lifetimes together.

[Mathieu, 2017-02-15 03:10:57]

Patchset #2 (id:40001) has been deleted

[Mathieu, 2017-02-15 03:14:59]

Description was changed from

==========
[Payments] Add combobox support to editors.

BUG=687601
==========

to

==========
[Payments] Add combobox support to editors.

* Refactored the ValidationDelegate to support Textfield and Combobox.
* Added the COMBOBOX type for editors to specify when returning field
definitions.
* Added a constructor to Combobox which takes ownership of its model.
* Added new ValidatingCombobox type which validates on first blur and subsequent
changes.

BUG=687601
TEST=CreditCardEditorViewController unit_tests, PaymentRequest*
interactive_ui_tests
==========

[Mathieu, 2017-02-15 03:14:59]

mathp@chromium.org changed reviewers:
+ rouslan@chromium.org

[Mathieu, 2017-02-15 03:15:57]

+rouslan

This is ready for review.

Rouslan, PTAL at the whole thing.
Scott, per discussion PTAL at the Combobox changes.

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.h
(right):

https://codereview.chromium.org/2689363004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.h:53:
std::unique_ptr<ui::SimpleComboboxModel> exp_month_model_;
On 2017/02/14 23:19:24, sky wrote:
> On 2017/02/14 21:23:54, Mathieu Perreault wrote:
> > Under the current code, it's problematic to have this here because the
> > controller is destroyed before the Combobox, and ComboboxMenuModel needs a
> valid
> > reference to this model in its destructor [1]. I think the controller should
> be
> > destroyed after the view (see other comment)
> > 
> > [1]
> >
>
https://cs.chromium.org/chromium/src/ui/views/controls/combobox/combobox.cc?r...
> 
> Indeed. You'll have to tie the lifetime of the model to the combobox. Can you
> explicitly delete the combobox before the model? If not, you'll need to
transfer
> ownership to an object that lives as long as the combobox. One option for that
> moving ownership to PaymentRequestDialogView as it lasts at least as long as
the
> combobox. Another option is to use class properties so that the combobox
> effectively owns the model. See
>
https://cs.chromium.org/chromium/src/ui/base/class_property.h?q=classproperty...
> . A last option is to add a constructor to Combobox() that takes a
> unique_ptr<ComboboxModel>. This is likely best option given others have
> encountered this in the past and you won't be the last that wants the combobox
> to own the model.

I've picked the last option, as I think that for most models it makes sense to
have the combobox hold on to it. PTAL

https://codereview.chromium.org/2689363004/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
(right):

https://codereview.chromium.org/2689363004/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:107: for
(const auto& field : comboboxes()) {
I'm not a huge fan of repeating this code here. I've thought of having a common
type for ValidatingTextfield and ValidatingCombobox (ValidatingInput?) which
defines value() and invalid() so that I could have one map, and iterate once to
validate. In the end it adds a lot of cruft for just these few repeated lines.
Let me know what you think

[Mathieu, 2017-02-15 12:47:21]

The CQ bit was checked by mathp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 12:47:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 13:48:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 13:48:11]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-02-15 15:59:40]

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
(right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:36: //
Returns the items that are in the expiration month dropdown.
Need to add a comment about why this needs a clock.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:58:
years.push_back(base::UTF8ToUTF16(std::to_string(now_exploded.year + i)));
now_exploded.year is an "int", so let's make both "i" and
"kNumberOfExpirationYears" to be "int" as well to avoid calculation errors when
summing up signed and unsigned integers.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:76:
EditorField::LengthHint::HINT_LONG, true},
What does "true" mean here? Would be good to have a comment, perhaps.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc
(right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc:50:
// current month.
Do we have mocks that show this? I wonder whether UX wants desktop to be
consistent with Android.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validating_combobox.cc (right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validating_combobox.cc:23: if (!was_validated_)
{
Perhaps a better name would be "has_blurred_once_" or "was_blurred_"?

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validating_combobox.h (right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validating_combobox.h:16: explicit
ValidatingCombobox(std::unique_ptr<ui::ComboboxModel> model,
No need for "explicit" keyword with more than one parameter.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validating_combobox.h:33: bool was_validated_ =
false;
Haven't see this done before. Perhaps it's best to init in the constructor,
where delegate_ is also being initialized.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validation_delegate.h (right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validation_delegate.h:16: ValidationDelegate()
{}
Interfaces should not need constructors and DISALLOW_COPY_AND_ASSIGN. Only the
pure methods (=0) and an empty destructor.

https://codereview.chromium.org/2689363004/diff/80001/ui/views/controls/combo...
File ui/views/controls/combobox/combobox.cc (right):

https://codereview.chromium.org/2689363004/diff/80001/ui/views/controls/combo...
ui/views/controls/combobox/combobox.cc:397: : Combobox(model.get(), style) {
: Combobox(model.get(), style), owned_model_(std::move(model)) {

[sky, 2017-02-15 16:50:52]

I only looked at the combobox change, LGTM (modulo rouslan's comment)

[Mathieu, 2017-02-15 17:04:16]

See my response about the combobox constructor (still addressing others)

https://codereview.chromium.org/2689363004/diff/80001/ui/views/controls/combo...
File ui/views/controls/combobox/combobox.cc (right):

https://codereview.chromium.org/2689363004/diff/80001/ui/views/controls/combo...
ui/views/controls/combobox/combobox.cc:397: : Combobox(model.get(), style) {
On 2017/02/15 15:59:39, rouslan wrote:
> : Combobox(model.get(), style), owned_model_(std::move(model)) {

When there is a delegated constructor, initializing other members is not allowed
by our compiler.

[sky, 2017-02-15 17:11:49]

Makes sense.

On Wed, Feb 15, 2017 at 9:04 AM,  <mathp@chromium.org> wrote:
> See my response about the combobox constructor (still addressing others)
>
>
>
https://codereview.chromium.org/2689363004/diff/80001/ui/views/controls/combo...
> File ui/views/controls/combobox/combobox.cc (right):
>
>
https://codereview.chromium.org/2689363004/diff/80001/ui/views/controls/combo...
> ui/views/controls/combobox/combobox.cc:397: : Combobox(model.get(),
> style) {
> On 2017/02/15 15:59:39, rouslan wrote:
>> : Combobox(model.get(), style), owned_model_(std::move(model)) {
>
> When there is a delegated constructor, initializing other members is not
> allowed by our compiler.
>
> https://codereview.chromium.org/2689363004/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mathieu, 2017-02-15 19:53:02]

The CQ bit was checked by mathp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 19:53:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mathieu, 2017-02-15 19:55:06]

Patchset #4 (id:20002) has been deleted

[Mathieu, 2017-02-15 19:55:10]

The CQ bit was checked by mathp@chromium.org to run a CQ dry run

[Mathieu, 2017-02-15 19:55:51]

PTAL

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
(right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:36: //
Returns the items that are in the expiration month dropdown.
On 2017/02/15 15:59:39, rouslan wrote:
> Need to add a comment about why this needs a clock.

Done.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:58:
years.push_back(base::UTF8ToUTF16(std::to_string(now_exploded.year + i)));
On 2017/02/15 15:59:39, rouslan wrote:
> now_exploded.year is an "int", so let's make both "i" and
> "kNumberOfExpirationYears" to be "int" as well to avoid calculation errors
when
> summing up signed and unsigned integers.

Done.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:76:
EditorField::LengthHint::HINT_LONG, true},
On 2017/02/15 15:59:39, rouslan wrote:
> What does "true" mean here? Would be good to have a comment, perhaps.

Done.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc
(right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc:50:
// current month.
On 2017/02/15 15:59:39, rouslan wrote:
> Do we have mocks that show this? I wonder whether UX wants desktop to be
> consistent with Android.

Changed to add the preselected_combobox_model. Will preselect the current month.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validating_combobox.cc (right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validating_combobox.cc:23: if (!was_validated_)
{
On 2017/02/15 15:59:39, rouslan wrote:
> Perhaps a better name would be "has_blurred_once_" or "was_blurred_"?

Done.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validating_combobox.h (right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validating_combobox.h:16: explicit
ValidatingCombobox(std::unique_ptr<ui::ComboboxModel> model,
On 2017/02/15 15:59:39, rouslan wrote:
> No need for "explicit" keyword with more than one parameter.

Done.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validating_combobox.h:33: bool was_validated_ =
false;
On 2017/02/15 15:59:39, rouslan wrote:
> Haven't see this done before. Perhaps it's best to init in the constructor,
> where delegate_ is also being initialized.

Done.

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validation_delegate.h (right):

https://codereview.chromium.org/2689363004/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validation_delegate.h:16: ValidationDelegate()
{}
On 2017/02/15 15:59:39, rouslan wrote:
> Interfaces should not need constructors and DISALLOW_COPY_AND_ASSIGN. Only the
> pure methods (=0) and an empty destructor.

Done.

[commit-bot: I haz the power, 2017-02-15 19:56:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 20:35:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 20:35:15]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Mathieu, 2017-02-15 21:04:38]

Patchset #4 (id:110001) has been deleted

[Mathieu, 2017-02-15 21:04:47]

The CQ bit was checked by mathp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 21:05:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 21:57:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 21:58:00]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[please use gerrit instead, 2017-02-16 14:52:19]

lgtm % comments

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
(right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:38: //
that the first month in the dropdown is the current month.
Update comment please.

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc
(right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc:24:
// Test that if we are in January, the months in the dropdown start at "01".
Throughout this file: please update the test descriptions in comments.

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/preselected_combobox_model.h (right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/preselected_combobox_model.h:9: #include
"ui/base/models/simple_combobox_model.h"
#include <vector>
#include "base/strings/string16.h"

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validation_delegate.h (right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validation_delegate.h:9: #include
"ui/views/controls/textfield/textfield.h"
Please forward-declare Textfield and Combobox instead.

[Mathieu, 2017-02-16 15:44:49]

The CQ bit was checked by mathp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-16 15:45:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mathieu, 2017-02-16 16:00:09]

Thanks. Also re-enabled a flaky test in this patchset

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
(right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:38: //
that the first month in the dropdown is the current month.
On 2017/02/16 14:52:18, rouslan wrote:
> Update comment please.

Done.

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc
(right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller_unittest.cc:24:
// Test that if we are in January, the months in the dropdown start at "01".
On 2017/02/16 14:52:18, rouslan wrote:
> Throughout this file: please update the test descriptions in comments.

Done.

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/preselected_combobox_model.h (right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/preselected_combobox_model.h:9: #include
"ui/base/models/simple_combobox_model.h"
On 2017/02/16 14:52:19, rouslan wrote:
> #include <vector>
> #include "base/strings/string16.h"

Done.

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/validation_delegate.h (right):

https://codereview.chromium.org/2689363004/diff/60002/chrome/browser/ui/views...
chrome/browser/ui/views/payments/validation_delegate.h:9: #include
"ui/views/controls/textfield/textfield.h"
On 2017/02/16 14:52:19, rouslan wrote:
> Please forward-declare Textfield and Combobox instead.

Done.

[Mathieu, 2017-02-16 16:01:06]

Description was changed from

==========
[Payments] Add combobox support to editors.

* Refactored the ValidationDelegate to support Textfield and Combobox.
* Added the COMBOBOX type for editors to specify when returning field
definitions.
* Added a constructor to Combobox which takes ownership of its model.
* Added new ValidatingCombobox type which validates on first blur and subsequent
changes.

BUG=687601
TEST=CreditCardEditorViewController unit_tests, PaymentRequest*
interactive_ui_tests
==========

to

==========
[Payments] Add combobox support to editors.

* Refactored the ValidationDelegate to support Textfield and Combobox.
* Added the COMBOBOX type for editors to specify when returning field
definitions.
* Added a constructor to Combobox which takes ownership of its model.
* Added new ValidatingCombobox type which validates on first blur and subsequent
changes.
* Re-enable flaky interactive_ui_tests PaymentRequest*CreditCard*Valid

BUG=687601, 691185
TEST=CreditCardEditorViewController unit_tests, PaymentRequest*
interactive_ui_tests
==========

[Mathieu, 2017-02-16 16:09:31]

The CQ bit was unchecked by mathp@chromium.org

[Mathieu, 2017-02-16 16:09:32]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2017-02-16 16:09:32]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2689363004/#ps140001
(title: "re-enable test, addressed comments")

[commit-bot: I haz the power, 2017-02-16 16:09:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 17:39:32]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1487261372252960,
"parent_rev": "34eede47d41b73afabd944056f67e6b5e49f2d6f", "commit_rev":
"0e33868f81c5c8a299e14c96b1ce6b49ce325ee5"}

[commit-bot: I haz the power, 2017-02-16 17:40:12]

Description was changed from

==========
[Payments] Add combobox support to editors.

* Refactored the ValidationDelegate to support Textfield and Combobox.
* Added the COMBOBOX type for editors to specify when returning field
definitions.
* Added a constructor to Combobox which takes ownership of its model.
* Added new ValidatingCombobox type which validates on first blur and subsequent
changes.
* Re-enable flaky interactive_ui_tests PaymentRequest*CreditCard*Valid

BUG=687601, 691185
TEST=CreditCardEditorViewController unit_tests, PaymentRequest*
interactive_ui_tests
==========

to

==========
[Payments] Add combobox support to editors.

* Refactored the ValidationDelegate to support Textfield and Combobox.
* Added the COMBOBOX type for editors to specify when returning field
definitions.
* Added a constructor to Combobox which takes ownership of its model.
* Added new ValidatingCombobox type which validates on first blur and subsequent
changes.
* Re-enable flaky interactive_ui_tests PaymentRequest*CreditCard*Valid

BUG=687601, 691185
TEST=CreditCardEditorViewController unit_tests, PaymentRequest*
interactive_ui_tests

Review-Url: https://codereview.chromium.org/2689363004
Cr-Commit-Position: refs/heads/master@{#451005}
Committed:
https://chromium.googlesource.com/chromium/src/+/0e33868f81c5c8a299e14c96b1ce...
==========

[commit-bot: I haz the power, 2017-02-16 17:40:13]

Committed patchset #5 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/0e33868f81c5c8a299e14c96b1ce...