From 794c9b67d3d519342aa7e15052766f7d4a99f551 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Sun, 4 Oct 2015 16:01:52 -0700 Subject: CJS_Timer should observe CJS_Runtime destruction. Also remove dead CJS_EmbedObj::{Begin,End}Timer code. BUG=539107 R=tsepez@chromium.org Review URL: https://codereview.chromium.org/1384883002 . --- fpdfsdk/include/javascript/JS_Object.h | 63 ++++++++++++--------------------- fpdfsdk/include/javascript/JS_Runtime.h | 12 +++++++ fpdfsdk/src/javascript/JS_Object.cpp | 58 +++++++++++++++++------------- fpdfsdk/src/javascript/JS_Runtime.cpp | 14 +++++++- fpdfsdk/src/javascript/app.cpp | 27 ++++++-------- 5 files changed, 92 insertions(+), 82 deletions(-) diff --git a/fpdfsdk/include/javascript/JS_Object.h b/fpdfsdk/include/javascript/JS_Object.h index c7f92ac61d..337da7ba20 100644 --- a/fpdfsdk/include/javascript/JS_Object.h +++ b/fpdfsdk/include/javascript/JS_Object.h @@ -15,11 +15,11 @@ #include "../fsdk_mgr.h" // For CPDFDoc_Environment #include "../fx_systemhandler.h" // For IFX_SystemHandler #include "../jsapi/fxjs_v8.h" +#include "JS_Runtime.h" class CPDFSDK_PageView; class CJS_Context; class CJS_Object; -class CJS_Runtime; class CJS_Timer; class CJS_EmbedObj { @@ -28,8 +28,6 @@ class CJS_EmbedObj { virtual ~CJS_EmbedObj(); virtual void TimerProc(CJS_Timer* pTimer) {} - CJS_Timer* BeginTimer(CPDFDoc_Environment* pApp, FX_UINT nElapse); - void EndTimer(CJS_Timer* pTimer); CJS_Object* GetJSObject() const { return m_pJSObject; } @@ -83,38 +81,22 @@ class CJS_Object { v8::Isolate* m_pIsolate; }; -class CJS_Timer { +class CJS_Timer : public CJS_Runtime::Observer { public: - CJS_Timer(CJS_EmbedObj* pObj, CPDFDoc_Environment* pApp) - : m_nTimerID(0), - m_pEmbedObj(pObj), - m_bProcessing(FALSE), - m_dwStartTime(0), - m_dwTimeOut(0), - m_dwElapse(0), - m_pRuntime(NULL), - m_nType(0), - m_pApp(pApp) {} - - virtual ~CJS_Timer() { KillJSTimer(); } + CJS_Timer(CJS_EmbedObj* pObj, + CPDFDoc_Environment* pApp, + CJS_Runtime* pRuntime, + int nType, + const CFX_WideString& script, + FX_DWORD dwElapse, + FX_DWORD dwTimeOut); + ~CJS_Timer() override; - public: - FX_UINT SetJSTimer(FX_UINT nElapse); void KillJSTimer(); - void SetType(int nType) { m_nType = nType; } int GetType() const { return m_nType; } - - void SetStartTime(FX_DWORD dwStartTime) { m_dwStartTime = dwStartTime; } - FX_DWORD GetStartTime() const { return m_dwStartTime; } - - void SetTimeOut(FX_DWORD dwTimeOut) { m_dwTimeOut = dwTimeOut; } FX_DWORD GetTimeOut() const { return m_dwTimeOut; } - - void SetRuntime(CJS_Runtime* pRuntime) { m_pRuntime = pRuntime; } - CJS_Runtime* GetRuntime() const { return m_pRuntime; } - - void SetJScript(const CFX_WideString& script) { m_swJScript = script; } + CJS_Runtime* GetRuntime() const { return m_bValid ? m_pRuntime : nullptr; } CFX_WideString GetJScript() const { return m_swJScript; } static void TimerProc(int idEvent); @@ -123,19 +105,20 @@ class CJS_Timer { using TimerMap = std::map; static TimerMap* GetGlobalTimerMap(); - FX_UINT m_nTimerID; - CJS_EmbedObj* m_pEmbedObj; - FX_BOOL m_bProcessing; + // CJS_Runtime::Observer + void OnDestroyed() override; + + FX_DWORD m_nTimerID; + CJS_EmbedObj* const m_pEmbedObj; + bool m_bProcessing; + bool m_bValid; // data - FX_DWORD m_dwStartTime; - FX_DWORD m_dwTimeOut; - FX_DWORD m_dwElapse; - CJS_Runtime* m_pRuntime; - CFX_WideString m_swJScript; - int m_nType; // 0:Interval; 1:TimeOut - - CPDFDoc_Environment* m_pApp; + const int m_nType; // 0:Interval; 1:TimeOut + const FX_DWORD m_dwTimeOut; + const CFX_WideString m_swJScript; + CJS_Runtime* const m_pRuntime; + CPDFDoc_Environment* const m_pApp; }; #endif // FPDFSDK_INCLUDE_JAVASCRIPT_JS_OBJECT_H_ diff --git a/fpdfsdk/include/javascript/JS_Runtime.h b/fpdfsdk/include/javascript/JS_Runtime.h index dd21e6d2f1..6fdc4b0708 100644 --- a/fpdfsdk/include/javascript/JS_Runtime.h +++ b/fpdfsdk/include/javascript/JS_Runtime.h @@ -19,6 +19,14 @@ class CJS_Context; class CJS_Runtime : public IFXJS_Runtime { public: + class Observer { + public: + virtual void OnDestroyed() = 0; + + protected: + virtual ~Observer() {} + }; + using FieldEvent = std::pair; explicit CJS_Runtime(CPDFDoc_Environment* pApp); @@ -44,6 +52,9 @@ class CJS_Runtime : public IFXJS_Runtime { v8::Isolate* GetIsolate() const { return m_isolate; } v8::Local NewJSContext(); + void AddObserver(Observer* observer); + void RemoveObserver(Observer* observer); + private: void DefineJSObjects(); @@ -55,6 +66,7 @@ class CJS_Runtime : public IFXJS_Runtime { v8::Isolate* m_isolate; bool m_isolateManaged; v8::Global m_context; + std::set m_observers; }; #endif // FPDFSDK_INCLUDE_JAVASCRIPT_JS_RUNTIME_H_ diff --git a/fpdfsdk/src/javascript/JS_Object.cpp b/fpdfsdk/src/javascript/JS_Object.cpp index 3cffec785d..7520a99778 100644 --- a/fpdfsdk/src/javascript/JS_Object.cpp +++ b/fpdfsdk/src/javascript/JS_Object.cpp @@ -56,20 +56,6 @@ void CJS_EmbedObj::Alert(CJS_Context* pContext, const FX_WCHAR* swMsg) { CJS_Object::Alert(pContext, swMsg); } -CJS_Timer* CJS_EmbedObj::BeginTimer(CPDFDoc_Environment* pApp, - FX_UINT nElapse) { - CJS_Timer* pTimer = new CJS_Timer(this, pApp); - pTimer->SetJSTimer(nElapse); - - return pTimer; -} - -void CJS_EmbedObj::EndTimer(CJS_Timer* pTimer) { - ASSERT(pTimer != NULL); - pTimer->KillJSTimer(); - delete pTimer; -} - void FreeObject(const v8::WeakCallbackInfo& data) { CJS_Object* pJSObj = data.GetParameter(); pJSObj->ExitInstance(); @@ -123,20 +109,40 @@ void CJS_Object::Alert(CJS_Context* pContext, const FX_WCHAR* swMsg) { } } -FX_UINT CJS_Timer::SetJSTimer(FX_UINT nElapse) { - if (m_nTimerID) - KillJSTimer(); +CJS_Timer::CJS_Timer(CJS_EmbedObj* pObj, + CPDFDoc_Environment* pApp, + CJS_Runtime* pRuntime, + int nType, + const CFX_WideString& script, + FX_DWORD dwElapse, + FX_DWORD dwTimeOut) + : m_nTimerID(0), + m_pEmbedObj(pObj), + m_bProcessing(false), + m_bValid(true), + m_nType(nType), + m_dwTimeOut(dwTimeOut), + m_pRuntime(pRuntime), + m_pApp(pApp) { IFX_SystemHandler* pHandler = m_pApp->GetSysHandler(); - m_nTimerID = pHandler->SetTimer(nElapse, TimerProc); + m_nTimerID = pHandler->SetTimer(dwElapse, TimerProc); (*GetGlobalTimerMap())[m_nTimerID] = this; - m_dwElapse = nElapse; - return m_nTimerID; + m_pRuntime->AddObserver(this); +} + +CJS_Timer::~CJS_Timer() { + CJS_Runtime* pRuntime = GetRuntime(); + if (pRuntime) + pRuntime->RemoveObserver(this); + KillJSTimer(); } void CJS_Timer::KillJSTimer() { if (m_nTimerID) { - IFX_SystemHandler* pHandler = m_pApp->GetSysHandler(); - pHandler->KillTimer(m_nTimerID); + if (m_bValid) { + IFX_SystemHandler* pHandler = m_pApp->GetSysHandler(); + pHandler->KillTimer(m_nTimerID); + } GetGlobalTimerMap()->erase(m_nTimerID); m_nTimerID = 0; } @@ -148,10 +154,10 @@ void CJS_Timer::TimerProc(int idEvent) { if (it != GetGlobalTimerMap()->end()) { CJS_Timer* pTimer = it->second; if (!pTimer->m_bProcessing) { - pTimer->m_bProcessing = TRUE; + CFX_AutoRestorer scoped_processing(&pTimer->m_bProcessing); + pTimer->m_bProcessing = true; if (pTimer->m_pEmbedObj) pTimer->m_pEmbedObj->TimerProc(pTimer); - pTimer->m_bProcessing = FALSE; } } } @@ -162,3 +168,7 @@ CJS_Timer::TimerMap* CJS_Timer::GetGlobalTimerMap() { static auto* s_TimerMap = new TimerMap; return s_TimerMap; } + +void CJS_Timer::OnDestroyed() { + m_bValid = false; +} diff --git a/fpdfsdk/src/javascript/JS_Runtime.cpp b/fpdfsdk/src/javascript/JS_Runtime.cpp index 5c463ce498..4c502a7b75 100644 --- a/fpdfsdk/src/javascript/JS_Runtime.cpp +++ b/fpdfsdk/src/javascript/JS_Runtime.cpp @@ -12,7 +12,6 @@ #include "../../include/javascript/JS_Define.h" #include "../../include/javascript/JS_Object.h" #include "../../include/javascript/JS_Value.h" -#include "../../include/javascript/Document.h" #include "../../include/javascript/app.h" #include "../../include/javascript/color.h" #include "../../include/javascript/Consts.h" @@ -54,6 +53,9 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) } CJS_Runtime::~CJS_Runtime() { + for (auto* obs : m_observers) + obs->OnDestroyed(); + for (int i = 0, sz = m_ContextArray.GetSize(); i < sz; i++) delete m_ContextArray.GetAt(i); @@ -176,6 +178,16 @@ v8::Local CJS_Runtime::NewJSContext() { return v8::Local::New(m_isolate, m_context); } +void CJS_Runtime::AddObserver(Observer* observer) { + ASSERT(m_observers.find(observer) == m_observers.end()); + m_observers.insert(observer); +} + +void CJS_Runtime::RemoveObserver(Observer* observer) { + ASSERT(m_observers.find(observer) != m_observers.end()); + m_observers.erase(observer); +} + CFX_WideString ChangeObjName(const CFX_WideString& str) { CFX_WideString sRet = str; sRet.Replace(L"_", L"."); diff --git a/fpdfsdk/src/javascript/app.cpp b/fpdfsdk/src/javascript/app.cpp index 9d4fa9ca13..067ae108bd 100644 --- a/fpdfsdk/src/javascript/app.cpp +++ b/fpdfsdk/src/javascript/app.cpp @@ -408,16 +408,10 @@ FX_BOOL app::setInterval(IFXJS_Context* cc, CPDFDoc_Environment* pApp = pRuntime->GetReaderApp(); ASSERT(pApp); - CJS_Timer* pTimer = new CJS_Timer(this, pApp); + CJS_Timer* pTimer = + new CJS_Timer(this, pApp, pRuntime, 0, script, dwInterval, 0); m_aTimer.Add(pTimer); - pTimer->SetType(0); - pTimer->SetRuntime(pRuntime); - pTimer->SetJScript(script); - pTimer->SetTimeOut(0); - // pTimer->SetStartTime(GetTickCount()); - pTimer->SetJSTimer(dwInterval); - v8::Local pRetObj = FXJS_NewFxDynamicObj( pRuntime->GetIsolate(), pContext, FXJS_GetObjDefnID(pRuntime->GetIsolate(), L"TimerObj")); @@ -462,15 +456,10 @@ FX_BOOL app::setTimeOut(IFXJS_Context* cc, CPDFDoc_Environment* pApp = pRuntime->GetReaderApp(); ASSERT(pApp); - CJS_Timer* pTimer = new CJS_Timer(this, pApp); + CJS_Timer* pTimer = + new CJS_Timer(this, pApp, pRuntime, 1, script, dwTimeOut, dwTimeOut); m_aTimer.Add(pTimer); - pTimer->SetType(1); - pTimer->SetRuntime(pRuntime); - pTimer->SetJScript(script); - pTimer->SetTimeOut(dwTimeOut); - pTimer->SetJSTimer(dwTimeOut); - v8::Local pRetObj = FXJS_NewFxDynamicObj( pRuntime->GetIsolate(), pContext, FXJS_GetObjDefnID(pRuntime->GetIsolate(), L"TimerObj")); @@ -585,13 +574,17 @@ FX_BOOL app::execMenuItem(IFXJS_Context* cc, void app::TimerProc(CJS_Timer* pTimer) { ASSERT(pTimer != NULL); + CJS_Runtime* pRuntime = pTimer->GetRuntime(); + switch (pTimer->GetType()) { case 0: // interval - RunJsScript(pTimer->GetRuntime(), pTimer->GetJScript()); + if (pRuntime) + RunJsScript(pRuntime, pTimer->GetJScript()); break; case 1: if (pTimer->GetTimeOut() > 0) { - RunJsScript(pTimer->GetRuntime(), pTimer->GetJScript()); + if (pRuntime) + RunJsScript(pRuntime, pTimer->GetJScript()); pTimer->KillJSTimer(); } break; -- cgit v1.2.3