AutoFillManagerTest.GetProfileSuggestionsEmptyValue crashes in official builds

BUG=50537
TEST=In the bug: unit test should work on any brand.

Review URL: http://codereview.chromium.org/3160022

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

3 files changed, 61 insertions, 44 deletions

diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 0d935ce..27113cb 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -99,7 +99,8 @@ const char* kAutoFillLearnMoreUrl =
 AutoFillManager::AutoFillManager(TabContents* tab_contents)
     : tab_contents_(tab_contents),
       personal_data_(NULL),
-      download_manager_(tab_contents_->profile()) {
+      download_manager_(tab_contents_->profile()),
+      disable_download_manager_requests_(false) {
   DCHECK(tab_contents);
 
   // |personal_data_| is NULL when using TestTabContents.
@@ -450,9 +451,10 @@ void AutoFillManager::HandleSubmit() {
 
 void AutoFillManager::UploadFormData() {
   // TODO(georgey): enable upload request when we make sure that our data is in
-  // line with toolbar data:
-  // download_manager_.StartUploadRequest(upload_form_structure_,
-  //                                      form_is_autofilled);
+  // line with toolbar data (bug #52501):
+  // if (!disable_download_manager_requests_)
+  //   download_manager_.StartUploadRequest(upload_form_structure_,
+  //                                        form_is_autofilled);
 }
 
 void AutoFillManager::OnInfoBarClosed(bool should_save) {
@@ -464,14 +466,16 @@ void AutoFillManager::OnInfoBarClosed(bool should_save) {
 AutoFillManager::AutoFillManager()
     : tab_contents_(NULL),
       personal_data_(NULL),
-      download_manager_(NULL) {
+      download_manager_(NULL),
+      disable_download_manager_requests_(false) {
 }
 
 AutoFillManager::AutoFillManager(TabContents* tab_contents,
                                  PersonalDataManager* personal_data)
     : tab_contents_(tab_contents),
       personal_data_(personal_data),
-      download_manager_(NULL) {
+      download_manager_(NULL),
+      disable_download_manager_requests_(false) {
   DCHECK(tab_contents);
 }
 
@@ -772,7 +776,7 @@ void AutoFillManager::ParseForms(
   }
 
   // If none of the forms were parsed, no use querying the server.
-  if (!form_structures_.empty())
+  if (!form_structures_.empty() && !disable_download_manager_requests_)
     download_manager_.StartQueryRequest(form_structures_);
 }
 
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 4858a5e..387e6df 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -94,6 +94,7 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill,
   AutoFillManager();
   AutoFillManager(TabContents* tab_contents,
                   PersonalDataManager* personal_data);
+
   void set_personal_data_manager(PersonalDataManager* personal_data) {
     personal_data_ = personal_data;
   }
@@ -174,6 +175,11 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill,
   static int PackIDs(int cc_id, int profile_id);
   static void UnpackIDs(int id, int* cc_id, int* profile_id);
 
+  // The following function is meant to be called from unit-test only.
+  void set_disable_download_manager_requests(bool value) {
+    disable_download_manager_requests_ = value;
+  }
+
   // The TabContents hosting this AutoFillManager.
   // Weak reference.
   // May not be NULL.
@@ -188,6 +194,11 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill,
   // Handles queries and uploads to AutoFill servers.
   AutoFillDownloadManager download_manager_;
 
+  // Should be set to true in AutoFillManagerTest and other tests, false in
+  // AutoFillDownloadManagerTest and in non-test environment. Is false by
+  // default.
+  bool disable_download_manager_requests_;
+
   // Our copy of the form data.
   ScopedVector<FormStructure> form_structures_;
 
@@ -197,7 +208,7 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill,
   // The InfoBar that asks for permission to store credit card information.
   scoped_ptr<AutoFillCCInfoBarDelegate> cc_infobar_;
 
-  friend class AutoFillManagerTest;
+  friend class TestAutoFillManager;
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillCreditCardForm);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillNonBillingFormSemicolon);
   FRIEND_TEST_ALL_PREFIXES(AutoFillManagerTest, FillBillFormSemicolon);
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index f54f23d..a5e7ab0 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -44,6 +44,7 @@ class TestPersonalDataManager : public PersonalDataManager {
 
   virtual void InitializeIfNeeded() {}
   virtual void SaveImportedFormData() {}
+  virtual bool IsDataLoaded() const { return true; }
 
   AutoFillProfile* GetLabeledProfile(const char* label) {
     for (std::vector<AutoFillProfile *>::iterator it = web_profiles_.begin();
@@ -108,10 +109,14 @@ class TestPersonalDataManager : public PersonalDataManager {
 
 class TestAutoFillManager : public AutoFillManager {
  public:
-  explicit TestAutoFillManager(TabContents* tab_contents)
+  TestAutoFillManager(TabContents* tab_contents,
+                      TestPersonalDataManager* personal_manager)
       : AutoFillManager(tab_contents, NULL) {
-    test_personal_data_ = new TestPersonalDataManager();
-    set_personal_data_manager(test_personal_data_.get());
+    test_personal_data_ = personal_manager;
+    set_personal_data_manager(personal_manager);
+    // Download manager requests are disabled for purposes of this unit-test.
+    // These request are tested in autofill_download_unittest.cc.
+    set_disable_download_manager_requests(true);
   }
 
   virtual bool IsAutoFillEnabled() const { return true; }
@@ -125,7 +130,7 @@ class TestAutoFillManager : public AutoFillManager {
   }
 
  private:
-  scoped_refptr<TestPersonalDataManager> test_personal_data_;
+  TestPersonalDataManager* test_personal_data_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAutoFillManager);
 };
@@ -231,10 +236,18 @@ void CreateTestFormDataBilling(FormData* form) {
 class AutoFillManagerTest : public RenderViewHostTestHarness {
  public:
   AutoFillManagerTest() {}
+  virtual ~AutoFillManagerTest() {
+    // Order of destruction is important as AutoFillManager relies on
+    // PersonalDataManager to be around when it gets destroyed.
+    autofill_manager_.reset(NULL);
+    test_personal_data_ = NULL;
+  }
 
   virtual void SetUp() {
     RenderViewHostTestHarness::SetUp();
-    autofill_manager_.reset(new TestAutoFillManager(contents()));
+    test_personal_data_ = new TestPersonalDataManager();
+    autofill_manager_.reset(new TestAutoFillManager(contents(),
+                                                    test_personal_data_.get()));
   }
 
   Profile* profile() { return contents()->profile(); }
@@ -276,20 +289,13 @@ class AutoFillManagerTest : public RenderViewHostTestHarness {
 
  protected:
   scoped_ptr<TestAutoFillManager> autofill_manager_;
+  scoped_refptr<TestPersonalDataManager> test_personal_data_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AutoFillManagerTest);
 };
 
-// TODO(georgey): All of these tests crash in official
-// builds. http://crbug.com/50537
-#if defined(GOOGLE_CHROME_BUILD)
-#define SKIP_BRANDED(test) DISABLED_##test
-#else
-#define SKIP_BRANDED(test) test
-#endif
-
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetProfileSuggestionsEmptyValue)) {
+TEST_F(AutoFillManagerTest, GetProfileSuggestionsEmptyValue) {
   FormData form;
   CreateTestFormData(&form);
 
@@ -327,7 +333,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetProfileSuggestionsEmptyValue)) {
   EXPECT_EQ(ASCIIToUTF16("123 Apple St."), labels[1]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetProfileSuggestionsMatchCharacter)) {
+TEST_F(AutoFillManagerTest, GetProfileSuggestionsMatchCharacter) {
   FormData form;
   CreateTestFormData(&form);
 
@@ -361,7 +367,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetProfileSuggestionsMatchCharacter)) {
   EXPECT_EQ(ASCIIToUTF16("3734 Elvis Presley Blvd."), labels[0]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsEmptyValue)) {
+TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsEmptyValue) {
   FormData form;
   CreateTestFormDataBilling(&form);
 
@@ -405,8 +411,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsEmptyValue)) {
   EXPECT_EQ(ASCIIToUTF16("*8765"), labels[5]);
 }
 
-TEST_F(AutoFillManagerTest,
-       SKIP_BRANDED(GetCreditCardSuggestionsMatchCharacter)) {
+TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsMatchCharacter) {
   FormData form;
   CreateTestFormDataBilling(&form);
 
@@ -444,7 +449,7 @@ TEST_F(AutoFillManagerTest,
   EXPECT_EQ(ASCIIToUTF16("*3456"), labels[2]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsNonCCNumber)) {
+TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsNonCCNumber) {
   FormData form;
   CreateTestFormDataBilling(&form);
 
@@ -488,7 +493,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsNonCCNumber)) {
   EXPECT_EQ(ASCIIToUTF16("*8765"), labels[5]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsSemicolon)) {
+TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsSemicolon) {
   // |profile| will be owned by the mock PersonalDataManager.
   AutoFillProfile* profile = new AutoFillProfile;
   autofill_unittest::SetProfileInfo(profile, "Home; 8765", "Joe", "", "Ely",
@@ -545,7 +550,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsSemicolon)) {
   EXPECT_EQ(ASCIIToUTF16("Joe Ely; *8765"), labels[7]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsNonHTTPS)) {
+TEST_F(AutoFillManagerTest, GetCreditCardSuggestionsNonHTTPS) {
   FormData form;
   CreateTestFormDataBilling(&form);
   form.origin = GURL("http://myform.com/form.html");
@@ -566,8 +571,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetCreditCardSuggestionsNonHTTPS)) {
       autofill_manager_->GetAutoFillSuggestions(kPageID, false, field));
 }
 
-TEST_F(AutoFillManagerTest,
-       SKIP_BRANDED(GetCombinedAutoFillAndAutocompleteSuggestions)) {
+TEST_F(AutoFillManagerTest, GetCombinedAutoFillAndAutocompleteSuggestions) {
   FormData form;
   CreateTestFormData(&form);
 
@@ -610,7 +614,7 @@ TEST_F(AutoFillManagerTest,
   EXPECT_EQ(string16(), labels[3]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetFieldSuggestionsFormIsAutoFilled)) {
+TEST_F(AutoFillManagerTest, GetFieldSuggestionsFormIsAutoFilled) {
   FormData form;
   CreateTestFormData(&form);
 
@@ -646,8 +650,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetFieldSuggestionsFormIsAutoFilled)) {
   EXPECT_EQ(string16(), labels[1]);
 }
 
-TEST_F(AutoFillManagerTest,
-       SKIP_BRANDED(GetFieldSuggestionsForAutocompleteOnly)) {
+TEST_F(AutoFillManagerTest, GetFieldSuggestionsForAutocompleteOnly) {
   FormData form;
   CreateTestFormData(&form);
 
@@ -683,8 +686,7 @@ TEST_F(AutoFillManagerTest,
   ASSERT_EQ(0U, labels.size());
 }
 
-TEST_F(AutoFillManagerTest,
-       SKIP_BRANDED(GetFieldSuggestionsWithDuplicateValues)) {
+TEST_F(AutoFillManagerTest, GetFieldSuggestionsWithDuplicateValues) {
   FormData form;
   CreateTestFormData(&form);
 
@@ -726,7 +728,7 @@ TEST_F(AutoFillManagerTest,
   EXPECT_EQ(string16(), labels[1]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetBillingSuggestionsAddress1)) {
+TEST_F(AutoFillManagerTest, GetBillingSuggestionsAddress1) {
   FormData form;
   CreateTestFormDataBilling(&form);
 
@@ -762,7 +764,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(GetBillingSuggestionsAddress1)) {
   EXPECT_EQ(ASCIIToUTF16("Charles Hardin Holley; *8765"), labels[1]);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillCreditCardForm)) {
+TEST_F(AutoFillManagerTest, FillCreditCardForm) {
   FormData form;
   CreateTestFormDataBilling(&form);
 
@@ -836,7 +838,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillCreditCardForm)) {
   EXPECT_TRUE(field.StrictlyEqualsHack(results.fields[14]));
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillNonBillingFormSemicolon)) {
+TEST_F(AutoFillManagerTest, FillNonBillingFormSemicolon) {
   // |profile| will be owned by the mock PersonalDataManager.
   AutoFillProfile* profile = new AutoFillProfile;
   autofill_unittest::SetProfileInfo(profile, "Home; 8765", "Joe", "", "Ely",
@@ -907,7 +909,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillNonBillingFormSemicolon)) {
   EXPECT_TRUE(field.StrictlyEqualsHack(results.fields[10]));
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillBillFormSemicolon)) {
+TEST_F(AutoFillManagerTest, FillBillFormSemicolon) {
   // |profile| will be owned by the mock PersonalDataManager.
   AutoFillProfile* profile = new AutoFillProfile;
   autofill_unittest::SetProfileInfo(profile, "Home; 8765", "Joe", "", "Ely",
@@ -991,7 +993,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillBillFormSemicolon)) {
   EXPECT_TRUE(field.StrictlyEqualsHack(results.fields[14]));
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillPhoneNumber)) {
+TEST_F(AutoFillManagerTest, FillPhoneNumber) {
   FormData form;
 
   form.name = ASCIIToUTF16("MyPhoneForm");
@@ -1064,7 +1066,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FillPhoneNumber)) {
   work_profile->SetInfo(phone_type, saved_phone);
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(FormChangesRemoveField)) {
+TEST_F(AutoFillManagerTest, FormChangesRemoveField) {
   FormData form;
   form.name = ASCIIToUTF16("MyForm");
   form.method = ASCIIToUTF16("POST");
@@ -1129,7 +1131,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FormChangesRemoveField)) {
   EXPECT_TRUE(field.StrictlyEqualsHack(results.fields[3]));
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(FormChangesAddField)) {
+TEST_F(AutoFillManagerTest, FormChangesAddField) {
   FormData form;
   form.name = ASCIIToUTF16("MyForm");
   form.method = ASCIIToUTF16("POST");
@@ -1197,7 +1199,7 @@ TEST_F(AutoFillManagerTest, SKIP_BRANDED(FormChangesAddField)) {
   EXPECT_TRUE(field.StrictlyEqualsHack(results.fields[4]));
 }
 
-TEST_F(AutoFillManagerTest, SKIP_BRANDED(HiddenFields)) {
+TEST_F(AutoFillManagerTest, HiddenFields) {
   FormData form;
   form.name = ASCIIToUTF16("MyForm");
   form.method = ASCIIToUTF16("POST");