[rAC, OSX] Allow controller to validate data.

[rAC, OSX] Allow controller to validate data.

This implements UpdateForErrors, which allows the
AutofillDialogController to initiate form validation. Necessary so
fields can be validated on e.g. instrument change.

BUG=277319
TEST=Use Wallet, use incomplete instrument, see error markers.

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

[groby-ooo-7-16, 2013-12-03 02:28:18]

Trying again, with a new and shiny CL. PTAL

[Ilya Sherman, 2013-12-03 03:51:34]

LGTM, thanks.  (Though, test coverage would be spiffy.)

[groby-ooo-7-16, 2013-12-03 04:06:31]

I take suggestions for a good test.

AutofillSectionContainerTest.ControllerInformsValidity already tests validation
per se.

AutofillDialogControllerTest.UpgradeMinimalAddress tests that UpdateForError is
invoked for minimal addresses.

That leaves testing that CocoaDialog forwards to WindowController forwards to
SectionContainer - which seems both fragile and trivial.

(Landing CL because it fixes a P1 bug, but happy to pick up work on the test if
we can define a good one)

[Ilya Sherman, 2013-12-03 04:07:49]

On 2013/12/03 04:06:31, groby wrote:
> I take suggestions for a good test.
> 
> AutofillSectionContainerTest.ControllerInformsValidity already tests
validation
> per se.
> 
> AutofillDialogControllerTest.UpgradeMinimalAddress tests that UpdateForError
is
> invoked for minimal addresses.
> 
> That leaves testing that CocoaDialog forwards to WindowController forwards to
> SectionContainer - which seems both fragile and trivial.
> 
> (Landing CL because it fixes a P1 bug, but happy to pick up work on the test
if
> we can define a good one)

I was hoping for some sort of platform-neutral test that would have already been
passing for the Views implementation.  Can we expose the appropriate state via
TestableAutofillView somehow?

[commit-bot: I haz the power, 2013-12-03 04:14:52]

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

[groby-ooo-7-16, 2013-12-03 04:30:46]

Well, the only observable effect of this change is to render a red outline.
There's no actual state to be exposed. All UpdateForErrors does is poll the
controller for validity state, reflecting it into the dialog.

We could expose the internals of the actual input fields, but this is
starting to feel really brittle.

As a gut feeling, I'd say the bigger issue is that both controller and view
can trigger a validation event. But I'm not quite sure how to fix that.




On Mon, Dec 2, 2013 at 8:07 PM, <isherman@chromium.org> wrote:

> On 2013/12/03 04:06:31, groby wrote:
>
>> I take suggestions for a good test.
>>
>
>  AutofillSectionContainerTest.ControllerInformsValidity already tests
>>
> validation
>
>> per se.
>>
>
>  AutofillDialogControllerTest.UpgradeMinimalAddress tests that
>> UpdateForError
>>
> is
>
>> invoked for minimal addresses.
>>
>
>  That leaves testing that CocoaDialog forwards to WindowController
>> forwards to
>> SectionContainer - which seems both fragile and trivial.
>>
>
>  (Landing CL because it fixes a P1 bug, but happy to pick up work on the
>> test
>>
> if
>
>> we can define a good one)
>>
>
> I was hoping for some sort of platform-neutral test that would have
> already been
> passing for the Views implementation.  Can we expose the appropriate state
> via
> TestableAutofillView somehow?
>
> https://codereview.chromium.org/100203003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ilya Sherman, 2013-12-03 04:39:20]

On 2013/12/03 04:30:46, groby wrote:
> Well, the only observable effect of this change is to render a red outline.
> There's no actual state to be exposed. All UpdateForErrors does is poll the
> controller for validity state, reflecting it into the dialog.
> 
> We could expose the internals of the actual input fields, but this is
> starting to feel really brittle.
> 
> As a gut feeling, I'd say the bigger issue is that both controller and view
> can trigger a validation event. But I'm not quite sure how to fix that.

Okay.  I'm fine with punting the test rather than writing a brittle one.

> On Mon, Dec 2, 2013 at 8:07 PM, <mailto:isherman@chromium.org> wrote:
> 
> > On 2013/12/03 04:06:31, groby wrote:
> >
> >> I take suggestions for a good test.
> >>
> >
> >  AutofillSectionContainerTest.ControllerInformsValidity already tests
> >>
> > validation
> >
> >> per se.
> >>
> >
> >  AutofillDialogControllerTest.UpgradeMinimalAddress tests that
> >> UpdateForError
> >>
> > is
> >
> >> invoked for minimal addresses.
> >>
> >
> >  That leaves testing that CocoaDialog forwards to WindowController
> >> forwards to
> >> SectionContainer - which seems both fragile and trivial.
> >>
> >
> >  (Landing CL because it fixes a P1 bug, but happy to pick up work on the
> >> test
> >>
> > if
> >
> >> we can define a good one)
> >>
> >
> > I was hoping for some sort of platform-neutral test that would have
> > already been
> > passing for the Views implementation.  Can we expose the appropriate state
> > via
> > TestableAutofillView somehow?
> >
> > https://codereview.chromium.org/100203003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-12-03 08:04:40]

Retried try job too often on win_rel for step(s) browser_tests,
interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-12-03 09:34:15]

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

[commit-bot: I haz the power, 2013-12-03 12:10:24]

Change committed as 238364