diff options
author | Tom Sepez <tsepez@chromium.org> | 2017-04-25 12:43:13 -0700 |
---|---|---|
committer | Chromium commit bot <commit-bot@chromium.org> | 2017-04-25 20:31:57 +0000 |
commit | 80547a165adf250f31ade57544771201bbc2690c (patch) | |
tree | 1c1669fdcbc6d2fc443172142b80c03cd1a4972a | |
parent | 233360e3ea0d6d4acea76a7d9e7ce7700ea80b1a (diff) | |
download | pdfium-80547a165adf250f31ade57544771201bbc2690c.tar.xz |
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>
-rw-r--r-- | fxjs/cfxjse_arguments.cpp | 12 | ||||
-rw-r--r-- | fxjs/cfxjse_arguments.h | 4 | ||||
-rw-r--r-- | fxjs/cfxjse_class.cpp | 34 | ||||
-rw-r--r-- | fxjs/cfxjse_class.h | 11 | ||||
-rw-r--r-- | fxjs/cfxjse_context.cpp | 44 | ||||
-rw-r--r-- | fxjs/cfxjse_context.h | 15 | ||||
-rw-r--r-- | xfa/fxfa/fm2js/xfa_fm2jscontext.cpp | 5 | ||||
-rw-r--r-- | xfa/fxfa/parser/cxfa_scriptcontext.cpp | 41 | ||||
-rw-r--r-- | 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_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. |