Extension syncing: Introduce a NeedsSync pref

chrome/browser/extensions/sync_bundle.cc

Index: chrome/browser/extensions/sync_bundle.cc
diff --git a/chrome/browser/extensions/sync_bundle.cc b/chrome/browser/extensions/sync_bundle.cc
index 09afb3ad83a4bca9226547f4e6fb3fddbcf1e151..22e4a342f78fc1a79511bce7d61e07c3d3566cf2 100644
--- a/chrome/browser/extensions/sync_bundle.cc
+++ b/chrome/browser/extensions/sync_bundle.cc
@@ -28,7 +28,11 @@ void SyncBundle::MergeDataAndStartSyncing(
         ExtensionSyncData::CreateFromSyncData(sync_data));
     if (extension_sync_data.get()) {
       AddSyncedExtension(extension_sync_data->id());
-      sync_service_->ApplySyncData(*extension_sync_data);
+
+      // If the extension has local state that needs to be synced, ignore this
+      // change (we assume the local state is more recent).
+      if (!sync_service_->ExtensionNeedsSync(extension_sync_data->id()))
+        sync_service_->ApplySyncData(*extension_sync_data);

[not at google - send to devlin, 2015/07/16 18:25:47]

It's odd to me that we make this check here, but the corresponding code which
gives it context is in ExtensionSyncService::MergeDataAndStartSyncing which
calls this method.

In fact can we just get rid of MergeDataAndStartSyncing here? Move it into
ExtensionSyncService::MergeDataAndStartSyncing?

[Marc Treib, 2015/07/17 10:24:06]

Not quite - this also calls AddSyncedExtension for all the items in
initial_sync_data, which still needs to happen. I've moved the actual applying
into ExtensionSyncService though and renamed this to Init.

[not at google - send to devlin, 2015/07/17 15:28:50]

Ah right, thanks. "StartSyncing" would be even better I think.

[Marc Treib, 2015/07/20 09:28:22]

Done.

     }
   }
 }
@@ -43,17 +47,6 @@ bool SyncBundle::IsSyncing() const {
   return sync_processor_ != nullptr;
 }
 
-bool SyncBundle::HasExtensionId(const std::string& id) const {
-  return synced_extensions_.find(id) != synced_extensions_.end();
-}
-
-bool SyncBundle::ShouldIncludeInLocalSyncDataList(
-    const Extension& extension) const {
-  // If there is pending data for this extension, then this version is out of
-  // date. We'll sync back the version we got from sync.
-  return IsSyncing() && !HasPendingExtensionId(extension.id());
-}
-
 void SyncBundle::PushSyncDataList(
     const syncer::SyncDataList& sync_data_list) {
   syncer::SyncChangeList sync_change_list;
@@ -71,6 +64,9 @@ void SyncBundle::PushSyncDataList(
 
 void SyncBundle::PushSyncDeletion(const std::string& extension_id,
                                   const syncer::SyncData& sync_data) {
+  if (!HasSyncedExtension(extension_id))
+    return;
+
   RemoveSyncedExtension(extension_id);
   PushSyncChanges(syncer::SyncChangeList(1,
       syncer::SyncChange(FROM_HERE,
@@ -78,13 +74,14 @@ void SyncBundle::PushSyncDeletion(const std::string& extension_id,
                          sync_data)));
 }
 
-void SyncBundle::PushSyncAddOrUpdate(const Extension& extension) {
-  syncer::SyncChangeList sync_change_list(
-      1,
-      CreateSyncChange(extension.id(),
-                       sync_service_->CreateSyncData(extension).GetSyncData()));
-  PushSyncChanges(sync_change_list);
-  MarkPendingExtensionSynced(extension.id());
+void SyncBundle::PushSyncAddOrUpdate(const std::string& extension_id,
+                                     const syncer::SyncData& sync_data) {
+  AddSyncedExtension(extension_id);
+  PushSyncChanges(syncer::SyncChangeList(
+      1, CreateSyncChange(extension_id, sync_data)));
+  // Now sync and local state agree. If we had any pending change from sync,
+  // clear it now.
+  pending_sync_data_.erase(extension_id);
 }
 
 void SyncBundle::ApplySyncChange(const syncer::SyncChange& sync_change) {
@@ -123,8 +120,8 @@ syncer::SyncChange SyncBundle::CreateSyncChange(
     const syncer::SyncData& sync_data) const {
   return syncer::SyncChange(
       FROM_HERE,
-      HasExtensionId(extension_id) ? syncer::SyncChange::ACTION_UPDATE
-                                   : syncer::SyncChange::ACTION_ADD,
+      HasSyncedExtension(extension_id) ? syncer::SyncChange::ACTION_UPDATE
+                                       : syncer::SyncChange::ACTION_ADD,
       sync_data);
 }
 
@@ -141,9 +138,8 @@ void SyncBundle::RemoveSyncedExtension(const std::string& id) {
   synced_extensions_.erase(id);
 }
 
-void SyncBundle::MarkPendingExtensionSynced(const std::string& id) {
-  pending_sync_data_.erase(id);
-  AddSyncedExtension(id);
+bool SyncBundle::HasSyncedExtension(const std::string& id) const {
+  return synced_extensions_.find(id) != synced_extensions_.end();
 }
 
 }  // namespace extensions