Change sync conflict resolution for extensions

sync/engine/conflict_resolver.cc

Index: sync/engine/conflict_resolver.cc
diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index 0af30977c331e8840fe7ef27a77362114de875af..c321c0e5660f1af12f27e579b372653f22a77a34 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -116,7 +116,10 @@ void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
   // 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.
-  if (!entry.GetServerIsDel()) {
+  // 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.
+  ModelType type = entry.GetModelType();
+  if (!entry.GetServerIsDel() || (type == EXTENSIONS || type == APPS)) {

[Nicolas Zea, 2016/02/18 19:28:55]

The more I think about it, the more I think it would be safer to not have this
condition here, and instead special case the extension/apps case down below in
the else.

The rationale is that the logic within the body of this condition is based on
the assumption that the entry is not deleted by the server. I think this is why
you ran into that subtle case with the specifics matching not triggering the
right logic.

     // 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