Use std::set<> to track active event handlers.
authorTom Sepez <tsepez@chromium.org>
Tue, 22 Sep 2015 22:39:15 +0000 (15:39 -0700)
committerTom Sepez <tsepez@chromium.org>
Tue, 22 Sep 2015 22:39:15 +0000 (15:39 -0700)
This avoids some custom linked-list code. Also note that
we use a local copy to be sure we removed the same thing
that was added no matter how our callees may muck with the
handler.

R=thestig@chromium.org

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

fpdfsdk/include/javascript/JS_Runtime.h
fpdfsdk/src/javascript/JS_Context.cpp
fpdfsdk/src/javascript/JS_Runtime.cpp

index a5d6954..67ce9c0 100644 (file)
@@ -7,6 +7,9 @@
 #ifndef FPDFSDK_INCLUDE_JAVASCRIPT_JS_RUNTIME_H_
 #define FPDFSDK_INCLUDE_JAVASCRIPT_JS_RUNTIME_H_
 
+#include <set>
+#include <utility>
+
 #include "../../../third_party/base/nonstd_unique_ptr.h"
 #include "../../../core/include/fxcrt/fx_basic.h"
 #include "../jsapi/fxjs_v8.h"
 
 class CJS_Context;
 
-class CJS_FieldEvent {
- public:
-  CFX_WideString sTargetName;
-  JS_EVENT_T eEventType;
-  CJS_FieldEvent* pNext;
-};
-
 class CJS_Runtime : public IFXJS_Runtime {
  public:
+  using FieldEvent = std::pair<CFX_WideString, JS_EVENT_T>;
+
   explicit CJS_Runtime(CPDFDoc_Environment* pApp);
   ~CJS_Runtime() override;
 
@@ -36,11 +34,9 @@ class CJS_Runtime : public IFXJS_Runtime {
 
   CPDFDoc_Environment* GetReaderApp() const { return m_pApp; }
 
-  FX_BOOL AddEventToLoop(const CFX_WideString& sTargetName,
-                         JS_EVENT_T eEventType);
-  void RemoveEventInLoop(const CFX_WideString& sTargetName,
-                         JS_EVENT_T eEventType);
-  void RemoveEventsInLoop(CJS_FieldEvent* pStart);
+  // Returns true if the event isn't already found in the set.
+  bool AddEventToSet(const FieldEvent& event);
+  void RemoveEventFromSet(const FieldEvent& event);
 
   void BeginBlock() { m_bBlocking = TRUE; }
   void EndBlock() { m_bBlocking = FALSE; }
@@ -56,7 +52,7 @@ class CJS_Runtime : public IFXJS_Runtime {
   CPDFDoc_Environment* m_pApp;
   CPDFSDK_Document* m_pDocument;
   FX_BOOL m_bBlocking;
-  CJS_FieldEvent* m_pFieldEventPath;
+  std::set<FieldEvent> m_FieldEventSet;
   v8::Isolate* m_isolate;
   bool m_isolateManaged;
   nonstd::unique_ptr<FXJS_ArrayBufferAllocator> m_pArrayBufferAllocator;
index 41146ed..342616d 100644 (file)
@@ -46,8 +46,9 @@ FX_BOOL CJS_Context::RunScript(const CFX_WideString& script,
   m_bBusy = TRUE;
 
   ASSERT(m_pEventHandler->IsValid());
-  if (!m_pRuntime->AddEventToLoop(m_pEventHandler->TargetName(),
-                                  m_pEventHandler->EventType())) {
+  CJS_Runtime::FieldEvent event(m_pEventHandler->TargetName(),
+                                m_pEventHandler->EventType());
+  if (!m_pRuntime->AddEventToSet(event)) {
     info = JSGetStringFromID(this, IDS_STRING_JSEVENT);
     return FALSE;
   }
@@ -68,9 +69,7 @@ FX_BOOL CJS_Context::RunScript(const CFX_WideString& script,
     info = JSGetStringFromID(this, IDS_STRING_RUN);
   }
 
-  m_pRuntime->RemoveEventInLoop(m_pEventHandler->TargetName(),
-                                m_pEventHandler->EventType());
-
+  m_pRuntime->RemoveEventFromSet(event);
   m_pEventHandler->Destroy();
   m_bBusy = FALSE;
 
index 43a4dcd..1e1486d 100644 (file)
@@ -33,7 +33,6 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp)
     : m_pApp(pApp),
       m_pDocument(NULL),
       m_bBlocking(FALSE),
-      m_pFieldEventPath(NULL),
       m_isolate(NULL),
       m_isolateManaged(false) {
   unsigned int embedderDataSlot = 0;
@@ -65,11 +64,9 @@ CJS_Runtime::~CJS_Runtime() {
 
   m_ContextArray.RemoveAll();
   FXJS_ReleaseRuntime(GetIsolate(), m_context);
-  RemoveEventsInLoop(m_pFieldEventPath);
 
   m_pApp = NULL;
   m_pDocument = NULL;
-  m_pFieldEventPath = NULL;
   m_context.Reset();
 
   if (m_isolateManaged)
@@ -172,75 +169,12 @@ void CJS_Runtime::SetReaderDocument(CPDFSDK_Document* pReaderDoc) {
   }
 }
 
-FX_BOOL CJS_Runtime::AddEventToLoop(const CFX_WideString& sTargetName,
-                                    JS_EVENT_T eEventType) {
-  if (m_pFieldEventPath == NULL) {
-    m_pFieldEventPath = new CJS_FieldEvent;
-    m_pFieldEventPath->sTargetName = sTargetName;
-    m_pFieldEventPath->eEventType = eEventType;
-    m_pFieldEventPath->pNext = NULL;
-
-    return TRUE;
-  }
-
-  // to search
-  CJS_FieldEvent* p = m_pFieldEventPath;
-  CJS_FieldEvent* pLast = m_pFieldEventPath;
-  while (p) {
-    if (p->eEventType == eEventType && p->sTargetName == sTargetName)
-      return FALSE;
-
-    pLast = p;
-    p = p->pNext;
-  }
-
-  // to add
-  CJS_FieldEvent* pNew = new CJS_FieldEvent;
-  pNew->sTargetName = sTargetName;
-  pNew->eEventType = eEventType;
-  pNew->pNext = NULL;
-
-  pLast->pNext = pNew;
-
-  return TRUE;
-}
-
-void CJS_Runtime::RemoveEventInLoop(const CFX_WideString& sTargetName,
-                                    JS_EVENT_T eEventType) {
-  FX_BOOL bFind = FALSE;
-
-  CJS_FieldEvent* p = m_pFieldEventPath;
-  CJS_FieldEvent* pLast = NULL;
-  while (p) {
-    if (p->eEventType == eEventType && p->sTargetName == sTargetName) {
-      bFind = TRUE;
-      break;
-    }
-
-    pLast = p;
-    p = p->pNext;
-  }
-
-  if (bFind) {
-    RemoveEventsInLoop(p);
-
-    if (p == m_pFieldEventPath)
-      m_pFieldEventPath = NULL;
-
-    if (pLast)
-      pLast->pNext = NULL;
-  }
+bool CJS_Runtime::AddEventToSet(const FieldEvent& event) {
+  return m_FieldEventSet.insert(event).second;
 }
 
-void CJS_Runtime::RemoveEventsInLoop(CJS_FieldEvent* pStart) {
-  CJS_FieldEvent* p = pStart;
-
-  while (p) {
-    CJS_FieldEvent* pOld = p;
-    p = pOld->pNext;
-
-    delete pOld;
-  }
+void CJS_Runtime::RemoveEventFromSet(const FieldEvent& event) {
+  m_FieldEventSet.erase(event);
 }
 
 v8::Local<v8::Context> CJS_Runtime::NewJSContext() {