Tidy up JS_Object.h and JS_Object.cpp.

R=thestig@chromium.org

Review URL: https://codereview.chromium.org/1293673003 .

2 files changed, 62 insertions, 69 deletions

diff --git a/fpdfsdk/include/javascript/JS_Object.h b/fpdfsdk/include/javascript/JS_Object.h
index 331475a548..adcedb837c 100644
--- a/fpdfsdk/include/javascript/JS_Object.h
+++ b/fpdfsdk/include/javascript/JS_Object.h
@@ -9,28 +9,29 @@
 
 #include <map>
 
+#include "../../../third_party/base/nonstd_unique_ptr.h"
+
 #include "../fsdk_define.h"       // For FX_UINT
 #include "../fsdk_mgr.h"          // For CPDFDoc_Environment
 #include "../fx_systemhandler.h"  // For IFX_SystemHandler
 #include "../jsapi/fxjs_v8.h"
 
 class CPDFSDK_PageView;
+class CJS_Context;
 class CJS_Object;
+class CJS_Runtime;
 class CJS_Timer;
-class CJS_Context;
 
 class CJS_EmbedObj {
  public:
-  CJS_EmbedObj(CJS_Object* pJSObject);
+  explicit CJS_EmbedObj(CJS_Object* pJSObject);
   virtual ~CJS_EmbedObj();
 
-  virtual void TimerProc(CJS_Timer* pTimer){};
-
+  virtual void TimerProc(CJS_Timer* pTimer) {}
   CJS_Timer* BeginTimer(CPDFDoc_Environment* pApp, FX_UINT nElapse);
   void EndTimer(CJS_Timer* pTimer);
 
-  CJS_Object* GetJSObject() { return m_pJSObject; };
-  operator CJS_Object*() { return m_pJSObject; };
+  CJS_Object* GetJSObject() const { return m_pJSObject; }
 
   CPDFSDK_PageView* JSGetPageView(IFXJS_Context* cc);
   int MsgBox(CPDFDoc_Environment* pApp,
@@ -47,25 +48,25 @@ class CJS_EmbedObj {
 
 class CJS_Object {
  public:
-  CJS_Object(JSFXObject pObject);
+  explicit CJS_Object(JSFXObject pObject);
   virtual ~CJS_Object(void);
 
   void MakeWeak();
   void Dispose();
 
-  virtual FX_BOOL IsType(const FX_CHAR* sClassName) { return TRUE; };
-  virtual CFX_ByteString GetClassName() { return ""; };
+  virtual FX_BOOL IsType(const FX_CHAR* sClassName) { return TRUE; }
+  virtual CFX_ByteString GetClassName() { return ""; }
 
-  virtual FX_BOOL InitInstance(IFXJS_Context* cc) { return TRUE; };
-  virtual FX_BOOL ExitInstance() { return TRUE; };
+  virtual FX_BOOL InitInstance(IFXJS_Context* cc) { return TRUE; }
+  virtual FX_BOOL ExitInstance() { return TRUE; }
 
   operator JSFXObject() {
     return v8::Local<v8::Object>::New(m_pIsolate, m_pObject);
   }
-  operator CJS_EmbedObj*() { return m_pEmbedObj; };
 
-  void SetEmbedObject(CJS_EmbedObj* pObj) { m_pEmbedObj = pObj; };
-  CJS_EmbedObj* GetEmbedObject() { return m_pEmbedObj; };
+  // Takes ownership of |pObj|.
+  void SetEmbedObject(CJS_EmbedObj* pObj) { m_pEmbedObj.reset(pObj); }
+  CJS_EmbedObj* GetEmbedObject() const { return m_pEmbedObj.get(); }
 
   static CPDFSDK_PageView* JSGetPageView(IFXJS_Context* cc);
   static int MsgBox(CPDFDoc_Environment* pApp,
@@ -79,16 +80,11 @@ class CJS_Object {
   v8::Isolate* GetIsolate() { return m_pIsolate; }
 
  protected:
-  CJS_EmbedObj* m_pEmbedObj;
+  nonstd::unique_ptr<CJS_EmbedObj> m_pEmbedObj;
   v8::Global<v8::Object> m_pObject;
   v8::Isolate* m_pIsolate;
 };
 
-using JSTimerMap = std::map<FX_UINT, CJS_Timer*>;
-JSTimerMap* GetGlobalTimerMap();
-
-class CJS_Runtime;
-
 class CJS_Timer {
  public:
   CJS_Timer(CJS_EmbedObj* pObj, CPDFDoc_Environment* pApp)
@@ -105,59 +101,30 @@ class CJS_Timer {
   virtual ~CJS_Timer() { KillJSTimer(); }
 
  public:
-  FX_UINT SetJSTimer(FX_UINT nElapse) {
-    if (m_nTimerID)
-      KillJSTimer();
-    IFX_SystemHandler* pHandler = m_pApp->GetSysHandler();
-    m_nTimerID = pHandler->SetTimer(nElapse, TimerProc);
-    (*GetGlobalTimerMap())[m_nTimerID] = this;
-    m_dwElapse = nElapse;
-    return m_nTimerID;
-  };
-
-  void KillJSTimer() {
-    if (m_nTimerID) {
-      IFX_SystemHandler* pHandler = m_pApp->GetSysHandler();
-      pHandler->KillTimer(m_nTimerID);
-      GetGlobalTimerMap()->erase(m_nTimerID);
-      m_nTimerID = 0;
-    }
-  };
+  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; }
-
   CFX_WideString GetJScript() const { return m_swJScript; }
 
-  static void TimerProc(int idEvent) {
-    const auto it = GetGlobalTimerMap()->find(idEvent);
-    if (it != GetGlobalTimerMap()->end()) {
-      CJS_Timer* pTimer = it->second;
-      if (!pTimer->m_bProcessing) {
-        pTimer->m_bProcessing = TRUE;
-        if (pTimer->m_pEmbedObj)
-          pTimer->m_pEmbedObj->TimerProc(pTimer);
-        pTimer->m_bProcessing = FALSE;
-      }
-    }
-  };
+  static void TimerProc(int idEvent);
 
  private:
+  using TimerMap = std::map<FX_UINT, CJS_Timer*>;
+  static TimerMap* GetGlobalTimerMap();
+
   FX_UINT m_nTimerID;
   CJS_EmbedObj* m_pEmbedObj;
   FX_BOOL m_bProcessing;
diff --git a/fpdfsdk/src/javascript/JS_Object.cpp b/fpdfsdk/src/javascript/JS_Object.cpp
index ef9bab971d..96e1d1d564 100644
--- a/fpdfsdk/src/javascript/JS_Object.cpp
+++ b/fpdfsdk/src/javascript/JS_Object.cpp
@@ -10,12 +10,6 @@
 #include "../../include/javascript/JS_Object.h"
 #include "../../include/javascript/JS_Context.h"
 
-JSTimerMap* GetGlobalTimerMap() {
-  // Leak the timer array at shutdown.
-  static auto* timeMap = new JSTimerMap;
-  return timeMap;
-}
-
 int FXJS_MsgBox(CPDFDoc_Environment* pApp,
                 CPDFSDK_PageView* pPageView,
                 const FX_WCHAR* swMsg,
@@ -39,9 +33,6 @@ CPDFSDK_PageView* FXJS_GetPageView(IFXJS_Context* cc) {
   return NULL;
 }
 
-/* ---------------------------------  CJS_EmbedObj
- * --------------------------------- */
-
 CJS_EmbedObj::CJS_EmbedObj(CJS_Object* pJSObject) : m_pJSObject(pJSObject) {}
 
 CJS_EmbedObj::~CJS_EmbedObj() {
@@ -79,8 +70,6 @@ void CJS_EmbedObj::EndTimer(CJS_Timer* pTimer) {
   delete pTimer;
 }
 
-/* ---------------------------------  CJS_Object
- * --------------------------------- */
 void FreeObject(const v8::WeakCallbackInfo<CJS_Object>& data) {
   CJS_Object* pJSObj = data.GetParameter();
   pJSObj->ExitInstance();
@@ -101,9 +90,6 @@ CJS_Object::CJS_Object(JSFXObject pObject) : m_pEmbedObj(NULL) {
 };
 
 CJS_Object::~CJS_Object(void) {
-  delete m_pEmbedObj;
-  m_pEmbedObj = NULL;
-
   m_pObject.Reset();
 };
 
@@ -137,3 +123,43 @@ void CJS_Object::Alert(CJS_Context* pContext, const FX_WCHAR* swMsg) {
       pApp->JS_appAlert(swMsg, NULL, 0, 3);
   }
 }
+
+FX_UINT CJS_Timer::SetJSTimer(FX_UINT nElapse) {
+  if (m_nTimerID)
+    KillJSTimer();
+  IFX_SystemHandler* pHandler = m_pApp->GetSysHandler();
+  m_nTimerID = pHandler->SetTimer(nElapse, TimerProc);
+  (*GetGlobalTimerMap())[m_nTimerID] = this;
+  m_dwElapse = nElapse;
+  return m_nTimerID;
+}
+
+void CJS_Timer::KillJSTimer() {
+  if (m_nTimerID) {
+    IFX_SystemHandler* pHandler = m_pApp->GetSysHandler();
+    pHandler->KillTimer(m_nTimerID);
+    GetGlobalTimerMap()->erase(m_nTimerID);
+    m_nTimerID = 0;
+  }
+}
+
+// static
+void CJS_Timer::TimerProc(int idEvent) {
+  const auto it = GetGlobalTimerMap()->find(idEvent);
+  if (it != GetGlobalTimerMap()->end()) {
+    CJS_Timer* pTimer = it->second;
+    if (!pTimer->m_bProcessing) {
+      pTimer->m_bProcessing = TRUE;
+      if (pTimer->m_pEmbedObj)
+        pTimer->m_pEmbedObj->TimerProc(pTimer);
+      pTimer->m_bProcessing = FALSE;
+    }
+  }
+}
+
+// static
+CJS_Timer::TimerMap* CJS_Timer::GetGlobalTimerMap() {
+  // Leak the timer array at shutdown.
+  static auto* s_TimerMap = new TimerMap;
+  return s_TimerMap;
+}