Add additional error logging to investigate Autocomplete Sync failures.

BUG=122687
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10310113

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@137617 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 64 insertions, 31 deletions

diff --git a/sync/internal_api/syncapi_unittest.cc b/sync/internal_api/syncapi_unittest.cc
index 27bb115..6b38a4f 100644
--- a/sync/internal_api/syncapi_unittest.cc
+++ b/sync/internal_api/syncapi_unittest.cc
@@ -131,7 +131,9 @@ int64 MakeNode(UserShare* share,
   ReadNode root_node(&trans);
   root_node.InitByRootLookup();
   WriteNode node(&trans);
-  EXPECT_TRUE(node.InitUniqueByCreation(model_type, root_node, client_tag));
+  sync_api::WriteNode::InitUniqueByCreationResult result =
+      node.InitUniqueByCreation(model_type, root_node, client_tag);
+  EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
   node.SetIsFolder(false);
   return node.GetId();
 }
@@ -146,7 +148,9 @@ int64 MakeNodeWithParent(UserShare* share,
   ReadNode parent_node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, parent_node.InitByIdLookup(parent_id));
   WriteNode node(&trans);
-  EXPECT_TRUE(node.InitUniqueByCreation(model_type, parent_node, client_tag));
+  sync_api::WriteNode::InitUniqueByCreationResult result =
+      node.InitUniqueByCreation(model_type, parent_node, client_tag);
+  EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
   node.SetIsFolder(false);
   return node.GetId();
 }
@@ -375,8 +379,9 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
     folder_id = folder_node.GetId();
 
     WriteNode wnode(&trans);
-    EXPECT_TRUE(wnode.InitUniqueByCreation(syncable::BOOKMARKS,
-        root_node, "testtag"));
+    sync_api::WriteNode::InitUniqueByCreationResult result =
+        wnode.InitUniqueByCreation(syncable::BOOKMARKS, root_node, "testtag");
+    EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
     wnode.SetIsFolder(false);
     wnode.SetTitle(UTF8ToWide(test_title));
 
@@ -416,8 +421,9 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 
     WriteNode wnode(&trans);
     // This will undelete the tag.
-    EXPECT_TRUE(wnode.InitUniqueByCreation(syncable::BOOKMARKS,
-        folder_node, "testtag"));
+    sync_api::WriteNode::InitUniqueByCreationResult result =
+        wnode.InitUniqueByCreation(syncable::BOOKMARKS, folder_node, "testtag");
+    EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
     EXPECT_EQ(wnode.GetIsFolder(), false);
     EXPECT_EQ(wnode.GetParentId(), folder_node.GetId());
     EXPECT_EQ(wnode.GetId(), node_id);
@@ -449,8 +455,10 @@ TEST_F(SyncApiTest, WriteAndReadPassword) {
     root_node.InitByRootLookup();
 
     WriteNode password_node(&trans);
-    EXPECT_TRUE(password_node.InitUniqueByCreation(syncable::PASSWORDS,
-                                                   root_node, "foo"));
+    sync_api::WriteNode::InitUniqueByCreationResult result =
+        password_node.InitUniqueByCreation(syncable::PASSWORDS,
+                                           root_node, "foo");
+    EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
     sync_pb::PasswordSpecificsData data;
     data.set_password_value("secret");
     password_node.SetPasswordSpecifics(data);
@@ -483,13 +491,16 @@ TEST_F(SyncApiTest, WriteEncryptedTitle) {
     root_node.InitByRootLookup();
 
     WriteNode bookmark_node(&trans);
-    EXPECT_TRUE(bookmark_node.InitUniqueByCreation(syncable::BOOKMARKS,
-                                                   root_node, "foo"));
+    sync_api::WriteNode::InitUniqueByCreationResult result =
+        bookmark_node.InitUniqueByCreation(syncable::BOOKMARKS,
+                                           root_node, "foo");
+    EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
     bookmark_node.SetTitle(UTF8ToWide("foo"));
 
     WriteNode pref_node(&trans);
-    EXPECT_TRUE(pref_node.InitUniqueByCreation(syncable::PREFERENCES,
-                                               root_node, "bar"));
+    result =
+        pref_node.InitUniqueByCreation(syncable::PREFERENCES, root_node, "bar");
+    EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
     pref_node.SetTitle(UTF8ToWide("bar"));
   }
   {
@@ -627,8 +638,9 @@ TEST_F(SyncApiTest, EmptyTags) {
   root_node.InitByRootLookup();
   WriteNode node(&trans);
   std::string empty_tag;
-  EXPECT_FALSE(node.InitUniqueByCreation(
-      syncable::TYPED_URLS, root_node, empty_tag));
+  sync_api::WriteNode::InitUniqueByCreationResult result =
+      node.InitUniqueByCreation(syncable::TYPED_URLS, root_node, empty_tag);
+  EXPECT_NE(sync_api::WriteNode::INIT_SUCCESS, result);
   EXPECT_EQ(BaseNode::INIT_FAILED_PRECONDITION,
             node.InitByTagLookup(empty_tag));
 }
@@ -1586,8 +1598,10 @@ TEST_F(SyncManagerTest, SetPassphraseWithPassword) {
     root_node.InitByRootLookup();
 
     WriteNode password_node(&trans);
-    EXPECT_TRUE(password_node.InitUniqueByCreation(syncable::PASSWORDS,
-                                                   root_node, "foo"));
+    sync_api::WriteNode::InitUniqueByCreationResult result =
+        password_node.InitUniqueByCreation(syncable::PASSWORDS,
+                                           root_node, "foo");
+    EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
     sync_pb::PasswordSpecificsData data;
     data.set_password_value("secret");
     password_node.SetPasswordSpecifics(data);
@@ -1818,8 +1832,9 @@ TEST_F(SyncManagerTest, SetPassphraseWithEmptyPasswordNode) {
     root_node.InitByRootLookup();
 
     WriteNode password_node(&trans);
-    EXPECT_TRUE(password_node.InitUniqueByCreation(syncable::PASSWORDS,
-                                                   root_node, tag));
+    sync_api::WriteNode::InitUniqueByCreationResult result =
+        password_node.InitUniqueByCreation(syncable::PASSWORDS, root_node, tag);
+    EXPECT_EQ(sync_api::WriteNode::INIT_SUCCESS, result);
     node_id = password_node.GetId();
   }
   EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_));
diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index ce20e31..68d3db7 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -364,13 +364,15 @@ bool WriteNode::InitByCreation(syncable::ModelType model_type,
 // we will attempt to undelete the node.
 // TODO(chron): Code datatype into hash tag.
 // TODO(chron): Is model type ever lost?
-bool WriteNode::InitUniqueByCreation(syncable::ModelType model_type,
-                                     const BaseNode& parent,
-                                     const std::string& tag) {
-  DCHECK(!entry_) << "Init called twice";
+WriteNode::InitUniqueByCreationResult WriteNode::InitUniqueByCreation(
+    syncable::ModelType model_type,
+    const BaseNode& parent,
+    const std::string& tag) {
+  // This DCHECK will only fail if init is called twice.
+  DCHECK(!entry_);
   if (tag.empty()) {
     LOG(WARNING) << "InitUniqueByCreation failed due to empty tag.";
-    return false;
+    return INIT_FAILED_EMPTY_TAG;
   }
 
   const std::string hash = GenerateSyncableHash(model_type, tag);
@@ -415,14 +417,13 @@ bool WriteNode::InitUniqueByCreation(syncable::ModelType model_type,
       existing_entry->Put(syncable::PARENT_ID, parent_id);
       entry_ = existing_entry.release();
     } else {
-      return false;
+      return INIT_FAILED_ENTRY_ALREADY_EXISTS;
     }
   } else {
     entry_ = new syncable::MutableEntry(transaction_->GetWrappedWriteTrans(),
                                         syncable::CREATE, parent_id, dummy);
-    if (!entry_->good()) {
-      return false;
-    }
+    if (!entry_->good())
+      return INIT_FAILED_COULD_NOT_CREATE_ENTRY;
 
     // Only set IS_DIR for new entries. Don't bitflip undeleted ones.
     entry_->Put(syncable::UNIQUE_CLIENT_TAG, hash);
@@ -435,7 +436,11 @@ bool WriteNode::InitUniqueByCreation(syncable::ModelType model_type,
   PutModelType(model_type);
 
   // Now set the predecessor, which sets IS_UNSYNCED as necessary.
-  return PutPredecessor(NULL);
+  bool success = PutPredecessor(NULL);
+  if (!success)
+    return INIT_FAILED_SET_PREDECESSOR;
+
+  return INIT_SUCCESS;
 }
 
 bool WriteNode::SetPosition(const BaseNode& new_parent,
diff --git a/sync/internal_api/write_node.h b/sync/internal_api/write_node.h
index a070a5c..a87a377 100644
--- a/sync/internal_api/write_node.h
+++ b/sync/internal_api/write_node.h
@@ -46,6 +46,18 @@ class WriteTransaction;
 // syncable::MutableEntry. A WriteTransaction is needed to create a WriteNode.
 class WriteNode : public BaseNode {
  public:
+  enum InitUniqueByCreationResult {
+    INIT_SUCCESS,
+    // The tag passed into this method was empty.
+    INIT_FAILED_EMPTY_TAG,
+    // An entry with this tag already exists.
+    INIT_FAILED_ENTRY_ALREADY_EXISTS,
+    // The constructor for a new MutableEntry with the specified data failed.
+    INIT_FAILED_COULD_NOT_CREATE_ENTRY,
+    // Setting the predecessor failed
+    INIT_FAILED_SET_PREDECESSOR,
+  };
+
   // Create a WriteNode using the given transaction.
   explicit WriteNode(WriteTransaction* transaction);
   virtual ~WriteNode();
@@ -75,9 +87,10 @@ class WriteNode : public BaseNode {
   // Most importantly, if it exists locally, this function will
   // actually undelete it
   // Client unique tagged nodes must NOT be folders.
-  bool InitUniqueByCreation(syncable::ModelType model_type,
-                            const BaseNode& parent,
-                            const std::string& client_tag);
+  InitUniqueByCreationResult InitUniqueByCreation(
+      syncable::ModelType model_type,
+      const BaseNode& parent,
+      const std::string& client_tag);
 
   // Each server-created permanent node is tagged with a unique string.
   // Look up the node with the particular tag.  If it does not exist,