From c9aa2f8bfdf74ffae633c7bb1a4d908dda9caed5 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 2 Jul 2018 23:08:53 +0000 Subject: Virtualize Observable::ObservedPtr::OnDestroy() for CPDF_Avail cleanup This enables more complicated cleanup when an observed object is destroyed. Use it to make documents observable and to allow the CPDF_Avail to cleanup without the need for intermediate class. Change-Id: I3a8e758b7ff542e0a58710eff1ac8017205cbd45 Reviewed-on: https://pdfium-review.googlesource.com/36373 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fpdfapi/parser/cpdf_data_avail.cpp | 18 ++++--------- core/fpdfapi/parser/cpdf_data_avail.h | 9 ++++--- core/fpdfapi/parser/cpdf_document.h | 4 ++- core/fxcrt/observable.h | 46 +++++++++++++++++++-------------- core/fxcrt/observable_unittest.cpp | 2 +- 5 files changed, 41 insertions(+), 38 deletions(-) (limited to 'core') diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp index 89b576bdea..a251d6ff61 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.cpp +++ b/core/fpdfapi/parser/cpdf_data_avail.cpp @@ -70,17 +70,6 @@ class HintsScope { UnownedPtr validator_; }; -class CPDF_DocumentForAvail : public CPDF_Document { - public: - explicit CPDF_DocumentForAvail(CPDF_DataAvail* avail) : m_pAvail(avail) { - DCHECK(avail); - } - ~CPDF_DocumentForAvail() override { m_pAvail->BeforeDocumentDestroyed(); } - - private: - UnownedPtr m_pAvail; -}; - } // namespace CPDF_DataAvail::FileAvail::~FileAvail() {} @@ -98,9 +87,11 @@ CPDF_DataAvail::CPDF_DataAvail( CPDF_DataAvail::~CPDF_DataAvail() { m_pHintTables.reset(); + if (m_pDocument) + m_pDocument->RemoveObserver(this); } -void CPDF_DataAvail::BeforeDocumentDestroyed() { +void CPDF_DataAvail::OnObservableDestroyed() { m_pDocument = nullptr; m_pFormAvail.reset(); m_PagesArray.clear(); @@ -1031,7 +1022,8 @@ CPDF_DataAvail::ParseDocument(const char* password) { // We already returned parsed document. return std::make_pair(CPDF_Parser::HANDLER_ERROR, nullptr); } - auto document = pdfium::MakeUnique(this); + auto document = pdfium::MakeUnique(); + document->AddObserver(this); CPDF_ReadValidator::Session read_session(GetValidator().Get()); CPDF_Parser::Error error = diff --git a/core/fpdfapi/parser/cpdf_data_avail.h b/core/fpdfapi/parser/cpdf_data_avail.h index 3c45116088..ea831d94db 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.h +++ b/core/fpdfapi/parser/cpdf_data_avail.h @@ -13,13 +13,13 @@ #include #include +#include "core/fpdfapi/parser/cpdf_document.h" #include "core/fpdfapi/parser/cpdf_parser.h" #include "core/fpdfapi/parser/cpdf_syntax_parser.h" #include "core/fxcrt/unowned_ptr.h" class CPDF_CrossRefAvail; class CPDF_Dictionary; -class CPDF_Document; class CPDF_HintTables; class CPDF_IndirectObjectHolder; class CPDF_LinearizedHeader; @@ -50,7 +50,7 @@ enum PDF_PAGENODE_TYPE { PDF_PAGENODE_ARRAY, }; -class CPDF_DataAvail final { +class CPDF_DataAvail final : public CPDF_Document::Observer { public: // Must match PDF_DATA_* definitions in public/fpdf_dataavail.h, but cannot // #include that header. fpdfsdk/fpdf_dataavail.cpp has static_asserts @@ -95,9 +95,10 @@ class CPDF_DataAvail final { CPDF_DataAvail(FileAvail* pFileAvail, const RetainPtr& pFileRead, bool bSupportHintTable); - ~CPDF_DataAvail(); + ~CPDF_DataAvail() override; - void BeforeDocumentDestroyed(); + // CPDF_Document::Observer: + void OnObservableDestroyed() override; DocAvailStatus IsDocAvail(DownloadHints* pHints); DocAvailStatus IsPageAvail(uint32_t dwPage, DownloadHints* pHints); diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h index 401f37d0cf..a40993da17 100644 --- a/core/fpdfapi/parser/cpdf_document.h +++ b/core/fpdfapi/parser/cpdf_document.h @@ -18,6 +18,7 @@ #include "core/fpdfapi/parser/cpdf_object.h" #include "core/fpdfapi/parser/cpdf_parser.h" #include "core/fpdfdoc/cpdf_linklist.h" +#include "core/fxcrt/observable.h" #include "core/fxcrt/retain_ptr.h" class CFX_Font; @@ -40,7 +41,8 @@ class JBig2_DocumentContext; #define FPDFPERM_FILL_FORM 0x0100 #define FPDFPERM_EXTRACT_ACCESS 0x0200 -class CPDF_Document : public CPDF_Parser::ParsedObjectsHolder { +class CPDF_Document : public Observable, + public CPDF_Parser::ParsedObjectsHolder { public: // Type from which the XFA extension can subclass itself. class Extension { diff --git a/core/fxcrt/observable.h b/core/fxcrt/observable.h index 2013b75be9..e99dadfc8e 100644 --- a/core/fxcrt/observable.h +++ b/core/fxcrt/observable.h @@ -15,26 +15,34 @@ namespace fxcrt { template class Observable { public: - class ObservedPtr { + // General-purpose interface for more complicated cleanup. + class Observer { + public: + virtual ~Observer() = default; + virtual void OnObservableDestroyed() = 0; + }; + + // Simple case of a self-nulling pointer. + class ObservedPtr final : public Observer { public: ObservedPtr() : m_pObservable(nullptr) {} explicit ObservedPtr(T* pObservable) : m_pObservable(pObservable) { if (m_pObservable) - m_pObservable->AddObservedPtr(this); + m_pObservable->AddObserver(this); } ObservedPtr(const ObservedPtr& that) : ObservedPtr(that.Get()) {} ~ObservedPtr() { if (m_pObservable) - m_pObservable->RemoveObservedPtr(this); + m_pObservable->RemoveObserver(this); } void Reset(T* pObservable = nullptr) { if (m_pObservable) - m_pObservable->RemoveObservedPtr(this); + m_pObservable->RemoveObserver(this); m_pObservable = pObservable; if (m_pObservable) - m_pObservable->AddObservedPtr(this); + m_pObservable->AddObserver(this); } - void OnDestroy() { + void OnObservableDestroyed() override { ASSERT(m_pObservable); m_pObservable = nullptr; } @@ -57,27 +65,27 @@ class Observable { Observable() = default; Observable(const Observable& that) = delete; - ~Observable() { NotifyObservedPtrs(); } - void AddObservedPtr(ObservedPtr* pObservedPtr) { - ASSERT(!pdfium::ContainsKey(m_ObservedPtrs, pObservedPtr)); - m_ObservedPtrs.insert(pObservedPtr); + ~Observable() { NotifyObservers(); } + void AddObserver(Observer* pObserver) { + ASSERT(!pdfium::ContainsKey(m_Observers, pObserver)); + m_Observers.insert(pObserver); } - void RemoveObservedPtr(ObservedPtr* pObservedPtr) { - ASSERT(pdfium::ContainsKey(m_ObservedPtrs, pObservedPtr)); - m_ObservedPtrs.erase(pObservedPtr); + void RemoveObserver(Observer* pObserver) { + ASSERT(pdfium::ContainsKey(m_Observers, pObserver)); + m_Observers.erase(pObserver); } - void NotifyObservedPtrs() { - for (auto* pObservedPtr : m_ObservedPtrs) - pObservedPtr->OnDestroy(); - m_ObservedPtrs.clear(); + void NotifyObservers() { + for (auto* pObserver : m_Observers) + pObserver->OnObservableDestroyed(); + m_Observers.clear(); } Observable& operator=(const Observable& that) = delete; protected: - size_t ActiveObservedPtrsForTesting() const { return m_ObservedPtrs.size(); } + size_t ActiveObserversForTesting() const { return m_Observers.size(); } private: - std::set m_ObservedPtrs; + std::set m_Observers; }; } // namespace fxcrt diff --git a/core/fxcrt/observable_unittest.cpp b/core/fxcrt/observable_unittest.cpp index 5e64743d86..63e61b8513 100644 --- a/core/fxcrt/observable_unittest.cpp +++ b/core/fxcrt/observable_unittest.cpp @@ -16,7 +16,7 @@ class PseudoObservable : public Observable { public: PseudoObservable() {} int SomeMethod() { return 42; } - size_t ActiveObservedPtrs() const { return ActiveObservedPtrsForTesting(); } + size_t ActiveObservedPtrs() const { return ActiveObserversForTesting(); } }; } // namespace -- cgit v1.2.3