Autofill/rAc: dispatch "input"/"change" on <input>, <textarea>, and <select>

Autofill/rAc: dispatch "input"/"change" on <input>, <textarea>, and <select>
elements when autofilled.

This more closely matches the spec behavior[1]:

"""
When the user agent is to change an input element's value on behalf of the user
(e.g. as part of a form prefilling feature), the user agent must queue a task to
first update the value accordingly, then fire a simple event that bubbles named
input at the input element, then fire a simple event that bubbles named change
at the input element.
"""

BUG=353691, 354257
R=isherman@chromium.org

[1] http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#event-input-input

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

[Dan Beam, 2014-03-28 01:38:25]

depends on https://codereview.chromium.org/215903003/

[Ilya Sherman, 2014-03-28 21:12:44]

Didn't look closely because your repo is out-of-date.  Please sync and rebase,
then I'll take another look.

https://codereview.chromium.org/214823011/diff/40001/components/autofill/cont...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/214823011/diff/40001/components/autofill/cont...
components/autofill/content/renderer/form_autofill_util.cc:549:
field->setValue(data.value, true);
Your repo is out of date.  Please sync and rebase.

[Dan Beam, 2014-03-28 23:03:57]

https://codereview.chromium.org/214823011/diff/40001/components/autofill/cont...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/214823011/diff/40001/components/autofill/cont...
components/autofill/content/renderer/form_autofill_util.cc:549:
field->setValue(data.value, true);
On 2014/03/28 21:12:44, Ilya Sherman wrote:
> Your repo is out of date.  Please sync and rebase.

Done.

[Ilya Sherman, 2014-03-29 00:07:06]

https://codereview.chromium.org/214823011/diff/80001/chrome/browser/autofill/...
File chrome/browser/autofill/autofill_interactive_uitest.cc (right):

https://codereview.chromium.org/214823011/diff/80001/chrome/browser/autofill/...
chrome/browser/autofill/autofill_interactive_uitest.cc:493:
IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, OnChangeAfterAutofill) {
Could you also add a test for oninput?

https://codereview.chromium.org/214823011/diff/80001/chrome/browser/autofill/...
chrome/browser/autofill/autofill_interactive_uitest.cc:496: const char*
kOnChangeScript =
nit: While you're here... can you update this to be "const char
kOnChangeScript[]"?

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/form_autofill_util.cc:546:
field->setValue(data.value, true);
I noticed that you've removed the check "if (field->value() != value)".  Does
that happen automatically now?

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
File components/autofill/content/renderer/password_autofill_agent.cc (left):

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/password_autofill_agent.cc:175: }
Why is this function safe to remove?  Does setAutofilled() now always dispatch a
change event?

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/password_autofill_agent.cc:841: // element
if a user gesture has not yet occured.
How is this case handled now?  We shouldn't trigger JS events when the user
simply previews the field.

[Dan Beam, 2014-03-29 00:51:14]

https://codereview.chromium.org/214823011/diff/80001/chrome/browser/autofill/...
File chrome/browser/autofill/autofill_interactive_uitest.cc (right):

https://codereview.chromium.org/214823011/diff/80001/chrome/browser/autofill/...
chrome/browser/autofill/autofill_interactive_uitest.cc:493:
IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, OnChangeAfterAutofill) {
On 2014/03/29 00:07:07, Ilya Sherman wrote:
> Could you also add a test for oninput?

Done.

https://codereview.chromium.org/214823011/diff/80001/chrome/browser/autofill/...
chrome/browser/autofill/autofill_interactive_uitest.cc:496: const char*
kOnChangeScript =
On 2014/03/29 00:07:07, Ilya Sherman wrote:
> nit: While you're here... can you update this to be "const char
> kOnChangeScript[]"?

Done.

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/form_autofill_util.cc:546:
field->setValue(data.value, true);
On 2014/03/29 00:07:07, Ilya Sherman wrote:
> I noticed that you've removed the check "if (field->value() != value)".  Does
> that happen automatically now?

yes

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
File components/autofill/content/renderer/password_autofill_agent.cc (left):

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/password_autofill_agent.cc:175: }
On 2014/03/29 00:07:07, Ilya Sherman wrote:
> Why is this function safe to remove?  Does setAutofilled() now always dispatch
a
> change event?

no, passing true to setValue() dispatches a change

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/password_autofill_agent.cc:815:
SetElementAutofilled(username_element, true);
^ this previously could've attempted to send 2 change events (but there's logic
in blink to stop this)

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/password_autofill_agent.cc:841: // element
if a user gesture has not yet occured.
On 2014/03/29 00:07:07, Ilya Sherman wrote:
> How is this case handled now?  We shouldn't trigger JS events when the user
> simply previews the field.

setAutofilled() doesn't dispatch a change

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/password_autofill_agent.cc:290: //
outdated.
^ there should already be a change dispatched in this case

https://codereview.chromium.org/214823011/diff/80001/components/autofill/cont...
components/autofill/content/renderer/password_autofill_agent.cc:296:
password.setValue(base::string16(), true);
sends change ^ (when appropriate)

[Ilya Sherman, 2014-03-29 00:58:19]

I see, thanks for 'splainin' stuff.  LGTM.

[Ilya Sherman, 2014-03-29 00:58:24]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2014-03-29 00:59:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/214823011/100001

[commit-bot: I haz the power, 2014-03-29 02:27:24]

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

[commit-bot: I haz the power, 2014-03-29 02:27:25]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[Dan Beam, 2014-03-29 04:27:15]

the tests are failing because this blink rev [1] needs to be rolled in first.

[1] https://src.chromium.org/viewvc/blink?revision=170393&view=revision

[Dan Beam, 2014-03-30 22:00:57]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2014-03-30 22:01:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/214823011/110001

[commit-bot: I haz the power, 2014-03-31 00:43:12]

Change committed as 260474