Split CJX_Node::RemoveMapModuleKey

This CL splits RemoveMapModuleKey to extract ClearMapModuleBuffer which
is called on destroy to clear the buffer.

This also fixes a potential bug where when we lookup the key, if the
hashcode for the key returns 0 we will end up with a nullptr key.

Change-Id: Ic2b0529a999f6f789dd81fbc4e02ecfbbf7a7632
Reviewed-on: https://pdfium-review.googlesource.com/17614
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

2 files changed, 29 insertions, 22 deletions

diff --git a/fxjs/cjx_node.cpp b/fxjs/cjx_node.cpp
index 7d919c1cff..c14e6f092c 100644
--- a/fxjs/cjx_node.cpp
+++ b/fxjs/cjx_node.cpp
@@ -180,7 +180,7 @@ CJX_Node::CJX_Node(CXFA_Node* node)
     : CJX_Object(node), map_module_data_(nullptr) {}
 
 CJX_Node::~CJX_Node() {
-  RemoveMapModuleKey();
+  ClearMapModuleBuffer();
 }
 
 CXFA_Node* CJX_Node::GetXFANode() {
@@ -320,10 +320,10 @@ bool CJX_Node::GetAttribute(const WideStringView& wsAttr,
   return true;
 }
 
-bool CJX_Node::RemoveAttribute(const WideStringView& wsAttr) {
+void CJX_Node::RemoveAttribute(const WideStringView& wsAttr) {
   void* pKey = GetMapKey_Custom(wsAttr);
-  RemoveMapModuleKey(pKey);
-  return true;
+  if (pKey)
+    RemoveMapModuleKey(pKey);
 }
 
 int32_t CJX_Node::Subform_and_SubformSet_InstanceIndex() {
@@ -3765,26 +3765,11 @@ bool CJX_Node::HasMapModuleKey(void* pKey) {
                      pdfium::ContainsKey(pModule->m_BufferMap, pKey));
 }
 
-void CJX_Node::RemoveMapModuleKey(void* pKey) {
+void CJX_Node::ClearMapModuleBuffer() {
   XFA_MAPMODULEDATA* pModule = GetMapModuleData();
   if (!pModule)
     return;
 
-  if (pKey) {
-    auto it = pModule->m_BufferMap.find(pKey);
-    if (it != pModule->m_BufferMap.end()) {
-      XFA_MAPDATABLOCK* pBuffer = it->second;
-      if (pBuffer) {
-        if (pBuffer->pCallbackInfo && pBuffer->pCallbackInfo->pFree)
-          pBuffer->pCallbackInfo->pFree(*(void**)pBuffer->GetData());
-        FX_Free(pBuffer);
-      }
-      pModule->m_BufferMap.erase(it);
-    }
-    pModule->m_ValueMap.erase(pKey);
-    return;
-  }
-
   for (auto& pair : pModule->m_BufferMap) {
     XFA_MAPDATABLOCK* pBuffer = pair.second;
     if (pBuffer) {
@@ -3798,6 +3783,27 @@ void CJX_Node::RemoveMapModuleKey(void* pKey) {
   delete pModule;
 }
 
+void CJX_Node::RemoveMapModuleKey(void* pKey) {
+  ASSERT(pKey);
+
+  XFA_MAPMODULEDATA* pModule = GetMapModuleData();
+  if (!pModule)
+    return;
+
+  auto it = pModule->m_BufferMap.find(pKey);
+  if (it != pModule->m_BufferMap.end()) {
+    XFA_MAPDATABLOCK* pBuffer = it->second;
+    if (pBuffer) {
+      if (pBuffer->pCallbackInfo && pBuffer->pCallbackInfo->pFree)
+        pBuffer->pCallbackInfo->pFree(*(void**)pBuffer->GetData());
+      FX_Free(pBuffer);
+    }
+    pModule->m_BufferMap.erase(it);
+  }
+  pModule->m_ValueMap.erase(pKey);
+  return;
+}
+
 void CJX_Node::MergeAllData(void* pDstModule) {
   XFA_MAPMODULEDATA* pDstModuleData =
       static_cast<CXFA_Node*>(pDstModule)->JSNode()->CreateMapModuleData();
diff --git a/fxjs/cjx_node.h b/fxjs/cjx_node.h
index 77092d434b..8a67d3f0fc 100644
--- a/fxjs/cjx_node.h
+++ b/fxjs/cjx_node.h
@@ -67,7 +67,7 @@ class CJX_Node : public CJX_Object {
                          const WideString& wsXMLValue,
                          bool bNotify,
                          bool bScriptModify);
-  bool RemoveAttribute(const WideStringView& wsAttr);
+  void RemoveAttribute(const WideStringView& wsAttr);
 
   CXFA_Node* GetProperty(int32_t index,
                          XFA_Element eType,
@@ -443,7 +443,8 @@ class CJX_Node : public CJX_Object {
                           int32_t& iBytes,
                           bool bProtoAlso) const;
   bool HasMapModuleKey(void* pKey);
-  void RemoveMapModuleKey(void* pKey = nullptr);
+  void ClearMapModuleBuffer();
+  void RemoveMapModuleKey(void* pKey);
   void MoveBufferMapData(CXFA_Node* pDstModule, void* pKey);
   void MoveBufferMapData(CXFA_Node* pSrcModule,
                          CXFA_Node* pDstModule,