Use unique_ptr in CXFA_ScriptContext::m_mapVariableToContext.

Remove unused CFXJSE_Arguments::GetRuntime().
Remove some default argument values.
Make members of CFXJSE_Context private.

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

9 files changed, 87 insertions, 84 deletions

diff --git a/fxjs/cfxjse_arguments.cpp b/fxjs/cfxjse_arguments.cpp
index 323134dcf5..f406ba34de 100644
--- a/fxjs/cfxjse_arguments.cpp
+++ b/fxjs/cfxjse_arguments.cpp
@@ -8,20 +8,16 @@
 
 #include "fxjs/cfxjse_context.h"
 #include "fxjs/cfxjse_value.h"
-
-v8::Isolate* CFXJSE_Arguments::GetRuntime() const {
-  return m_pRetValue->GetIsolate();
-}
+#include "third_party/base/ptr_util.h"
 
 int32_t CFXJSE_Arguments::GetLength() const {
   return m_pInfo->Length();
 }
 
 std::unique_ptr<CFXJSE_Value> CFXJSE_Arguments::GetValue(int32_t index) const {
-  std::unique_ptr<CFXJSE_Value> lpArgValue(
-      new CFXJSE_Value(v8::Isolate::GetCurrent()));
-  lpArgValue->ForceSetValue((*m_pInfo)[index]);
-  return lpArgValue;
+  auto pArgValue = pdfium::MakeUnique<CFXJSE_Value>(v8::Isolate::GetCurrent());
+  pArgValue->ForceSetValue((*m_pInfo)[index]);
+  return pArgValue;
 }
 
 bool CFXJSE_Arguments::GetBoolean(int32_t index) const {
diff --git a/fxjs/cfxjse_arguments.h b/fxjs/cfxjse_arguments.h
index 4f18082f84..beaf1525bb 100644
--- a/fxjs/cfxjse_arguments.h
+++ b/fxjs/cfxjse_arguments.h
@@ -9,17 +9,15 @@
 
 #include <memory>
 
+#include "fxjs/cfxjse_class.h"
 #include "fxjs/fxjse.h"
 
-class CFXJSE_Class;
-
 class CFXJSE_Arguments {
  public:
   CFXJSE_Arguments(const v8::FunctionCallbackInfo<v8::Value>* pInfo,
                    CFXJSE_Value* pRetValue)
       : m_pInfo(pInfo), m_pRetValue(pRetValue) {}
 
-  v8::Isolate* GetRuntime() const;
   int32_t GetLength() const;
   std::unique_ptr<CFXJSE_Value> GetValue(int32_t index) const;
   bool GetBoolean(int32_t index) const;
diff --git a/fxjs/cfxjse_class.cpp b/fxjs/cfxjse_class.cpp
index 8924a48a47..da70583f45 100644
--- a/fxjs/cfxjse_class.cpp
+++ b/fxjs/cfxjse_class.cpp
@@ -7,7 +7,9 @@
 #include "fxjs/cfxjse_class.h"
 
 #include <memory>
+#include <utility>
 
+#include "fxjs/cfxjse_arguments.h"
 #include "fxjs/cfxjse_context.h"
 #include "fxjs/cfxjse_value.h"
 #include "third_party/base/ptr_util.h"
@@ -311,13 +313,13 @@ CFXJSE_Class* CFXJSE_Class::Create(
   if (!lpContext || !lpClassDefinition)
     return nullptr;
 
-  CFXJSE_Class* pClass =
-      GetClassFromContext(lpContext, lpClassDefinition->name);
-  if (pClass)
-    return pClass;
+  CFXJSE_Class* pExistingClass =
+      lpContext->GetClassByName(lpClassDefinition->name);
+  if (pExistingClass)
+    return pExistingClass;
 
-  v8::Isolate* pIsolate = lpContext->m_pIsolate;
-  pClass = new CFXJSE_Class(lpContext);
+  v8::Isolate* pIsolate = lpContext->GetIsolate();
+  auto pClass = pdfium::MakeUnique<CFXJSE_Class>(lpContext);
   pClass->m_szClassName = lpClassDefinition->name;
   pClass->m_lpClassDefinition = lpClassDefinition;
   CFXJSE_ScopeUtil_IsolateHandleRootContext scope(pIsolate);
@@ -369,8 +371,7 @@ CFXJSE_Class* CFXJSE_Class::Create(
                                               lpClassDefinition))),
           static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete));
     } else {
-      v8::Local<v8::Context> hLocalContext =
-          v8::Local<v8::Context>::New(pIsolate, lpContext->m_hContext);
+      v8::Local<v8::Context> hLocalContext = lpContext->GetContext();
       FXJSE_GetGlobalObjectFromContext(hLocalContext)
           ->Set(v8::String::NewFromUtf8(pIsolate, lpClassDefinition->name),
                 v8::Function::New(
@@ -388,19 +389,10 @@ CFXJSE_Class* CFXJSE_Class::Create(
     fun->RemovePrototype();
     hObjectTemplate->Set(v8::String::NewFromUtf8(pIsolate, "toString"), fun);
   }
-  pClass->m_hTemplate.Reset(lpContext->m_pIsolate, hFunctionTemplate);
-  lpContext->m_rgClasses.push_back(std::unique_ptr<CFXJSE_Class>(pClass));
-  return pClass;
-}
-
-// static
-CFXJSE_Class* CFXJSE_Class::GetClassFromContext(CFXJSE_Context* pContext,
-                                                const CFX_ByteStringC& szName) {
-  for (const auto& pClass : pContext->m_rgClasses) {
-    if (pClass->m_szClassName == szName)
-      return pClass.get();
-  }
-  return nullptr;
+  pClass->m_hTemplate.Reset(lpContext->GetIsolate(), hFunctionTemplate);
+  CFXJSE_Class* pResult = pClass.get();
+  lpContext->AddClass(std::move(pClass));
+  return pResult;
 }
 
 // static
diff --git a/fxjs/cfxjse_class.h b/fxjs/cfxjse_class.h
index 1b3528a5c0..04354941fa 100644
--- a/fxjs/cfxjse_class.h
+++ b/fxjs/cfxjse_class.h
@@ -7,7 +7,6 @@
 #ifndef FXJS_CFXJSE_CLASS_H_
 #define FXJS_CFXJSE_CLASS_H_
 
-#include "fxjs/cfxjse_arguments.h"
 #include "fxjs/fxjse.h"
 #include "v8/include/v8.h"
 
@@ -18,22 +17,20 @@ class CFXJSE_Class {
  public:
   static CFXJSE_Class* Create(CFXJSE_Context* pContext,
                               const FXJSE_CLASS_DESCRIPTOR* lpClassDefintion,
-                              bool bIsJSGlobal = false);
-  static CFXJSE_Class* GetClassFromContext(CFXJSE_Context* pContext,
-                                           const CFX_ByteStringC& szName);
+                              bool bIsJSGlobal);
+
   static void SetUpNamedPropHandler(
       v8::Isolate* pIsolate,
       v8::Local<v8::ObjectTemplate>& hObjectTemplate,
       const FXJSE_CLASS_DESCRIPTOR* lpClassDefinition);
 
+  explicit CFXJSE_Class(CFXJSE_Context* lpContext);
   ~CFXJSE_Class();
 
-  CFXJSE_Context* GetContext() { return m_pContext; }
+  CFXJSE_Context* GetContext() const { return m_pContext; }
   v8::Global<v8::FunctionTemplate>& GetTemplate() { return m_hTemplate; }
 
  protected:
-  explicit CFXJSE_Class(CFXJSE_Context* lpContext);
-
   CFX_ByteString m_szClassName;
   const FXJSE_CLASS_DESCRIPTOR* m_lpClassDefinition;
   CFXJSE_Context* m_pContext;
diff --git a/fxjs/cfxjse_context.cpp b/fxjs/cfxjse_context.cpp
index d3b55ff43e..183d6363eb 100644
--- a/fxjs/cfxjse_context.cpp
+++ b/fxjs/cfxjse_context.cpp
@@ -6,6 +6,8 @@
 
 #include "fxjs/cfxjse_context.h"
 
+#include <utility>
+
 #include "fxjs/cfxjse_class.h"
 #include "fxjs/cfxjse_value.h"
 #include "third_party/base/ptr_util.h"
@@ -102,7 +104,7 @@ CFXJSE_HostObject* FXJSE_RetrieveObjectBinding(
   if (lpClass) {
     v8::Local<v8::FunctionTemplate> hClass =
         v8::Local<v8::FunctionTemplate>::New(
-            lpClass->GetContext()->GetRuntime(), lpClass->GetTemplate());
+            lpClass->GetContext()->GetIsolate(), lpClass->GetTemplate());
     if (!hClass->HasInstance(hObject))
       return nullptr;
   }
@@ -148,20 +150,20 @@ v8::Local<v8::Object> FXJSE_CreateReturnValue(v8::Isolate* pIsolate,
 }
 
 // static
-CFXJSE_Context* CFXJSE_Context::Create(
+std::unique_ptr<CFXJSE_Context> CFXJSE_Context::Create(
     v8::Isolate* pIsolate,
-    const FXJSE_CLASS_DESCRIPTOR* lpGlobalClass,
-    CFXJSE_HostObject* lpGlobalObject) {
+    const FXJSE_CLASS_DESCRIPTOR* pGlobalClass,
+    CFXJSE_HostObject* pGlobalObject) {
   CFXJSE_ScopeUtil_IsolateHandle scope(pIsolate);
-  CFXJSE_Context* pContext = new CFXJSE_Context(pIsolate);
-  CFXJSE_Class* lpGlobalClassObj = nullptr;
+  auto pContext = pdfium::MakeUnique<CFXJSE_Context>(pIsolate);
+  CFXJSE_Class* pGlobalClassObj = nullptr;
   v8::Local<v8::ObjectTemplate> hObjectTemplate;
-  if (lpGlobalClass) {
-    lpGlobalClassObj = CFXJSE_Class::Create(pContext, lpGlobalClass, true);
-    ASSERT(lpGlobalClassObj);
+  if (pGlobalClass) {
+    pGlobalClassObj = CFXJSE_Class::Create(pContext.get(), pGlobalClass, true);
+    ASSERT(pGlobalClassObj);
     v8::Local<v8::FunctionTemplate> hFunctionTemplate =
         v8::Local<v8::FunctionTemplate>::New(pIsolate,
-                                             lpGlobalClassObj->m_hTemplate);
+                                             pGlobalClassObj->m_hTemplate);
     hObjectTemplate = hFunctionTemplate->InstanceTemplate();
   } else {
     hObjectTemplate = v8::ObjectTemplate::New(pIsolate);
@@ -178,7 +180,7 @@ CFXJSE_Context* CFXJSE_Context::Create(
   hNewContext->SetSecurityToken(hRootContext->GetSecurityToken());
   v8::Local<v8::Object> hGlobalObject =
       FXJSE_GetGlobalObjectFromContext(hNewContext);
-  FXJSE_UpdateObjectBinding(hGlobalObject, lpGlobalObject);
+  FXJSE_UpdateObjectBinding(hGlobalObject, pGlobalObject);
   pContext->m_hContext.Reset(pIsolate, hNewContext);
   return pContext;
 }
@@ -189,16 +191,32 @@ CFXJSE_Context::~CFXJSE_Context() {}
 
 std::unique_ptr<CFXJSE_Value> CFXJSE_Context::GetGlobalObject() {
   auto pValue = pdfium::MakeUnique<CFXJSE_Value>(m_pIsolate);
-
   CFXJSE_ScopeUtil_IsolateHandleContext scope(this);
   v8::Local<v8::Context> hContext =
       v8::Local<v8::Context>::New(m_pIsolate, m_hContext);
   v8::Local<v8::Object> hGlobalObject = hContext->Global();
   pValue->ForceSetValue(hGlobalObject);
-
   return pValue;
 }
 
+v8::Local<v8::Context> CFXJSE_Context::GetContext() {
+  return v8::Local<v8::Context>::New(m_pIsolate, m_hContext);
+}
+
+void CFXJSE_Context::AddClass(std::unique_ptr<CFXJSE_Class> pClass) {
+  m_rgClasses.push_back(std::move(pClass));
+}
+
+CFXJSE_Class* CFXJSE_Context::GetClassByName(
+    const CFX_ByteStringC& szName) const {
+  auto pClass =
+      std::find_if(m_rgClasses.begin(), m_rgClasses.end(),
+                   [szName](const std::unique_ptr<CFXJSE_Class>& item) {
+                     return szName == item->m_szClassName;
+                   });
+  return pClass != m_rgClasses.end() ? pClass->get() : nullptr;
+}
+
 void CFXJSE_Context::EnableCompatibleMode() {
   ExecuteScript(szCompatibleModeScript, nullptr, nullptr);
 }
diff --git a/fxjs/cfxjse_context.h b/fxjs/cfxjse_context.h
index 7b83aee4aa..35d5b59cea 100644
--- a/fxjs/cfxjse_context.h
+++ b/fxjs/cfxjse_context.h
@@ -20,28 +20,29 @@ struct FXJSE_CLASS_DESCRIPTOR;
 
 class CFXJSE_Context {
  public:
-  static CFXJSE_Context* Create(
+  static std::unique_ptr<CFXJSE_Context> Create(
       v8::Isolate* pIsolate,
-      const FXJSE_CLASS_DESCRIPTOR* lpGlobalClass = nullptr,
-      CFXJSE_HostObject* lpGlobalObject = nullptr);
+      const FXJSE_CLASS_DESCRIPTOR* pGlobalClass,
+      CFXJSE_HostObject* pGlobalObject);
 
+  explicit CFXJSE_Context(v8::Isolate* pIsolate);
   ~CFXJSE_Context();
 
-  v8::Isolate* GetRuntime() { return m_pIsolate; }
+  v8::Isolate* GetIsolate() const { return m_pIsolate; }
+  v8::Local<v8::Context> GetContext();
   std::unique_ptr<CFXJSE_Value> GetGlobalObject();
+  void AddClass(std::unique_ptr<CFXJSE_Class> pClass);
+  CFXJSE_Class* GetClassByName(const CFX_ByteStringC& szName) const;
   void EnableCompatibleMode();
   bool ExecuteScript(const char* szScript,
                      CFXJSE_Value* lpRetValue,
                      CFXJSE_Value* lpNewThisObject = nullptr);
 
  protected:
-  friend class CFXJSE_Class;
   friend class CFXJSE_ScopeUtil_IsolateHandleContext;
 
   CFXJSE_Context();
   CFXJSE_Context(const CFXJSE_Context&);
-  explicit CFXJSE_Context(v8::Isolate* pIsolate);
-
   CFXJSE_Context& operator=(const CFXJSE_Context&);
 
   v8::Global<v8::Context> m_hContext;
diff --git a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp
index 5c67418218..172c234aca 100644
--- a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp
+++ b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp
@@ -6133,8 +6133,9 @@ CXFA_FM2JSContext::CXFA_FM2JSContext(v8::Isolate* pScriptIsolate,
                                      CFXJSE_Context* pScriptContext,
                                      CXFA_Document* pDoc)
     : m_pIsolate(pScriptIsolate),
-      m_pFMClass(
-          CFXJSE_Class::Create(pScriptContext, &formcalc_fm2js_descriptor)),
+      m_pFMClass(CFXJSE_Class::Create(pScriptContext,
+                                      &formcalc_fm2js_descriptor,
+                                      false)),
       m_pValue(pdfium::MakeUnique<CFXJSE_Value>(pScriptIsolate)),
       m_pDocument(pDoc) {
   m_pValue.get()->SetNull();
diff --git a/xfa/fxfa/parser/cxfa_scriptcontext.cpp b/xfa/fxfa/parser/cxfa_scriptcontext.cpp
index b0b52299e7..e7697f5fab 100644
--- a/xfa/fxfa/parser/cxfa_scriptcontext.cpp
+++ b/xfa/fxfa/parser/cxfa_scriptcontext.cpp
@@ -129,12 +129,8 @@ CXFA_ScriptContext::CXFA_ScriptContext(CXFA_Document* pDocument)
       m_eRunAtType(XFA_ATTRIBUTEENUM_Client) {}
 
 CXFA_ScriptContext::~CXFA_ScriptContext() {
-  for (const auto& pair : m_mapVariableToContext) {
-    CFXJSE_Context* pVariableContext = pair.second;
-    delete ToThisProxy(pVariableContext->GetGlobalObject().get(), nullptr);
-    delete pVariableContext;
-  }
-  m_mapVariableToContext.clear();
+  for (const auto& pair : m_mapVariableToContext)
+    delete ToThisProxy(pair.second->GetGlobalObject().get(), nullptr);
 }
 
 void CXFA_ScriptContext::Initialize(v8::Isolate* pIsolate) {
@@ -440,25 +436,28 @@ XFA_SCRIPTLANGTYPE CXFA_ScriptContext::GetType() {
   return m_eScriptType;
 }
 void CXFA_ScriptContext::DefineJsContext() {
-  m_JsContext.reset(CFXJSE_Context::Create(m_pIsolate, &GlobalClassDescriptor,
-                                           m_pDocument->GetRoot()));
+  m_JsContext = CFXJSE_Context::Create(m_pIsolate, &GlobalClassDescriptor,
+                                       m_pDocument->GetRoot());
   RemoveBuiltInObjs(m_JsContext.get());
   m_JsContext->EnableCompatibleMode();
 }
+
 CFXJSE_Context* CXFA_ScriptContext::CreateVariablesContext(
     CXFA_Node* pScriptNode,
     CXFA_Node* pSubform) {
   if (!pScriptNode || !pSubform)
     return nullptr;
 
-  CFXJSE_Context* pVariablesContext =
+  auto pNewContext =
       CFXJSE_Context::Create(m_pIsolate, &VariablesClassDescriptor,
                              new CXFA_ThisProxy(pSubform, pScriptNode));
-  RemoveBuiltInObjs(pVariablesContext);
-  pVariablesContext->EnableCompatibleMode();
-  m_mapVariableToContext[pScriptNode] = pVariablesContext;
-  return pVariablesContext;
+  RemoveBuiltInObjs(pNewContext.get());
+  pNewContext->EnableCompatibleMode();
+  CFXJSE_Context* pResult = pNewContext.get();
+  m_mapVariableToContext[pScriptNode] = std::move(pNewContext);
+  return pResult;
 }
+
 CXFA_Object* CXFA_ScriptContext::GetVariablesThis(CXFA_Object* pObject,
                                                   bool bScriptNode) {
   if (!pObject->IsVariablesThis())
@@ -517,15 +516,14 @@ bool CXFA_ScriptContext::QueryVariableValue(CXFA_Node* pScriptNode,
   if (it == m_mapVariableToContext.end() || !it->second)
     return false;
 
-  void* lpVariables = it->second;
-  bool bRes = false;
-  CFXJSE_Context* pVariableContext = static_cast<CFXJSE_Context*>(lpVariables);
+  CFXJSE_Context* pVariableContext = it->second.get();
   std::unique_ptr<CFXJSE_Value> pObject = pVariableContext->GetGlobalObject();
   auto hVariableValue = pdfium::MakeUnique<CFXJSE_Value>(m_pIsolate);
   if (!bGetter) {
     pObject->SetObjectOwnProperty(szPropName, pValue);
-    bRes = true;
-  } else if (pObject->HasObjectOwnProperty(szPropName, false)) {
+    return true;
+  }
+  if (pObject->HasObjectOwnProperty(szPropName, false)) {
     pObject->GetObjectProperty(szPropName, hVariableValue.get());
     if (hVariableValue->IsFunction())
       pValue->SetFunctionBind(hVariableValue.get(), pObject.get());
@@ -533,13 +531,14 @@ bool CXFA_ScriptContext::QueryVariableValue(CXFA_Node* pScriptNode,
       pValue->Assign(hVariableValue.get());
     else
       hVariableValue.get()->Assign(pValue);
-    bRes = true;
+    return true;
   }
-  return bRes;
+  return false;
 }
 
 void CXFA_ScriptContext::DefineJsClass() {
-  m_pJsClass = CFXJSE_Class::Create(m_JsContext.get(), &NormalClassDescriptor);
+  m_pJsClass =
+      CFXJSE_Class::Create(m_JsContext.get(), &NormalClassDescriptor, false);
 }
 
 void CXFA_ScriptContext::RemoveBuiltInObjs(CFXJSE_Context* pContext) const {
diff --git a/xfa/fxfa/parser/cxfa_scriptcontext.h b/xfa/fxfa/parser/cxfa_scriptcontext.h
index 3f81bb8ebe..4a497ca22a 100644
--- a/xfa/fxfa/parser/cxfa_scriptcontext.h
+++ b/xfa/fxfa/parser/cxfa_scriptcontext.h
@@ -103,13 +103,14 @@ class CXFA_ScriptContext {
   void DefineJsClass();
   void RemoveBuiltInObjs(CFXJSE_Context* pContext) const;
 
-  CXFA_Document* m_pDocument;
+  CXFA_Document* const m_pDocument;
   std::unique_ptr<CFXJSE_Context> m_JsContext;
   v8::Isolate* m_pIsolate;
   CFXJSE_Class* m_pJsClass;
   XFA_SCRIPTLANGTYPE m_eScriptType;
   std::map<CXFA_Object*, std::unique_ptr<CFXJSE_Value>> m_mapObjectToValue;
-  std::map<CXFA_Object*, CFXJSE_Context*> m_mapVariableToContext;
+  std::map<CXFA_Object*, std::unique_ptr<CFXJSE_Context>>
+      m_mapVariableToContext;
   CXFA_EventParam m_eventParam;
   std::vector<CXFA_Node*> m_upObjectArray;
   // CacheList holds the NodeList items so we can clean them up when we're done.