[Sync] WriteNode::SetTitle doesn't update bookmark title when it is empty string.

sync/internal_api/write_node.cc

Index: sync/internal_api/write_node.cc
diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index cc445e42ff5ae74bd4790e7f9f926dca017c90da..a95146af961c0f963ccdae25d611b4516c5226c1 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -77,20 +77,24 @@ void WriteNode::SetTitle(const std::string& title) {
   bool title_matches = (current_legal_title == new_legal_title);
   bool encrypted_without_overwriting_name = (needs_encryption &&
       entry_->GetNonUniqueName() != kEncryptedString);
-
-  // If the title matches and the NON_UNIQUE_NAME is properly overwritten as
-  // necessary, nothing needs to change.
-  if (title_matches && !encrypted_without_overwriting_name) {
-    DVLOG(2) << "Title matches, dropping change.";
-    return;
-  }
+  bool bookmark_title_updated = false;

[Nicolas Zea, 2015/05/15 20:15:17]

Does it make sense to, instead of having this new boolean, to just update
title_matches above to be false if specifics.bookmark().title() !=
new_legal_title?

[pavely, 2015/05/15 21:45:28]

I used title_matches first, but decided to use separate variable because
title_matches reflects comparison of NonUniqueName(), not bookmark's title. I
think updating title_matches is fine. I'll change code.

 
   // For bookmarks, we also set the title field in the specifics.
   // TODO(zea): refactor bookmarks to not need this functionality.
-  if (GetModelType() == BOOKMARKS) {
-    sync_pb::EntitySpecifics specifics = GetEntitySpecifics();
+  sync_pb::EntitySpecifics specifics = GetEntitySpecifics();
+  if (GetModelType() == BOOKMARKS &&
+      specifics.bookmark().title() != new_legal_title) {
     specifics.mutable_bookmark()->set_title(new_legal_title);
     SetEntitySpecifics(specifics);  // Does it's own encryption checking.
+    bookmark_title_updated = true;
+  }
+
+  // If the title matches and the NON_UNIQUE_NAME is properly overwritten as
+  // necessary, nothing needs to change.
+  if (title_matches && !encrypted_without_overwriting_name &&
+      !bookmark_title_updated) {
+    DVLOG(2) << "Title matches, dropping change.";
+    return;
   }
 
   // For bookmarks, this has to happen after we set the title in the specifics,