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

Side by Side Diff: components/sync_driver/generic_change_processor.cc

Issue 538403004: Revert of Replace AttachmentStore's StoreAttachments with UploadAttachments. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 6 years, 3 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 unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/sync_driver/generic_change_processor.h" 5 #include "components/sync_driver/generic_change_processor.h"
6 6
7 #include "base/location.h" 7 #include "base/location.h"
8 #include "base/strings/string_number_conversions.h" 8 #include "base/strings/string_number_conversions.h"
9 #include "base/strings/utf_string_conversions.h" 9 #include "base/strings/utf_string_conversions.h"
10 #include "components/sync_driver/sync_api_component_factory.h" 10 #include "components/sync_driver/sync_api_component_factory.h"
(...skipping 67 matching lines...) Expand 10 before | Expand all | Expand 10 after
78 sync_pb::EntitySpecifics password_holder; 78 sync_pb::EntitySpecifics password_holder;
79 password_holder.mutable_password()->mutable_client_only_encrypted_data()-> 79 password_holder.mutable_password()->mutable_client_only_encrypted_data()->
80 CopyFrom(read_node.GetPasswordSpecifics()); 80 CopyFrom(read_node.GetPasswordSpecifics());
81 return syncer::SyncData::CreateRemoteData(sync_id, 81 return syncer::SyncData::CreateRemoteData(sync_id,
82 password_holder, 82 password_holder,
83 read_node.GetModificationTime(), 83 read_node.GetModificationTime(),
84 attachment_ids, 84 attachment_ids,
85 attachment_service_proxy); 85 attachment_service_proxy);
86 } 86 }
87 87
88 const syncer::AttachmentId& AttachmentToAttachmentId(
89 const syncer::Attachment& attachment) {
90 return attachment.GetId();
91 }
92
93 } // namespace 88 } // namespace
94 89
95 GenericChangeProcessor::GenericChangeProcessor( 90 GenericChangeProcessor::GenericChangeProcessor(
96 DataTypeErrorHandler* error_handler, 91 DataTypeErrorHandler* error_handler,
97 const base::WeakPtr<syncer::SyncableService>& local_service, 92 const base::WeakPtr<syncer::SyncableService>& local_service,
98 const base::WeakPtr<syncer::SyncMergeResult>& merge_result, 93 const base::WeakPtr<syncer::SyncMergeResult>& merge_result,
99 syncer::UserShare* user_share, 94 syncer::UserShare* user_share,
100 SyncApiComponentFactory* sync_factory) 95 SyncApiComponentFactory* sync_factory)
101 : ChangeProcessor(error_handler), 96 : ChangeProcessor(error_handler),
102 local_service_(local_service), 97 local_service_(local_service),
103 merge_result_(merge_result), 98 merge_result_(merge_result),
104 share_handle_(user_share), 99 share_handle_(user_share),
105 attachment_service_( 100 attachment_service_(
106 sync_factory->CreateAttachmentService(*user_share, this)), 101 sync_factory->CreateAttachmentService(*user_share, this)),
107 attachment_service_weak_ptr_factory_(attachment_service_.get()), 102 attachment_service_weak_ptr_factory_(attachment_service_.get()),
108 attachment_service_proxy_( 103 attachment_service_proxy_(
109 base::MessageLoopProxy::current(), 104 base::MessageLoopProxy::current(),
110 attachment_service_weak_ptr_factory_.GetWeakPtr()), 105 attachment_service_weak_ptr_factory_.GetWeakPtr()) {
111 weak_ptr_factory_(this) {
112 DCHECK(CalledOnValidThread()); 106 DCHECK(CalledOnValidThread());
113 DCHECK(attachment_service_); 107 DCHECK(attachment_service_);
114 } 108 }
115 109
116 GenericChangeProcessor::~GenericChangeProcessor() { 110 GenericChangeProcessor::~GenericChangeProcessor() {
117 DCHECK(CalledOnValidThread()); 111 DCHECK(CalledOnValidThread());
118 } 112 }
119 113
120 void GenericChangeProcessor::ApplyChangesFromSyncModel( 114 void GenericChangeProcessor::ApplyChangesFromSyncModel(
121 const syncer::BaseTransaction* trans, 115 const syncer::BaseTransaction* trans,
(...skipping 264 matching lines...) Expand 10 before | Expand all | Expand 10 after
386 type, error_handler); 380 type, error_handler);
387 } 381 }
388 } 382 }
389 if (IsActOnceDataType(type)) 383 if (IsActOnceDataType(type))
390 node->Drop(); 384 node->Drop();
391 else 385 else
392 node->Tombstone(); 386 node->Tombstone();
393 return syncer::SyncError(); 387 return syncer::SyncError();
394 } 388 }
395 389
390 // A callback invoked on completion of AttachmentService::StoreAttachment.
391 void IgnoreStoreResult(const syncer::AttachmentService::StoreResult&) {
392 // TODO(maniscalco): Here is where we're going to update the in-directory
393 // entry to indicate that the attachments have been successfully stored on
394 // disk. Why do we care? Because we might crash after persisting the
395 // directory to disk, but before we have persisted its attachments, leaving us
396 // with danging attachment ids. Having a flag that indicates we've stored the
397 // entry will allow us to detect and filter entries with dangling attachment
398 // ids (bug 368353).
399 }
400
401 void StoreAttachments(syncer::AttachmentService* attachment_service,
402 const syncer::AttachmentList& attachments) {
403 DCHECK(attachment_service);
404 syncer::AttachmentService::StoreCallback ignore_store_result =
405 base::Bind(&IgnoreStoreResult);
406 attachment_service->StoreAttachments(attachments, ignore_store_result);
407 }
408
396 } // namespace 409 } // namespace
397 410
398 syncer::SyncError GenericChangeProcessor::ProcessSyncChanges( 411 syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
399 const tracked_objects::Location& from_here, 412 const tracked_objects::Location& from_here,
400 const syncer::SyncChangeList& list_of_changes) { 413 const syncer::SyncChangeList& list_of_changes) {
401 DCHECK(CalledOnValidThread()); 414 DCHECK(CalledOnValidThread());
402 415
403 // Keep track of brand new attachments so we can persist them on this device 416 // Keep track of brand new attachments so we can persist them on this device
404 // and upload them to the server. 417 // and upload them to the server.
405 syncer::AttachmentList new_attachments; 418 syncer::AttachmentList new_attachments;
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
445 change.location().ToString(), 458 change.location().ToString(),
446 type); 459 type);
447 error_handler()->OnSingleDataTypeUnrecoverableError(error); 460 error_handler()->OnSingleDataTypeUnrecoverableError(error);
448 NOTREACHED(); 461 NOTREACHED();
449 LOG(ERROR) << "Unset sync change."; 462 LOG(ERROR) << "Unset sync change.";
450 return error; 463 return error;
451 } 464 }
452 } 465 }
453 466
454 if (!new_attachments.empty()) { 467 if (!new_attachments.empty()) {
455 StoreAndUploadAttachments(new_attachments); 468 StoreAttachments(attachment_service_.get(), new_attachments);
456 } 469 }
457 470
458 return syncer::SyncError(); 471 return syncer::SyncError();
459 } 472 }
460 473
461 // WARNING: this code is sensitive to compiler optimizations. Be careful 474 // WARNING: this code is sensitive to compiler optimizations. Be careful
462 // modifying any code around an OnSingleDataTypeUnrecoverableError call, else 475 // modifying any code around an OnSingleDataTypeUnrecoverableError call, else
463 // the compiler attempts to merge it with other calls, losing useful information 476 // the compiler attempts to merge it with other calls, losing useful information
464 // in breakpad uploads. 477 // in breakpad uploads.
465 syncer::SyncError GenericChangeProcessor::HandleActionAdd( 478 syncer::SyncError GenericChangeProcessor::HandleActionAdd(
(...skipping 225 matching lines...) Expand 10 before | Expand all | Expand 10 after
691 } 704 }
692 705
693 void GenericChangeProcessor::StartImpl() { 706 void GenericChangeProcessor::StartImpl() {
694 } 707 }
695 708
696 syncer::UserShare* GenericChangeProcessor::share_handle() const { 709 syncer::UserShare* GenericChangeProcessor::share_handle() const {
697 DCHECK(CalledOnValidThread()); 710 DCHECK(CalledOnValidThread());
698 return share_handle_; 711 return share_handle_;
699 } 712 }
700 713
701 void GenericChangeProcessor::StoreAndUploadAttachments(
702 const syncer::AttachmentList& attachments) {
703 DCHECK(CalledOnValidThread());
704 attachment_service_->GetStore()->Write(
705 attachments,
706 base::Bind(&GenericChangeProcessor::WriteAttachmentsDone,
707 weak_ptr_factory_.GetWeakPtr(),
708 attachments));
709 }
710
711 void GenericChangeProcessor::WriteAttachmentsDone(
712 const syncer::AttachmentList& attachments,
713 const syncer::AttachmentStore::Result& result) {
714 DCHECK(CalledOnValidThread());
715 if (result != syncer::AttachmentStore::SUCCESS) {
716 // TODO(maniscalco): Deal with case where an error occurred (bug 361251).
717 return;
718 }
719
720 // TODO(maniscalco): Here is where we're going to update the in-directory
721 // entry to indicate that the attachments have been successfully stored on
722 // disk. Why do we care? Because we might crash after persisting the
723 // directory to disk, but before we have persisted its attachments, leaving us
724 // with danging attachment ids. Having a flag that indicates we've stored the
725 // entry will allow us to detect and filter entries with dangling attachment
726 // ids (bug 368353).
727
728 // Begin uploading the attachments now that they are safe on disk.
729 syncer::AttachmentIdSet attachment_ids;
730 std::transform(attachments.begin(),
731 attachments.end(),
732 std::inserter(attachment_ids, attachment_ids.end()),
733 AttachmentToAttachmentId);
734 attachment_service_->UploadAttachments(attachment_ids);
735 }
736
737 } // namespace sync_driver 714 } // namespace sync_driver
OLDNEW
« 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