Commit 8aac2d78 authored by isherman@chromium.org's avatar isherman@chromium.org

[rAc] Persist selection of newly added cards and addresses.

BUG=285458
TEST=(see bug)
R=estade@chromium.org

Review URL: https://chromiumcodereview.appspot.com/23882013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@223519 0039d316-1c4b-4281-b951-d872f2087c98
parent 5da472a4
......@@ -2678,7 +2678,8 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
GetValueFromSection(SECTION_BILLING, NAME_FULL));
if (ShouldSaveDetailsLocally()) {
GetManager()->SaveImportedCreditCard(card);
std::string guid = GetManager()->SaveImportedCreditCard(card);
newly_saved_data_model_guids_[section] = guid;
DCHECK(!profile()->IsOffTheRecord());
newly_saved_card_.reset(new CreditCard(card));
}
......@@ -2695,8 +2696,10 @@ void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
profile.set_origin(kAutofillDialogOrigin);
FillFormGroupFromOutputs(output, &profile);
if (ShouldSaveDetailsLocally())
SaveProfileGleanedFromSection(profile, section);
if (ShouldSaveDetailsLocally()) {
std::string guid = GetManager()->SaveImportedProfile(profile);
newly_saved_data_model_guids_[section] = guid;
}
AutofillProfileWrapper profile_wrapper(&profile);
profile_wrapper.FillFormStructure(inputs, compare, &form_structure_);
......@@ -2749,12 +2752,6 @@ string16 AutofillDialogControllerImpl::GetValueFromSection(
return string16();
}
void AutofillDialogControllerImpl::SaveProfileGleanedFromSection(
const AutofillProfile& profile,
DialogSection section) {
GetManager()->SaveImportedProfile(profile);
}
SuggestionsMenuModel* AutofillDialogControllerImpl::
SuggestionsMenuModelForSection(DialogSection section) {
switch (section) {
......@@ -3143,8 +3140,12 @@ void AutofillDialogControllerImpl::DoFinishSubmit() {
SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);
std::string item_key = model->GetItemKeyForCheckedItem();
if (IsASuggestionItemKey(item_key) || item_key == kSameAsBillingKey)
if (IsASuggestionItemKey(item_key) || item_key == kSameAsBillingKey) {
PersistAutofillChoice(section, item_key);
} else if (item_key == kAddNewItemKey && ShouldSaveDetailsLocally()) {
DCHECK(newly_saved_data_model_guids_.count(section));
PersistAutofillChoice(section, newly_saved_data_model_guids_[section]);
}
}
profile_->GetPrefs()->SetBoolean(::prefs::kAutofillDialogSaveData,
......
......@@ -407,11 +407,6 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
string16 GetValueFromSection(DialogSection section,
ServerFieldType type);
// Saves the data in |profile| to the personal data manager. This may add
// a new profile or tack onto an existing profile.
void SaveProfileGleanedFromSection(const AutofillProfile& profile,
DialogSection section);
// Gets the SuggestionsMenuModel for |section|.
SuggestionsMenuModel* SuggestionsMenuModelForSection(DialogSection section);
const SuggestionsMenuModel* SuggestionsMenuModelForSection(
......@@ -712,6 +707,11 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
// to Wallet.
std::string wallet_cookie_value_;
// A map from dialog sections to the GUID of a newly saved Autofill data
// models for that section. No entries present that don't have newly saved
// data models.
std::map<DialogSection, std::string> newly_saved_data_model_guids_;
// Populated if the user chose to save a newly inputted credit card. Used to
// show a bubble as the dialog closes to confirm a user's new card info was
// saved. Never populated while incognito (as nothing's actually saved).
......
......@@ -460,6 +460,30 @@ class AutofillDialogControllerTest : public ChromeRenderViewHostTestHarness {
controller()->GetView()->SetUserInput(SECTION_CC, cc_outputs);
}
// Activates the 'Add new foo' option from the |section|'s suggestions
// dropdown and fills the |section|'s inputs with the data from the
// |data_model|. If |section| is SECTION_CC, also fills in '123' for the CVC.
void FillInputs(DialogSection section, const AutofillDataModel& data_model) {
// Select the 'Add new foo' option.
ui::MenuModel* model = GetMenuModelForSection(section);
model->ActivatedAt(model->GetItemCount() - 2);
// Fill the inputs.
DetailOutputMap outputs;
const DetailInputs& inputs =
controller()->RequestedFieldsForSection(section);
for (size_t i = 0; i < inputs.size(); ++i) {
ServerFieldType type = inputs[i].type;
base::string16 output;
if (type == CREDIT_CARD_VERIFICATION_CODE)
output = ASCIIToUTF16("123");
else
output = data_model.GetInfo(AutofillType(type), "en-US");
outputs[&inputs[i]] = output;
}
controller()->GetView()->SetUserInput(section, outputs);
}
std::vector<DialogNotification> NotificationsOfType(
DialogNotification::Type type) {
std::vector<DialogNotification> right_type;
......@@ -538,6 +562,11 @@ class AutofillDialogControllerTest : public ChromeRenderViewHostTestHarness {
return false;
}
SuggestionsMenuModel* GetMenuModelForSection(DialogSection section) {
ui::MenuModel* model = controller()->MenuModelForSection(section);
return static_cast<SuggestionsMenuModel*>(model);
}
TestAutofillDialogController* controller() { return controller_.get(); }
const FormStructure* form_structure() { return form_structure_; }
......@@ -906,31 +935,27 @@ TEST_F(AutofillDialogControllerTest, AutofillProfiles) {
// Makes sure that the choice of which Autofill profile to use for each section
// is sticky.
TEST_F(AutofillDialogControllerTest, AutofillProfileDefaults) {
AutofillProfile full_profile(test::GetFullProfile());
full_profile.set_origin(kSettingsOrigin);
controller()->GetTestingManager()->AddTestingProfile(&full_profile);
AutofillProfile full_profile2(test::GetFullProfile2());
full_profile2.set_origin(kSettingsOrigin);
controller()->GetTestingManager()->AddTestingProfile(&full_profile2);
AutofillProfile profile(test::GetVerifiedProfile());
AutofillProfile profile2(test::GetVerifiedProfile2());
controller()->GetTestingManager()->AddTestingProfile(&profile);
controller()->GetTestingManager()->AddTestingProfile(&profile2);
// Until a selection has been made, the default shipping suggestion is the
// first one (after "use billing").
SuggestionsMenuModel* shipping_model = static_cast<SuggestionsMenuModel*>(
controller()->MenuModelForSection(SECTION_SHIPPING));
SuggestionsMenuModel* shipping_model =
GetMenuModelForSection(SECTION_SHIPPING);
EXPECT_EQ(1, shipping_model->checked_item());
for (int i = 2; i >= 0; --i) {
shipping_model = static_cast<SuggestionsMenuModel*>(
controller()->MenuModelForSection(SECTION_SHIPPING));
shipping_model = GetMenuModelForSection(SECTION_SHIPPING);
shipping_model->ExecuteCommand(i, 0);
FillCreditCardInputs();
controller()->OnAccept();
Reset();
controller()->GetTestingManager()->AddTestingProfile(&full_profile);
controller()->GetTestingManager()->AddTestingProfile(&full_profile2);
shipping_model = static_cast<SuggestionsMenuModel*>(
controller()->MenuModelForSection(SECTION_SHIPPING));
controller()->GetTestingManager()->AddTestingProfile(&profile);
controller()->GetTestingManager()->AddTestingProfile(&profile2);
shipping_model = GetMenuModelForSection(SECTION_SHIPPING);
EXPECT_EQ(i, shipping_model->checked_item());
}
......@@ -940,12 +965,54 @@ TEST_F(AutofillDialogControllerTest, AutofillProfileDefaults) {
FillCreditCardInputs();
controller()->OnAccept();
Reset();
controller()->GetTestingManager()->AddTestingProfile(&full_profile);
shipping_model = static_cast<SuggestionsMenuModel*>(
controller()->MenuModelForSection(SECTION_SHIPPING));
controller()->GetTestingManager()->AddTestingProfile(&profile);
shipping_model = GetMenuModelForSection(SECTION_SHIPPING);
EXPECT_EQ(1, shipping_model->checked_item());
}
// Makes sure that a newly added Autofill profile becomes set as the default
// choice for the next run.
TEST_F(AutofillDialogControllerTest, NewAutofillProfileIsDefault) {
SwitchToAutofill();
AutofillProfile profile(test::GetVerifiedProfile());
CreditCard credit_card(test::GetVerifiedCreditCard());
controller()->GetTestingManager()->AddTestingProfile(&profile);
controller()->GetTestingManager()->AddTestingCreditCard(&credit_card);
// Until a selection has been made, the default suggestion is the first one.
// For the shipping section, this follows the "use billing" suggestion.
EXPECT_EQ(0, GetMenuModelForSection(SECTION_CC)->checked_item());
EXPECT_EQ(1, GetMenuModelForSection(SECTION_SHIPPING)->checked_item());
// Fill in the shipping and credit card sections with new data.
AutofillProfile new_profile(test::GetVerifiedProfile2());
CreditCard new_credit_card(test::GetVerifiedCreditCard2());
FillInputs(SECTION_SHIPPING, new_profile);
FillInputs(SECTION_CC, new_credit_card);
controller()->GetView()->CheckSaveDetailsLocallyCheckbox(true);
controller()->OnAccept();
// Update the |new_profile| and |new_credit_card|'s guids to the saved ones.
new_profile.set_guid(
controller()->GetTestingManager()->imported_profile().guid());
new_credit_card.set_guid(
controller()->GetTestingManager()->imported_credit_card().guid());
// Reload the dialog. The newly added address and credit card should now be
// set as the defaults.
Reset();
controller()->GetTestingManager()->AddTestingProfile(&profile);
controller()->GetTestingManager()->AddTestingProfile(&new_profile);
controller()->GetTestingManager()->AddTestingCreditCard(&credit_card);
controller()->GetTestingManager()->AddTestingCreditCard(&new_credit_card);
// Until a selection has been made, the default suggestion is the first one.
// For the shipping section, this follows the "use billing" suggestion.
EXPECT_EQ(1, GetMenuModelForSection(SECTION_CC)->checked_item());
EXPECT_EQ(2, GetMenuModelForSection(SECTION_SHIPPING)->checked_item());
}
TEST_F(AutofillDialogControllerTest, AutofillProfileVariants) {
EXPECT_CALL(*controller()->GetView(), ModelChanged()).Times(1);
ui::MenuModel* shipping_model =
......@@ -1425,8 +1492,7 @@ TEST_F(AutofillDialogControllerTest, ManageItem) {
controller()->GetTestingManager()->AddTestingProfile(&full_profile);
SwitchToAutofill();
SuggestionsMenuModel* shipping = static_cast<SuggestionsMenuModel*>(
controller()->MenuModelForSection(SECTION_SHIPPING));
SuggestionsMenuModel* shipping = GetMenuModelForSection(SECTION_SHIPPING);
shipping->ExecuteCommand(shipping->GetItemCount() - 1, 0);
GURL autofill_manage_url = controller()->open_tab_url();
EXPECT_EQ("chrome", autofill_manage_url.scheme());
......@@ -1440,8 +1506,7 @@ TEST_F(AutofillDialogControllerTest, ManageItem) {
GURL wallet_manage_addresses_url = controller()->open_tab_url();
EXPECT_EQ("https", wallet_manage_addresses_url.scheme());
SuggestionsMenuModel* billing = static_cast<SuggestionsMenuModel*>(
controller()->MenuModelForSection(SECTION_CC_BILLING));
SuggestionsMenuModel* billing = GetMenuModelForSection(SECTION_CC_BILLING);
controller()->SuggestionItemSelected(billing, billing->GetItemCount() - 1);
GURL wallet_manage_instruments_url = controller()->open_tab_url();
EXPECT_EQ("https", wallet_manage_instruments_url.scheme());
......
......@@ -128,12 +128,25 @@ CreditCard GetCreditCard() {
return credit_card;
}
CreditCard GetCreditCard2() {
CreditCard credit_card(base::GenerateGUID(), "https://www.example.com");
SetCreditCardInfo(
&credit_card, "Someone Else", "378282246310005" /* AmEx */, "07", "2019");
return credit_card;
}
CreditCard GetVerifiedCreditCard() {
CreditCard credit_card(GetCreditCard());
credit_card.set_origin(kSettingsOrigin);
return credit_card;
}
CreditCard GetVerifiedCreditCard2() {
CreditCard credit_card(GetCreditCard2());
credit_card.set_origin(kSettingsOrigin);
return credit_card;
}
void SetProfileInfo(AutofillProfile* profile,
const char* first_name, const char* middle_name,
const char* last_name, const char* email, const char* company,
......
......@@ -46,9 +46,15 @@ AutofillProfile GetVerifiedProfile2();
// Returns a credit card full of dummy info.
CreditCard GetCreditCard();
// Returns a credit card full of dummy info, different to the above.
CreditCard GetCreditCard2();
// Returns a verified credit card full of dummy info.
CreditCard GetVerifiedCreditCard();
// Returns a verified credit card full of dummy info, different to the above.
CreditCard GetVerifiedCreditCard2();
// A unit testing utility that is common to a number of the Autofill unit
// tests. |SetProfileInfo| provides a quick way to populate a profile with
// c-strings.
......
......@@ -739,8 +739,9 @@ void AutofillManager::ImportFormData(const FormStructure& submitted_form) {
manager_delegate_->ConfirmSaveCreditCard(
*metric_logger_,
*imported_credit_card,
base::Bind(&PersonalDataManager::SaveImportedCreditCard,
base::Unretained(personal_data_), *imported_credit_card));
base::Bind(
base::IgnoreResult(&PersonalDataManager::SaveImportedCreditCard),
base::Unretained(personal_data_), *imported_credit_card));
}
}
......
......@@ -84,7 +84,7 @@ class TestPersonalDataManager : public PersonalDataManager {
return NULL;
}
MOCK_METHOD1(SaveImportedProfile, void(const AutofillProfile&));
MOCK_METHOD1(SaveImportedProfile, std::string(const AutofillProfile&));
AutofillProfile* GetProfileWithGUID(const char* guid) {
for (std::vector<AutofillProfile *>::iterator it = web_profiles_.begin();
......
......@@ -88,7 +88,8 @@ class PersonalDataManagerMock : public PersonalDataManager {
void Reset();
// PersonalDataManager:
virtual void SaveImportedProfile(const AutofillProfile& profile) OVERRIDE;
virtual std::string SaveImportedProfile(
const AutofillProfile& profile) OVERRIDE;
virtual const std::vector<AutofillProfile*>& web_profiles() const OVERRIDE;
private:
......@@ -108,11 +109,14 @@ void PersonalDataManagerMock::Reset() {
profiles_.clear();
}
void PersonalDataManagerMock::SaveImportedProfile(
std::string PersonalDataManagerMock::SaveImportedProfile(
const AutofillProfile& profile) {
std::vector<AutofillProfile> profiles;
if (!MergeProfile(profile, profiles_.get(), "en-US", &profiles))
std::string merged_guid =
MergeProfile(profile, profiles_.get(), "en-US", &profiles);
if (merged_guid == profile.guid())
profiles_.push_back(new AutofillProfile(profile));
return merged_guid;
}
const std::vector<AutofillProfile*>& PersonalDataManagerMock::web_profiles()
......
......@@ -125,7 +125,7 @@ class TestPersonalDataManager : public PersonalDataManager {
}
MOCK_METHOD1(SaveImportedCreditCard,
void(const CreditCard& imported_credit_card));
std::string(const CreditCard& imported_credit_card));
private:
void CreateTestAutofillProfiles(ScopedVector<AutofillProfile>* profiles) {
......@@ -319,8 +319,9 @@ scoped_ptr<ConfirmInfoBarDelegate> AutofillMetricsTest::CreateDelegate(
CreditCard credit_card;
return AutofillCCInfoBarDelegate::Create(
metric_logger,
base::Bind(&TestPersonalDataManager::SaveImportedCreditCard,
base::Unretained(personal_data_.get()), credit_card));
base::Bind(
base::IgnoreResult(&TestPersonalDataManager::SaveImportedCreditCard),
base::Unretained(personal_data_.get()), credit_card));
}
// Test that we log quality metrics appropriately.
......
......@@ -726,7 +726,7 @@ bool PersonalDataManager::IsValidLearnableProfile(
}
// static
bool PersonalDataManager::MergeProfile(
std::string PersonalDataManager::MergeProfile(
const AutofillProfile& new_profile,
const std::vector<AutofillProfile*>& existing_profiles,
const std::string& app_locale,
......@@ -735,6 +735,7 @@ bool PersonalDataManager::MergeProfile(
// Set to true if |existing_profiles| already contains an equivalent profile.
bool matching_profile_found = false;
std::string guid = new_profile.guid();
// If we have already saved this address, merge in any missing values.
// Only merge with the first match.
......@@ -751,6 +752,7 @@ bool PersonalDataManager::MergeProfile(
// data. If an automatically aggregated profile would overwrite a
// verified profile, just drop it.
matching_profile_found = true;
guid = existing_profile->guid();
if (!existing_profile->IsVerified() || new_profile.IsVerified())
existing_profile->OverwriteWithOrAddTo(new_profile, app_locale);
}
......@@ -761,7 +763,7 @@ bool PersonalDataManager::MergeProfile(
if (!matching_profile_found)
merged_profiles->push_back(new_profile);
return matching_profile_found;
return guid;
}
void PersonalDataManager::SetProfiles(std::vector<AutofillProfile>* profiles) {
......@@ -962,10 +964,10 @@ void PersonalDataManager::CancelPendingQuery(
*handle = 0;
}
void PersonalDataManager::SaveImportedProfile(
std::string PersonalDataManager::SaveImportedProfile(
const AutofillProfile& imported_profile) {
if (browser_context_->IsOffTheRecord())
return;
return std::string();
// Don't save a web profile if the data in the profile is a subset of an
// auxiliary profile.
......@@ -973,32 +975,39 @@ void PersonalDataManager::SaveImportedProfile(
auxiliary_profiles_.begin();
iter != auxiliary_profiles_.end(); ++iter) {
if (imported_profile.IsSubsetOf(**iter, app_locale_))
return;
return (*iter)->guid();
}
std::vector<AutofillProfile> profiles;
MergeProfile(imported_profile, web_profiles_.get(), app_locale_, &profiles);
std::string guid =
MergeProfile(imported_profile, web_profiles_.get(), app_locale_,
&profiles);
SetProfiles(&profiles);
return guid;
}
void PersonalDataManager::SaveImportedCreditCard(
std::string PersonalDataManager::SaveImportedCreditCard(
const CreditCard& imported_card) {
DCHECK(!imported_card.number().empty());
if (browser_context_->IsOffTheRecord())
return;
return std::string();
// Set to true if |imported_card| is merged into the credit card list.
bool merged = false;
std::string guid = imported_card.guid();
std::vector<CreditCard> credit_cards;
for (std::vector<CreditCard*>::const_iterator card = credit_cards_.begin();
card != credit_cards_.end();
++card) {
// If |imported_card| has not yet been merged, check whether it should be
// with the current |card|.
if (!merged && (*card)->UpdateFromImportedCard(imported_card, app_locale_))
if (!merged &&
(*card)->UpdateFromImportedCard(imported_card, app_locale_)) {
guid = (*card)->guid();
merged = true;
}
credit_cards.push_back(**card);
}
......@@ -1007,6 +1016,7 @@ void PersonalDataManager::SaveImportedCreditCard(
credit_cards.push_back(imported_card);
SetCreditCards(&credit_cards);
return guid;
}
void PersonalDataManager::LogProfileCount() const {
......
......@@ -79,11 +79,15 @@ class PersonalDataManager : public WebDataServiceConsumer,
bool ImportFormData(const FormStructure& form,
const CreditCard** credit_card);
// Saves |imported_profile| to the WebDB if it exists.
virtual void SaveImportedProfile(const AutofillProfile& imported_profile);
// Saves |imported_profile| to the WebDB if it exists. Returns the guid of
// the new or updated profile, or the empty string if no profile was saved.
virtual std::string SaveImportedProfile(
const AutofillProfile& imported_profile);
// Saves a credit card value detected in |ImportedFormData|.
virtual void SaveImportedCreditCard(const CreditCard& imported_credit_card);
// Saves a credit card value detected in |ImportedFormData|. Returns the guid
// of the new or updated card, or the empty string if no card was saved.
virtual std::string SaveImportedCreditCard(
const CreditCard& imported_credit_card);
// Adds |profile| to the web database.
void AddProfile(const AutofillProfile& profile);
......@@ -169,8 +173,9 @@ class PersonalDataManager : public WebDataServiceConsumer,
// Merges |new_profile| into one of the |existing_profiles| if possible;
// otherwise appends |new_profile| to the end of that list. Fills
// |merged_profiles| with the result.
static bool MergeProfile(
// |merged_profiles| with the result. Returns the |guid| of the new or updated
// profile.
static std::string MergeProfile(
const AutofillProfile& new_profile,
const std::vector<AutofillProfile*>& existing_profiles,
const std::string& app_locale,
......
......@@ -34,9 +34,16 @@ const std::vector<CreditCard*>& TestPersonalDataManager::
return credit_cards_;
}
void TestPersonalDataManager::SaveImportedProfile(
std::string TestPersonalDataManager::SaveImportedProfile(
const AutofillProfile& imported_profile) {
imported_profile_ = imported_profile;
return imported_profile.guid();
}
std::string TestPersonalDataManager::SaveImportedCreditCard(
const CreditCard& imported_credit_card) {
imported_credit_card_ = imported_credit_card;
return imported_credit_card.guid();
}
} // namespace autofill
......@@ -27,16 +27,21 @@ class TestPersonalDataManager : public PersonalDataManager {
void AddTestingCreditCard(CreditCard* credit_card);
virtual const std::vector<AutofillProfile*>& GetProfiles() OVERRIDE;
virtual void SaveImportedProfile(const AutofillProfile& imported_profile)
OVERRIDE;
virtual const std::vector<CreditCard*>& GetCreditCards() const OVERRIDE;
virtual std::string SaveImportedProfile(
const AutofillProfile& imported_profile) OVERRIDE;
virtual std::string SaveImportedCreditCard(
const CreditCard& imported_credit_card) OVERRIDE;
const AutofillProfile& imported_profile() { return imported_profile_; }
virtual const std::vector<CreditCard*>& GetCreditCards() const OVERRIDE;
const CreditCard& imported_credit_card() { return imported_credit_card_; }
private:
std::vector<AutofillProfile*> profiles_;
std::vector<CreditCard*> credit_cards_;
AutofillProfile imported_profile_;
CreditCard imported_credit_card_;
};
} // namespace autofill
......
......@@ -2095,7 +2095,7 @@ bool AutofillTable::MigrateToVersion37MergeAndCullOlderProfiles() {
if (PersonalDataManager::IsValidLearnableProfile(*profile, app_locale_)) {
std::vector<AutofillProfile> merged_profiles;
bool merged = PersonalDataManager::MergeProfile(
std::string merged_guid = PersonalDataManager::MergeProfile(
*profile, accumulated_profiles_p, app_locale_, &merged_profiles);
std::swap(accumulated_profiles, merged_profiles);
......@@ -2108,9 +2108,8 @@ bool AutofillTable::MigrateToVersion37MergeAndCullOlderProfiles() {
address_of<AutofillProfile>);
// If the profile got merged trash the original.
if (merged)
if (merged_guid != profile->guid())
AddAutofillGUIDToTrash(profile->guid());
} else {
// An invalid profile, so trash it.
AddAutofillGUIDToTrash(profile->guid());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment