Do not autofill element when there is no autofill suggestion from profile

Do not autofill element when there is no autofill suggestion from profile.

R=isherman@chromium.org
BUG=377705
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275417

[ziran.sun, 2014-06-02 17:07:47]

Please review. Thanks!

[Ilya Sherman, 2014-06-02 23:51:00]

Thanks, Ziran!  We should also implement this same behavior for filling the form
(as well as for previewing).

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

https://codereview.chromium.org/309063006/diff/1/components/autofill/content/...
components/autofill/content/renderer/form_autofill_util.cc:589:
field->setSuggestedValue(data.value);
We should only set the suggested value if the field is being autofilled.

https://codereview.chromium.org/309063006/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/autofill_driver.h (right):

https://codereview.chromium.org/309063006/diff/1/components/autofill/core/bro...
components/autofill/core/browser/autofill_driver.h:50: // Returns true if the
renderer is available for communication.
nit: "iff" was correct -- it stands for "if and only if"

https://codereview.chromium.org/309063006/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/309063006/diff/1/components/autofill/core/bro...
components/autofill/core/browser/autofill_manager.cc:561: // assigned to it.
Is there any harm to doing this for all the autofilled fields, rather than only
for <select> fields?

[ziran.sun, 2014-06-03 12:55:09]

Updated code as per review comments. All review comments have been addressed.
Please review. Thanks!

[Ilya Sherman, 2014-06-03 20:39:32]

Thanks, Ziran!

https://codereview.chromium.org/309063006/diff/20001/chrome/renderer/autofill...
File chrome/renderer/autofill/form_autofill_browsertest.cc (right):

https://codereview.chromium.org/309063006/diff/20001/chrome/renderer/autofill...
chrome/renderer/autofill/form_autofill_browsertest.cc:54: bool is_autofilled; 
// Whether the FormFieldData field is autofilled.
It seems like this is always identical to should_be_autofilled.  Do you really
need the new member in the struct, or can you simply reuse the existing member?

https://codereview.chromium.org/309063006/diff/20001/chrome/renderer/autofill...
chrome/renderer/autofill/form_autofill_browsertest.cc:84: "  <SELECT
id=\"select-nonewsuggestion\">"
nit: Perhaps "select-unchanged"?

https://codereview.chromium.org/309063006/diff/20001/components/autofill/core...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/309063006/diff/20001/components/autofill/core...
components/autofill/core/browser/autofill_manager.cc:559:
form_structure->field(i)->is_autofilled = true;
nit: Please leave a blank line after this one.

https://codereview.chromium.org/309063006/diff/20001/components/autofill/core...
components/autofill/core/browser/autofill_manager.cc:561: // it.
nit: Please extend this comment with "This allows the renderer to distinguish
autofilled fields from fields with non-empty values, such as select-one fields."

[ziran.sun, 2014-06-04 10:15:40]

Updated code as per further review comments. All comments have been addressed.
Please review. Thanks!

[Ilya Sherman, 2014-06-04 19:53:51]

LGTM.  Thanks!

[Ilya Sherman, 2014-06-04 19:53:55]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2014-06-04 19:55:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ziran.sun@samsung.com/309063006/30001

[commit-bot: I haz the power, 2014-06-04 23:29:27]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[Ilya Sherman, 2014-06-04 23:30:49]

The CQ bit was unchecked by isherman@chromium.org

[Ilya Sherman, 2014-06-04 23:31:03]

Looks like there are failing tests.  PTAL.

[ziran.sun, 2014-06-05 16:42:44]

On 2014/06/04 23:31:03, Ilya Sherman wrote:
> Looks like there are failing tests.  PTAL.

Ilya, I tried two solutions - Patch Set 4 and Patch Set 5. I wonder what's you
opinion on them. Thanks.

[ziran.sun, 2014-06-05 16:43:39]

Updated code. Please review. Thanks!

[Ilya Sherman, 2014-06-06 04:19:07]

Hi Ziran, I like the solution in patch set 5 better.  That LGTM with the
following nit addressed.  Thanks!

https://codereview.chromium.org/309063006/diff/70001/components/autofill/cont...
File components/autofill/content/browser/request_autocomplete_manager.cc
(right):

https://codereview.chromium.org/309063006/diff/70001/components/autofill/cont...
components/autofill/content/browser/request_autocomplete_manager.cc:79: FormData
form_data = FormData();
nit: No need for the "= FormData()".

[ziran.sun, 2014-06-06 09:06:16]

The CQ bit was checked by ziran.sun@samsung.com

[commit-bot: I haz the power, 2014-06-06 09:07:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ziran.sun@samsung.com/309063006/90001

[commit-bot: I haz the power, 2014-06-06 11:50:32]

Change committed as 275417