Adds the ability for ObserverList to not notify observers added during
notification. I need this for bookmarks. If a new observer is added while the bookmark model is in the process of sending out notification the newly added observer gets confused. BUG=674 TEST=none Review URL: http://codereview.chromium.org/8919 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@4267 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
@ -5,8 +5,9 @@
|
||||
#ifndef BASE_OBSERVER_LIST_H__
|
||||
#define BASE_OBSERVER_LIST_H__
|
||||
|
||||
#include <vector>
|
||||
#include <algorithm>
|
||||
#include <limits>
|
||||
#include <vector>
|
||||
|
||||
#include "base/basictypes.h"
|
||||
#include "base/logging.h"
|
||||
@ -52,12 +53,25 @@
|
||||
// ObserverList<Observer> observer_list_;
|
||||
// };
|
||||
//
|
||||
//
|
||||
///////////////////////////////////////////////////////////////////////////////
|
||||
|
||||
template <class ObserverType, bool check_empty = false>
|
||||
class ObserverList {
|
||||
public:
|
||||
ObserverList() : notify_depth_(0) {}
|
||||
// Enumeration of which observers are notified.
|
||||
enum NotificationType {
|
||||
// Specifies that any observers added during notification are notified.
|
||||
// This is the default type if non type is provided to the constructor.
|
||||
NOTIFY_ALL,
|
||||
|
||||
// Specifies that observers added while sending out notification are not
|
||||
// notified.
|
||||
NOTIFY_EXISTING_ONLY
|
||||
};
|
||||
|
||||
ObserverList() : notify_depth_(0), type_(NOTIFY_ALL) {}
|
||||
ObserverList(NotificationType type) : notify_depth_(0), type_(type) {}
|
||||
~ObserverList() {
|
||||
// When check_empty is true, assert that the list is empty on destruction.
|
||||
if (check_empty) {
|
||||
@ -98,7 +112,12 @@ class ObserverList {
|
||||
// also the FOREACH_OBSERVER macro defined below.
|
||||
class Iterator {
|
||||
public:
|
||||
Iterator(const ObserverList<ObserverType>& list) : list_(list), index_(0) {
|
||||
Iterator(const ObserverList<ObserverType>& list)
|
||||
: list_(list),
|
||||
index_(0),
|
||||
max_index_(list.type_ == NOTIFY_ALL ?
|
||||
std::numeric_limits<size_t>::max() :
|
||||
list.observers_.size()) {
|
||||
++list_.notify_depth_;
|
||||
}
|
||||
|
||||
@ -110,14 +129,16 @@ class ObserverList {
|
||||
ObserverType* GetNext() {
|
||||
ListType& observers = list_.observers_;
|
||||
// Advance if the current element is null
|
||||
while (index_ < observers.size() && !observers[index_])
|
||||
size_t max_index = std::min(max_index_, observers.size());
|
||||
while (index_ < max_index && !observers[index_])
|
||||
++index_;
|
||||
return index_ < observers.size() ? observers[index_++] : NULL;
|
||||
return index_ < max_index ? observers[index_++] : NULL;
|
||||
}
|
||||
|
||||
private:
|
||||
const ObserverList<ObserverType>& list_;
|
||||
size_t index_;
|
||||
size_t max_index_;
|
||||
};
|
||||
|
||||
private:
|
||||
@ -137,6 +158,7 @@ class ObserverList {
|
||||
// These are marked mutable to facilitate having NotifyAll be const.
|
||||
mutable ListType observers_;
|
||||
mutable int notify_depth_;
|
||||
NotificationType type_;
|
||||
|
||||
friend class ObserverList::Iterator;
|
||||
|
||||
|
@ -60,6 +60,26 @@ class ThreadSafeDisrupter : public Foo {
|
||||
Foo* doomed_;
|
||||
};
|
||||
|
||||
class AddInObserve : public Foo {
|
||||
public:
|
||||
AddInObserve(ObserverList<Foo>* observer_list)
|
||||
: added(false),
|
||||
observer_list(observer_list),
|
||||
adder(1) {
|
||||
}
|
||||
virtual void Observe(int x) {
|
||||
if (!added) {
|
||||
added = true;
|
||||
observer_list->AddObserver(&adder);
|
||||
}
|
||||
}
|
||||
|
||||
bool added;
|
||||
ObserverList<Foo>* observer_list;
|
||||
Adder adder;
|
||||
};
|
||||
|
||||
|
||||
class ObserverListThreadSafeTest : public testing::Test {
|
||||
};
|
||||
|
||||
@ -262,3 +282,23 @@ TEST(ObserverListThreadSafeTest, CrossThreadNotifications) {
|
||||
// the main thread and all 3 observer threads.
|
||||
ThreadSafeObserverHarness(3, true);
|
||||
}
|
||||
|
||||
TEST(ObserverListTest, Existing) {
|
||||
ObserverList<Foo> observer_list(ObserverList<Foo>::NOTIFY_EXISTING_ONLY);
|
||||
Adder a(1);
|
||||
AddInObserve b(&observer_list);
|
||||
|
||||
observer_list.AddObserver(&a);
|
||||
observer_list.AddObserver(&b);
|
||||
|
||||
FOR_EACH_OBSERVER(Foo, observer_list, Observe(1));
|
||||
|
||||
EXPECT_TRUE(b.added);
|
||||
// B's adder should not have been notified because it was added during
|
||||
// notificaiton.
|
||||
EXPECT_EQ(0, b.adder.total);
|
||||
|
||||
// Notify again to make sure b's adder is notified.
|
||||
FOR_EACH_OBSERVER(Foo, observer_list, Observe(1));
|
||||
EXPECT_EQ(1, b.adder.total);
|
||||
}
|
||||
|
@ -77,6 +77,7 @@ BookmarkModel::BookmarkModel(Profile* profile)
|
||||
root_(this, GURL()),
|
||||
bookmark_bar_node_(NULL),
|
||||
other_node_(NULL),
|
||||
observers_(ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY),
|
||||
waiting_for_history_load_(false),
|
||||
loaded_signal_(CreateEvent(NULL, TRUE, FALSE, NULL)) {
|
||||
// Create the bookmark bar and other bookmarks folders. These always exist.
|
||||
|
Reference in New Issue
Block a user