From 1c62054a42cf0759148501a36c541de5d5769d32 Mon Sep 17 00:00:00 2001 From: tsepez Date: Mon, 12 Sep 2016 09:47:52 -0700 Subject: Make CFX_Obeservable::Observer into a pointer-ish type; This may be a better design because it avoids having a level of indirection that the Observer required. Review-Url: https://codereview.chromium.org/2326763002 --- core/fxcrt/cfx_observable_unittest.cpp | 114 +++++++++++++++++++++++++++++++++ core/fxcrt/include/cfx_observable.h | 74 ++++++++++++--------- 2 files changed, 156 insertions(+), 32 deletions(-) create mode 100644 core/fxcrt/cfx_observable_unittest.cpp (limited to 'core/fxcrt') diff --git a/core/fxcrt/cfx_observable_unittest.cpp b/core/fxcrt/cfx_observable_unittest.cpp new file mode 100644 index 0000000000..eaadd95bb6 --- /dev/null +++ b/core/fxcrt/cfx_observable_unittest.cpp @@ -0,0 +1,114 @@ +// Copyright 2016 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/include/cfx_observable.h" + +#include + +#include "testing/fx_string_testhelpers.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class PseudoObservable : public CFX_Observable { + public: + PseudoObservable() {} + int SomeMethod() { return 42; } + size_t ActiveObservedPtrs() const { return ActiveObservedPtrsForTesting(); } +}; + +} // namespace + +TEST(fxcrt, ObservePtrNull) { + PseudoObservable::ObservedPtr ptr; + EXPECT_EQ(nullptr, ptr.Get()); +} + +TEST(fxcrt, ObservePtrLivesLonger) { + PseudoObservable* pObs = new PseudoObservable; + PseudoObservable::ObservedPtr ptr(pObs); + EXPECT_NE(nullptr, ptr.Get()); + EXPECT_EQ(1u, pObs->ActiveObservedPtrs()); + delete pObs; + EXPECT_EQ(nullptr, ptr.Get()); +} + +TEST(fxcrt, ObservePtrLivesShorter) { + PseudoObservable obs; + { + PseudoObservable::ObservedPtr ptr(&obs); + EXPECT_NE(nullptr, ptr.Get()); + EXPECT_EQ(1u, obs.ActiveObservedPtrs()); + } + EXPECT_EQ(0u, obs.ActiveObservedPtrs()); +} + +TEST(fxcrt, ObservePtrResetNull) { + PseudoObservable obs; + PseudoObservable::ObservedPtr ptr(&obs); + EXPECT_EQ(1u, obs.ActiveObservedPtrs()); + ptr.Reset(); + EXPECT_EQ(0u, obs.ActiveObservedPtrs()); +} + +TEST(fxcrt, ObservePtrReset) { + PseudoObservable obs1; + PseudoObservable obs2; + PseudoObservable::ObservedPtr ptr(&obs1); + EXPECT_EQ(1u, obs1.ActiveObservedPtrs()); + EXPECT_EQ(0u, obs2.ActiveObservedPtrs()); + ptr.Reset(&obs2); + EXPECT_EQ(0u, obs1.ActiveObservedPtrs()); + EXPECT_EQ(1u, obs2.ActiveObservedPtrs()); +} + +TEST(fxcrt, ObservePtrEquals) { + PseudoObservable obj1; + PseudoObservable obj2; + PseudoObservable::ObservedPtr null_ptr1; + PseudoObservable::ObservedPtr obj1_ptr1(&obj1); + PseudoObservable::ObservedPtr obj2_ptr1(&obj2); + { + PseudoObservable::ObservedPtr null_ptr2; + EXPECT_TRUE(null_ptr1 == null_ptr2); + + PseudoObservable::ObservedPtr obj1_ptr2(&obj1); + EXPECT_TRUE(obj1_ptr1 == obj1_ptr2); + + PseudoObservable::ObservedPtr obj2_ptr2(&obj2); + EXPECT_TRUE(obj2_ptr1 == obj2_ptr2); + } + EXPECT_FALSE(null_ptr1 == obj1_ptr1); + EXPECT_FALSE(null_ptr1 == obj2_ptr1); + EXPECT_FALSE(obj1_ptr1 == obj2_ptr1); +} + +TEST(fxcrt, ObservePtrNotEquals) { + PseudoObservable obj1; + PseudoObservable obj2; + PseudoObservable::ObservedPtr null_ptr1; + PseudoObservable::ObservedPtr obj1_ptr1(&obj1); + PseudoObservable::ObservedPtr obj2_ptr1(&obj2); + { + PseudoObservable::ObservedPtr null_ptr2; + PseudoObservable::ObservedPtr obj1_ptr2(&obj1); + PseudoObservable::ObservedPtr obj2_ptr2(&obj2); + EXPECT_FALSE(null_ptr1 != null_ptr2); + EXPECT_FALSE(obj1_ptr1 != obj1_ptr2); + EXPECT_FALSE(obj2_ptr1 != obj2_ptr2); + } + EXPECT_TRUE(null_ptr1 != obj1_ptr1); + EXPECT_TRUE(null_ptr1 != obj2_ptr1); + EXPECT_TRUE(obj1_ptr1 != obj2_ptr1); +} + +TEST(fxcrt, ObservePtrBool) { + PseudoObservable obj1; + PseudoObservable::ObservedPtr null_ptr; + PseudoObservable::ObservedPtr obj1_ptr(&obj1); + bool null_bool = !!null_ptr; + bool obj1_bool = !!obj1_ptr; + EXPECT_FALSE(null_bool); + EXPECT_TRUE(obj1_bool); +} diff --git a/core/fxcrt/include/cfx_observable.h b/core/fxcrt/include/cfx_observable.h index 99a9951284..a5053f3952 100644 --- a/core/fxcrt/include/cfx_observable.h +++ b/core/fxcrt/include/cfx_observable.h @@ -13,56 +13,66 @@ template class CFX_Observable { public: - class Observer { + class ObservedPtr { public: - Observer() : m_pWatchedPtr(nullptr) {} - Observer(T** pWatchedPtr) : m_pWatchedPtr(pWatchedPtr) { - if (m_pWatchedPtr) - (*m_pWatchedPtr)->AddObserver(this); + ObservedPtr() : m_pObservedPtr(nullptr) {} + explicit ObservedPtr(T* pObservedPtr) : m_pObservedPtr(pObservedPtr) { + if (m_pObservedPtr) + m_pObservedPtr->AddObservedPtr(this); } - Observer(const Observer& that) = delete; - ~Observer() { - if (m_pWatchedPtr) - (*m_pWatchedPtr)->RemoveObserver(this); + ObservedPtr(const ObservedPtr& that) = delete; + ~ObservedPtr() { + if (m_pObservedPtr) + m_pObservedPtr->RemoveObservedPtr(this); } - void SetWatchedPtr(T** pWatchedPtr) { - if (m_pWatchedPtr) - (*m_pWatchedPtr)->RemoveObserver(this); - m_pWatchedPtr = pWatchedPtr; - if (m_pWatchedPtr) - (*m_pWatchedPtr)->AddObserver(this); + void Reset(T* pObservedPtr = nullptr) { + if (m_pObservedPtr) + m_pObservedPtr->RemoveObservedPtr(this); + m_pObservedPtr = pObservedPtr; + if (m_pObservedPtr) + m_pObservedPtr->AddObservedPtr(this); } void OnDestroy() { - ASSERT(m_pWatchedPtr); - *m_pWatchedPtr = nullptr; - m_pWatchedPtr = nullptr; + ASSERT(m_pObservedPtr); + m_pObservedPtr = nullptr; } - Observer& operator=(const Observer& that) = delete; + ObservedPtr& operator=(const ObservedPtr& that) = delete; + bool operator==(const ObservedPtr& that) const { + return m_pObservedPtr == that.m_pObservedPtr; + } + bool operator!=(const ObservedPtr& that) const { return !(*this == that); } + explicit operator bool() const { return !!m_pObservedPtr; } + T* Get() const { return m_pObservedPtr; } + T& operator*() const { return *m_pObservedPtr; } + T* operator->() const { return m_pObservedPtr; } private: - T** m_pWatchedPtr; + T* m_pObservedPtr; }; CFX_Observable() {} CFX_Observable(const CFX_Observable& that) = delete; - ~CFX_Observable() { NotifyObservers(); } - void AddObserver(Observer* pObserver) { - ASSERT(!pdfium::ContainsKey(m_Observers, pObserver)); - m_Observers.insert(pObserver); + ~CFX_Observable() { NotifyObservedPtrs(); } + void AddObservedPtr(ObservedPtr* pObservedPtr) { + ASSERT(!pdfium::ContainsKey(m_ObservedPtrs, pObservedPtr)); + m_ObservedPtrs.insert(pObservedPtr); } - void RemoveObserver(Observer* pObserver) { - ASSERT(pdfium::ContainsKey(m_Observers, pObserver)); - m_Observers.erase(pObserver); + void RemoveObservedPtr(ObservedPtr* pObservedPtr) { + ASSERT(pdfium::ContainsKey(m_ObservedPtrs, pObservedPtr)); + m_ObservedPtrs.erase(pObservedPtr); } - void NotifyObservers() { - for (auto* pObserver : m_Observers) - pObserver->OnDestroy(); - m_Observers.clear(); + void NotifyObservedPtrs() { + for (auto* pObservedPtr : m_ObservedPtrs) + pObservedPtr->OnDestroy(); + m_ObservedPtrs.clear(); } CFX_Observable& operator=(const CFX_Observable& that) = delete; + protected: + size_t ActiveObservedPtrsForTesting() const { return m_ObservedPtrs.size(); } + private: - std::set m_Observers; + std::set m_ObservedPtrs; }; #endif // CORE_FXCRT_INCLUDE_CFX_OBSERVABLE_H_ -- cgit v1.2.3