Explode Autofill sync error messages for debugging

chrome/browser/sync/glue/generic_change_processor.cc

Index: chrome/browser/sync/glue/generic_change_processor.cc
diff --git a/chrome/browser/sync/glue/generic_change_processor.cc b/chrome/browser/sync/glue/generic_change_processor.cc
index 3317c89cfe2d6d5c4b1de96b03e7540977fd3d29..894aba3e89041de3662ea8302c5e5a02e4cd56b0 100644
--- a/chrome/browser/sync/glue/generic_change_processor.cc
+++ b/chrome/browser/sync/glue/generic_change_processor.cc
@@ -128,21 +128,48 @@ SyncError GenericChangeProcessor::GetSyncDataForType(
 
 namespace {
 
-bool AttemptDelete(const SyncChange& change, sync_api::WriteNode* node) {
+// TODO(isherman): Investigating http://crbug.com/121592
+std::string GetErrorExplanation(sync_api::BaseNode::InitByLookupResult result) {
+  switch (result) {
+    case sync_api::BaseNode::INIT_FAILED_ENTRY_NOT_GOOD:
+      return " could not find entry matching the lookup criteria.";
+    case sync_api::BaseNode::INIT_FAILED_ENTRY_IS_DEL:
+      return " entry is already deleted.";
+    case sync_api::BaseNode::INIT_FAILED_DECRYPT_IF_NECESSARY:
+      return " unable to decrypt.";
+    case sync_api::BaseNode::INIT_FAILED_PRECONDITION:
+      return " a precondition was not met for calling init.";
+    default:
+      NOTREACHED();
+  }
+
+  // Should have listed all the possible error cases above.
+  NOTREACHED();

[tim (not reviewing), 2012/05/14 22:50:08]

*extra* unreachable :) One NR() might suffice, but okay either way.

[Ilya Sherman, 2012/05/15 00:22:15]

Done.

+  return "";
+}
+
+bool AttemptDelete(const SyncChange& change,
+                   sync_api::WriteNode* node,
+                   std::string* error_message) {
   DCHECK_EQ(change.change_type(), SyncChange::ACTION_DELETE);
   if (change.sync_data().IsLocal()) {
     const std::string& tag = change.sync_data().GetTag();
     if (tag.empty()) {
+      *error_message = "Local data, empty tag.";
       return false;
     }
-    if (node->InitByClientTagLookup(
-            change.sync_data().GetDataType(), tag) !=
-            sync_api::BaseNode::INIT_OK) {
+
+    sync_api::BaseNode::InitByLookupResult result =
+        node->InitByClientTagLookup(change.sync_data().GetDataType(), tag);
+    if (result != sync_api::BaseNode::INIT_OK) {
+      *error_message = "Local data, " + GetErrorExplanation(result);
       return false;
     }
   } else {
-    if (node->InitByIdLookup(change.sync_data().GetRemoteId()) !=
-            sync_api::BaseNode::INIT_OK) {
+    sync_api::BaseNode::InitByLookupResult result =
+        node->InitByIdLookup(change.sync_data().GetRemoteId());
+    if (result != sync_api::BaseNode::INIT_OK) {
+      *error_message = "Non-local data, " + GetErrorExplanation(result);
       return false;
     }
   }
@@ -167,11 +194,13 @@ SyncError GenericChangeProcessor::ProcessSyncChanges(
     std::string type_str = syncable::ModelTypeToString(type);
     sync_api::WriteNode sync_node(&trans);
     if (change.change_type() == SyncChange::ACTION_DELETE) {
-      if (!AttemptDelete(change, &sync_node)) {
+      std::string error_message;
+      if (!AttemptDelete(change, &sync_node, &error_message)) {
         NOTREACHED();
-        SyncError error(FROM_HERE,
-                        "Failed to delete " + type_str + " node.",
-                        type);
+        SyncError error(
+            FROM_HERE,
+            "Failed to delete " + type_str + " node. " + error_message,
+            type);
         error_handler()->OnSingleDatatypeUnrecoverableError(error.location(),

[tim (not reviewing), 2012/05/14 22:50:08]

So, unfortunately this is actually the line that will create the
NONCONTINUABLE_EXCEPTION that gets uploaded to crash/.  So what you want to do
is likely have a different loc depending on the error that happened within
AttemptDelete -- so instead of a message string, you may want an enum or extra
bool param.

[Ilya Sherman, 2012/05/15 00:22:15]

Ok, assuming I've understood things correctly, I believe this should be fixed in
the latest patch set.  PTAL :)

                                                             error.message());
         return error;