CFX_MapByteStringToPtr considered harmful.

R=thestig@chromium.org

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

2 files changed, 61 insertions, 93 deletions

diff --git a/fpdfsdk/src/fsdk_annothandler.cpp b/fpdfsdk/src/fsdk_annothandler.cpp
index 40938c7592..bb999e1f08 100644
--- a/fpdfsdk/src/fsdk_annothandler.cpp
+++ b/fpdfsdk/src/fsdk_annothandler.cpp
@@ -23,25 +23,20 @@ CPDFSDK_AnnotHandlerMgr::~CPDFSDK_AnnotHandlerMgr() {
     delete pHandler;
   }
   m_Handlers.RemoveAll();
-  m_mapType2Handler.RemoveAll();
+  m_mapType2Handler.clear();
 }
 
 void CPDFSDK_AnnotHandlerMgr::RegisterAnnotHandler(
     IPDFSDK_AnnotHandler* pAnnotHandler) {
-  ASSERT(pAnnotHandler != NULL);
-
-  ASSERT(GetAnnotHandler(pAnnotHandler->GetType()) == NULL);
+  ASSERT(!GetAnnotHandler(pAnnotHandler->GetType()));
 
   m_Handlers.Add(pAnnotHandler);
-  m_mapType2Handler.SetAt(pAnnotHandler->GetType(), (void*)pAnnotHandler);
+  m_mapType2Handler[pAnnotHandler->GetType()] = pAnnotHandler;
 }
 
 void CPDFSDK_AnnotHandlerMgr::UnRegisterAnnotHandler(
     IPDFSDK_AnnotHandler* pAnnotHandler) {
-  ASSERT(pAnnotHandler != NULL);
-
-  m_mapType2Handler.RemoveKey(pAnnotHandler->GetType());
-
+  m_mapType2Handler.erase(pAnnotHandler->GetType());
   for (int i = 0, sz = m_Handlers.GetSize(); i < sz; i++) {
     if (m_Handlers.GetAt(i) == pAnnotHandler) {
       m_Handlers.RemoveAt(i);
@@ -110,9 +105,8 @@ IPDFSDK_AnnotHandler* CPDFSDK_AnnotHandlerMgr::GetAnnotHandler(
 
 IPDFSDK_AnnotHandler* CPDFSDK_AnnotHandlerMgr::GetAnnotHandler(
     const CFX_ByteString& sType) const {
-  void* pRet = NULL;
-  m_mapType2Handler.Lookup(sType, pRet);
-  return (IPDFSDK_AnnotHandler*)pRet;
+  auto it = m_mapType2Handler.find(sType);
+  return it != m_mapType2Handler.end() ? it->second : nullptr;
 }
 
 void CPDFSDK_AnnotHandlerMgr::Annot_OnDraw(CPDFSDK_PageView* pPageView,
diff --git a/fpdfsdk/src/javascript/global.cpp b/fpdfsdk/src/javascript/global.cpp
index aa3bfa026a..cbb04241ae 100644
--- a/fpdfsdk/src/javascript/global.cpp
+++ b/fpdfsdk/src/javascript/global.cpp
@@ -125,15 +125,12 @@ FX_BOOL JSGlobalAlternate::QueryProperty(const FX_WCHAR* propname) {
 FX_BOOL JSGlobalAlternate::DelProperty(IFXJS_Context* cc,
                                        const FX_WCHAR* propname,
                                        CFX_WideString& sError) {
-  JSGlobalData* pData = NULL;
-  CFX_ByteString sPropName = CFX_ByteString::FromUnicode(propname);
-
-  if (m_mapGlobal.Lookup(sPropName, (void*&)pData)) {
-    pData->bDeleted = TRUE;
-    return TRUE;
-  }
+  auto it = m_mapGlobal.find(CFX_ByteString::FromUnicode(propname));
+  if (it == m_mapGlobal.end())
+    return FALSE;
 
-  return FALSE;
+  it->second->bDeleted = TRUE;
+  return TRUE;
 }
 
 FX_BOOL JSGlobalAlternate::DoProperty(IFXJS_Context* cc,
@@ -179,19 +176,16 @@ FX_BOOL JSGlobalAlternate::DoProperty(IFXJS_Context* cc,
         break;
     }
   } else {
-    void* pVoid = nullptr;
-    if (!m_mapGlobal.Lookup(CFX_ByteString::FromUnicode(propname), pVoid)) {
+    auto it = m_mapGlobal.find(CFX_ByteString::FromUnicode(propname));
+    if (it == m_mapGlobal.end()) {
       vp.SetNull();
       return TRUE;
     }
-    if (!pVoid) {
+    JSGlobalData* pData = it->second;
+    if (pData->bDeleted) {
       vp.SetNull();
       return TRUE;
     }
-    JSGlobalData* pData = (JSGlobalData*)pVoid;
-    if (pData->bDeleted)
-      return TRUE;
-
     switch (pData->nType) {
       case JS_GLOBALDATA_TYPE_NUMBER:
         vp << pData->dData;
@@ -228,11 +222,10 @@ FX_BOOL JSGlobalAlternate::setPersistent(IFXJS_Context* cc,
     return FALSE;
   }
 
-  CFX_ByteString sName = params[0].ToCFXByteString();
-
-  JSGlobalData* pData = NULL;
-  if (m_mapGlobal.Lookup(sName, (void*&)pData)) {
-    if (pData && !pData->bDeleted) {
+  auto it = m_mapGlobal.find(params[0].ToCFXByteString());
+  if (it != m_mapGlobal.end()) {
+    JSGlobalData* pData = it->second;
+    if (!pData->bDeleted) {
       pData->bPersistent = params[1].ToBool();
       return TRUE;
     }
@@ -298,52 +291,42 @@ void JSGlobalAlternate::UpdateGlobalPersistentVariables() {
 }
 
 void JSGlobalAlternate::CommitGlobalPersisitentVariables() {
-  ASSERT(m_pGlobalData != NULL);
-
-  FX_POSITION pos = m_mapGlobal.GetStartPosition();
-  while (pos) {
-    CFX_ByteString name;
-    JSGlobalData* pData = NULL;
-    m_mapGlobal.GetNextAssoc(pos, name, (void*&)pData);
-
-    if (pData) {
-      if (pData->bDeleted) {
-        m_pGlobalData->DeleteGlobalVariable(name);
-      } else {
-        switch (pData->nType) {
-          case JS_GLOBALDATA_TYPE_NUMBER:
-            m_pGlobalData->SetGlobalVariableNumber(name, pData->dData);
+  ASSERT(m_pGlobalData);
+  for (auto it = m_mapGlobal.begin(); it != m_mapGlobal.end(); ++it) {
+    CFX_ByteString name = it->first;
+    JSGlobalData* pData = it->second;
+    if (pData->bDeleted) {
+      m_pGlobalData->DeleteGlobalVariable(name);
+    } else {
+      switch (pData->nType) {
+        case JS_GLOBALDATA_TYPE_NUMBER:
+          m_pGlobalData->SetGlobalVariableNumber(name, pData->dData);
+          m_pGlobalData->SetGlobalVariablePersistent(name, pData->bPersistent);
+          break;
+        case JS_GLOBALDATA_TYPE_BOOLEAN:
+          m_pGlobalData->SetGlobalVariableBoolean(name, pData->bData);
+          m_pGlobalData->SetGlobalVariablePersistent(name, pData->bPersistent);
+          break;
+        case JS_GLOBALDATA_TYPE_STRING:
+          m_pGlobalData->SetGlobalVariableString(name, pData->sData);
+          m_pGlobalData->SetGlobalVariablePersistent(name, pData->bPersistent);
+          break;
+        case JS_GLOBALDATA_TYPE_OBJECT:
+          // if (pData->pData)
+          {
+            CJS_GlobalVariableArray array;
+            v8::Local<v8::Object> obj = v8::Local<v8::Object>::New(
+                GetJSObject()->GetIsolate(), pData->pData);
+            ObjectToArray(obj, array);
+            m_pGlobalData->SetGlobalVariableObject(name, array);
             m_pGlobalData->SetGlobalVariablePersistent(name,
                                                        pData->bPersistent);
-            break;
-          case JS_GLOBALDATA_TYPE_BOOLEAN:
-            m_pGlobalData->SetGlobalVariableBoolean(name, pData->bData);
-            m_pGlobalData->SetGlobalVariablePersistent(name,
-                                                       pData->bPersistent);
-            break;
-          case JS_GLOBALDATA_TYPE_STRING:
-            m_pGlobalData->SetGlobalVariableString(name, pData->sData);
-            m_pGlobalData->SetGlobalVariablePersistent(name,
-                                                       pData->bPersistent);
-            break;
-          case JS_GLOBALDATA_TYPE_OBJECT:
-            // if (pData->pData)
-            {
-              CJS_GlobalVariableArray array;
-              v8::Local<v8::Object> obj = v8::Local<v8::Object>::New(
-                  GetJSObject()->GetIsolate(), pData->pData);
-              ObjectToArray(obj, array);
-              m_pGlobalData->SetGlobalVariableObject(name, array);
-              m_pGlobalData->SetGlobalVariablePersistent(name,
-                                                         pData->bPersistent);
-            }
-            break;
-          case JS_GLOBALDATA_TYPE_NULL:
-            m_pGlobalData->SetGlobalVariableNull(name);
-            m_pGlobalData->SetGlobalVariablePersistent(name,
-                                                       pData->bPersistent);
-            break;
-        }
+          }
+          break;
+        case JS_GLOBALDATA_TYPE_NULL:
+          m_pGlobalData->SetGlobalVariableNull(name);
+          m_pGlobalData->SetGlobalVariablePersistent(name, pData->bPersistent);
+          break;
       }
     }
   }
@@ -447,15 +430,10 @@ void JSGlobalAlternate::PutObjectProperty(v8::Local<v8::Object> pObj,
 }
 
 void JSGlobalAlternate::DestroyGlobalPersisitentVariables() {
-  FX_POSITION pos = m_mapGlobal.GetStartPosition();
-  while (pos) {
-    CFX_ByteString name;
-    JSGlobalData* pData = NULL;
-    m_mapGlobal.GetNextAssoc(pos, name, (void*&)pData);
-    delete pData;
+  for (const auto& pair : m_mapGlobal) {
+    delete pair.second;
   }
-
-  m_mapGlobal.RemoveAll();
+  m_mapGlobal.clear();
 }
 
 FX_BOOL JSGlobalAlternate::SetGlobalVariables(const FX_CHAR* propname,
@@ -465,13 +443,12 @@ FX_BOOL JSGlobalAlternate::SetGlobalVariables(const FX_CHAR* propname,
                                               const CFX_ByteString& sData,
                                               JSObject pData,
                                               bool bDefaultPersistent) {
-  if (propname == NULL)
+  if (!propname)
     return FALSE;
 
-  JSGlobalData* pTemp = NULL;
-  m_mapGlobal.Lookup(propname, (void*&)pTemp);
-
-  if (pTemp) {
+  auto it = m_mapGlobal.find(propname);
+  if (it != m_mapGlobal.end()) {
+    JSGlobalData* pTemp = it->second;
     if (pTemp->bDeleted || pTemp->nType != nType) {
       pTemp->dData = 0;
       pTemp->bData = 0;
@@ -480,7 +457,6 @@ FX_BOOL JSGlobalAlternate::SetGlobalVariables(const FX_CHAR* propname,
     }
 
     pTemp->bDeleted = FALSE;
-
     switch (nType) {
       case JS_GLOBALDATA_TYPE_NUMBER: {
         pTemp->dData = dData;
@@ -499,7 +475,6 @@ FX_BOOL JSGlobalAlternate::SetGlobalVariables(const FX_CHAR* propname,
       default:
         return FALSE;
     }
-
     return TRUE;
   }
 
@@ -539,8 +514,7 @@ FX_BOOL JSGlobalAlternate::SetGlobalVariables(const FX_CHAR* propname,
       return FALSE;
   }
 
-  m_mapGlobal.SetAt(propname, (void*)pNewData);
-
+  m_mapGlobal[propname] = pNewData;
   return TRUE;
 }