diff options
author | lipalani@chromium.org <lipalani@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-02-25 19:28:24 +0000 |
---|---|---|
committer | lipalani@chromium.org <lipalani@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-02-25 19:28:24 +0000 |
commit | 72f10ae3436be5443ae8ed72fc4a1ece230a081b (patch) | |
tree | 4b4b136b4a05603089d6f4e7934448c915a1767c /chrome/browser/sync | |
parent | 9a643ab28724555bd0c840dd49b0e8d523a7493f (diff) | |
download | chromium_src-72f10ae3436be5443ae8ed72fc4a1ece230a081b.zip chromium_src-72f10ae3436be5443ae8ed72fc4a1ece230a081b.tar.gz chromium_src-72f10ae3436be5443ae8ed72fc4a1ece230a081b.tar.bz2 |
Fixing all possible scenarios in which a autofill profile sync node with a null guid being inserted.
The fix is to do a DCHECK on debug builds and then return with out committing(both retail and debug).
Passing a autofill profile node with out a valid guid is a bug. The fix here would help us catch those bugs and prevent sync from having to deal with invalid guids.
BUG=71581,73088
TEST=
Review URL: http://codereview.chromium.org/6533010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76082 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync')
6 files changed, 397 insertions, 350 deletions
diff --git a/chrome/browser/sync/glue/autofill_change_processor.cc b/chrome/browser/sync/glue/autofill_change_processor.cc index 6837f68..aef3b99 100644 --- a/chrome/browser/sync/glue/autofill_change_processor.cc +++ b/chrome/browser/sync/glue/autofill_change_processor.cc @@ -340,6 +340,10 @@ void AutofillChangeProcessor::ApplySyncAutofillProfileChange( switch (action) { case sync_api::SyncManager::ChangeRecord::ACTION_ADD: { std::string guid(guid::GenerateGUID()); + if (guid::IsValidGUID(guid) == false) { + DCHECK(false) << "Guid generated is invalid " << guid; + return; + } scoped_ptr<AutoFillProfile> p(new AutoFillProfile); p->set_guid(guid); AutofillModelAssociator::FillProfileWithServerData(p.get(), diff --git a/chrome/browser/sync/glue/autofill_model_associator.cc b/chrome/browser/sync/glue/autofill_model_associator.cc index 4a204af..e081538 100644 --- a/chrome/browser/sync/glue/autofill_model_associator.cc +++ b/chrome/browser/sync/glue/autofill_model_associator.cc @@ -360,6 +360,10 @@ void AutofillModelAssociator::AddNativeProfileIfNeeded( VLOG(1) << "[AUTOFILL MIGRATION]" << "Node not found in web db so creating and associating"; std::string guid = guid::GenerateGUID(); + if (guid::IsValidGUID(guid) == false) { + DCHECK(false) << "Guid generated is invalid " << guid; + return; + } Associate(&guid, node.GetId()); AutoFillProfile* p = new AutoFillProfile(guid); FillProfileWithServerData(p, profile); diff --git a/chrome/browser/sync/glue/autofill_profile_change_processor.cc b/chrome/browser/sync/glue/autofill_profile_change_processor.cc index 5484805..1ba896b 100644 --- a/chrome/browser/sync/glue/autofill_profile_change_processor.cc +++ b/chrome/browser/sync/glue/autofill_profile_change_processor.cc @@ -1,338 +1,358 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/sync/glue/autofill_profile_change_processor.h"
-
-#include <string>
-#include <vector>
-
-#include "base/string_util.h"
-#include "base/utf_string_conversions.h"
-#include "chrome/browser/autofill/autofill_profile.h"
-#include "chrome/browser/autofill/personal_data_manager.h"
-#include "chrome/browser/sync/engine/syncapi.h"
-#include "chrome/browser/sync/glue/autofill_profile_model_associator.h"
-#include "chrome/browser/sync/glue/change_processor.h"
-#include "chrome/browser/sync/glue/do_optimistic_refresh_task.h"
-#include "chrome/browser/sync/unrecoverable_error_handler.h"
-#include "chrome/browser/webdata/autofill_change.h"
-#include "chrome/browser/webdata/web_database.h"
-#include "chrome/common/notification_registrar.h"
-#include "chrome/common/notification_service.h"
-#include "chrome/common/notification_type.h"
-
-namespace browser_sync {
-
-AutofillProfileChangeProcessor::AutofillProfileChangeProcessor(
- AutofillProfileModelAssociator *model_associator,
- WebDatabase* web_database,
- PersonalDataManager* personal_data_manager,
- UnrecoverableErrorHandler* error_handler)
- : ChangeProcessor(error_handler),
- model_associator_(model_associator),
- observing_(false),
- web_database_(web_database),
- personal_data_(personal_data_manager) {
- DCHECK(model_associator);
- DCHECK(web_database);
- DCHECK(error_handler);
- DCHECK(personal_data_manager);
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-
- StartObserving();
-}
-
-AutofillProfileChangeProcessor::~AutofillProfileChangeProcessor() {}
-
-AutofillProfileChangeProcessor::ScopedStopObserving::ScopedStopObserving(
- AutofillProfileChangeProcessor* processor) {
- processor_ = processor;
- processor_->StopObserving();
-}
-
-AutofillProfileChangeProcessor::ScopedStopObserving::~ScopedStopObserving() {
- processor_->StartObserving();
-}
-
-void AutofillProfileChangeProcessor::ApplyChangesFromSyncModel(
- const sync_api::BaseTransaction *write_trans,
- const sync_api::SyncManager::ChangeRecord* changes,
- int change_count) {
-
- ScopedStopObserving observer(this);
-
- sync_api::ReadNode autofill_profile_root(write_trans);
- if (!autofill_profile_root.InitByTagLookup(kAutofillProfileTag)) {
- error_handler()->OnUnrecoverableError(FROM_HERE,
- "Autofill Profile root node lookup failed");
- return;
- }
-
- for (int i = 0; i < change_count; ++i) {
- if (sync_api::SyncManager::ChangeRecord::ACTION_DELETE ==
- changes[i].action) {
- DCHECK(changes[i].specifics.HasExtension(
- sync_pb::autofill_profile));
-
- const sync_pb::AutofillProfileSpecifics& specifics =
- changes[i].specifics.GetExtension(sync_pb::autofill_profile);
-
- autofill_changes_.push_back(AutofillProfileChangeRecord(changes[i].action,
- changes[i].id,
- specifics));
- continue;
- }
-
- // If it is not a delete.
- sync_api::ReadNode sync_node(write_trans);
- if (!sync_node.InitByIdLookup(changes[i].id)) {
- LOG(ERROR) << "Could not find the id in sync db " << changes[i].id;
- continue;
- }
-
- const sync_pb::AutofillProfileSpecifics& autofill(
- sync_node.GetAutofillProfileSpecifics());
-
- autofill_changes_.push_back(AutofillProfileChangeRecord(changes[i].action,
- changes[i].id,
- autofill));
- }
-}
-
-void AutofillProfileChangeProcessor::Observe(NotificationType type,
- const NotificationSource& source,
- const NotificationDetails& details) {
- DCHECK_EQ(type.value, NotificationType::AUTOFILL_PROFILE_CHANGED);
- WebDataService* wds = Source<WebDataService>(source).ptr();
-
- if (!wds || wds->GetDatabase() != web_database_)
- return;
-
- sync_api::WriteTransaction trans(share_handle());
- sync_api::ReadNode autofill_root(&trans);
- if (!autofill_root.InitByTagLookup(kAutofillProfileTag)) {
- error_handler()->OnUnrecoverableError(FROM_HERE,
- "Server did not create a tolp level node");
- return;
- }
-
- AutofillProfileChange* change = Details<AutofillProfileChange>(details).ptr();
-
- ActOnChange(change, &trans, autofill_root);
-}
-
-void AutofillProfileChangeProcessor::ActOnChange(
- AutofillProfileChange* change,
- sync_api::WriteTransaction* trans,
- sync_api::ReadNode& autofill_root) {
- DCHECK(change->type() == AutofillProfileChange::REMOVE || change->profile());
- switch (change->type()) {
- case AutofillProfileChange::ADD: {
- AddAutofillProfileSyncNode(trans, autofill_root, *(change->profile()));
- break;
- }
- case AutofillProfileChange::UPDATE: {
- int64 sync_id = model_associator_->GetSyncIdFromChromeId(change->key());
- if (sync_api::kInvalidId == sync_id) {
- LOG(ERROR) << "Sync id is not found for " << change->key();
- break;
- }
- sync_api::WriteNode node(trans);
- if (!node.InitByIdLookup(sync_id)) {
- LOG(ERROR) << "Could not find sync node for id " << sync_id;
- break;
- }
-
- WriteAutofillProfile(*(change->profile()), &node);
- break;
- }
- case AutofillProfileChange::REMOVE: {
- int64 sync_id = model_associator_->GetSyncIdFromChromeId(change->key());
- if (sync_api::kInvalidId == sync_id) {
- LOG(ERROR) << "Sync id is not found for " << change->key();
- break;
- }
- sync_api::WriteNode node(trans);
- if (!node.InitByIdLookup(sync_id)) {
- LOG(ERROR) << "Could not find sync node for id " << sync_id;
- break;
- }
- node.Remove();
- model_associator_->Disassociate(sync_id);
- break;
- }
- default:
- NOTREACHED();
- }
-}
-
-void AutofillProfileChangeProcessor::CommitChangesFromSyncModel() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-
- if (!running())
- return;
-
- ScopedStopObserving observer(this);
-
- for (unsigned int i = 0;i < autofill_changes_.size(); ++i) {
- if (sync_api::SyncManager::ChangeRecord::ACTION_DELETE ==
- autofill_changes_[i].action_) {
- if (!web_database_->RemoveAutoFillProfile(
- autofill_changes_[i].profile_specifics_.guid())) {
- LOG(ERROR) << "could not delete the profile " <<
- autofill_changes_[i].profile_specifics_.guid();
- continue;
- }
- continue;
- }
-
- // Now for updates and adds.
- ApplyAutofillProfileChange(autofill_changes_[i].action_,
- autofill_changes_[i].profile_specifics_,
- autofill_changes_[i].id_);
- }
-
- autofill_changes_.clear();
-
- PostOptimisticRefreshTask();
-}
-
-void AutofillProfileChangeProcessor::PostOptimisticRefreshTask() {
- BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
- new DoOptimisticRefreshForAutofill(
- personal_data_));
-}
-
-void AutofillProfileChangeProcessor::ApplyAutofillProfileChange(
- sync_api::SyncManager::ChangeRecord::Action action,
- const sync_pb::AutofillProfileSpecifics& profile_specifics,
- int64 sync_id) {
-
- DCHECK_NE(sync_api::SyncManager::ChangeRecord::ACTION_DELETE, action);
- switch (action) {
- case sync_api::SyncManager::ChangeRecord::ACTION_ADD: {
- AutoFillProfile p(profile_specifics.guid());
- AutofillProfileModelAssociator::OverwriteProfileWithServerData(&p,
- profile_specifics);
- if (!web_database_->AddAutoFillProfile(p)) {
- LOG(ERROR) << "could not add autofill profile for guid " << p.guid();
- break;
- }
-
- // Now that the node has been succesfully created we can associate it.
- std::string guid = p.guid();
- model_associator_->Associate(&guid, sync_id);
- break;
- }
- case sync_api::SyncManager::ChangeRecord::ACTION_UPDATE: {
- AutoFillProfile *p;
- if (!web_database_->GetAutoFillProfile(profile_specifics.guid(), &p)) {
- LOG(ERROR) << "Could not find the autofill profile to update for " <<
- profile_specifics.guid();
- break;
- }
- scoped_ptr<AutoFillProfile> autofill_pointer(p);
- AutofillProfileModelAssociator::OverwriteProfileWithServerData(
- autofill_pointer.get(),
- profile_specifics);
-
- if (!web_database_->UpdateAutoFillProfile(*(autofill_pointer.get()))) {
- LOG(ERROR) << "Could not update autofill profile for " <<
- profile_specifics.guid();
- break;
- }
- break;
- }
- default: {
- NOTREACHED();
- break;
- }
- }
-}
-
-void AutofillProfileChangeProcessor::RemoveSyncNode(const std::string& guid,
- sync_api::WriteTransaction* trans) {
- sync_api::WriteNode node(trans);
- int64 sync_id = model_associator_->GetSyncIdFromChromeId(guid);
- if (sync_api::kInvalidId == sync_id) {
- LOG(ERROR) << "Could not find the node in associator " << guid;
- return;
- }
-
- if (!node.InitByIdLookup(sync_id)) {
- LOG(ERROR) << "Could not find the sync node for " << guid;
- return;
- }
-
- model_associator_->Disassociate(sync_id);
- node.Remove();
-}
-
-void AutofillProfileChangeProcessor::AddAutofillProfileSyncNode(
- sync_api::WriteTransaction* trans,
- sync_api::BaseNode& autofill_profile_root,
- const AutoFillProfile& profile) {
- sync_api::WriteNode node(trans);
- if (!node.InitUniqueByCreation(syncable::AUTOFILL_PROFILE,
- autofill_profile_root,
- profile.guid())) {
- LOG(ERROR) << "could not create a sync node ";
- return;
- }
-
- node.SetTitle(UTF8ToWide(profile.guid()));
-
- WriteAutofillProfile(profile, &node);
-
- std::string guid = profile.guid();
- model_associator_->Associate(&guid, node.GetId());
-}
-
-void AutofillProfileChangeProcessor::StartObserving() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
- notification_registrar_.Add(this,
- NotificationType::AUTOFILL_PROFILE_CHANGED,
- NotificationService::AllSources());
-}
-
-void AutofillProfileChangeProcessor::StopObserving() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
- notification_registrar_.RemoveAll();
-}
-
-void AutofillProfileChangeProcessor::WriteAutofillProfile(
- const AutoFillProfile& profile,
- sync_api::WriteNode* node) {
- sync_pb::AutofillProfileSpecifics specifics;
- specifics.set_guid(profile.guid());
- specifics.set_name_first(UTF16ToUTF8(
- profile.GetFieldText(AutoFillType(NAME_FIRST))));
- specifics.set_name_middle(UTF16ToUTF8(
- profile.GetFieldText(AutoFillType(NAME_MIDDLE))));
- specifics.set_name_last(
- UTF16ToUTF8(profile.GetFieldText(AutoFillType(NAME_LAST))));
- specifics.set_address_home_line1(
- UTF16ToUTF8(profile.GetFieldText(AutoFillType(ADDRESS_HOME_LINE1))));
- specifics.set_address_home_line2(
- UTF16ToUTF8(profile.GetFieldText(AutoFillType(ADDRESS_HOME_LINE2))));
- specifics.set_address_home_city(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(ADDRESS_HOME_CITY))));
- specifics.set_address_home_state(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(ADDRESS_HOME_STATE))));
- specifics.set_address_home_country(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(ADDRESS_HOME_COUNTRY))));
- specifics.set_address_home_zip(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(ADDRESS_HOME_ZIP))));
- specifics.set_email_address(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(EMAIL_ADDRESS))));
- specifics.set_company_name(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(COMPANY_NAME))));
- specifics.set_phone_fax_whole_number(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(PHONE_FAX_WHOLE_NUMBER))));
- specifics.set_phone_home_whole_number(UTF16ToUTF8(profile.GetFieldText(
- AutoFillType(PHONE_HOME_WHOLE_NUMBER))));
- node->SetAutofillProfileSpecifics(specifics);
-}
-
-} // namespace browser_sync
+// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/sync/glue/autofill_profile_change_processor.h" + +#include <string> +#include <vector> + +#include "base/string_util.h" +#include "base/utf_string_conversions.h" +#include "chrome/browser/autofill/autofill_profile.h" +#include "chrome/browser/autofill/personal_data_manager.h" +#include "chrome/browser/sync/engine/syncapi.h" +#include "chrome/browser/sync/glue/autofill_profile_model_associator.h" +#include "chrome/browser/sync/glue/change_processor.h" +#include "chrome/browser/sync/glue/do_optimistic_refresh_task.h" +#include "chrome/browser/sync/unrecoverable_error_handler.h" +#include "chrome/browser/webdata/autofill_change.h" +#include "chrome/browser/webdata/web_database.h" +#include "chrome/common/guid.h" +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/notification_type.h" + +namespace browser_sync { + +AutofillProfileChangeProcessor::AutofillProfileChangeProcessor( + AutofillProfileModelAssociator *model_associator, + WebDatabase* web_database, + PersonalDataManager* personal_data_manager, + UnrecoverableErrorHandler* error_handler) + : ChangeProcessor(error_handler), + model_associator_(model_associator), + observing_(false), + web_database_(web_database), + personal_data_(personal_data_manager) { + DCHECK(model_associator); + DCHECK(web_database); + DCHECK(error_handler); + DCHECK(personal_data_manager); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); + + StartObserving(); +} + +AutofillProfileChangeProcessor::~AutofillProfileChangeProcessor() {} + +AutofillProfileChangeProcessor::ScopedStopObserving::ScopedStopObserving( + AutofillProfileChangeProcessor* processor) { + processor_ = processor; + processor_->StopObserving(); +} + +AutofillProfileChangeProcessor::ScopedStopObserving::~ScopedStopObserving() { + processor_->StartObserving(); +} + +void AutofillProfileChangeProcessor::ApplyChangesFromSyncModel( + const sync_api::BaseTransaction *write_trans, + const sync_api::SyncManager::ChangeRecord* changes, + int change_count) { + + ScopedStopObserving observer(this); + + sync_api::ReadNode autofill_profile_root(write_trans); + if (!autofill_profile_root.InitByTagLookup(kAutofillProfileTag)) { + error_handler()->OnUnrecoverableError(FROM_HERE, + "Autofill Profile root node lookup failed"); + return; + } + + for (int i = 0; i < change_count; ++i) { + if (sync_api::SyncManager::ChangeRecord::ACTION_DELETE == + changes[i].action) { + DCHECK(changes[i].specifics.HasExtension( + sync_pb::autofill_profile)); + + const sync_pb::AutofillProfileSpecifics& specifics = + changes[i].specifics.GetExtension(sync_pb::autofill_profile); + + autofill_changes_.push_back(AutofillProfileChangeRecord(changes[i].action, + changes[i].id, + specifics)); + continue; + } + + // If it is not a delete. + sync_api::ReadNode sync_node(write_trans); + if (!sync_node.InitByIdLookup(changes[i].id)) { + LOG(ERROR) << "Could not find the id in sync db " << changes[i].id; + continue; + } + + const sync_pb::AutofillProfileSpecifics& autofill( + sync_node.GetAutofillProfileSpecifics()); + + autofill_changes_.push_back(AutofillProfileChangeRecord(changes[i].action, + changes[i].id, + autofill)); + } +} + +void AutofillProfileChangeProcessor::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK_EQ(type.value, NotificationType::AUTOFILL_PROFILE_CHANGED); + WebDataService* wds = Source<WebDataService>(source).ptr(); + + if (!wds || wds->GetDatabase() != web_database_) + return; + + sync_api::WriteTransaction trans(share_handle()); + sync_api::ReadNode autofill_root(&trans); + if (!autofill_root.InitByTagLookup(kAutofillProfileTag)) { + error_handler()->OnUnrecoverableError(FROM_HERE, + "Server did not create a tolp level node"); + return; + } + + AutofillProfileChange* change = Details<AutofillProfileChange>(details).ptr(); + + ActOnChange(change, &trans, autofill_root); +} + +void AutofillProfileChangeProcessor::ActOnChange( + AutofillProfileChange* change, + sync_api::WriteTransaction* trans, + sync_api::ReadNode& autofill_root) { + DCHECK(change->type() == AutofillProfileChange::REMOVE || change->profile()); + switch (change->type()) { + case AutofillProfileChange::ADD: { + AddAutofillProfileSyncNode(trans, autofill_root, *(change->profile())); + break; + } + case AutofillProfileChange::UPDATE: { + int64 sync_id = model_associator_->GetSyncIdFromChromeId(change->key()); + if (sync_api::kInvalidId == sync_id) { + LOG(ERROR) << "Sync id is not found for " << change->key(); + break; + } + sync_api::WriteNode node(trans); + if (!node.InitByIdLookup(sync_id)) { + LOG(ERROR) << "Could not find sync node for id " << sync_id; + break; + } + + WriteAutofillProfile(*(change->profile()), &node); + break; + } + case AutofillProfileChange::REMOVE: { + int64 sync_id = model_associator_->GetSyncIdFromChromeId(change->key()); + if (sync_api::kInvalidId == sync_id) { + LOG(ERROR) << "Sync id is not found for " << change->key(); + break; + } + sync_api::WriteNode node(trans); + if (!node.InitByIdLookup(sync_id)) { + LOG(ERROR) << "Could not find sync node for id " << sync_id; + break; + } + node.Remove(); + model_associator_->Disassociate(sync_id); + break; + } + default: + NOTREACHED(); + } +} + +void AutofillProfileChangeProcessor::CommitChangesFromSyncModel() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); + + if (!running()) + return; + + ScopedStopObserving observer(this); + + for (unsigned int i = 0;i < autofill_changes_.size(); ++i) { + if (sync_api::SyncManager::ChangeRecord::ACTION_DELETE == + autofill_changes_[i].action_) { + if (!web_database_->RemoveAutoFillProfile( + autofill_changes_[i].profile_specifics_.guid())) { + LOG(ERROR) << "could not delete the profile " << + autofill_changes_[i].profile_specifics_.guid(); + continue; + } + continue; + } + + // Now for updates and adds. + ApplyAutofillProfileChange(autofill_changes_[i].action_, + autofill_changes_[i].profile_specifics_, + autofill_changes_[i].id_); + } + + autofill_changes_.clear(); + + PostOptimisticRefreshTask(); +} + +void AutofillProfileChangeProcessor::PostOptimisticRefreshTask() { + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + new DoOptimisticRefreshForAutofill( + personal_data_)); +} + +void AutofillProfileChangeProcessor::ApplyAutofillProfileChange( + sync_api::SyncManager::ChangeRecord::Action action, + const sync_pb::AutofillProfileSpecifics& profile_specifics, + int64 sync_id) { + + DCHECK_NE(sync_api::SyncManager::ChangeRecord::ACTION_DELETE, action); + switch (action) { + case sync_api::SyncManager::ChangeRecord::ACTION_ADD: { + if(guid::IsValidGUID(profile_specifics.guid()) == false) { + NOTREACHED() << "Guid from the server is invalid " << + profile_specifics.guid(); + return; + } + AutoFillProfile p(profile_specifics.guid()); + AutofillProfileModelAssociator::OverwriteProfileWithServerData(&p, + profile_specifics); + if (!web_database_->AddAutoFillProfile(p)) { + LOG(ERROR) << "could not add autofill profile for guid " << p.guid(); + break; + } + + // Now that the node has been succesfully created we can associate it. + std::string guid = p.guid(); + model_associator_->Associate(&guid, sync_id); + break; + } + case sync_api::SyncManager::ChangeRecord::ACTION_UPDATE: { + AutoFillProfile *p; + if (!web_database_->GetAutoFillProfile(profile_specifics.guid(), &p)) { + LOG(ERROR) << "Could not find the autofill profile to update for " << + profile_specifics.guid(); + break; + } + scoped_ptr<AutoFillProfile> autofill_pointer(p); + AutofillProfileModelAssociator::OverwriteProfileWithServerData( + autofill_pointer.get(), + profile_specifics); + + if (!web_database_->UpdateAutoFillProfile(*(autofill_pointer.get()))) { + LOG(ERROR) << "Could not update autofill profile for " << + profile_specifics.guid(); + break; + } + break; + } + default: { + NOTREACHED(); + break; + } + } +} + +void AutofillProfileChangeProcessor::RemoveSyncNode(const std::string& guid, + sync_api::WriteTransaction* trans) { + sync_api::WriteNode node(trans); + int64 sync_id = model_associator_->GetSyncIdFromChromeId(guid); + if (sync_api::kInvalidId == sync_id) { + LOG(ERROR) << "Could not find the node in associator " << guid; + return; + } + + if (!node.InitByIdLookup(sync_id)) { + LOG(ERROR) << "Could not find the sync node for " << guid; + return; + } + + model_associator_->Disassociate(sync_id); + node.Remove(); +} + +void AutofillProfileChangeProcessor::AddAutofillProfileSyncNode( + sync_api::WriteTransaction* trans, + sync_api::BaseNode& autofill_profile_root, + const AutoFillProfile& profile) { + + std::string guid = profile.guid(); + + if(guid::IsValidGUID(guid) == false) { + DCHECK(false) << "Guid set on the profile is invalid " << guid; + return; + } + + sync_api::WriteNode node(trans); + if (!node.InitUniqueByCreation(syncable::AUTOFILL_PROFILE, + autofill_profile_root, + profile.guid())) { + LOG(ERROR) << "could not create a sync node "; + return; + } + + node.SetTitle(UTF8ToWide(profile.guid())); + + WriteAutofillProfile(profile, &node); + + model_associator_->Associate(&guid, node.GetId()); +} + +void AutofillProfileChangeProcessor::StartObserving() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); + notification_registrar_.Add(this, + NotificationType::AUTOFILL_PROFILE_CHANGED, + NotificationService::AllSources()); +} + +void AutofillProfileChangeProcessor::StopObserving() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); + notification_registrar_.RemoveAll(); +} + +void AutofillProfileChangeProcessor::WriteAutofillProfile( + const AutoFillProfile& profile, + sync_api::WriteNode* node) { + sync_pb::AutofillProfileSpecifics specifics; + + // This would get compiled out in official builds. The caller is expected to + // pass in a valid profile object with valid guid.(i.e., the caller might + // have to a DCHECK and log before calling. Having to check in 2 places is + // not optimal.) + DCHECK(guid::IsValidGUID(profile.guid())); + + specifics.set_guid(profile.guid()); + specifics.set_name_first(UTF16ToUTF8( + profile.GetFieldText(AutoFillType(NAME_FIRST)))); + specifics.set_name_middle(UTF16ToUTF8( + profile.GetFieldText(AutoFillType(NAME_MIDDLE)))); + specifics.set_name_last( + UTF16ToUTF8(profile.GetFieldText(AutoFillType(NAME_LAST)))); + specifics.set_address_home_line1( + UTF16ToUTF8(profile.GetFieldText(AutoFillType(ADDRESS_HOME_LINE1)))); + specifics.set_address_home_line2( + UTF16ToUTF8(profile.GetFieldText(AutoFillType(ADDRESS_HOME_LINE2)))); + specifics.set_address_home_city(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(ADDRESS_HOME_CITY)))); + specifics.set_address_home_state(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(ADDRESS_HOME_STATE)))); + specifics.set_address_home_country(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(ADDRESS_HOME_COUNTRY)))); + specifics.set_address_home_zip(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(ADDRESS_HOME_ZIP)))); + specifics.set_email_address(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(EMAIL_ADDRESS)))); + specifics.set_company_name(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(COMPANY_NAME)))); + specifics.set_phone_fax_whole_number(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(PHONE_FAX_WHOLE_NUMBER)))); + specifics.set_phone_home_whole_number(UTF16ToUTF8(profile.GetFieldText( + AutoFillType(PHONE_HOME_WHOLE_NUMBER)))); + node->SetAutofillProfileSpecifics(specifics); +} + +} // namespace browser_sync diff --git a/chrome/browser/sync/glue/autofill_profile_model_associator.cc b/chrome/browser/sync/glue/autofill_profile_model_associator.cc index 295a7ca..8502dcc 100644 --- a/chrome/browser/sync/glue/autofill_profile_model_associator.cc +++ b/chrome/browser/sync/glue/autofill_profile_model_associator.cc @@ -9,6 +9,7 @@ #include "chrome/browser/sync/glue/do_optimistic_refresh_task.h" #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/webdata/web_database.h" +#include "chrome/common/guid.h" using sync_api::ReadNode; namespace browser_sync { @@ -76,6 +77,10 @@ bool AutofillProfileModelAssociator::TraverseAndAssociateChromeAutoFillProfiles( ix != profiles.end(); ++ix) { std::string guid((*ix)->guid()); + if (guid::IsValidGUID(guid) == false) { + DCHECK(false) << "Guid in the web db is invalid " << guid; + continue; + } ReadNode node(write_trans); if (node.InitByClientTagLookup(syncable::AUTOFILL_PROFILE, guid) && @@ -304,6 +309,11 @@ bool AutofillProfileModelAssociator::MakeNewAutofillProfileSyncNodeIfNeeded( } const sync_pb::AutofillProfileSpecifics& autofill_specifics( read_node.GetAutofillProfileSpecifics()); + if (guid::IsValidGUID(autofill_specifics.guid()) == false) { + NOTREACHED() << "Guid in the web db is invalid " << + autofill_specifics.guid(); + return false; + } AutoFillProfile* p = new AutoFillProfile(autofill_specifics.guid()); OverwriteProfileWithServerData(p, autofill_specifics); new_profiles->push_back(p); @@ -319,6 +329,10 @@ bool AutofillProfileModelAssociator::MakeNewAutofillProfileSyncNodeIfNeeded( << profile.guid(); } else { sync_api::WriteNode node(trans); + + // The profile.guid() is expected to be a valid guid. The caller is expected + // to pass in a valid profile object with a valid guid. Having to check in + // 2 places(the caller and here) is not optimal. if (!node.InitUniqueByCreation( syncable::AUTOFILL_PROFILE, autofill_root, profile.guid())) { LOG(ERROR) << "Failed to create autofill sync node."; @@ -381,6 +395,11 @@ void AutofillProfileModelAssociator::AddNativeProfileIfNeeded( << " Guid " << profile.guid() << " in the web db"; + if (guid::IsValidGUID(profile.guid()) == false) { + DCHECK(false) << "Guid in the sync db is invalid " << profile.guid(); + return; + } + if (bundle->current_profiles.find(profile.guid()) == bundle->current_profiles.end()) { std::string guid(profile.guid()); diff --git a/chrome/browser/sync/glue/autofill_profile_model_associator_unittest.cc b/chrome/browser/sync/glue/autofill_profile_model_associator_unittest.cc index ee6a9fc..8843b5a 100644 --- a/chrome/browser/sync/glue/autofill_profile_model_associator_unittest.cc +++ b/chrome/browser/sync/glue/autofill_profile_model_associator_unittest.cc @@ -95,7 +95,7 @@ class AutofillProfileModelAssociatorTest : public testing::Test { TEST_F(AutofillProfileModelAssociatorTest, TestAssociateProfileInWebDBWithSyncDB) { ScopedVector<AutoFillProfile> profiles_from_web_db; - std::string guid = "abc"; + std::string guid = "EDC609ED-7EEE-4F27-B00C-423242A9C44B"; sync_pb::EntitySpecifics specifics; MockDirectory mock_directory; @@ -147,7 +147,7 @@ TEST_F(AutofillProfileModelAssociatorTest, TestAssociatingMissingWebDBProfile) { sync_api::ReadNode autofill_root(&write_trans); - std::string guid = "abc"; + std::string guid = "EDC609ED-7EEE-4F27-B00C-423242A9C44A"; std::set<std::string> current_profiles; AutoFillProfile *profile = new AutoFillProfile(guid); @@ -216,7 +216,7 @@ TEST_F(AutofillProfileModelAssociatorTest, TestDontNeedToAddNativeProfile) { ::testing::StrictMock<MockAutofillProfileModelAssociator> associator; sync_pb::AutofillProfileSpecifics profile_specifics; ReadNodeMock read_node; - std::string guid = "abc"; + std::string guid = "EDC609ED-7EEE-4F27-B00C-423242A9C44C"; std::set<std::string> current_profiles; browser_sync::AutofillProfileModelAssociator::DataBundle bundle; @@ -234,7 +234,7 @@ TEST_F(AutofillProfileModelAssociatorTest, TestDontNeedToAddNativeProfile) { TEST_F(AutofillProfileModelAssociatorTest, TestNeedToAddNativeProfile) { sync_pb::AutofillProfileSpecifics profile_specifics; ReadNodeMock read_node; - std::string guid = "abc"; + std::string guid = "EDC609ED-7EEE-4F27-B00C-423242A9C44D"; std::set<std::string> current_profiles; browser_sync::AutofillProfileModelAssociator::DataBundle bundle; std::string first_name = "lingesh"; diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc index 069c501..4b4604e 100644 --- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc @@ -657,7 +657,7 @@ TEST_F(ProfileSyncServiceAutofillTest, HasProfileEmptySync) { // Owned by GetAutoFillProfiles caller. AutoFillProfile* profile0 = new AutoFillProfile; autofill_test::SetProfileInfoWithGuid(profile0, - "{54B3F9AA-335E-4f71-A27D-719C41564230}", "Billing", + "54B3F9AA-335E-4F71-A27D-719C41564230", "Billing", "Mitchell", "Morrison", "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA", "91601", "US", "12345678910", "01987654321"); @@ -767,14 +767,14 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeEntry) { TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeProfile) { AutoFillProfile sync_profile; autofill_test::SetProfileInfoWithGuid(&sync_profile, - "{23355099-1170-4b71-8ED4-144470CC9EBE}", "Billing", + "23355099-1170-4B71-8ED4-144470CC9EBE", "Billing", "Mitchell", "Morrison", "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA", "91601", "US", "12345678910", "01987654321"); AutoFillProfile* native_profile = new AutoFillProfile; autofill_test::SetProfileInfoWithGuid(native_profile, - "{23355099-1170-4b71-8ED4-144470CC9EBE}", "Billing", "Alicia", "Saenz", + "23355099-1170-4B71-8ED4-144470CC9EBE", "Billing", "Alicia", "Saenz", "joewayne@me.xyz", "Fox", "1212 Center.", "Bld. 5", "Orlando", "FL", "32801", "US", "19482937549", "13502849239"); @@ -804,12 +804,12 @@ TEST_F(ProfileSyncServiceAutofillTest, MergeProfileWithDifferentGuid) { AutoFillProfile sync_profile; autofill_test::SetProfileInfoWithGuid(&sync_profile, - "{23355099-1170-4b71-8ED4-144470CC9EBE}", "Billing", + "23355099-1170-4B71-8ED4-144470CC9EBE", "Billing", "Mitchell", "Morrison", "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA", "91601", "US", "12345678910", "01987654321"); - std::string native_guid = "{EDC609ED-7EEE-4f27-B00C-423242A9C44B}"; + std::string native_guid = "EDC609ED-7EEE-4F27-B00C-423242A9C44B"; AutoFillProfile* native_profile = new AutoFillProfile; autofill_test::SetProfileInfoWithGuid(native_profile, native_guid.c_str(), "Billing", @@ -882,7 +882,7 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeAddProfile) { AutoFillProfile added_profile; autofill_test::SetProfileInfoWithGuid(&added_profile, - "{D6ADA912-D374-4c0a-917D-F5C8EBE43011}", "Josephine", "Alicia", "Saenz", + "D6ADA912-D374-4C0A-917D-F5C8EBE43011", "Josephine", "Alicia", "Saenz", "joewayne@me.xyz", "Fox", "1212 Center.", "Bld. 5", "Orlando", "FL", "32801", "US", "19482937549", "13502849239"); @@ -967,12 +967,12 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeRemoveEntry) { TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeRemoveProfile) { AutoFillProfile sync_profile; autofill_test::SetProfileInfoWithGuid(&sync_profile, - "{3BA5FA1B-1EC4-4bb3-9B57-EC92BE3C1A09}", "Josephine", "Alicia", "Saenz", + "3BA5FA1B-1EC4-4BB3-9B57-EC92BE3C1A09", "Josephine", "Alicia", "Saenz", "joewayne@me.xyz", "Fox", "1212 Center.", "Bld. 5", "Orlando", "FL", "32801", "US", "19482937549", "13502849239"); AutoFillProfile* native_profile = new AutoFillProfile; autofill_test::SetProfileInfoWithGuid(native_profile, - "{3BA5FA1B-1EC4-4bb3-9B57-EC92BE3C1A09}", "Josephine", "Alicia", "Saenz", + "3BA5FA1B-1EC4-4BB3-9B57-EC92BE3C1A09", "Josephine", "Alicia", "Saenz", "joewayne@me.xyz", "Fox", "1212 Center.", "Bld. 5", "Orlando", "FL", "32801", "US", "19482937549", "13502849239"); |