Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1)

Side by Side Diff: sync/internal_api/write_node.cc

Issue 1134793006: [Sync] WriteNode::SetTitle doesn't update bookmark title when it is empty string. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Small change after Nicolas' feedback. Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « sync/internal_api/sync_manager_impl_unittest.cc ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "sync/internal_api/public/write_node.h" 5 #include "sync/internal_api/public/write_node.h"
6 6
7 #include "base/strings/string_util.h" 7 #include "base/strings/string_util.h"
8 #include "base/strings/utf_string_conversions.h" 8 #include "base/strings/utf_string_conversions.h"
9 #include "base/values.h" 9 #include "base/values.h"
10 #include "sync/internal_api/public/base_transaction.h" 10 #include "sync/internal_api/public/base_transaction.h"
(...skipping 60 matching lines...) Expand 10 before | Expand all | Expand 10 after
71 // Non-bookmarks and legacy bookmarks (those with no title in their 71 // Non-bookmarks and legacy bookmarks (those with no title in their
72 // specifics) store their title in NON_UNIQUE_NAME. Non-legacy bookmarks 72 // specifics) store their title in NON_UNIQUE_NAME. Non-legacy bookmarks
73 // store their title in specifics as well as NON_UNIQUE_NAME. 73 // store their title in specifics as well as NON_UNIQUE_NAME.
74 current_legal_title = entry_->GetNonUniqueName(); 74 current_legal_title = entry_->GetNonUniqueName();
75 } 75 }
76 76
77 bool title_matches = (current_legal_title == new_legal_title); 77 bool title_matches = (current_legal_title == new_legal_title);
78 bool encrypted_without_overwriting_name = (needs_encryption && 78 bool encrypted_without_overwriting_name = (needs_encryption &&
79 entry_->GetNonUniqueName() != kEncryptedString); 79 entry_->GetNonUniqueName() != kEncryptedString);
80 80
81 // For bookmarks, we also set the title field in the specifics.
82 // TODO(zea): refactor bookmarks to not need this functionality.
83 sync_pb::EntitySpecifics specifics = GetEntitySpecifics();
84 if (GetModelType() == BOOKMARKS &&
85 specifics.bookmark().title() != new_legal_title) {
86 specifics.mutable_bookmark()->set_title(new_legal_title);
87 SetEntitySpecifics(specifics); // Does it's own encryption checking.
88 title_matches = false;
89 }
90
81 // If the title matches and the NON_UNIQUE_NAME is properly overwritten as 91 // If the title matches and the NON_UNIQUE_NAME is properly overwritten as
82 // necessary, nothing needs to change. 92 // necessary, nothing needs to change.
83 if (title_matches && !encrypted_without_overwriting_name) { 93 if (title_matches && !encrypted_without_overwriting_name) {
84 DVLOG(2) << "Title matches, dropping change."; 94 DVLOG(2) << "Title matches, dropping change.";
85 return; 95 return;
86 } 96 }
87 97
88 // For bookmarks, we also set the title field in the specifics.
89 // TODO(zea): refactor bookmarks to not need this functionality.
90 if (GetModelType() == BOOKMARKS) {
91 sync_pb::EntitySpecifics specifics = GetEntitySpecifics();
92 specifics.mutable_bookmark()->set_title(new_legal_title);
93 SetEntitySpecifics(specifics); // Does it's own encryption checking.
94 }
95
96 // For bookmarks, this has to happen after we set the title in the specifics, 98 // For bookmarks, this has to happen after we set the title in the specifics,
97 // because the presence of a title in the NON_UNIQUE_NAME is what controls 99 // because the presence of a title in the NON_UNIQUE_NAME is what controls
98 // the logic deciding whether this is an empty node or a legacy bookmark. 100 // the logic deciding whether this is an empty node or a legacy bookmark.
99 // See BaseNode::GetUnencryptedSpecific(..). 101 // See BaseNode::GetUnencryptedSpecific(..).
100 if (needs_encryption) 102 if (needs_encryption)
101 entry_->PutNonUniqueName(kEncryptedString); 103 entry_->PutNonUniqueName(kEncryptedString);
102 else 104 else
103 entry_->PutNonUniqueName(new_legal_title); 105 entry_->PutNonUniqueName(new_legal_title);
104 106
105 DVLOG(1) << "Overwriting title of type " 107 DVLOG(1) << "Overwriting title of type "
(...skipping 429 matching lines...) Expand 10 before | Expand all | Expand 10 after
535 syncable::Id predecessor_id = predecessor ? 537 syncable::Id predecessor_id = predecessor ?
536 predecessor->GetEntry()->GetId() : syncable::Id(); 538 predecessor->GetEntry()->GetId() : syncable::Id();
537 return entry_->PutPredecessor(predecessor_id); 539 return entry_->PutPredecessor(predecessor_id);
538 } 540 }
539 541
540 void WriteNode::MarkForSyncing() { 542 void WriteNode::MarkForSyncing() {
541 syncable::MarkForSyncing(entry_); 543 syncable::MarkForSyncing(entry_);
542 } 544 }
543 545
544 } // namespace syncer 546 } // namespace syncer
OLDNEW
« no previous file with comments | « sync/internal_api/sync_manager_impl_unittest.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698