Change sync conflict resolution for extensions

sync/engine/conflict_resolver.cc

 // Copyright 2012 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 "sync/engine/conflict_resolver.h"
 
 #include <list>
 #include <set>
 #include <string>
 
@@ skipping 98 matching lines @@
   // d) If the server_specifics match specifics, server_specifics are
   //    encrypted with the default key, and all other visible properties match,
   //    then we can safely ignore the local changes as redundant.
   // e) Otherwise if the base_server_specifics match the server_specifics, no
   //    functional change must have been made server-side (else
   //    base_server_specifics would have been cleared), and we can therefore
   //    safely ignore the server changes as redundant.
   // f) Otherwise, it's in general safer to ignore local changes, with the
   //    exception of deletion conflicts (choose to undelete) and conflicts
   //    where the non_unique_name or parent don't match.
+  // e) Except for the case of extensions and apps, where we want uninstalls to
+  //    win over local modifications to avoid "back from the dead" reinstalls.
   if (!entry.GetServerIsDel()) {
     // TODO(nick): The current logic is arbitrary; instead, it ought to be made
     // consistent with the ModelAssociator behavior for a datatype.  It would
     // be nice if we could route this back to ModelAssociator code to pick one
     // of three options: CLIENT, SERVER, or MERGE.  Some datatypes (autofill)
     // are easily mergeable.
     // See http://crbug.com/77339.
     bool name_matches = entry.GetNonUniqueName() ==
         entry.GetServerNonUniqueName();
     // The parent is implicit type root folder or the parent ID matches.
@@ skipping 92 matching lines @@
       status->increment_num_local_overwrites();
       counters->num_local_overwrites++;
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 OVERWRITE_LOCAL,
                                 CONFLICT_RESOLUTION_SIZE);
     }
     // Now that we've resolved the conflict, clear the prev server
     // specifics.
     entry.PutBaseServerSpecifics(sync_pb::EntitySpecifics());
   } else {  // SERVER_IS_DEL is true
-    if (entry.GetIsDir()) {
-      Directory::Metahandles children;
-      trans->directory()->GetChildHandlesById(trans,
-                                              entry.GetId(),
-                                              &children);
-      // If a server deleted folder has local contents it should be a hierarchy
-      // conflict.  Hierarchy conflicts should not be processed by this
-      // function.
-      DCHECK(children.empty());
+    ModelType type = entry.GetModelType();
+    if (type == EXTENSIONS || type == APPS) {
+      // Ignore local changes for extensions/apps when server had a delete, to
+      // avoid unwanted reinstall of an uninstalled extension.
+      DVLOG(1) << "Resolving simple conflict, ignoring local changes for "
+               << "extension/app: " << entry;
+      conflict_util::IgnoreLocalChanges(&entry);
+      status->increment_num_local_overwrites();
+      counters->num_local_overwrites++;
+      UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
+                                OVERWRITE_LOCAL,
+                                CONFLICT_RESOLUTION_SIZE);
+    } else {
+      if (entry.GetIsDir()) {
+        Directory::Metahandles children;
+        trans->directory()->GetChildHandlesById(trans,
+                                                entry.GetId(),
+                                                &children);
+        // If a server deleted folder has local contents it should be a
+        // hierarchy conflict.  Hierarchy conflicts should not be processed by
+        // this function.
+        DCHECK(children.empty());
+      }
+
+      // The entry is deleted on the server but still exists locally.
+      // We undelete it by overwriting the server's tombstone with the local
+      // data.
+      conflict_util::OverwriteServerChanges(&entry);
+      status->increment_num_server_overwrites();
+      counters->num_server_overwrites++;
+      DVLOG(1) << "Resolving simple conflict, undeleting server entry: "
+               << entry;
+      UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
+                                UNDELETE,
+                                CONFLICT_RESOLUTION_SIZE);
     }
-
-    // The entry is deleted on the server but still exists locally.
-    // We undelete it by overwriting the server's tombstone with the local
-    // data.
-    conflict_util::OverwriteServerChanges(&entry);
-    status->increment_num_server_overwrites();
-    counters->num_server_overwrites++;
-    DVLOG(1) << "Resolving simple conflict, undeleting server entry: "
-             << entry;
-    UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
-                              UNDELETE,
-                              CONFLICT_RESOLUTION_SIZE);
   }
 }
 
 void ConflictResolver::ResolveConflicts(
     syncable::WriteTransaction* trans,
     const Cryptographer* cryptographer,
     const std::set<syncable::Id>& simple_conflict_ids,
     sessions::StatusController* status,
     UpdateCounters* counters) {
   // Iterate over simple conflict items.
   set<Id>::const_iterator it;
   for (it = simple_conflict_ids.begin();
        it != simple_conflict_ids.end();
        ++it) {
     // We don't resolve conflicts for control types here.
     Entry conflicting_node(trans, syncable::GET_BY_ID, *it);
     CHECK(conflicting_node.good());
     if (IsControlType(
         GetModelTypeFromSpecifics(conflicting_node.GetSpecifics()))) {
       continue;
     }
 
     ProcessSimpleConflict(trans, *it, cryptographer, status, counters);
   }
   return;
 }
 
 }  // namespace syncer