Add support for autofilling radio buttons and checkboxes.

chrome/browser/autofill/autofill_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/autofill/autofill_manager.h"
 
 #include <stddef.h>
 
 #include <limits>
 #include <map>
@@ skipping 630 matching lines @@
       if (result.fields[i] == field ||
           field_group_type == initiating_group_type) {
         use_variant = variant;
       }
       form_group->FillFormField(*cached_field,
                                 use_variant,
                                 &result.fields[i]);
       // Mark the cached field as autofilled, so that we can detect when a user
       // edits an autofilled field (for metrics).
       form_structure->field(i)->is_autofilled = true;
+    } else {
+      // Check if the field type is one with default value.
+      if (cached_field->type() == FIELD_WITH_DEFAULT_VALUE &&

[Ilya Sherman, 2012/12/01 00:54:12]

nit: Combine this with the else on line 651, as in "else if (...) {"

[Raman Kakilate, 2012/12/06 01:54:05]

Done.

+          cached_field->is_checkable) {
+        string16 default_value = ASCIIToUTF16(cached_field->default_value());

[Ilya Sherman, 2012/12/01 00:54:12]

How do you know that the cached_field's default value is ASCII?

[Raman Kakilate, 2012/12/06 01:54:05]

Changed it to UTF8ToUTF16. I am not entirely sure if this is good enough. Do you
know if the XML response from autofill server is UTF8 encoded ?

[Ilya Sherman, 2012/12/07 01:47:22]

I'm not certain, but it's likely.  Vadim Berman would be the best person to ask.

[Raman Kakilate, 2012/12/10 18:36:45]

Vadim was also not sure. Code says its UTF8. 

In a different CL will be adding a test to server code to assert response is
UTF8 as well.

+        result.fields[i].is_checked = (default_value == result.fields[i].value);

[Ilya Sherman, 2012/12/01 00:54:12]

I'm confused by this line.  Why isn't it just "result.fields[i].value =
default_value"?

[Raman Kakilate, 2012/12/06 01:54:05]

as I understand, for checkboxes and radio buttons - a user cannot change the
values of them, only thing user controls is its checked state.

Idea is maintain default values to mark checked in the autofill server. When the
client sees the values to be checked, client checks if the values of the field
is same as default value, if yes it marks as checked. else, un-checks it.

[Ilya Sherman, 2012/12/07 01:47:22]

If the user can't set the value, why doesn't the server just return a boolean
for whether the field should be checked or not?  I find the current logic quite
confusing...

Moreover, if we need to keep the current logic, it seems like something that the
renderer should be responsible for, rather than having the browser need to get
involved, since the renderer is where the form actually gets filled.

[Raman Kakilate, 2012/12/10 18:36:45]

For a form with radio buttons, eg.

<form>
<input type="radio" name="sex" value="male">Male<br>
<input type="radio" name="sex" value="female">Female
</form>

field signature for both the radio type fields will be same (as the signature is
based on name; radio buttons share common name).

If field mapping uses a boolean checked value, and server wants it clients to
pick "Female" option in the above example, then it would return something like:

(fieldtype: 61, value: false)
(fieldtype: 61, value: true)

This will make the implementation sensitive to order of fields, and if webmaster
decides to change the order of her radio buttons, autofill would fail and would
have to regenerate field mappings. I am trying to avoid it by using value and
doing client side check.

Regarding renderer process or browser process: I was under the impression that
renderer process is mostly handling the view part and decisions are to be made
in the browser process. Please correct me if I am wrong.

[Ilya Sherman, 2012/12/11 05:56:19]

Perhaps this is a reason to include the value in the signature for radio
buttons?  I don't see any backwards compatibility issues that would be worse
than the ones we get from simply adding radio buttons in the first place (i.e.,
that the server will "forget" all that it has learned about any forms that
contain radio buttons).
I don't see how passing the value makes this any different -- the information
content is still essentially the same.  Either way, you're relying on the order
of the fields.  That is, if the server were to return

(fieldtype: 61, value: dont-fill-me)
(fieldtype: 61, value: female)

and the field order were to be reversed, Chrome still wouldn't be able to fill
the form correctly.  Or are you suggesting that the server should return

(fieldtype: 61, value: female)
(fieldtype: 61, value: female)

?  If so, that makes a bit of sense, but I think it would be helpful to add a
comment to this code explaining how this works, as it's not obvious (at least it
wasn't to me).
It depends on what sort of decision is being made.  If the decision is pretty
much just about how to talk to WebKit, then the renderer is a more appropriate
place.  If the decision is more complex than that, then the browser is more
appropriate.  Now that I better understand what you're trying to achieve (i.e.
handling the fact that distinct radio buttons in a single group have the same
signature), I'm more ok with leaving this logic in the browser process.

[Raman Kakilate, 2012/12/11 17:45:45]

In this CL, I am counting radio-buttons/checkboxes towards form signature only
when experiment is on (in a longer run, only for whitelisted sites). When we
count checkable elements towards form signature, we also mark the request to
autofill server as experimental. Based on the marker in the autofill request,
autofill server looks at different SSTable to respond. (all this changes have
been submitted)

This way we wouldn't be breaking any of the existing forms with radio buttons.
Sorry for the confusion, added more comments. Hopefully they address your point.
Ack.

+        form_structure->field(i)->is_autofilled = true;

[Ilya Sherman, 2012/12/01 00:54:12]

nit: Please reproduce the comment from line 648-649 here, assuming that's why we
need this line.

[Raman Kakilate, 2012/12/06 01:54:05]

Done.

+      }
     }
   }
 
   autofilled_form_signatures_.push_front(form_structure->FormSignature());
   // Only remember the last few forms that we've seen, both to avoid false
   // positives and to avoid wasting memory.
   if (autofilled_form_signatures_.size() > kMaxRecentFormSignaturesToRemember)
     autofilled_form_signatures_.pop_back();
 
   host->Send(new AutofillMsg_FormDataFilled(
@@ skipping 653 matching lines @@
   *profile_guid = IDToGUID(profile_id);
 }
 
 void AutofillManager::UpdateInitialInteractionTimestamp(
     const TimeTicks& interaction_timestamp) {
   if (initial_interaction_timestamp_.is_null() ||
       interaction_timestamp < initial_interaction_timestamp_) {
     initial_interaction_timestamp_ = interaction_timestamp;
   }
 }