[USS] Change the place where SharedModelTypeProcessor got created

components/sync_driver/device_info_service.cc

Index: components/sync_driver/device_info_service.cc
diff --git a/components/sync_driver/device_info_service.cc b/components/sync_driver/device_info_service.cc
index 6a63dcbeb0a0b68746b37410b56a3c585b3c3574..4318616d93b7f3080edc1570d213a940db597530 100644
--- a/components/sync_driver/device_info_service.cc
+++ b/components/sync_driver/device_info_service.cc
@@ -42,8 +42,10 @@ using WriteBatch = ModelTypeStore::WriteBatch;
 
 DeviceInfoService::DeviceInfoService(
     sync_driver::LocalDeviceInfoProvider* local_device_info_provider,
-    const StoreFactoryFunction& callback)
-    : local_device_info_provider_(local_device_info_provider),
+    const StoreFactoryFunction& callback,
+    const ChangeProcessorFactory& change_processor_factory)
+    : ModelTypeService(change_processor_factory),
+      local_device_info_provider_(local_device_info_provider),
       weak_factory_(this) {
   DCHECK(local_device_info_provider);
 
@@ -370,6 +372,9 @@ void DeviceInfoService::OnReadAllMetadata(
     Result result,
     scoped_ptr<RecordList> metadata_records,
     const std::string& global_metadata) {
+  if (metadata_records->size() > 0) {

[skym, 2016/03/24 16:08:52]

Should also create a proc if the global_metadata isn't empty.

[Gang Wu, 2016/03/24 22:18:11]

Done.

+    GetOrCreateChangeProcessor();

[skym, 2016/03/24 16:08:51]

This GetOrCreateChangeProcessor() method is turning out to be more awkward than
I thought it was going to be, I may have been wrong when I thought this was the
right way to do things. Lets leave it as is though until we remove the processor
from the controller, then we can clean it up.

[Gang Wu, 2016/03/24 22:18:10]

Done.

+  }
   if (!change_processor()) {

[skym, 2016/03/24 16:08:51]

This race condition doesn't exist anymore. Can remove this block.

[Gang Wu, 2016/03/24 22:18:11]

I think we should keep it if metadata_records->size() == 0 &&
global_metadata.size() == 0

[skym, 2016/03/24 22:51:37]

Okay, you're right. Ugh, the correct behavior here is really subtle. I don't
like that we've got ourselves into this scenario. Not sure how to make it better
though.

     // This datatype was disabled while this read was oustanding.
     return;
@@ -421,10 +426,14 @@ void DeviceInfoService::TryReconcileLocalAndStored() {
 }
 
 void DeviceInfoService::TryLoadAllMetadata() {
-  if (has_data_loaded_ && change_processor()) {
+  if (has_data_loaded_) {

[skym, 2016/03/24 16:08:52]

This is significantly wrong now. We're going to double load metadata and the
proc is going to fail a DCHECK. I'm okay with you just adding a TODO in here and
I'll fix this right after you land this. It's very tightly coupled to changes
I'm in the process of making.

[Gang Wu, 2016/03/24 22:18:10]

Done.

     store_->ReadAllMetadata(base::Bind(&DeviceInfoService::OnReadAllMetadata,
                                        weak_factory_.GetWeakPtr()));
   }
 }
 
+syncer::ModelType DeviceInfoService::type() const {

[skym, 2016/03/24 16:08:51]

Another way to do this. You could move this into the base class and have each
impl pass in their type upon construction. 

[maxbogue, 2016/03/24 23:28:47]

+1, I like the idea of just tossing the type in the base constructor.

[Gang Wu, 2016/03/25 02:07:00]

Done.

+  return syncer::DEVICE_INFO;
+}
+
 }  // namespace sync_driver_v2