Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(22)

Unified Diff: components/sync_driver/generic_change_processor.cc

Issue 265853004: Revert of Keep track of which attachments are referenced by which sync entries. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/sync_driver/generic_change_processor.cc
diff --git a/components/sync_driver/generic_change_processor.cc b/components/sync_driver/generic_change_processor.cc
index 49889a69c1b2234b94d27826d40e9fa9cbc76ca3..adc4c90ed42d96897330b9168588cea05beb58b3 100644
--- a/components/sync_driver/generic_change_processor.cc
+++ b/components/sync_driver/generic_change_processor.cc
@@ -34,27 +34,6 @@
} else {
write_node->SetEntitySpecifics(entity_specifics);
}
-}
-
-// Helper function to convert AttachmentId to AttachmentMetadataRecord.
-sync_pb::AttachmentMetadataRecord AttachmentIdToRecord(
- const syncer::AttachmentId& attachment_id) {
- sync_pb::AttachmentMetadataRecord record;
- *record.mutable_id() = attachment_id.GetProto();
- return record;
-}
-
-// Replace |write_nodes|'s attachment ids with |attachment_ids|.
-void SetAttachmentMetadata(const syncer::AttachmentIdList& attachment_ids,
- syncer::WriteNode* write_node) {
- DCHECK(write_node);
- sync_pb::AttachmentMetadata attachment_metadata;
- std::transform(
- attachment_ids.begin(),
- attachment_ids.end(),
- RepeatedFieldBackInserter(attachment_metadata.mutable_record()),
- AttachmentIdToRecord);
- write_node->SetAttachmentMetadata(attachment_metadata);
}
syncer::SyncData BuildRemoteSyncData(
@@ -340,11 +319,12 @@
}
}
-syncer::SyncError AttemptDelete(const syncer::SyncChange& change,
- syncer::ModelType type,
- const std::string& type_str,
- syncer::WriteNode* node,
- DataTypeErrorHandler* error_handler) {
+syncer::SyncError AttemptDelete(
+ const syncer::SyncChange& change,
+ syncer::ModelType type,
+ const std::string& type_str,
+ syncer::WriteNode* node,
+ DataTypeErrorHandler* error_handler) {
DCHECK_EQ(change.change_type(), syncer::SyncChange::ACTION_DELETE);
if (change.sync_data().IsLocal()) {
const std::string& tag = syncer::SyncDataLocal(change.sync_data()).GetTag();
@@ -388,36 +368,12 @@
return syncer::SyncError();
}
-// A callback invoked on completion of AttachmentService::StoreAttachment.
-void IgnoreStoreResult(const syncer::AttachmentService::StoreResult&) {
- // TODO(maniscalco): Here is where we're going to update the in-directory
- // entry to indicate that the attachments have been successfully stored on
- // disk. Why do we care? Because we might crash after persisting the
- // directory to disk, but before we have persisted its attachments, leaving us
- // with danging attachment ids. Having a flag that indicates we've stored the
- // entry will allow us to detect and filter entries with dangling attachment
- // ids (bug 368353).
-}
-
-void StoreAttachments(syncer::AttachmentService* attachment_service,
- const syncer::AttachmentList& attachments) {
- DCHECK(attachment_service);
- syncer::AttachmentService::StoreCallback ignore_store_result =
- base::Bind(&IgnoreStoreResult);
- attachment_service->StoreAttachments(attachments, ignore_store_result);
-}
-
} // namespace
syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
const tracked_objects::Location& from_here,
const syncer::SyncChangeList& list_of_changes) {
DCHECK(CalledOnValidThread());
-
- // Keep track of brand new attachments so we can persist them on this device
- // and upload them to the server.
- syncer::AttachmentList new_attachments;
-
syncer::WriteTransaction trans(from_here, share_handle());
for (syncer::SyncChangeList::const_iterator iter = list_of_changes.begin();
@@ -435,19 +391,20 @@
NOTREACHED();
return error;
}
+ attachment_service_->OnSyncDataDelete(change.sync_data());
if (merge_result_.get()) {
merge_result_->set_num_items_deleted(
merge_result_->num_items_deleted() + 1);
}
} else if (change.change_type() == syncer::SyncChange::ACTION_ADD) {
- syncer::SyncError error = HandleActionAdd(
- change, type_str, type, trans, &sync_node, &new_attachments);
+ syncer::SyncError error =
+ HandleActionAdd(change, type_str, type, trans, &sync_node);
if (error.IsSet()) {
return error;
}
} else if (change.change_type() == syncer::SyncChange::ACTION_UPDATE) {
- syncer::SyncError error = HandleActionUpdate(
- change, type_str, type, trans, &sync_node, &new_attachments);
+ syncer::SyncError error =
+ HandleActionUpdate(change, type_str, type, trans, &sync_node);
if (error.IsSet()) {
return error;
}
@@ -465,11 +422,6 @@
return error;
}
}
-
- if (!new_attachments.empty()) {
- StoreAttachments(attachment_service_.get(), new_attachments);
- }
-
return syncer::SyncError();
}
@@ -482,14 +434,12 @@
const std::string& type_str,
const syncer::ModelType& type,
const syncer::WriteTransaction& trans,
- syncer::WriteNode* sync_node,
- syncer::AttachmentList* new_attachments) {
+ syncer::WriteNode* sync_node) {
// TODO(sync): Handle other types of creation (custom parents, folders,
// etc.).
syncer::ReadNode root_node(&trans);
- const syncer::SyncDataLocal sync_data_local(change.sync_data());
if (root_node.InitByTagLookup(syncer::ModelTypeToRootTag(
- sync_data_local.GetDataType())) != syncer::BaseNode::INIT_OK) {
+ change.sync_data().GetDataType())) != syncer::BaseNode::INIT_OK) {
syncer::SyncError error(FROM_HERE,
syncer::SyncError::DATATYPE_ERROR,
"Failed to look up root node for type " + type_str,
@@ -502,7 +452,9 @@
}
syncer::WriteNode::InitUniqueByCreationResult result =
sync_node->InitUniqueByCreation(
- sync_data_local.GetDataType(), root_node, sync_data_local.GetTag());
+ change.sync_data().GetDataType(),
+ root_node,
+ syncer::SyncDataLocal(change.sync_data()).GetTag());
if (result != syncer::WriteNode::INIT_SUCCESS) {
std::string error_prefix = "Failed to create " + type_str + " node: " +
change.location().ToString() + ", ";
@@ -551,18 +503,8 @@
}
}
sync_node->SetTitle(change.sync_data().GetTitle());
- SetNodeSpecifics(sync_data_local.GetSpecifics(), sync_node);
-
- syncer::AttachmentIdList attachment_ids = sync_data_local.GetAttachmentIds();
- SetAttachmentMetadata(attachment_ids, sync_node);
-
- // Return any newly added attachments.
- const syncer::AttachmentList& local_attachments_for_upload =
- sync_data_local.GetLocalAttachmentsForUpload();
- new_attachments->insert(new_attachments->end(),
- local_attachments_for_upload.begin(),
- local_attachments_for_upload.end());
-
+ SetNodeSpecifics(change.sync_data().GetSpecifics(), sync_node);
+ attachment_service_->OnSyncDataAdd(change.sync_data());
if (merge_result_.get()) {
merge_result_->set_num_items_added(merge_result_->num_items_added() + 1);
}
@@ -577,14 +519,12 @@
const std::string& type_str,
const syncer::ModelType& type,
const syncer::WriteTransaction& trans,
- syncer::WriteNode* sync_node,
- syncer::AttachmentList* new_attachments) {
+ syncer::WriteNode* sync_node) {
// TODO(zea): consider having this logic for all possible changes?
-
- const syncer::SyncDataLocal sync_data_local(change.sync_data());
syncer::BaseNode::InitByLookupResult result =
- sync_node->InitByClientTagLookup(sync_data_local.GetDataType(),
- sync_data_local.GetTag());
+ sync_node->InitByClientTagLookup(
+ change.sync_data().GetDataType(),
+ syncer::SyncDataLocal(change.sync_data()).GetTag());
if (result != syncer::BaseNode::INIT_OK) {
std::string error_prefix = "Failed to load " + type_str + " node. " +
change.location().ToString() + ", ";
@@ -666,16 +606,9 @@
}
sync_node->SetTitle(change.sync_data().GetTitle());
- SetNodeSpecifics(sync_data_local.GetSpecifics(), sync_node);
- SetAttachmentMetadata(sync_data_local.GetAttachmentIds(), sync_node);
-
- // Return any newly added attachments.
- const syncer::AttachmentList& local_attachments_for_upload =
- sync_data_local.GetLocalAttachmentsForUpload();
- new_attachments->insert(new_attachments->end(),
- local_attachments_for_upload.begin(),
- local_attachments_for_upload.end());
-
+ SetNodeSpecifics(change.sync_data().GetSpecifics(), sync_node);
+ attachment_service_->OnSyncDataUpdate(sync_node->GetAttachmentIds(),
+ change.sync_data());
if (merge_result_.get()) {
merge_result_->set_num_items_modified(merge_result_->num_items_modified() +
1);
« no previous file with comments | « components/sync_driver/generic_change_processor.h ('k') | components/sync_driver/generic_change_processor_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698