Skip to content

Commit

Permalink
[Autofill] Add a notion of "verified" profiles.
Browse files Browse the repository at this point in the history
Each Autofill profile and credit card is now associated with an origin.  For
automatically aggregated profiles, this is the domain they were aggregated from.
For manually entered profiles, this is something more like
"chrome://settings/autofill".  Profiles with unknown origin have an empty string
as their origin.

A profile is considered to be "verified" if its origin is a non-empty,
non-web-URL string.  When a user makes an edit to a profile via
chrome://settings/autofill, it becomes verified.  (If the user simply views the
profile, but does not make any changes, it is not upgraded to verified status.)

Once a profile is in the verified state, nothing can knock it back into the
unverified state.  The only way to add variants to a verified profile is to
enter them directly via Chrome's settings UI.  Any automatically aggregated
profiles that would be merged to a verified profile are simply discarded, since
we don't want to mess up profiles that users have directly edited, nor do we
want to pollute the global namespace of profiles.  We could instead keep track
of verified/unverified state per variant, but this adds a relatively large
amount of code complexity for relatively little gain.

This CL is just the first part of the work.  Follow-up CLs will:
  (1) Extend the WebDB to be origin-aware.
  (2) Extend Sync to be origin-aware (requires a Sync backend change).
  (3a) Write the origin to the WebDB.
  (3b) Ensure that web origins are cleared from the WebDB when users select to
       erase history.
  (4) Ensure that all clients of the PersonalDataManager are setting the origin
      correctly for any profiles they add.

BUG=170401, 231029
TEST=TBD

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@197653 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
isherman@chromium.org committed May 1, 2013
1 parent a1a6636 commit 012b8f3
Show file tree
Hide file tree
Showing 19 changed files with 483 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ namespace {

const char kFakeEmail[] = "user@example.com";
const char* kFieldsFromPage[] = { "email", "cc-number" };
const char kSettingsOrigin[] = "Chrome settings";

using content::BrowserThread;

Expand Down Expand Up @@ -369,14 +370,15 @@ TEST_F(AutofillDialogControllerTest, AutofillProfiles) {
EXPECT_CALL(*controller()->GetView(), ModelChanged()).Times(2);

// Empty profiles are ignored.
AutofillProfile empty_profile(base::GenerateGUID());
AutofillProfile empty_profile(base::GenerateGUID(), kSettingsOrigin);
empty_profile.SetRawInfo(NAME_FULL, ASCIIToUTF16("John Doe"));
controller()->GetTestingManager()->AddTestingProfile(&empty_profile);
shipping_model = controller()->MenuModelForSection(SECTION_SHIPPING);
EXPECT_EQ(3, shipping_model->GetItemCount());

// A full profile should be picked up.
AutofillProfile full_profile(test::GetFullProfile());
full_profile.set_origin(kSettingsOrigin);
full_profile.SetRawInfo(ADDRESS_HOME_LINE2, string16());
controller()->GetTestingManager()->AddTestingProfile(&full_profile);
shipping_model = controller()->MenuModelForSection(SECTION_SHIPPING);
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/ui/webui/options/autofill_options_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ using autofill::PersonalDataManager;

namespace {

const char kSettingsOrigin[] = "Chrome settings";

// Sets data related to the country <select>.
void SetCountryData(DictionaryValue* localized_strings) {
std::string default_country_code = AutofillCountry::CountryCodeForLocale(
Expand Down Expand Up @@ -510,7 +512,7 @@ void AutofillOptionsHandler::SetAddress(const ListValue* args) {
return;
}

AutofillProfile profile(guid);
AutofillProfile profile(guid, kSettingsOrigin);

std::string country_code;
string16 value;
Expand Down Expand Up @@ -564,7 +566,7 @@ void AutofillOptionsHandler::SetCreditCard(const ListValue* args) {
return;
}

CreditCard credit_card(guid);
CreditCard credit_card(guid, kSettingsOrigin);

string16 value;
if (args->GetString(1, &value))
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/webdata/autofill_profile_syncable_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,9 @@ AutofillProfileSyncableService::CreateOrUpdateProfile(
}
} else {
// New profile synced.
// TODO(isherman): Read the origin from |autofill_specifics|.
AutofillProfile* new_profile(
new AutofillProfile(autofill_specifics.guid()));
new AutofillProfile(autofill_specifics.guid(), std::string()));
OverwriteProfileWithServerData(
autofill_specifics, new_profile, app_locale_);

Expand Down Expand Up @@ -504,7 +505,7 @@ void AutofillProfileSyncableService::ActOnChange(
break;
}
case AutofillProfileChange::REMOVE: {
AutofillProfile empty_profile(change.key());
AutofillProfile empty_profile(change.key(), std::string());
new_changes.push_back(
syncer::SyncChange(FROM_HERE,
syncer::SyncChange::ACTION_DELETE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@ using autofill::AutofillProfileChange;
using content::BrowserThread;

// Some guids for testing.
std::string kGuid1 = "EDC609ED-7EEE-4F27-B00C-423242A9C44B";
std::string kGuid2 = "EDC609ED-7EEE-4F27-B00C-423242A9C44C";
std::string kGuid3 = "EDC609ED-7EEE-4F27-B00C-423242A9C44D";
std::string kGuid4 = "EDC609ED-7EEE-4F27-B00C-423242A9C44E";
const char kGuid1[] = "EDC609ED-7EEE-4F27-B00C-423242A9C44B";
const char kGuid2[] = "EDC609ED-7EEE-4F27-B00C-423242A9C44C";
const char kGuid3[] = "EDC609ED-7EEE-4F27-B00C-423242A9C44D";
const char kGuid4[] = "EDC609ED-7EEE-4F27-B00C-423242A9C44E";
const char kHttpOrigin[] = "http://www.example.com/";
const char kHttpsOrigin[] = "https://www.example.com/";
const char kSettingsOrigin[] = "Chrome settings";

class MockAutofillProfileSyncableService
: public AutofillProfileSyncableService {
Expand Down Expand Up @@ -122,27 +125,33 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeDataAndStartSyncing) {
std::string guid_present2 = kGuid2;
std::string guid_synced1 = kGuid3;
std::string guid_synced2 = kGuid4;
std::string origin_present1 = kHttpOrigin;
std::string origin_present2 = std::string();
std::string origin_synced1 = kHttpsOrigin;
std::string origin_synced2 = kSettingsOrigin;

profiles_from_web_db.push_back(new AutofillProfile(guid_present1));
profiles_from_web_db.push_back(
new AutofillProfile(guid_present1, origin_present1));
profiles_from_web_db.back()->SetRawInfo(
autofill::NAME_FIRST, UTF8ToUTF16("John"));
profiles_from_web_db.back()->SetRawInfo(
autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("1 1st st"));
profiles_from_web_db.push_back(new AutofillProfile(guid_present2));
profiles_from_web_db.push_back(
new AutofillProfile(guid_present2, origin_present2));
profiles_from_web_db.back()->SetRawInfo(
autofill::NAME_FIRST, UTF8ToUTF16("Tom"));
profiles_from_web_db.back()->SetRawInfo(
autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("2 2nd st"));

syncer::SyncDataList data_list;
AutofillProfile profile1(guid_synced1);
AutofillProfile profile1(guid_synced1, origin_synced1);
profile1.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Jane"));
data_list.push_back(autofill_syncable_service_.CreateData(profile1));
AutofillProfile profile2(guid_synced2);
AutofillProfile profile2(guid_synced2, origin_synced2);
profile2.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Harry"));
data_list.push_back(autofill_syncable_service_.CreateData(profile2));
// This one will have the name updated.
AutofillProfile profile3(guid_present2);
AutofillProfile profile3(guid_present2, origin_synced2);
profile3.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Tom Doe"));
data_list.push_back(autofill_syncable_service_.CreateData(profile3));

Expand Down Expand Up @@ -185,10 +194,12 @@ TEST_F(AutofillProfileSyncableServiceTest, GetAllSyncData) {
std::string guid_present1 = kGuid1;
std::string guid_present2 = kGuid2;

profiles_from_web_db.push_back(new AutofillProfile(guid_present1));
profiles_from_web_db.push_back(
new AutofillProfile(guid_present1, kHttpOrigin));
profiles_from_web_db.back()->SetRawInfo(
autofill::NAME_FIRST, UTF8ToUTF16("John"));
profiles_from_web_db.push_back(new AutofillProfile(guid_present2));
profiles_from_web_db.push_back(
new AutofillProfile(guid_present2, kHttpsOrigin));
profiles_from_web_db.back()->SetRawInfo(
autofill::NAME_FIRST, UTF8ToUTF16("Jane"));

Expand Down Expand Up @@ -217,11 +228,14 @@ TEST_F(AutofillProfileSyncableServiceTest, GetAllSyncData) {
syncer::SyncDataList data =
autofill_syncable_service_.GetAllSyncData(syncer::AUTOFILL_PROFILE);

EXPECT_EQ(2U, data.size());
EXPECT_EQ(guid_present1, data.front().GetSpecifics()
.autofill_profile().guid());
EXPECT_EQ(guid_present2, data.back().GetSpecifics()
.autofill_profile().guid());
ASSERT_EQ(2U, data.size());
EXPECT_EQ(guid_present1, data[0].GetSpecifics().autofill_profile().guid());
EXPECT_EQ(guid_present2, data[1].GetSpecifics().autofill_profile().guid());
// TODO(isherman): Verify that the origins match once they are saved and read
// from the database. http://crbug.com/170401
// EXPECT_EQ(kHttpOrigin, data[0].GetSpecifics().autofill_profile().origin());
// EXPECT_EQ(kHttpsOrigin,
// data[1].GetSpecifics().autofill_profile().origin());
}

TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
Expand All @@ -230,13 +244,13 @@ TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
std::string guid_synced = kGuid2;

syncer::SyncChangeList change_list;
AutofillProfile profile(guid_synced);
AutofillProfile profile(guid_synced, kHttpOrigin);
profile.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Jane"));
change_list.push_back(
syncer::SyncChange(FROM_HERE,
syncer::SyncChange::ACTION_ADD,
AutofillProfileSyncableService::CreateData(profile)));
AutofillProfile empty_profile(guid_present);
AutofillProfile empty_profile(guid_present, kHttpsOrigin);
change_list.push_back(
syncer::SyncChange(
FROM_HERE,
Expand All @@ -260,7 +274,7 @@ TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
}

TEST_F(AutofillProfileSyncableServiceTest, ActOnChange) {
AutofillProfile profile(kGuid1);
AutofillProfile profile(kGuid1, std::string());
profile.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Jane"));
AutofillProfileChange change1(AutofillProfileChange::ADD, kGuid1, &profile);
AutofillProfileChange change2(AutofillProfileChange::REMOVE, kGuid2, NULL);
Expand All @@ -276,7 +290,7 @@ TEST_F(AutofillProfileSyncableServiceTest, ActOnChange) {
}

TEST_F(AutofillProfileSyncableServiceTest, UpdateField) {
AutofillProfile profile(kGuid1);
AutofillProfile profile(kGuid1, kSettingsOrigin);
std::string company1 = "A Company";
std::string company2 = "Another Company";
profile.SetRawInfo(autofill::COMPANY_NAME, UTF8ToUTF16(company1));
Expand All @@ -292,7 +306,7 @@ TEST_F(AutofillProfileSyncableServiceTest, UpdateField) {
}

TEST_F(AutofillProfileSyncableServiceTest, UpdateMultivaluedField) {
AutofillProfile profile(kGuid1);
AutofillProfile profile(kGuid1, kHttpsOrigin);

std::vector<string16> values;
values.push_back(UTF8ToUTF16("1@1.com"));
Expand Down Expand Up @@ -326,7 +340,7 @@ TEST_F(AutofillProfileSyncableServiceTest, UpdateMultivaluedField) {
}

TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
AutofillProfile profile1(kGuid1);
AutofillProfile profile1(kGuid1, kHttpOrigin);
profile1.SetRawInfo(
autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));

Expand All @@ -335,7 +349,7 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
values.push_back(UTF8ToUTF16("2@1.com"));
profile1.SetRawMultiInfo(autofill::EMAIL_ADDRESS, values);

AutofillProfile profile2(kGuid2);
AutofillProfile profile2(kGuid2, std::string());
profile2.SetRawInfo(
autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));

Expand Down Expand Up @@ -383,7 +397,7 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
ASSERT_EQ(values.size(), 1U);
EXPECT_EQ(values[0], UTF8ToUTF16("650234567"));

AutofillProfile profile3(kGuid3);
AutofillProfile profile3(kGuid3, kHttpOrigin);
profile3.SetRawInfo(
autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));

Expand Down
1 change: 1 addition & 0 deletions chrome/chrome_tests_unit.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,7 @@
'../components/autofill/browser/autocheckout_page_meta_data_unittest.cc',
'../components/autofill/browser/autocomplete_history_manager_unittest.cc',
'../components/autofill/browser/autofill_country_unittest.cc',
'../components/autofill/browser/autofill_data_model_unittest.cc',
'../components/autofill/browser/autofill_download_unittest.cc',
'../components/autofill/browser/autofill_download_url_unittest.cc',
'../components/autofill/browser/autofill_external_delegate_unittest.cc',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
// unique guid or else things break.
const char kAndroidMeContactA[] = "9A9E1C06-7A3B-48FA-AA4F-135CA6FC25D9";

// The origin string for all profiles loaded from the Android contacts list.
const char kAndroidContactsOrigin[] = "Android Contacts";

namespace {

// Takes misc. address information strings from Android API and collapses
Expand Down Expand Up @@ -58,7 +61,8 @@ AuxiliaryProfilesAndroid::~AuxiliaryProfilesAndroid() {
}

scoped_ptr<AutofillProfile> AuxiliaryProfilesAndroid::LoadContactsProfile() {
scoped_ptr<AutofillProfile> profile(new AutofillProfile(kAndroidMeContactA));
scoped_ptr<AutofillProfile> profile(
new AutofillProfile(kAndroidMeContactA, kAndroidContactsOrigin));
LoadName(profile.get());
LoadEmailAddress(profile.get());
LoadPhoneNumbers(profile.get());
Expand Down
4 changes: 2 additions & 2 deletions components/autofill/browser/autofill_common_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ inline void check_and_set(
}

AutofillProfile GetFullProfile() {
AutofillProfile profile(base::GenerateGUID());
AutofillProfile profile(base::GenerateGUID(), "http://www.example.com/");
SetProfileInfo(&profile,
"John",
"H.",
Expand All @@ -53,7 +53,7 @@ AutofillProfile GetFullProfile() {
}

AutofillProfile GetFullProfile2() {
AutofillProfile profile(base::GenerateGUID());
AutofillProfile profile(base::GenerateGUID(), "https://www.example.com/");
SetProfileInfo(&profile,
"Jane",
"A.",
Expand Down
10 changes: 9 additions & 1 deletion components/autofill/browser/autofill_data_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "components/autofill/browser/state_names.h"
#include "components/autofill/browser/validation.h"
#include "components/autofill/common/form_field_data.h"
#include "googleurl/src/gurl.h"
#include "grit/generated_resources.h"
#include "ui/base/l10n/l10n_util.h"

Expand Down Expand Up @@ -120,7 +121,10 @@ bool FillCreditCardTypeSelectControl(const base::string16& value,

} // namespace

AutofillDataModel::AutofillDataModel(const std::string& guid) : guid_(guid) {}
AutofillDataModel::AutofillDataModel(const std::string& guid,
const std::string& origin)
: guid_(guid),
origin_(origin) {}
AutofillDataModel::~AutofillDataModel() {}

void AutofillDataModel::FillSelectControl(AutofillFieldType type,
Expand Down Expand Up @@ -179,4 +183,8 @@ bool AutofillDataModel::FillCountrySelectControl(
return false;
}

bool AutofillDataModel::IsVerified() const {
return !origin_.empty() && !GURL(origin_).is_valid();
}

} // namespace autofill
17 changes: 16 additions & 1 deletion components/autofill/browser/autofill_data_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ struct FormFieldData;
// PersonalDataManager.
class AutofillDataModel : public FormGroup {
public:
explicit AutofillDataModel(const std::string& guid);
AutofillDataModel(const std::string& guid, const std::string& origin);
virtual ~AutofillDataModel();

// Set |field_data|'s value based on |field| and contents of |this| (using
Expand All @@ -36,9 +36,16 @@ class AutofillDataModel : public FormGroup {
const std::string& app_locale,
FormFieldData* field_data) const;

// Returns true if the data in this model was entered directly by the user,
// rather than automatically aggregated.
bool IsVerified() const;

std::string guid() const { return guid_; }
void set_guid(const std::string& guid) { guid_ = guid; }

std::string origin() const { return origin_; }
void set_origin(const std::string& origin) { origin_ = origin; }

protected:
// Fills in a select control for a country from data in |this|. Returns true
// for success.
Expand All @@ -48,6 +55,14 @@ class AutofillDataModel : public FormGroup {
private:
// A globally unique ID for this object.
std::string guid_;

// The origin of this data. This should be
// (a) a web URL for the domain of the form from which the data was
// automatically aggregated, e.g. https://www.example.com/register,
// (b) some other non-empty string, which cannot be interpreted as a web
// URL, identifying the origin for non-aggregated data, or
// (c) an empty string, indicating that the origin for this data is unknown.
std::string origin_;
};

} // namespace autofill
Expand Down
Loading

0 comments on commit 012b8f3

Please sign in to comment.