Commit 551c70ba authored by Rushan Suleymanov's avatar Rushan Suleymanov Committed by Commit Bot

Remove DCHECKs validating non-duplicate GUIDs in bookmarks

The DCHECK could be violated during a sync incremental update when
remote updates have entities having different sync ids and the same
GUIDs.

This is a temporary measure until it is guaranteed that GUIDs are unique
across all bookmarks.

Bug: 1142790
Change-Id: Ic1279eee89290f4245f5e7f76528e888ff0dec39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506030Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Cr-Commit-Position: refs/heads/master@{#821816}
parent 4a99ae87
...@@ -110,18 +110,6 @@ class EmptyUndoDelegate : public BookmarkUndoDelegate { ...@@ -110,18 +110,6 @@ class EmptyUndoDelegate : public BookmarkUndoDelegate {
DISALLOW_COPY_AND_ASSIGN(EmptyUndoDelegate); DISALLOW_COPY_AND_ASSIGN(EmptyUndoDelegate);
}; };
#if DCHECK_IS_ON()
void AddGuidsToIndexRecursive(const BookmarkNode* node,
std::set<std::string>* guid_index) {
bool success = guid_index->insert(node->guid()).second;
DCHECK(success);
// Recurse through children.
for (size_t i = node->children().size(); i > 0; --i)
AddGuidsToIndexRecursive(node->children()[i - 1].get(), guid_index);
}
#endif // DCHECK_IS_ON()
} // namespace } // namespace
// BookmarkModel -------------------------------------------------------------- // BookmarkModel --------------------------------------------------------------
...@@ -779,12 +767,6 @@ void BookmarkModel::RemoveNodeFromIndexRecursive(BookmarkNode* node) { ...@@ -779,12 +767,6 @@ void BookmarkModel::RemoveNodeFromIndexRecursive(BookmarkNode* node) {
if (node->is_url()) if (node->is_url())
titled_url_index_->Remove(node); titled_url_index_->Remove(node);
// Note that |guid_index_| is used for DCHECK-enabled builds only.
#if DCHECK_IS_ON()
DCHECK(guid_index_.erase(node->guid()))
<< "Bookmark GUID missing in index: " << node->guid();
#endif // DCHECK_IS_ON()
CancelPendingFaviconLoadRequests(node); CancelPendingFaviconLoadRequests(node);
// Recurse through children. // Recurse through children.
...@@ -818,10 +800,6 @@ void BookmarkModel::DoneLoading(std::unique_ptr<BookmarkLoadDetails> details) { ...@@ -818,10 +800,6 @@ void BookmarkModel::DoneLoading(std::unique_ptr<BookmarkLoadDetails> details) {
other_node_ = details->other_folder_node(); other_node_ = details->other_folder_node();
mobile_node_ = details->mobile_folder_node(); mobile_node_ = details->mobile_folder_node();
#if DCHECK_IS_ON()
AddGuidsToIndexRecursive(root_, &guid_index_);
#endif // DCHECK_IS_ON()
titled_url_index_->SetNodeSorter( titled_url_index_->SetNodeSorter(
std::make_unique<TypedCountSorter>(client_.get())); std::make_unique<TypedCountSorter>(client_.get()));
// Sorting the permanent nodes has to happen on the main thread, so we do it // Sorting the permanent nodes has to happen on the main thread, so we do it
...@@ -865,16 +843,12 @@ BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent, ...@@ -865,16 +843,12 @@ BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent,
void BookmarkModel::AddNodeToIndexRecursive(const BookmarkNode* node) { void BookmarkModel::AddNodeToIndexRecursive(const BookmarkNode* node) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// TODO(crbug.com/1143246): add a DCHECK to validate that all nodes have
// unique GUID when it is guaranteed.
if (node->is_url()) if (node->is_url())
titled_url_index_->Add(node); titled_url_index_->Add(node);
// The node's GUID must be unique. Note that |guid_index_| is used for
// DCHECK-enabled builds only.
#if DCHECK_IS_ON()
DCHECK(guid_index_.insert(node->guid()).second)
<< "Duplicate bookmark GUID: " << node->guid();
#endif // DCHECK_IS_ON()
for (const auto& child : node->children()) for (const auto& child : node->children())
AddNodeToIndexRecursive(child.get()); AddNodeToIndexRecursive(child.get());
} }
......
...@@ -402,11 +402,6 @@ class BookmarkModel : public BookmarkUndoProvider, ...@@ -402,11 +402,6 @@ class BookmarkModel : public BookmarkUndoProvider,
std::unique_ptr<TitledUrlIndex> titled_url_index_; std::unique_ptr<TitledUrlIndex> titled_url_index_;
#if DCHECK_IS_ON()
// GUID index used to verify uniqueness in DCHECK-enabled builds.
std::set<std::string> guid_index_;
#endif // DCHECK_IS_ON()
// Owned by |model_loader_|. // Owned by |model_loader_|.
// WARNING: in some tests this does *not* refer to // WARNING: in some tests this does *not* refer to
// |ModelLoader::history_bookmark_model_|. This is because some tests // |ModelLoader::history_bookmark_model_|. This is because some tests
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment