Commit 8750dd91 authored by oshima's avatar oshima Committed by Commit bot

Make sure observers are not nullptr

I believe there is no good reason to allow nullptr observer,
and the bug seems to be hitting this. This is to catch such bug early.

BUG=471649

Review URL: https://codereview.chromium.org/1059383003

Cr-Commit-Position: refs/heads/master@{#324299}
parent 7f46f7bf
...@@ -154,6 +154,7 @@ ObserverType* ObserverListBase<ObserverType>::Iterator::GetNext() { ...@@ -154,6 +154,7 @@ ObserverType* ObserverListBase<ObserverType>::Iterator::GetNext() {
template <class ObserverType> template <class ObserverType>
void ObserverListBase<ObserverType>::AddObserver(ObserverType* obs) { void ObserverListBase<ObserverType>::AddObserver(ObserverType* obs) {
DCHECK(obs);
if (std::find(observers_.begin(), observers_.end(), obs) if (std::find(observers_.begin(), observers_.end(), obs)
!= observers_.end()) { != observers_.end()) {
NOTREACHED() << "Observers can only be added once!"; NOTREACHED() << "Observers can only be added once!";
...@@ -164,6 +165,7 @@ void ObserverListBase<ObserverType>::AddObserver(ObserverType* obs) { ...@@ -164,6 +165,7 @@ void ObserverListBase<ObserverType>::AddObserver(ObserverType* obs) {
template <class ObserverType> template <class ObserverType>
void ObserverListBase<ObserverType>::RemoveObserver(ObserverType* obs) { void ObserverListBase<ObserverType>::RemoveObserver(ObserverType* obs) {
DCHECK(obs);
typename ListType::iterator it = typename ListType::iterator it =
std::find(observers_.begin(), observers_.end(), obs); std::find(observers_.begin(), observers_.end(), obs);
if (it != observers_.end()) { if (it != observers_.end()) {
......
...@@ -70,13 +70,6 @@ TEST(DisplayChangeNotifierTest, AddObserver_Smoke) { ...@@ -70,13 +70,6 @@ TEST(DisplayChangeNotifierTest, AddObserver_Smoke) {
EXPECT_EQ(1, observer.display_added()); EXPECT_EQ(1, observer.display_added());
} }
TEST(DisplayChangeNotifierTest, AddObserver_Null) {
DisplayChangeNotifier change_notifier;
change_notifier.AddObserver(NULL);
// Should not crash.
}
TEST(DisplayChangeNotifier, RemoveObserver_Smoke) { TEST(DisplayChangeNotifier, RemoveObserver_Smoke) {
DisplayChangeNotifier change_notifier; DisplayChangeNotifier change_notifier;
MockDisplayObserver observer; MockDisplayObserver observer;
...@@ -93,13 +86,6 @@ TEST(DisplayChangeNotifier, RemoveObserver_Smoke) { ...@@ -93,13 +86,6 @@ TEST(DisplayChangeNotifier, RemoveObserver_Smoke) {
EXPECT_EQ(0, observer.display_added()); EXPECT_EQ(0, observer.display_added());
} }
TEST(DisplayChangeNotifierTest, RemoveObserver_Null) {
DisplayChangeNotifier change_notifier;
change_notifier.RemoveObserver(NULL);
// Should not crash.
}
TEST(DisplayChangeNotifierTest, RemoveObserver_Unknown) { TEST(DisplayChangeNotifierTest, RemoveObserver_Unknown) {
DisplayChangeNotifier change_notifier; DisplayChangeNotifier change_notifier;
MockDisplayObserver observer; MockDisplayObserver observer;
......
...@@ -389,6 +389,8 @@ gfx::NativeWindow Widget::GetNativeWindow() const { ...@@ -389,6 +389,8 @@ gfx::NativeWindow Widget::GetNativeWindow() const {
} }
void Widget::AddObserver(WidgetObserver* observer) { void Widget::AddObserver(WidgetObserver* observer) {
// Make sure that there is no nullptr in observer list. crbug.com/471649.
CHECK(observer);
observers_.AddObserver(observer); observers_.AddObserver(observer);
} }
......
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