Fix for autofill crash (all systems) - created unit tests for all touched code as well.

BUG=44594
TEST=in the bug: the phone should be *empty* (or shorter than 3 characters) to cause the old crash.
Review URL: http://codereview.chromium.org/2066013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48156 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 141 insertions, 8 deletions

diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index c54d910..f0b3988 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -512,11 +512,15 @@ void AutoFillManager::FillPhoneNumberField(const AutoFillProfile* profile,
   // If we are filling a phone number, check to see if the size field
   // matches the "prefix" or "suffix" sizes and fill accordingly.
   string16 number = profile->GetFieldText(AutoFillType(PHONE_HOME_NUMBER));
-  if (field->size() == kAutoFillPhoneNumberPrefixCount) {
+  bool has_valid_suffix_and_prefix = (number.length() ==
+      (kAutoFillPhoneNumberPrefixCount + kAutoFillPhoneNumberSuffixCount));
+  if (has_valid_suffix_and_prefix &&
+      field->size() == kAutoFillPhoneNumberPrefixCount) {
     number = number.substr(kAutoFillPhoneNumberPrefixOffset,
                            kAutoFillPhoneNumberPrefixCount);
     field->set_value(number);
-  } else if (field->size() == kAutoFillPhoneNumberSuffixCount) {
+  } else if (has_valid_suffix_and_prefix &&
+             field->size() == kAutoFillPhoneNumberSuffixCount) {
     number = number.substr(kAutoFillPhoneNumberSuffixOffset,
                            kAutoFillPhoneNumberSuffixCount);
     field->set_value(number);
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index dccba87..9d018d5 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -33,6 +33,15 @@ class TestPersonalDataManager : public PersonalDataManager {
 
   virtual void InitializeIfNeeded() {}
 
+  AutoFillProfile* GetLabeledProfile(const char* label) {
+    for (std::vector<AutoFillProfile *>::iterator it = web_profiles_.begin();
+         it != web_profiles_.end(); ++it) {
+       if (!(*it)->Label().compare(ASCIIToUTF16(label)))
+         return *it;
+    }
+    return NULL;
+  }
+
  private:
   void CreateTestAutoFillProfiles(ScopedVector<AutoFillProfile>* profiles) {
     AutoFillProfile* profile = new AutoFillProfile;
@@ -83,6 +92,10 @@ class TestAutoFillManager : public AutoFillManager {
 
   virtual bool IsAutoFillEnabled() const { return true; }
 
+  AutoFillProfile* GetLabeledProfile(const char* label) {
+    return test_personal_data_.GetLabeledProfile(label);
+  }
+
  private:
   TestPersonalDataManager test_personal_data_;
 
@@ -451,6 +464,71 @@ TEST_F(AutoFillManagerTest, FillCreditCardForm) {
   EXPECT_TRUE(field.StrictlyEqualsHack(results.fields[14]));
 }
 
+TEST_F(AutoFillManagerTest, FillPhoneNumberTest) {
+  FormData form;
+
+  form.name = ASCIIToUTF16("MyPhoneForm");
+  form.method = ASCIIToUTF16("POST");
+  form.origin = GURL("http://myform.com/phone_form.html");
+  form.action = GURL("http://myform.com/phone_submit.html");
+
+  webkit_glue::FormField field;
+
+  CreateTestFormField("country code", "country code", "", "text", &field);
+  field.set_size(1);
+  form.fields.push_back(field);
+  CreateTestFormField("area code", "area code", "", "text", &field);
+  field.set_size(3);
+  form.fields.push_back(field);
+  CreateTestFormField("phone", "phone prefix", "1", "text", &field);
+  field.set_size(3);
+  form.fields.push_back(field);
+  CreateTestFormField("-", "phone suffix", "", "text", &field);
+  field.set_size(4);
+  form.fields.push_back(field);
+  CreateTestFormField("Phone Extension", "ext", "", "text", &field);
+  field.set_size(3);
+  form.fields.push_back(field);
+
+  // Set up our FormStructures.
+  std::vector<FormData> forms;
+  forms.push_back(form);
+  autofill_manager_->FormsSeen(forms);
+
+  AutoFillProfile *work_profile = autofill_manager_->GetLabeledProfile("Work");
+  EXPECT_TRUE(work_profile != NULL);
+  const AutoFillType phone_type(PHONE_HOME_NUMBER);
+  string16 saved_phone = work_profile->GetFieldText(phone_type);
+
+  char test_data[] = "1234567890123456";
+  for (int i = arraysize(test_data) - 1; i >= 0; --i) {
+    test_data[i] = 0;
+    work_profile->SetInfo(phone_type, ASCIIToUTF16(test_data));
+    // The page ID sent to the AutoFillManager from the RenderView, used to send
+    // an IPC message back to the renderer.
+    int page_id = 100 - i;
+    process()->sink().ClearMessages();
+    EXPECT_TRUE(autofill_manager_->FillAutoFillFormData(page_id,
+         form,
+         ASCIIToUTF16(test_data),
+         ASCIIToUTF16("Work")));
+    page_id = 0;
+    FormData results;
+    EXPECT_TRUE(GetAutoFillFormDataFilledMessage(&page_id, &results));
+
+    if (i != 7) {
+      EXPECT_EQ(ASCIIToUTF16(test_data), results.fields[2].value());
+      EXPECT_EQ(ASCIIToUTF16(test_data), results.fields[3].value());
+    } else {
+      // The only size that is parsed and split, right now is 7:
+      EXPECT_EQ(ASCIIToUTF16("123"), results.fields[2].value());
+      EXPECT_EQ(ASCIIToUTF16("4567"), results.fields[3].value());
+    }
+  }
+
+  work_profile->SetInfo(phone_type, saved_phone);
+}
+
 TEST_F(AutoFillManagerTest, FillCreditCardFormWithBilling) {
   FormData form;
   CreateTestFormDataBilling(&form);
diff --git a/chrome/browser/autofill/phone_number.cc b/chrome/browser/autofill/phone_number.cc
index 2f7557d..a32bb57 100644
--- a/chrome/browser/autofill/phone_number.cc
+++ b/chrome/browser/autofill/phone_number.cc
@@ -228,21 +228,21 @@ bool PhoneNumber::FindInfoMatchesHelper(const FieldTypeSubGroup& subgroup,
 }
 
 bool PhoneNumber::IsNumber(const string16& text) const {
-  if (text.length() <= number_.length())
+  if (text.length() > number_.length())
     return false;
 
   return StartsWith(number_, text, false);
 }
 
 bool PhoneNumber::IsCityCode(const string16& text) const {
-  if (text.length() <= city_code_.length())
+  if (text.length() > city_code_.length())
     return false;
 
   return StartsWith(city_code_, text, false);
 }
 
 bool PhoneNumber::IsCountryCode(const string16& text) const {
-  if (text.length() <= country_code_.length())
+  if (text.length() > country_code_.length())
     return false;
 
   return StartsWith(country_code_, text, false);
diff --git a/chrome/browser/autofill/phone_number.h b/chrome/browser/autofill/phone_number.h
index 60e5c96..da6f051 100644
--- a/chrome/browser/autofill/phone_number.h
+++ b/chrome/browser/autofill/phone_number.h
@@ -39,6 +39,9 @@ class PhoneNumber : public FormGroup {
   explicit PhoneNumber(const PhoneNumber& phone_number);
 
  private:
+  // For test.
+  friend class PhoneNumberTest;
+
   void operator=(const PhoneNumber& phone_number);
 
   const string16& country_code() const { return country_code_; }
diff --git a/chrome/browser/autofill/phone_number_unittest.cc b/chrome/browser/autofill/phone_number_unittest.cc
index 878031f..be61665 100644
--- a/chrome/browser/autofill/phone_number_unittest.cc
+++ b/chrome/browser/autofill/phone_number_unittest.cc
@@ -2,13 +2,37 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/autofill/home_phone_number.h"
 #include "chrome/browser/autofill/phone_number.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-namespace {
+class PhoneNumberTest : public testing::Test {
+ public:
+  PhoneNumberTest() {}
+
+  void set_whole_number(PhoneNumber* phone_number,
+                        const string16& whole_number) {
+    phone_number->set_whole_number(whole_number);
+  }
+
+  bool IsNumber(PhoneNumber* phone_number, const string16& text) const {
+    return phone_number->IsNumber(text);
+  }
+
+  bool IsCityCode(PhoneNumber* phone_number, const string16& text) const {
+    return phone_number->IsCityCode(text);
+  }
+
+  bool IsCountryCode(PhoneNumber* phone_number, const string16& text) const {
+    return phone_number->IsCountryCode(text);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PhoneNumberTest);
+};
 
 // Tests the phone number parser.
-TEST(PhoneNumberTest, Parser) {
+TEST_F(PhoneNumberTest, Parser) {
   string16 number;
   string16 city_code;
   string16 country_code;
@@ -94,5 +118,29 @@ TEST(PhoneNumberTest, Parser) {
   EXPECT_EQ(ASCIIToUTF16("01"), country_code);
 }
 
-}  // namespace
+TEST_F(PhoneNumberTest, Matcher) {
+  string16 phone(ASCIIToUTF16("121231234567"));
+  HomePhoneNumber phone_number;
+  set_whole_number(&phone_number, phone);
+  // Phone number is now country_code == 12, city_code = 123, number = 1234567.
+  char test_number[] = "1234567890";
+  for (int i = arraysize(test_number) - 1; i >= 0; --i) {
+    test_number[i] = 0;  // Cut the string.
+    if (i > 7) {
+      EXPECT_FALSE(IsNumber(&phone_number, ASCIIToUTF16(test_number)));
+    } else {
+      EXPECT_TRUE(IsNumber(&phone_number, ASCIIToUTF16(test_number)));
+    }
+    if (i > 3) {
+      EXPECT_FALSE(IsCityCode(&phone_number, ASCIIToUTF16(test_number)));
+    } else {
+      EXPECT_TRUE(IsCityCode(&phone_number, ASCIIToUTF16(test_number)));
+    }
+    if (i > 2) {
+      EXPECT_FALSE(IsCountryCode(&phone_number, ASCIIToUTF16(test_number)));
+    } else {
+      EXPECT_TRUE(IsCountryCode(&phone_number, ASCIIToUTF16(test_number)));
+    }
+  }
+}