From 8832fbf53cf71d4d4cb53986d9cfc024edd2bf1f Mon Sep 17 00:00:00 2001
From: tsepez <tsepez@chromium.org>
Date: Thu, 8 Sep 2016 10:25:55 -0700
Subject: Replace CJS_Runtime::Observer with CFX_Runtime<CJS_Runtime>::Observer

Previously, the observer set a flag which conditionalized a null
pointer return.  Now, we just clear the pointer.

Destruction order matters, so add a method to trigger notifications.

Review-Url: https://codereview.chromium.org/2322743002
---
 fpdfsdk/javascript/app.cpp         | 23 +++++------------------
 fpdfsdk/javascript/cjs_runtime.cpp | 14 +-------------
 fpdfsdk/javascript/cjs_runtime.h   | 17 ++++-------------
 3 files changed, 10 insertions(+), 44 deletions(-)

(limited to 'fpdfsdk')

diff --git a/fpdfsdk/javascript/app.cpp b/fpdfsdk/javascript/app.cpp
index f95ec0b09b..89e8b563da 100644
--- a/fpdfsdk/javascript/app.cpp
+++ b/fpdfsdk/javascript/app.cpp
@@ -30,7 +30,7 @@ class GlobalTimer : public CJS_Runtime::Observer {
               const CFX_WideString& script,
               uint32_t dwElapse,
               uint32_t dwTimeOut);
-  ~GlobalTimer() override;
+  ~GlobalTimer();
 
   static void Trigger(int nTimerID);
   static void Cancel(int nTimerID);
@@ -38,26 +38,22 @@ class GlobalTimer : public CJS_Runtime::Observer {
   bool IsOneShot() const { return m_nType == 1; }
   uint32_t GetTimeOut() const { return m_dwTimeOut; }
   int GetTimerID() const { return m_nTimerID; }
-  CJS_Runtime* GetRuntime() const { return m_bValid ? m_pRuntime : nullptr; }
+  CJS_Runtime* GetRuntime() const { return m_pRuntime; }
   CFX_WideString GetJScript() const { return m_swJScript; }
 
  private:
   using TimerMap = std::map<FX_UINT, GlobalTimer*>;
   static TimerMap* GetGlobalTimerMap();
 
-  // CJS_Runtime::Observer
-  void OnDestroyed() override;
-
   uint32_t m_nTimerID;
   app* const m_pEmbedObj;
   bool m_bProcessing;
-  bool m_bValid;
 
   // data
   const int m_nType;  // 0:Interval; 1:TimeOut
   const uint32_t m_dwTimeOut;
   const CFX_WideString m_swJScript;
-  CJS_Runtime* const m_pRuntime;
+  CJS_Runtime* m_pRuntime;
   CPDFDoc_Environment* const m_pApp;
 };
 
@@ -71,7 +67,6 @@ GlobalTimer::GlobalTimer(app* pObj,
     : m_nTimerID(0),
       m_pEmbedObj(pObj),
       m_bProcessing(false),
-      m_bValid(true),
       m_nType(nType),
       m_dwTimeOut(dwTimeOut),
       m_swJScript(script),
@@ -80,18 +75,14 @@ GlobalTimer::GlobalTimer(app* pObj,
   CFX_SystemHandler* pHandler = m_pApp->GetSysHandler();
   m_nTimerID = pHandler->SetTimer(dwElapse, Trigger);
   (*GetGlobalTimerMap())[m_nTimerID] = this;
-  m_pRuntime->AddObserver(this);
+  SetWatchedPtr(&m_pRuntime);
 }
 
 GlobalTimer::~GlobalTimer() {
-  CJS_Runtime* pRuntime = GetRuntime();
-  if (pRuntime)
-    pRuntime->RemoveObserver(this);
-
   if (!m_nTimerID)
     return;
 
-  if (m_bValid)
+  if (GetRuntime())
     m_pApp->GetSysHandler()->KillTimer(m_nTimerID);
 
   GetGlobalTimerMap()->erase(m_nTimerID);
@@ -139,10 +130,6 @@ GlobalTimer::TimerMap* GlobalTimer::GetGlobalTimerMap() {
   return s_TimerMap;
 }
 
-void GlobalTimer::OnDestroyed() {
-  m_bValid = false;
-}
-
 BEGIN_JS_STATIC_CONST(CJS_TimerObj)
 END_JS_STATIC_CONST()
 
diff --git a/fpdfsdk/javascript/cjs_runtime.cpp b/fpdfsdk/javascript/cjs_runtime.cpp
index b12a3f5eeb..67d45d5e4a 100644
--- a/fpdfsdk/javascript/cjs_runtime.cpp
+++ b/fpdfsdk/javascript/cjs_runtime.cpp
@@ -124,9 +124,7 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp)
 }
 
 CJS_Runtime::~CJS_Runtime() {
-  for (auto* obs : m_observers)
-    obs->OnDestroyed();
-
+  NotifyObservers();
   ReleaseEngine();
   if (m_isolateManaged) {
     GetIsolate()->Dispose();
@@ -255,16 +253,6 @@ void CJS_Runtime::RemoveEventFromSet(const FieldEvent& event) {
   m_FieldEventSet.erase(event);
 }
 
-void CJS_Runtime::AddObserver(Observer* observer) {
-  ASSERT(!pdfium::ContainsKey(m_observers, observer));
-  m_observers.insert(observer);
-}
-
-void CJS_Runtime::RemoveObserver(Observer* observer) {
-  ASSERT(pdfium::ContainsKey(m_observers, observer));
-  m_observers.erase(observer);
-}
-
 #ifdef PDF_ENABLE_XFA
 CFX_WideString ChangeObjName(const CFX_WideString& str) {
   CFX_WideString sRet = str;
diff --git a/fpdfsdk/javascript/cjs_runtime.h b/fpdfsdk/javascript/cjs_runtime.h
index 33d2956ce3..5157c416ad 100644
--- a/fpdfsdk/javascript/cjs_runtime.h
+++ b/fpdfsdk/javascript/cjs_runtime.h
@@ -13,6 +13,7 @@
 #include <utility>
 #include <vector>
 
+#include "core/fxcrt/include/cfx_observable.h"
 #include "core/fxcrt/include/fx_basic.h"
 #include "fpdfsdk/javascript/JS_EventHandler.h"
 #include "fpdfsdk/javascript/ijs_runtime.h"
@@ -20,16 +21,10 @@
 
 class CJS_Context;
 
-class CJS_Runtime : public IJS_Runtime, public CFXJS_Engine {
+class CJS_Runtime : public IJS_Runtime,
+                    public CFXJS_Engine,
+                    public CFX_Observable<CJS_Runtime> {
  public:
-  class Observer {
-   public:
-    virtual void OnDestroyed() = 0;
-
-   protected:
-    virtual ~Observer() {}
-  };
-
   using FieldEvent = std::pair<CFX_WideString, JS_EVENT_T>;
 
   static CJS_Runtime* FromContext(const IJS_Context* cc);
@@ -64,9 +59,6 @@ class CJS_Runtime : public IJS_Runtime, public CFXJS_Engine {
                          CFXJSE_Value* pValue) override;
 #endif  // PDF_ENABLE_XFA
 
-  void AddObserver(Observer* observer);
-  void RemoveObserver(Observer* observer);
-
  private:
   void DefineJSObjects();
 
@@ -76,7 +68,6 @@ class CJS_Runtime : public IJS_Runtime, public CFXJS_Engine {
   bool m_bBlocking;
   bool m_isolateManaged;
   std::set<FieldEvent> m_FieldEventSet;
-  std::set<Observer*> m_observers;
 };
 
 #endif  // FPDFSDK_JAVASCRIPT_CJS_RUNTIME_H_
-- 
cgit v1.2.3