From dddc1b44f4ae8346098bfadecd71d23e7de9299e Mon Sep 17 00:00:00 2001 From: "sky@google.com" Date: Thu, 9 Oct 2008 20:56:59 +0000 Subject: Changes BookmarkModel to not grab the same lock twice. BUG=none TEST=none Review URL: http://codereview.chromium.org/6391 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@3127 0039d316-1c4b-4281-b951-d872f2087c98 --- base/lock.cc | 3 +-- chrome/browser/bookmarks/bookmark_model.cc | 12 ++++++++---- chrome/browser/bookmarks/bookmark_model.h | 9 ++++++++- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/base/lock.cc b/base/lock.cc index 1dd4dc1..c2f79c7 100644 --- a/base/lock.cc +++ b/base/lock.cc @@ -43,8 +43,7 @@ void Lock::Acquire() { acquisition_count_++; if (2 == recursion_count_shadow_ && !recursion_used_) { recursion_used_ = true; - // TODO(sky): Uncomment this DCHECK after fixing test cases. - // DCHECK(false); // Catch accidental redundant lock acquisition. + DCHECK(false); // Catch accidental redundant lock acquisition. } #endif // NDEBUG } diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc index 5e29095..fba0629 100644 --- a/chrome/browser/bookmarks/bookmark_model.cc +++ b/chrome/browser/bookmarks/bookmark_model.cc @@ -302,9 +302,7 @@ void BookmarkModel::GetBookmarks(std::vector* urls) { bool BookmarkModel::IsBookmarked(const GURL& url) { AutoLock url_lock(url_lock_); - BookmarkNode tmp_node(this, url); - return (nodes_ordered_by_url_set_.find(&tmp_node) != - nodes_ordered_by_url_set_.end()); + return IsBookmarkedNoLock(url); } BookmarkNode* BookmarkModel::GetNodeByID(int id) { @@ -389,6 +387,12 @@ void BookmarkModel::ResetDateGroupModified(BookmarkNode* node) { SetDateGroupModified(node, Time()); } +bool BookmarkModel::IsBookmarkedNoLock(const GURL& url) { + BookmarkNode tmp_node(this, url); + return (nodes_ordered_by_url_set_.find(&tmp_node) != + nodes_ordered_by_url_set_.end()); +} + void BookmarkModel::FavIconLoaded(BookmarkNode* node) { // Send out notification to the observer. FOR_EACH_OBSERVER(BookmarkModelObserver, observers_, @@ -520,7 +524,7 @@ void BookmarkModel::RemoveAndDeleteNode(BookmarkNode* delete_me) { // allow duplicates we need to remove any entries that are still bookmarked. for (std::set::iterator i = details.changed_urls.begin(); i != details.changed_urls.end(); ){ - if (IsBookmarked(*i)) + if (IsBookmarkedNoLock(*i)) i = details.changed_urls.erase(i); else ++i; diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h index a9f6308..7c2a6ba 100644 --- a/chrome/browser/bookmarks/bookmark_model.h +++ b/chrome/browser/bookmarks/bookmark_model.h @@ -202,7 +202,8 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { // modified groups. This never returns an empty vector. std::vector GetMostRecentlyModifiedGroups(size_t max_count); - // Returns the most recently added bookmarks. + // Returns the most recently added bookmarks. This does not return groups, + // only nodes of type url. void GetMostRecentlyAddedEntries(size_t count, std::vector* nodes); @@ -294,6 +295,8 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { // combobox of most recently modified groups. void ResetDateGroupModified(BookmarkNode* node); + Profile* profile() const { return profile_; } + private: // Used to order BookmarkNodes by URL. class NodeURLComparator { @@ -303,6 +306,10 @@ class BookmarkModel : public NotificationObserver, public BookmarkService { } }; + // Implementation of IsBookmarked. Before calling this the caller must + // obtain a lock on url_lock_. + bool IsBookmarkedNoLock(const GURL& url); + // Overriden to notify the observer the favicon has been loaded. void FavIconLoaded(BookmarkNode* node); -- cgit v1.1