Move AttachmentStore ownership to datatype

components/sync_driver/generic_change_processor.cc

 // Copyright 2014 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 "components/sync_driver/generic_change_processor.h"
 
 #include "base/location.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/sync_driver/sync_api_component_factory.h"
@@ skipping 74 matching lines @@
                                             attachment_service_proxy);
 }
 
 }  // namespace
 
 GenericChangeProcessor::GenericChangeProcessor(
     DataTypeErrorHandler* error_handler,
     const base::WeakPtr<syncer::SyncableService>& local_service,
     const base::WeakPtr<syncer::SyncMergeResult>& merge_result,
     syncer::UserShare* user_share,
-    SyncApiComponentFactory* sync_factory)
+    SyncApiComponentFactory* sync_factory,
+    const scoped_refptr<syncer::AttachmentStore>& attachment_store)
     : ChangeProcessor(error_handler),
       local_service_(local_service),
       merge_result_(merge_result),
-      share_handle_(user_share),
-      attachment_service_(
-          sync_factory->CreateAttachmentService(*user_share, this)),
-      attachment_service_weak_ptr_factory_(attachment_service_.get()),
-      attachment_service_proxy_(
-          base::MessageLoopProxy::current(),
-          attachment_service_weak_ptr_factory_.GetWeakPtr()) {
+      share_handle_(user_share) {
   DCHECK(CalledOnValidThread());
-  DCHECK(attachment_service_);
+  if (attachment_store) {

[maniscalco, 2014/09/08 21:56:35]

I thought scoped_refptr no longer automatically converted to *.  I *think*
you'll need to call .get() explicitly.

[pavely, 2014/09/09 23:52:30]

Done.

+    attachment_service_ = sync_factory->CreateAttachmentService(
+        attachment_store, *user_share, this);
+    attachment_service_weak_ptr_factory_.reset(
+        new base::WeakPtrFactory<syncer::AttachmentService>(
+            attachment_service_.get()));
+    attachment_service_proxy_.reset(new syncer::AttachmentServiceProxy(
+        base::MessageLoopProxy::current(),
+        attachment_service_weak_ptr_factory_->GetWeakPtr()));
+  } else {
+    attachment_service_proxy_.reset(new syncer::AttachmentServiceProxy(
+        base::MessageLoopProxy::current(),
+        base::WeakPtr<syncer::AttachmentService>()));
+  }
 }
 
 GenericChangeProcessor::~GenericChangeProcessor() {
   DCHECK(CalledOnValidThread());
 }
 
 void GenericChangeProcessor::ApplyChangesFromSyncModel(
     const syncer::BaseTransaction* trans,
     int64 model_version,
     const syncer::ImmutableChangeRecordList& changes) {
@@ skipping 11 matching lines @@
       }
       const syncer::AttachmentIdList empty_list_of_attachment_ids;
       syncer_changes_.push_back(
           syncer::SyncChange(FROM_HERE,
                              syncer::SyncChange::ACTION_DELETE,
                              syncer::SyncData::CreateRemoteData(
                                  it->id,
                                  specifics ? *specifics : it->specifics,
                                  base::Time(),
                                  empty_list_of_attachment_ids,
-                                 attachment_service_proxy_)));
+                                 *attachment_service_proxy_)));
     } else {
       syncer::SyncChange::SyncChangeType action =
           (it->action == syncer::ChangeRecord::ACTION_ADD) ?
           syncer::SyncChange::ACTION_ADD : syncer::SyncChange::ACTION_UPDATE;
       // Need to load specifics from node.
       syncer::ReadNode read_node(trans);
       if (read_node.InitByIdLookup(it->id) != syncer::BaseNode::INIT_OK) {
         syncer::SyncError error(
             FROM_HERE,
             syncer::SyncError::DATATYPE_ERROR,
             "Failed to look up data for received change with id " +
                 base::Int64ToString(it->id),
             syncer::GetModelTypeFromSpecifics(it->specifics));
         error_handler()->OnSingleDataTypeUnrecoverableError(error);
         return;
       }
       syncer_changes_.push_back(syncer::SyncChange(
           FROM_HERE,
           action,
-          BuildRemoteSyncData(it->id, read_node, attachment_service_proxy_)));
+          BuildRemoteSyncData(it->id, read_node, *attachment_service_proxy_)));
     }
   }
 }
 
 void GenericChangeProcessor::CommitChangesFromSyncModel() {
   DCHECK(CalledOnValidThread());
   if (syncer_changes_.empty())
     return;
   if (!local_service_.get()) {
     syncer::ModelType type = syncer_changes_[0].sync_data().GetDataType();
@@ skipping 76 matching lines @@
     if (sync_child_node.InitByIdLookup(*it) !=
             syncer::BaseNode::INIT_OK) {
       syncer::SyncError error(FROM_HERE,
                               syncer::SyncError::DATATYPE_ERROR,
                               "Failed to fetch child node for type " +
                                   type_name + ".",
                               type);
       return error;
     }
     current_sync_data->push_back(BuildRemoteSyncData(
-        sync_child_node.GetId(), sync_child_node, attachment_service_proxy_));
+        sync_child_node.GetId(), sync_child_node, *attachment_service_proxy_));
   }
   return syncer::SyncError();
 }
 
 bool GenericChangeProcessor::GetDataTypeContext(syncer::ModelType type,
                                                 std::string* context) const {
   syncer::ReadTransaction trans(FROM_HERE, share_handle());
   sync_pb::DataTypeContext context_proto;
   trans.GetDataTypeContext(type, &context_proto);
   if (!context_proto.has_context())
@@ skipping 191 matching lines @@
               change.location().ToString(),
           type);
       error_handler()->OnSingleDataTypeUnrecoverableError(error);
       NOTREACHED();
       LOG(ERROR) << "Unset sync change.";
       return error;
     }
   }
 
   if (!new_attachments.empty()) {
     StoreAttachments(attachment_service_.get(), new_attachments);

[maniscalco, 2014/09/08 21:56:35]

If we get to this point and attachment_service_ is null, I think we have a
problem.  WDYT about triggering an unrecoverable error here?

[pavely, 2014/09/09 23:52:30]

Done.

   }
 
   return syncer::SyncError();
 }
 
 // WARNING: this code is sensitive to compiler optimizations. Be careful
 // modifying any code around an OnSingleDataTypeUnrecoverableError call, else
 // the compiler attempts to merge it with other calls, losing useful information
 // in breakpad uploads.
 syncer::SyncError GenericChangeProcessor::HandleActionAdd(
@@ skipping 226 matching lines @@
 
 void GenericChangeProcessor::StartImpl() {
 }
 
 syncer::UserShare* GenericChangeProcessor::share_handle() const {
   DCHECK(CalledOnValidThread());
   return share_handle_;
 }
 
 }  // namespace sync_driver