Virtualize Observable<T>::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 <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

5 files changed, 41 insertions, 38 deletions

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<CPDF_ReadValidator> 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<CPDF_DataAvail> 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<CPDF_DocumentForAvail>(this);
+  auto document = pdfium::MakeUnique<CPDF_Document>();
+  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 <utility>
 #include <vector>
 
+#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<IFX_SeekableReadStream>& 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<CPDF_Document>,
+                      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 T>
 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<ObservedPtr*> m_ObservedPtrs;
+  std::set<Observer*> 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<PseudoObservable> {
  public:
   PseudoObservable() {}
   int SomeMethod() { return 42; }
-  size_t ActiveObservedPtrs() const { return ActiveObservedPtrsForTesting(); }
+  size_t ActiveObservedPtrs() const { return ActiveObserversForTesting(); }
 };
 
 }  // namespace