Use a map for app::m_Timers

fpdfsdk/javascript/app.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "fpdfsdk/javascript/app.h"
 
 #include <memory>
 #include <vector>
@@ skipping 491 matching lines @@
 
   CFX_WideString script =
       params.size() > 0 ? params[0].ToCFXWideString(pRuntime) : L"";
   if (script.IsEmpty()) {
     sError = JSGetStringFromID(pContext, IDS_STRING_JSAFNUMBER_KEYSTROKE);
     return TRUE;
   }
 
   uint32_t dwInterval = params.size() > 1 ? params[1].ToInt(pRuntime) : 1000;
   CPDFDoc_Environment* pApp = pRuntime->GetReaderApp();
-  m_Timers.push_back(std::unique_ptr<GlobalTimer>(
-      new GlobalTimer(this, pApp, pRuntime, 0, script, dwInterval, 0)));
+
+  std::unique_ptr<GlobalTimer> timer(
+      new GlobalTimer(this, pApp, pRuntime, 0, script, dwInterval, 0));
+  GlobalTimer* timerRef = timer.get();
+  m_Timers[timerRef] = std::move(timer);
 
   v8::Local<v8::Object> pRetObj =
       pRuntime->NewFxDynamicObj(CJS_TimerObj::g_nObjDefnID);
   CJS_TimerObj* pJS_TimerObj =
       static_cast<CJS_TimerObj*>(pRuntime->GetObjectPrivate(pRetObj));
   TimerObj* pTimerObj = static_cast<TimerObj*>(pJS_TimerObj->GetEmbedObject());
-  pTimerObj->SetTimer(m_Timers.back().get());
+  pTimerObj->SetTimer(timerRef);
 
   vRet = CJS_Value(pRuntime, pRetObj);
   return TRUE;
 }
 
 FX_BOOL app::setTimeOut(IJS_Context* cc,
                         const std::vector<CJS_Value>& params,
                         CJS_Value& vRet,
                         CFX_WideString& sError) {
   CJS_Context* pContext = static_cast<CJS_Context*>(cc);
   CJS_Runtime* pRuntime = pContext->GetJSRuntime();
 
   if (params.size() > 2 || params.size() == 0) {
     sError = JSGetStringFromID(pContext, IDS_STRING_JSPARAMERROR);
     return FALSE;
   }
 
   CFX_WideString script = params[0].ToCFXWideString(pRuntime);
   if (script.IsEmpty()) {
     sError = JSGetStringFromID(pContext, IDS_STRING_JSAFNUMBER_KEYSTROKE);
     return TRUE;
   }
 
   uint32_t dwTimeOut = params.size() > 1 ? params[1].ToInt(pRuntime) : 1000;
   CPDFDoc_Environment* pApp = pRuntime->GetReaderApp();
-  m_Timers.push_back(std::unique_ptr<GlobalTimer>(
-      new GlobalTimer(this, pApp, pRuntime, 1, script, dwTimeOut, dwTimeOut)));
+
+  std::unique_ptr<GlobalTimer> timer(
+      new GlobalTimer(this, pApp, pRuntime, 1, script, dwTimeOut, dwTimeOut));
+  GlobalTimer* timerRef = timer.get();
+  m_Timers[timerRef] = std::move(timer);

[Tom Sepez, 2016/08/18 17:17:43]

This feels strange, mapping a pointer to itself, Why isn't this just
std::set<std::unique_ptr<GlobalTimer> > ???

[dsinclair, 2016/08/18 17:20:21]

If this was a set I'd have the same issue as before in that it would be a linear
search to find the GlobalTimer* in the remove method. I need the key to be the
pointer, but we want the unique_ptr for the memory cleanup happy-ness.

 
   v8::Local<v8::Object> pRetObj =
       pRuntime->NewFxDynamicObj(CJS_TimerObj::g_nObjDefnID);
 
   CJS_TimerObj* pJS_TimerObj =
       static_cast<CJS_TimerObj*>(pRuntime->GetObjectPrivate(pRetObj));
 
   TimerObj* pTimerObj = static_cast<TimerObj*>(pJS_TimerObj->GetEmbedObject());
-  pTimerObj->SetTimer(m_Timers.back().get());
+  pTimerObj->SetTimer(timerRef);
 
   vRet = CJS_Value(pRuntime, pRetObj);
   return TRUE;
 }
 
 FX_BOOL app::clearTimeOut(IJS_Context* cc,
                           const std::vector<CJS_Value>& params,
                           CJS_Value& vRet,
                           CFX_WideString& sError) {
   CJS_Context* pContext = (CJS_Context*)cc;
@@ skipping 46 matching lines @@
   return FALSE;
 }
 
 void app::TimerProc(GlobalTimer* pTimer) {
   CJS_Runtime* pRuntime = pTimer->GetRuntime();
   if (pRuntime && (!pTimer->IsOneShot() || pTimer->GetTimeOut() > 0))
     RunJsScript(pRuntime, pTimer->GetJScript());
 }
 
 void app::CancelProc(GlobalTimer* pTimer) {
-  auto iter = std::find_if(m_Timers.begin(), m_Timers.end(),
-                           [pTimer](const std::unique_ptr<GlobalTimer>& that) {
-                             return pTimer == that.get();
-                           });
-
-  if (iter != m_Timers.end())
-    m_Timers.erase(iter);
+  m_Timers.erase(pTimer);
 }
 
 void app::RunJsScript(CJS_Runtime* pRuntime, const CFX_WideString& wsScript) {
   if (!pRuntime->IsBlocking()) {
     IJS_Context* pContext = pRuntime->NewContext();
     pContext->OnExternal_Exec();
     CFX_WideString wtInfo;
     pContext->RunScript(wsScript, &wtInfo);
     pRuntime->ReleaseContext(pContext);
   }
@@ skipping 194 matching lines @@
 FX_BOOL app::media(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) {
   return FALSE;
 }
 
 FX_BOOL app::execDialog(IJS_Context* cc,
                         const std::vector<CJS_Value>& params,
                         CJS_Value& vRet,
                         CFX_WideString& sError) {
   return TRUE;
 }