[Autofill] Fill from the last digits when filling a phone number with a maximum length.

components/autofill/core/browser/autofill_field.cc

Index: components/autofill/core/browser/autofill_field.cc
diff --git a/components/autofill/core/browser/autofill_field.cc b/components/autofill/core/browser/autofill_field.cc
index 145d7a04dd692d4c82c087076cd385bd6b86fae4..285a5876883491ddf9681cbf8a4fecbcb2905883 100644
--- a/components/autofill/core/browser/autofill_field.cc
+++ b/components/autofill/core/browser/autofill_field.cc
@@ -531,7 +531,8 @@ bool AutofillField::FillFormField(const AutofillField& field,
     return false;
   }
 
-  if (type.GetStorableType() == PHONE_HOME_NUMBER) {
+  if (type.GetStorableType() == PHONE_HOME_NUMBER ||
+      type.GetStorableType() == PHONE_HOME_WHOLE_NUMBER) {
     FillPhoneNumberField(field, value, field_data);
     return true;
   } else if (field_data->form_control_type == "select-one") {
@@ -556,21 +557,28 @@ base::string16 AutofillField::GetPhoneNumberValue(
     const FormFieldData& field_data) {
   // Check to see if the size field matches the "prefix" or "suffix" size.
   // If so, return the appropriate substring.
-  if (number.length() !=
-          PhoneNumber::kPrefixLength + PhoneNumber::kSuffixLength) {
-    return number;
-  }
+  if (number.length() ==

[Mathieu, 2016/01/26 14:43:01]

In which cases will the phone number size be 7? I'm thinking almost never. This
appears to be well intentioned logic that may never work. Let's put a TODO to
investigate if libphonenumber can help here. It can be done in a subsequent CL
since you are just refactoring this block.

[sebsg, 2016/01/26 23:56:26]

Done.

+      PhoneNumber::kPrefixLength + PhoneNumber::kSuffixLength) {
+    if (field.phone_part() == AutofillField::PHONE_PREFIX ||
+        field_data.max_length == PhoneNumber::kPrefixLength) {
+      return number.substr(PhoneNumber::kPrefixOffset,
+                           PhoneNumber::kPrefixLength);
+    }
 
-  if (field.phone_part() == AutofillField::PHONE_PREFIX ||
-      field_data.max_length == PhoneNumber::kPrefixLength) {
-    return
-        number.substr(PhoneNumber::kPrefixOffset, PhoneNumber::kPrefixLength);
+    if (field.phone_part() == AutofillField::PHONE_SUFFIX ||
+        field_data.max_length == PhoneNumber::kSuffixLength) {
+      return number.substr(PhoneNumber::kSuffixOffset,
+                           PhoneNumber::kSuffixLength);
+    }
   }
 
-  if (field.phone_part() == AutofillField::PHONE_SUFFIX ||
-      field_data.max_length == PhoneNumber::kSuffixLength) {
-    return
-        number.substr(PhoneNumber::kSuffixOffset, PhoneNumber::kSuffixLength);
+  // Check to see if the max length of the field matches the "city and number"
+  // size. If |number| exceeds that size, cut the first part to provide a valid
+  // number for the field.
+  if (field_data.max_length == PhoneNumber::kCityAndNumberLength &&

[Mathieu, 2016/01/26 14:43:02]

Why should we check for max_length == 10? It feels like it could be applicable
to other situations such as european phone numbers.

[sebsg, 2016/01/26 23:56:26]

I thought it was safer that way, to only cut the front part when we are pretty
sure it's gonna help. However by looking at the test, I realize that the result
would be bad anyway if we cut the from. However the current way is pretty much
always wrong (cutting the back). We might event catch some nice use case in
Europe like you mentioned now. Thanks

+      number.length() > field_data.max_length) {
+    return number.substr(number.length() - field_data.max_length,
+                         field_data.max_length);
   }
 
   return number;