From 8832fbf53cf71d4d4cb53986d9cfc024edd2bf1f Mon Sep 17 00:00:00 2001 From: tsepez Date: Thu, 8 Sep 2016 10:25:55 -0700 Subject: Replace CJS_Runtime::Observer with CFX_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 --- core/fxcrt/include/cfx_observable.h | 10 ++++++---- fpdfsdk/javascript/app.cpp | 23 +++++------------------ fpdfsdk/javascript/cjs_runtime.cpp | 14 +------------- fpdfsdk/javascript/cjs_runtime.h | 17 ++++------------- 4 files changed, 16 insertions(+), 48 deletions(-) diff --git a/core/fxcrt/include/cfx_observable.h b/core/fxcrt/include/cfx_observable.h index b669e0d3d5..99a9951284 100644 --- a/core/fxcrt/include/cfx_observable.h +++ b/core/fxcrt/include/cfx_observable.h @@ -45,10 +45,7 @@ class CFX_Observable { CFX_Observable() {} CFX_Observable(const CFX_Observable& that) = delete; - ~CFX_Observable() { - for (auto* pObserver : m_Observers) - pObserver->OnDestroy(); - } + ~CFX_Observable() { NotifyObservers(); } void AddObserver(Observer* pObserver) { ASSERT(!pdfium::ContainsKey(m_Observers, pObserver)); m_Observers.insert(pObserver); @@ -57,6 +54,11 @@ class CFX_Observable { ASSERT(pdfium::ContainsKey(m_Observers, pObserver)); m_Observers.erase(pObserver); } + void NotifyObservers() { + for (auto* pObserver : m_Observers) + pObserver->OnDestroy(); + m_Observers.clear(); + } CFX_Observable& operator=(const CFX_Observable& that) = delete; private: 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; 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 #include +#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 { public: - class Observer { - public: - virtual void OnDestroyed() = 0; - - protected: - virtual ~Observer() {} - }; - using FieldEvent = std::pair; 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 m_FieldEventSet; - std::set m_observers; }; #endif // FPDFSDK_JAVASCRIPT_CJS_RUNTIME_H_ -- cgit v1.2.3