diff options
author | estade@chromium.org <estade@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-16 22:10:39 +0000 |
---|---|---|
committer | estade@chromium.org <estade@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-16 22:10:39 +0000 |
commit | 8da04b85b1e9c88d636175c01c218adf6dc4b768 (patch) | |
tree | 2ba197d18ba61861da39e2a03dd3a5df8efd9c42 | |
parent | 97525370580abfdde511db31db159aa064b2cbda (diff) | |
download | chromium_src-8da04b85b1e9c88d636175c01c218adf6dc4b768.zip chromium_src-8da04b85b1e9c88d636175c01c218adf6dc4b768.tar.gz chromium_src-8da04b85b1e9c88d636175c01c218adf6dc4b768.tar.bz2 |
retry r168247: more autofill refactoring
makes AutofillProfile and CreditCard better resemble one another. Much code deleted.
BUG=none
fix: revert change to guid() that made it return a reference.
Review URL: https://codereview.chromium.org/11299046
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@168304 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/autofill/autofill_manager.cc | 112 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager.h | 15 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager_unittest.cc | 24 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_profile.cc | 4 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_profile.h | 14 | ||||
-rw-r--r-- | chrome/browser/autofill/credit_card.cc | 24 | ||||
-rw-r--r-- | chrome/browser/autofill/credit_card.h | 12 | ||||
-rw-r--r-- | chrome/browser/autofill/form_group.cc | 11 | ||||
-rw-r--r-- | chrome/browser/autofill/form_group.h | 12 | ||||
-rw-r--r-- | chrome/browser/autofill/personal_data_manager.cc | 36 | ||||
-rw-r--r-- | chrome/browser/autofill/personal_data_manager.h | 7 | ||||
-rw-r--r-- | chrome/browser/autofill/personal_data_manager_unittest.cc | 4 | ||||
-rw-r--r-- | chrome/browser/resources/options/autofill_options.js | 23 | ||||
-rw-r--r-- | chrome/browser/resources/options/autofill_options_list.js | 4 | ||||
-rw-r--r-- | chrome/browser/ui/webui/options/autofill_options_handler.cc | 24 | ||||
-rw-r--r-- | chrome/browser/ui/webui/options/autofill_options_handler.h | 10 |
16 files changed, 137 insertions, 199 deletions
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc index d0ff784..f8888e3 100644 --- a/chrome/browser/autofill/autofill_manager.cc +++ b/chrome/browser/autofill/autofill_manager.cc @@ -507,8 +507,7 @@ void AutofillManager::OnQueryFormFieldAutofill(int query_id, RenderViewHost* host = NULL; FormStructure* form_structure = NULL; AutofillField* autofill_field = NULL; - if (GetHost( - personal_data_->profiles(), personal_data_->credit_cards(), &host) && + if (GetHost(&host) && GetCachedFormAndField(form, field, &form_structure, &autofill_field) && // Don't send suggestions for forms that aren't auto-fillable. form_structure->IsAutofillable(false)) { @@ -583,24 +582,19 @@ void AutofillManager::OnFillAutofillFormData(int query_id, const FormData& form, const FormFieldData& field, int unique_id) { - const std::vector<AutofillProfile*>& profiles = personal_data_->profiles(); - const std::vector<CreditCard*>& credit_cards = personal_data_->credit_cards(); - const AutofillProfile* profile = NULL; - const CreditCard* credit_card = NULL; + const FormGroup* form_group = NULL; size_t variant = 0; RenderViewHost* host = NULL; FormStructure* form_structure = NULL; AutofillField* autofill_field = NULL; - if (!GetProfileOrCreditCard(unique_id, profiles, credit_cards, &profile, - &credit_card, &variant) || - !GetHost(profiles, credit_cards, &host) || + if (!GetProfileOrCreditCard(unique_id, &form_group, &variant) || + !GetHost(&host) || !GetCachedFormAndField(form, field, &form_structure, &autofill_field)) return; DCHECK(host); DCHECK(form_structure); DCHECK(autofill_field); - DCHECK(profile || credit_card); FormData result = form; @@ -610,17 +604,7 @@ void AutofillManager::OnFillAutofillFormData(int query_id, for (std::vector<FormFieldData>::iterator iter = result.fields.begin(); iter != result.fields.end(); ++iter) { if ((*iter) == field) { - AutofillFieldType field_type = autofill_field->type(); - if (profile) { - DCHECK_NE(AutofillType::CREDIT_CARD, - AutofillType(field_type).group()); - profile->FillFormField(*autofill_field, variant, &(*iter)); - } else { - DCHECK_EQ(AutofillType::CREDIT_CARD, - AutofillType(field_type).group()); - credit_card->FillFormField(field_type, &(*iter)); - } - + form_group->FillFormField(*autofill_field, variant, &(*iter)); // Mark the cached field as autofilled, so that we can detect when a // user edits an autofilled field (for metrics). autofill_field->is_autofilled = true; @@ -644,27 +628,22 @@ void AutofillManager::OnFillAutofillFormData(int query_id, DCHECK_EQ(*form_structure->field(i), result.fields[i]); const AutofillField* cached_field = form_structure->field(i); - AutofillFieldType field_type = cached_field->type(); - FieldTypeGroup field_group_type = AutofillType(field_type).group(); + FieldTypeGroup field_group_type = + AutofillType(cached_field->type()).group(); if (field_group_type != AutofillType::NO_GROUP) { - if (profile) { - DCHECK_NE(AutofillType::CREDIT_CARD, field_group_type); - // If the field being filled is either - // (a) the field that the user initiated the fill from, or - // (b) part of the same logical unit, e.g. name or phone number, - // then take the multi-profile "variant" into account. - // Otherwise fill with the default (zeroth) variant. - size_t use_variant = 0; - if (result.fields[i] == field || - field_group_type == initiating_group_type) { - use_variant = variant; - } - profile->FillFormField(*cached_field, use_variant, &result.fields[i]); - } else { - DCHECK_EQ(AutofillType::CREDIT_CARD, field_group_type); - credit_card->FillFormField(field_type, &result.fields[i]); + // If the field being filled is either + // (a) the field that the user initiated the fill from, or + // (b) part of the same logical unit, e.g. name or phone number, + // then take the multi-profile "variant" into account. + // Otherwise fill with the default (zeroth) variant. + size_t use_variant = 0; + 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; @@ -740,13 +719,9 @@ void AutofillManager::OnShowPasswordGenerationPopup( } void AutofillManager::RemoveAutofillProfileOrCreditCard(int unique_id) { - const std::vector<AutofillProfile*>& profiles = personal_data_->profiles(); - const std::vector<CreditCard*>& credit_cards = personal_data_->credit_cards(); - const AutofillProfile* profile = NULL; - const CreditCard* credit_card = NULL; + const FormGroup* form_group = NULL; size_t variant = 0; - if (!GetProfileOrCreditCard(unique_id, profiles, credit_cards, &profile, - &credit_card, &variant)) { + if (!GetProfileOrCreditCard(unique_id, &form_group, &variant)) { NOTREACHED(); return; } @@ -757,10 +732,7 @@ void AutofillManager::RemoveAutofillProfileOrCreditCard(int unique_id) { if (variant != 0) return; - if (profile) - personal_data_->RemoveProfile(profile->guid()); - else - personal_data_->RemoveCreditCard(credit_card->guid()); + personal_data_->RemoveByGUID(form_group->GetGUID()); } void AutofillManager::RemoveAutocompleteEntry(const string16& name, @@ -944,15 +916,15 @@ void AutofillManager::set_metric_logger(const AutofillMetrics* metric_logger) { metric_logger_.reset(metric_logger); } -bool AutofillManager::GetHost(const std::vector<AutofillProfile*>& profiles, - const std::vector<CreditCard*>& credit_cards, - RenderViewHost** host) const { +bool AutofillManager::GetHost(RenderViewHost** host) const { if (!IsAutofillEnabled()) return false; // No autofill data to return if the profiles are empty. - if (profiles.empty() && credit_cards.empty()) + if (personal_data_->profiles().empty() && + personal_data_->credit_cards().empty()) { return false; + } *host = web_contents()->GetRenderViewHost(); if (!*host) @@ -963,10 +935,7 @@ bool AutofillManager::GetHost(const std::vector<AutofillProfile*>& profiles, bool AutofillManager::GetProfileOrCreditCard( int unique_id, - const std::vector<AutofillProfile*>& profiles, - const std::vector<CreditCard*>& credit_cards, - const AutofillProfile** profile, - const CreditCard** credit_card, + const FormGroup** form_group, size_t* variant) const { // Unpack the |unique_id| into component parts. GUIDPair credit_card_guid; @@ -977,33 +946,22 @@ bool AutofillManager::GetProfileOrCreditCard( // Find the profile that matches the |profile_guid|, if one is specified. if (base::IsValidGUID(profile_guid.first)) { - for (std::vector<AutofillProfile*>::const_iterator iter = profiles.begin(); - iter != profiles.end(); ++iter) { - if ((*iter)->guid() == profile_guid.first) { - *profile = *iter; - break; - } - } - DCHECK(*profile); - + *form_group = personal_data_->GetProfileByGUID(profile_guid.first); + DCHECK(*form_group); *variant = profile_guid.second; + return true; } // Find the credit card that matches the |credit_card_guid|, if specified. if (base::IsValidGUID(credit_card_guid.first)) { - for (std::vector<CreditCard*>::const_iterator iter = credit_cards.begin(); - iter != credit_cards.end(); ++iter) { - if ((*iter)->guid() == credit_card_guid.first) { - *credit_card = *iter; - break; - } - } - DCHECK(*credit_card); - + *form_group = + personal_data_->GetCreditCardByGUID(credit_card_guid.first); + DCHECK(*form_group); *variant = credit_card_guid.second; + return true; } - return (*profile) || (*credit_card); + return false; } bool AutofillManager::FindCachedForm(const FormData& form, diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h index 5dc0fcc..6c332b0 100644 --- a/chrome/browser/autofill/autofill_manager.h +++ b/chrome/browser/autofill/autofill_manager.h @@ -32,6 +32,7 @@ class AutofillField; class AutofillProfile; class AutofillMetrics; class CreditCard; +class FormGroup; class PersonalDataManager; class PrefService; class ProfileSyncService; @@ -229,17 +230,13 @@ class AutofillManager : public content::WebContentsObserver, // Fills |host| with the RenderViewHost for this tab. // Returns false if Autofill is disabled or if the host is unavailable. - bool GetHost(const std::vector<AutofillProfile*>& profiles, - const std::vector<CreditCard*>& credit_cards, - content::RenderViewHost** host) const WARN_UNUSED_RESULT; + bool GetHost(content::RenderViewHost** host) const WARN_UNUSED_RESULT; - // Unpacks |unique_id| and fills |profile| or |credit_card| with the - // appropriate data source. Returns false if the unpacked id cannot be found. + // Unpacks |unique_id| and fills |form_group| and |variant| with the + // appropriate data source and variant index. Returns false if the unpacked + // id cannot be found. bool GetProfileOrCreditCard(int unique_id, - const std::vector<AutofillProfile*>& profiles, - const std::vector<CreditCard*>& credit_cards, - const AutofillProfile** profile, - const CreditCard** credit_card, + const FormGroup** form_group, size_t* variant) const WARN_UNUSED_RESULT; // Fills |form_structure| cached element corresponding to |form|. diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc index 9da139f..e355947 100644 --- a/chrome/browser/autofill/autofill_manager_unittest.cc +++ b/chrome/browser/autofill/autofill_manager_unittest.cc @@ -106,20 +106,20 @@ class TestPersonalDataManager : public PersonalDataManager { credit_cards_.push_back(credit_card); } - virtual void RemoveProfile(const std::string& guid) OVERRIDE { - AutofillProfile* profile = GetProfileWithGUID(guid.c_str()); - - web_profiles_.erase( - std::remove(web_profiles_.begin(), web_profiles_.end(), profile), - web_profiles_.end()); - } - - virtual void RemoveCreditCard(const std::string& guid) OVERRIDE { + virtual void RemoveByGUID(const std::string& guid) OVERRIDE { CreditCard* credit_card = GetCreditCardWithGUID(guid.c_str()); + if (credit_card) { + credit_cards_.erase( + std::remove(credit_cards_.begin(), credit_cards_.end(), credit_card), + credit_cards_.end()); + } - credit_cards_.erase( - std::remove(credit_cards_.begin(), credit_cards_.end(), credit_card), - credit_cards_.end()); + AutofillProfile* profile = GetProfileWithGUID(guid.c_str()); + if (profile) { + web_profiles_.erase( + std::remove(web_profiles_.begin(), web_profiles_.end(), profile), + web_profiles_.end()); + } } void ClearAutofillProfiles() { diff --git a/chrome/browser/autofill/autofill_profile.cc b/chrome/browser/autofill/autofill_profile.cc index fd54c4d..ff7356c 100644 --- a/chrome/browser/autofill/autofill_profile.cc +++ b/chrome/browser/autofill/autofill_profile.cc @@ -253,6 +253,10 @@ AutofillProfile& AutofillProfile::operator=(const AutofillProfile& profile) { return *this; } +std::string AutofillProfile::GetGUID() const { + return guid(); +} + void AutofillProfile::GetMatchingTypes(const string16& text, FieldTypeSet* matching_types) const { FormGroupList info = FormGroups(); diff --git a/chrome/browser/autofill/autofill_profile.h b/chrome/browser/autofill/autofill_profile.h index 927a3c6..9cdd5b7 100644 --- a/chrome/browser/autofill/autofill_profile.h +++ b/chrome/browser/autofill/autofill_profile.h @@ -20,7 +20,6 @@ #include "chrome/browser/autofill/form_group.h" #include "chrome/browser/autofill/phone_number.h" -class AutofillField; struct FormFieldData; // A collection of FormGroups stored in a profile. AutofillProfile also @@ -39,6 +38,7 @@ class AutofillProfile : public FormGroup { AutofillProfile& operator=(const AutofillProfile& profile); // FormGroup: + virtual std::string GetGUID() const OVERRIDE; virtual void GetMatchingTypes(const string16& text, FieldTypeSet* matching_types) const OVERRIDE; virtual string16 GetRawInfo(AutofillFieldType type) const OVERRIDE; @@ -47,6 +47,9 @@ class AutofillProfile : public FormGroup { virtual string16 GetCanonicalizedInfo(AutofillFieldType type) const OVERRIDE; virtual bool SetCanonicalizedInfo(AutofillFieldType type, const string16& value) OVERRIDE; + virtual void FillFormField(const AutofillField& field, + size_t variant, + FormFieldData* field_data) const OVERRIDE; // Multi-value equivalents to |GetInfo| and |SetInfo|. void SetMultiInfo(AutofillFieldType type, @@ -56,13 +59,6 @@ class AutofillProfile : public FormGroup { void GetCanonicalizedMultiInfo(AutofillFieldType type, std::vector<string16>* values) const; - // Set |field_data|'s value based on |field|'s type and contents of the - // |this|. The |variant| parameter specifies which value to use from a - // multi-valued profile. - void FillFormField(const AutofillField& field, - size_t variant, - FormFieldData* field_data) const; - // Set |field_data|'s value for phone number based on contents of |this|. // The |field| specifies the type of the phone and whether this is a // phone prefix or suffix. The |variant| parameter specifies which value in a @@ -77,6 +73,8 @@ class AutofillProfile : public FormGroup { const string16 Label() const; // This guid is the primary identifier for |AutofillProfile| objects. + // TODO(estade): remove this and just use GetGUID(). |guid_| can probably + // be moved to FormGroup. const std::string guid() const { return guid_; } void set_guid(const std::string& guid) { guid_ = guid; } diff --git a/chrome/browser/autofill/credit_card.cc b/chrome/browser/autofill/credit_card.cc index 924bfc2..d3631eb 100644 --- a/chrome/browser/autofill/credit_card.cc +++ b/chrome/browser/autofill/credit_card.cc @@ -18,6 +18,7 @@ #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/autofill/autofill_country.h" +#include "chrome/browser/autofill/autofill_field.h" #include "chrome/browser/autofill/autofill_regexes.h" #include "chrome/browser/autofill/autofill_type.h" #include "chrome/browser/autofill/field_types.h" @@ -233,6 +234,10 @@ CreditCard::CreditCard(const CreditCard& credit_card) : FormGroup() { CreditCard::~CreditCard() {} +std::string CreditCard::GetGUID() const { + return guid(); +} + string16 CreditCard::GetRawInfo(AutofillFieldType type) const { switch (type) { case CREDIT_CARD_NAME: @@ -448,24 +453,25 @@ bool CreditCard::UpdateFromImportedCard(const CreditCard& imported_card) { return true; } -void CreditCard::FillFormField(AutofillFieldType type, - FormFieldData* field) const { - DCHECK_EQ(AutofillType::CREDIT_CARD, AutofillType(type).group()); - DCHECK(field); +void CreditCard::FillFormField(const AutofillField& field, + size_t /*variant*/, + FormFieldData* field_data) const { + DCHECK_EQ(AutofillType::CREDIT_CARD, AutofillType(field.type()).group()); + DCHECK(field_data); - if (field->form_control_type == "select-one") { - FillSelectControl(type, field); - } else if (field->form_control_type == "month") { + if (field_data->form_control_type == "select-one") { + FillSelectControl(field.type(), field_data); + } else if (field_data->form_control_type == "month") { // HTML5 input="month" consists of year-month. string16 year = GetCanonicalizedInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR); string16 month = GetCanonicalizedInfo(CREDIT_CARD_EXP_MONTH); if (!year.empty() && !month.empty()) { // Fill the value only if |this| includes both year and month // information. - field->value = year + ASCIIToUTF16("-") + month; + field_data->value = year + ASCIIToUTF16("-") + month; } } else { - field->value = GetCanonicalizedInfo(type); + field_data->value = GetCanonicalizedInfo(field.type()); } } diff --git a/chrome/browser/autofill/credit_card.h b/chrome/browser/autofill/credit_card.h index 256a511..714a4c5 100644 --- a/chrome/browser/autofill/credit_card.h +++ b/chrome/browser/autofill/credit_card.h @@ -26,14 +26,18 @@ class CreditCard : public FormGroup { virtual ~CreditCard(); // FormGroup implementation: + virtual std::string GetGUID() const OVERRIDE; + virtual void GetMatchingTypes(const string16& text, + FieldTypeSet* matching_types) const OVERRIDE; virtual string16 GetRawInfo(AutofillFieldType type) const OVERRIDE; virtual void SetRawInfo(AutofillFieldType type, const string16& value) OVERRIDE; virtual string16 GetCanonicalizedInfo(AutofillFieldType type) const OVERRIDE; virtual bool SetCanonicalizedInfo(AutofillFieldType type, const string16& value) OVERRIDE; - virtual void GetMatchingTypes(const string16& text, - FieldTypeSet* matching_types) const OVERRIDE; + virtual void FillFormField(const AutofillField& field, + size_t variant, + FormFieldData* field_data) const OVERRIDE; // Credit card preview summary, for example: ******1234, Exp: 01/2020 const string16 Label() const; @@ -49,6 +53,7 @@ class CreditCard : public FormGroup { const std::string& type() const { return type_; } // The guid is the primary identifier for |CreditCard| objects. + // TODO(estade): remove this and just use GetGUID(). const std::string guid() const { return guid_; } void set_guid(const std::string& guid) { guid_ = guid; } @@ -61,9 +66,6 @@ class CreditCard : public FormGroup { bool UpdateFromImportedCard(const CreditCard& imported_card) WARN_UNUSED_RESULT; - // Set |field|'s value based on |type| and contents of |this|. - void FillFormField(AutofillFieldType type, FormFieldData* field) const; - // Comparison for Sync. Returns 0 if the credit card is the same as |this|, // or < 0, or > 0 if it is different. The implied ordering can be used for // culling duplicates. The ordering is based on collation order of the diff --git a/chrome/browser/autofill/form_group.cc b/chrome/browser/autofill/form_group.cc index 49ebb88..b814c42 100644 --- a/chrome/browser/autofill/form_group.cc +++ b/chrome/browser/autofill/form_group.cc @@ -207,6 +207,11 @@ bool FillCreditCardTypeSelectControl(const string16& value, } // namespace +std::string FormGroup::GetGUID() const { + NOTREACHED(); + return std::string(); +} + void FormGroup::GetMatchingTypes(const string16& text, FieldTypeSet* matching_types) const { if (text.empty()) { @@ -246,6 +251,12 @@ bool FormGroup::SetCanonicalizedInfo(AutofillFieldType type, return true; } +void FormGroup::FillFormField(const AutofillField& field, + size_t variant, + FormFieldData* field_data) const { + NOTREACHED(); +} + void FormGroup::FillSelectControl(AutofillFieldType type, FormFieldData* field) const { DCHECK(field); diff --git a/chrome/browser/autofill/form_group.h b/chrome/browser/autofill/form_group.h index e84cff2..807be86 100644 --- a/chrome/browser/autofill/form_group.h +++ b/chrome/browser/autofill/form_group.h @@ -5,12 +5,14 @@ #ifndef CHROME_BROWSER_AUTOFILL_FORM_GROUP_H_ #define CHROME_BROWSER_AUTOFILL_FORM_GROUP_H_ +#include <string> #include <vector> #include "base/string16.h" #include "base/string_util.h" #include "chrome/browser/autofill/field_types.h" +class AutofillField; struct FormFieldData; // This class is an interface for collections of form fields, grouped by type. @@ -20,6 +22,10 @@ class FormGroup { public: virtual ~FormGroup() {} + // Returns a globally unique ID for this object. It is an error to call the + // default implementation. + virtual std::string GetGUID() const; + // Used to determine the type of a field based on the text that a user enters // into the field. The field types can then be reported back to the server. // This method is additive on |matching_types|. @@ -49,6 +55,12 @@ class FormGroup { virtual bool SetCanonicalizedInfo(AutofillFieldType type, const string16& value); + // Set |field_data|'s value based on |field| and contents of |this| (using + // data variant |variant|). It is an error to call the default implementation. + virtual void FillFormField(const AutofillField& field, + size_t variant, + FormFieldData* field_data) const; + // Fills in select control with data matching |type| from |this|. // Public for testing purposes. void FillSelectControl(AutofillFieldType type, diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc index 580521d..a5b8d8f 100644 --- a/chrome/browser/autofill/personal_data_manager.cc +++ b/chrome/browser/autofill/personal_data_manager.cc @@ -379,7 +379,7 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) { return; if (profile.IsEmpty()) { - RemoveProfile(profile.guid()); + RemoveByGUID(profile.guid()); return; } @@ -395,25 +395,6 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) { Refresh(); } -void PersonalDataManager::RemoveProfile(const std::string& guid) { - if (browser_context_->IsOffTheRecord()) - return; - - if (!FindByGUID<AutofillProfile>(web_profiles_, guid)) - return; - - scoped_ptr<AutofillWebDataService> autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) - return; - - // Remove the profile. - autofill_data->RemoveAutofillProfile(guid); - - // Refresh our local cache and send notifications to observers. - Refresh(); -} - AutofillProfile* PersonalDataManager::GetProfileByGUID( const std::string& guid) { for (std::vector<AutofillProfile*>::iterator iter = web_profiles_.begin(); @@ -458,7 +439,7 @@ void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) { return; if (credit_card.IsEmpty()) { - RemoveCreditCard(credit_card.guid()); + RemoveByGUID(credit_card.guid()); return; } @@ -474,11 +455,14 @@ void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) { Refresh(); } -void PersonalDataManager::RemoveCreditCard(const std::string& guid) { +void PersonalDataManager::RemoveByGUID(const std::string& guid) { if (browser_context_->IsOffTheRecord()) return; - if (!FindByGUID<CreditCard>(credit_cards_, guid)) + bool is_credit_card = FindByGUID<CreditCard>(credit_cards_, guid); + bool is_profile = !is_credit_card && + FindByGUID<AutofillProfile>(web_profiles_, guid); + if (!is_credit_card && !is_profile) return; scoped_ptr<AutofillWebDataService> autofill_data( @@ -486,8 +470,10 @@ void PersonalDataManager::RemoveCreditCard(const std::string& guid) { if (!autofill_data.get()) return; - // Remove the credit card. - autofill_data->RemoveCreditCard(guid); + if (is_credit_card) + autofill_data->RemoveCreditCard(guid); + else + autofill_data->RemoveAutofillProfile(guid); // Refresh our local cache and send notifications to observers. Refresh(); diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h index 4530262..8259be1 100644 --- a/chrome/browser/autofill/personal_data_manager.h +++ b/chrome/browser/autofill/personal_data_manager.h @@ -87,8 +87,8 @@ class PersonalDataManager // Updates |profile| which already exists in the web database. void UpdateProfile(const AutofillProfile& profile); - // Removes the profile represented by |guid|. - virtual void RemoveProfile(const std::string& guid); + // Removes the profile or credit card represented by |guid|. + virtual void RemoveByGUID(const std::string& guid); // Returns the profile with the specified |guid|, or NULL if there is no // profile with the specified |guid|. @@ -100,9 +100,6 @@ class PersonalDataManager // Updates |credit_card| which already exists in the web database. void UpdateCreditCard(const CreditCard& credit_card); - // Removes the credit card represented by |guid|. - virtual void RemoveCreditCard(const std::string& guid); - // Returns the credit card with the specified |guid|, or NULL if there is // no credit card with the specified |guid|. CreditCard* GetCreditCardByGUID(const std::string& guid); diff --git a/chrome/browser/autofill/personal_data_manager_unittest.cc b/chrome/browser/autofill/personal_data_manager_unittest.cc index de96d3f..3a7e8b0 100644 --- a/chrome/browser/autofill/personal_data_manager_unittest.cc +++ b/chrome/browser/autofill/personal_data_manager_unittest.cc @@ -193,7 +193,7 @@ TEST_F(PersonalDataManagerTest, AddUpdateRemoveProfiles) { // Update, remove, and add. profile0.SetRawInfo(NAME_FIRST, ASCIIToUTF16("John")); personal_data_->UpdateProfile(profile0); - personal_data_->RemoveProfile(profile1.guid()); + personal_data_->RemoveByGUID(profile1.guid()); personal_data_->AddProfile(profile2); // Verify that the web database has been updated and the notification sent. @@ -248,7 +248,7 @@ TEST_F(PersonalDataManagerTest, AddUpdateRemoveCreditCards) { // Update, remove, and add. credit_card0.SetRawInfo(CREDIT_CARD_NAME, ASCIIToUTF16("Joe")); personal_data_->UpdateCreditCard(credit_card0); - personal_data_->RemoveCreditCard(credit_card1.guid()); + personal_data_->RemoveByGUID(credit_card1.guid()); personal_data_->AddCreditCard(credit_card2); // Verify that the web database has been updated and the notification sent. diff --git a/chrome/browser/resources/options/autofill_options.js b/chrome/browser/resources/options/autofill_options.js index 82d8c24..2c30ade 100644 --- a/chrome/browser/resources/options/autofill_options.js +++ b/chrome/browser/resources/options/autofill_options.js @@ -126,21 +126,12 @@ cr.define('options', function() { }, /** - * Removes the Autofill address represented by |guid|. + * Removes the Autofill address or credit card represented by |guid|. * @param {String} guid The GUID of the address to remove. * @private */ - removeAddress_: function(guid) { - chrome.send('removeAddress', [guid]); - }, - - /** - * Removes the Autofill credit card represented by |guid|. - * @param {String} guid The GUID of the credit card to remove. - * @private - */ - removeCreditCard_: function(guid) { - chrome.send('removeCreditCard', [guid]); + removeData_: function(guid) { + chrome.send('removeData', [guid]); }, /** @@ -200,12 +191,8 @@ cr.define('options', function() { AutofillOptions.getInstance().setCreditCardList_(entries); }; - AutofillOptions.removeAddress = function(guid) { - AutofillOptions.getInstance().removeAddress_(guid); - }; - - AutofillOptions.removeCreditCard = function(guid) { - AutofillOptions.getInstance().removeCreditCard_(guid); + AutofillOptions.removeData = function(guid) { + AutofillOptions.getInstance().removeData_(guid); }; AutofillOptions.loadAddressEditor = function(guid) { diff --git a/chrome/browser/resources/options/autofill_options_list.js b/chrome/browser/resources/options/autofill_options_list.js index 7bb9c7b..7b2aaeb 100644 --- a/chrome/browser/resources/options/autofill_options_list.js +++ b/chrome/browser/resources/options/autofill_options_list.js @@ -355,7 +355,7 @@ cr.define('options.autofillOptions', function() { /** @inheritDoc */ deleteItemAtIndex: function(index) { - AutofillOptions.removeAddress(this.dataModel.item(index)[0]); + AutofillOptions.removeData(this.dataModel.item(index)[0]); }, }; @@ -385,7 +385,7 @@ cr.define('options.autofillOptions', function() { /** @inheritDoc */ deleteItemAtIndex: function(index) { - AutofillOptions.removeCreditCard(this.dataModel.item(index)[0]); + AutofillOptions.removeData(this.dataModel.item(index)[0]); }, }; diff --git a/chrome/browser/ui/webui/options/autofill_options_handler.cc b/chrome/browser/ui/webui/options/autofill_options_handler.cc index 29f00d6..4976c82 100644 --- a/chrome/browser/ui/webui/options/autofill_options_handler.cc +++ b/chrome/browser/ui/webui/options/autofill_options_handler.cc @@ -318,12 +318,8 @@ void AutofillOptionsHandler::InitializePage() { void AutofillOptionsHandler::RegisterMessages() { web_ui()->RegisterMessageCallback( - "removeAddress", - base::Bind(&AutofillOptionsHandler::RemoveAddress, - base::Unretained(this))); - web_ui()->RegisterMessageCallback( - "removeCreditCard", - base::Bind(&AutofillOptionsHandler::RemoveCreditCard, + "removeData", + base::Bind(&AutofillOptionsHandler::RemoveData, base::Unretained(this))); web_ui()->RegisterMessageCallback( "loadAddressEditor", @@ -440,19 +436,7 @@ void AutofillOptionsHandler::LoadAutofillData() { credit_cards); } -void AutofillOptionsHandler::RemoveAddress(const ListValue* args) { - DCHECK(IsPersonalDataLoaded()); - - std::string guid; - if (!args->GetString(0, &guid)) { - NOTREACHED(); - return; - } - - personal_data_->RemoveProfile(guid); -} - -void AutofillOptionsHandler::RemoveCreditCard(const ListValue* args) { +void AutofillOptionsHandler::RemoveData(const ListValue* args) { DCHECK(IsPersonalDataLoaded()); std::string guid; @@ -461,7 +445,7 @@ void AutofillOptionsHandler::RemoveCreditCard(const ListValue* args) { return; } - personal_data_->RemoveCreditCard(guid); + personal_data_->RemoveByGUID(guid); } void AutofillOptionsHandler::LoadAddressEditor(const ListValue* args) { diff --git a/chrome/browser/ui/webui/options/autofill_options_handler.h b/chrome/browser/ui/webui/options/autofill_options_handler.h index 6356487..b88639f 100644 --- a/chrome/browser/ui/webui/options/autofill_options_handler.h +++ b/chrome/browser/ui/webui/options/autofill_options_handler.h @@ -44,13 +44,9 @@ class AutofillOptionsHandler : public OptionsPageUIHandler, // Loads Autofill addresses and credit cards using the PersonalDataManager. void LoadAutofillData(); - // Removes an address from the PersonalDataManager. - // |args| - A string, the GUID of the address to remove. - void RemoveAddress(const base::ListValue* args); - - // Removes a credit card from the PersonalDataManager. - // |args| - A string, the GUID of the credit card to remove. - void RemoveCreditCard(const base::ListValue* args); + // Removes data from the PersonalDataManager. + // |args| - A string, the GUID of the address or credit card to remove. + void RemoveData(const base::ListValue* args); // Requests profile data for a specific address. Calls into WebUI with the // loaded profile data to open the address editor. |