Index: components/bookmarks/browser/bookmark_model.cc |
diff --git a/components/bookmarks/browser/bookmark_model.cc b/components/bookmarks/browser/bookmark_model.cc |
index 60e143caf4f38ccb184bca21ef82b37750874da4..3ec8670701343756a52c3ae39c99169861234607 100644 |
--- a/components/bookmarks/browser/bookmark_model.cc |
+++ b/components/bookmarks/browser/bookmark_model.cc |
@@ -371,9 +371,9 @@ void BookmarkModel::SetURL(const BookmarkNode* node, const GURL& url) { |
{ |
base::AutoLock url_lock(url_lock_); |
- RemoveNodeFromURLSet(mutable_node); |
+ RemoveNodeFromInternalMaps(mutable_node); |
mutable_node->set_url(url); |
- nodes_ordered_by_url_set_.insert(mutable_node); |
+ AddNodeToInternalMaps(mutable_node); |
} |
if (store_.get()) |
@@ -617,12 +617,6 @@ const BookmarkNode* BookmarkModel::AddURLWithCreationTimeAndMetaInfo( |
if (meta_info) |
new_node->SetMetaInfoMap(*meta_info); |
- { |
- // Only hold the lock for the duration of the insert. |
- base::AutoLock url_lock(url_lock_); |
- nodes_ordered_by_url_set_.insert(new_node); |
- } |
- |
return AddNode(AsMutable(parent), index, new_node); |
} |
@@ -740,9 +734,8 @@ void BookmarkModel::RemoveNode(BookmarkNode* node, |
url_lock_.AssertAcquired(); |
if (node->is_url()) { |
- RemoveNodeFromURLSet(node); |
+ RemoveNodeFromInternalMaps(node); |
removed_urls->insert(node->url()); |
- index_->Remove(node); |
} |
CancelPendingFaviconLoadRequests(node); |
@@ -837,7 +830,8 @@ void BookmarkModel::RemoveAndDeleteNode(BookmarkNode* delete_me) { |
BookmarkNodeRemoved(this, parent, index, node.get(), removed_urls)); |
} |
-void BookmarkModel::RemoveNodeFromURLSet(BookmarkNode* node) { |
+void BookmarkModel::RemoveNodeFromInternalMaps(BookmarkNode* node) { |
+ index_->Remove(node); |
// NOTE: this is called in such a way that url_lock_ is already held. As |
// such, this doesn't explicitly grab the lock. |
NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(node); |
@@ -886,11 +880,21 @@ BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent, |
FOR_EACH_OBSERVER(BookmarkModelObserver, observers_, |
BookmarkNodeAdded(this, parent, index)); |
- index_->Add(node); |
+ if (node->type() == BookmarkNode::URL) { |
danduong
2014/11/04 01:48:06
Is this right? It seems to match the previous logi
sky
2014/11/04 03:41:45
I don't think there is a good reason why this is a
danduong
2014/11/04 03:52:48
Done.
|
+ base::AutoLock url_lock(url_lock_); |
+ AddNodeToInternalMaps(node); |
danduong
2014/11/04 01:48:05
Moving this here actually makes you hold on to url
sky
2014/11/04 03:41:45
I don't think it'll make much of a difference.
|
+ } else { |
+ index_->Add(node); |
+ } |
return node; |
} |
+void BookmarkModel::AddNodeToInternalMaps(BookmarkNode* node) { |
+ index_->Add(node); |
+ nodes_ordered_by_url_set_.insert(node); |
+} |
+ |
bool BookmarkModel::IsValidIndex(const BookmarkNode* parent, |
int index, |
bool allow_end) { |