From 80547a165adf250f31ade57544771201bbc2690c Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 25 Apr 2017 12:43:13 -0700 Subject: 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 Reviewed-by: dsinclair --- fxjs/cfxjse_arguments.cpp | 12 ++++------ fxjs/cfxjse_arguments.h | 4 +--- fxjs/cfxjse_class.cpp | 34 ++++++++++---------------- fxjs/cfxjse_class.h | 11 ++++----- fxjs/cfxjse_context.cpp | 44 ++++++++++++++++++++++++---------- fxjs/cfxjse_context.h | 15 ++++++------ xfa/fxfa/fm2js/xfa_fm2jscontext.cpp | 5 ++-- xfa/fxfa/parser/cxfa_scriptcontext.cpp | 41 ++++++++++++++++--------------- xfa/fxfa/parser/cxfa_scriptcontext.h | 5 ++-- 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_Arguments::GetValue(int32_t index) const { - std::unique_ptr lpArgValue( - new CFXJSE_Value(v8::Isolate::GetCurrent())); - lpArgValue->ForceSetValue((*m_pInfo)[index]); - return lpArgValue; + auto pArgValue = pdfium::MakeUnique(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 +#include "fxjs/cfxjse_class.h" #include "fxjs/fxjse.h" -class CFXJSE_Class; - class CFXJSE_Arguments { public: CFXJSE_Arguments(const v8::FunctionCallbackInfo* pInfo, CFXJSE_Value* pRetValue) : m_pInfo(pInfo), m_pRetValue(pRetValue) {} - v8::Isolate* GetRuntime() const; int32_t GetLength() const; std::unique_ptr 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 +#include +#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(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::ReadOnly | v8::DontDelete)); } else { - v8::Local hLocalContext = - v8::Local::New(pIsolate, lpContext->m_hContext); + v8::Local 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(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& 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& 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 + #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 hClass = v8::Local::New( - lpClass->GetContext()->GetRuntime(), lpClass->GetTemplate()); + lpClass->GetContext()->GetIsolate(), lpClass->GetTemplate()); if (!hClass->HasInstance(hObject)) return nullptr; } @@ -148,20 +150,20 @@ v8::Local FXJSE_CreateReturnValue(v8::Isolate* pIsolate, } // static -CFXJSE_Context* CFXJSE_Context::Create( +std::unique_ptr 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(pIsolate); + CFXJSE_Class* pGlobalClassObj = nullptr; v8::Local 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 hFunctionTemplate = v8::Local::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 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_Context::GetGlobalObject() { auto pValue = pdfium::MakeUnique(m_pIsolate); - CFXJSE_ScopeUtil_IsolateHandleContext scope(this); v8::Local hContext = v8::Local::New(m_pIsolate, m_hContext); v8::Local hGlobalObject = hContext->Global(); pValue->ForceSetValue(hGlobalObject); - return pValue; } +v8::Local CFXJSE_Context::GetContext() { + return v8::Local::New(m_pIsolate, m_hContext); +} + +void CFXJSE_Context::AddClass(std::unique_ptr 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& 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 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 GetContext(); std::unique_ptr GetGlobalObject(); + void AddClass(std::unique_ptr 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 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(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(lpVariables); + CFXJSE_Context* pVariableContext = it->second.get(); std::unique_ptr pObject = pVariableContext->GetGlobalObject(); auto hVariableValue = pdfium::MakeUnique(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 m_JsContext; v8::Isolate* m_pIsolate; CFXJSE_Class* m_pJsClass; XFA_SCRIPTLANGTYPE m_eScriptType; std::map> m_mapObjectToValue; - std::map m_mapVariableToContext; + std::map> + m_mapVariableToContext; CXFA_EventParam m_eventParam; std::vector m_upObjectArray; // CacheList holds the NodeList items so we can clean them up when we're done. -- cgit v1.2.3